fix: removing files from e2ee rooms do not remove their messages #38379
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: c5e11b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a typed Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant UploadService
participant MessagesDB
participant Decryptor
Note over Client,MessagesDB: File removal and encrypted-message reconciliation
Client->>Server: Request delete message or file
activate Server
Server->>UploadService: updateMessageRemovingFiles(messageId)
activate UploadService
UploadService->>MessagesDB: Update message attachments -> { type: 'removed-file', color, text, fileId? }
deactivate UploadService
Server->>Client: Broadcast updated message
deactivate Server
Note over Client,Decryptor: Client receives updated message and decrypts if needed
Client->>Decryptor: decryptMessageContent(updatedMessage)
activate Decryptor
Decryptor->>Decryptor: Check attachments (isRemovedFileAttachment / isFileAttachment) and replace reconciled items
Decryptor-->>Client: Return decrypted message with placeholders
deactivate Decryptor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #38379 +/- ##
===========================================
- Coverage 70.40% 70.35% -0.05%
===========================================
Files 3162 3174 +12
Lines 110706 110882 +176
Branches 19926 19941 +15
===========================================
+ Hits 77942 78016 +74
- Misses 30737 30826 +89
- Partials 2027 2040 +13
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/11 00:50 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Wed, 11 Feb 2026 00:50:49 GMT |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/client/lib/e2ee/rocketchat.e2e.room.ts`:
- Around line 672-678: The second early-return check "if
(!deletedAttachments.length) { return message; }" is redundant because
"!deletedAttachments.length" is already part of the earlier compound condition;
remove the duplicate if-block and keep only the initial guard that checks
deletedAttachments, message.attachments, and content.attachments so the function
returns once when there are no deleted attachments or missing attachments (refer
to the variables deletedAttachments, message.attachments, and
content.attachments to locate the redundant check).
🧹 Nitpick comments (1)
apps/meteor/client/lib/e2ee/rocketchat.e2e.room.ts (1)
680-701: Consider using.find()instead of manual loop for cleaner code.The manual
for...ofloop can be simplified.♻️ Suggested refactor
message.attachments = message.attachments.map((att) => { if (!isFileAttachment(att)) { return att; } if (deletedAllAttachment) { return deletedAllAttachment; } const fileId = att.fileId || message.file?._id; if (!fileId) { return att; } - for (const removedAttachment of deletedAttachments) { - if (removedAttachment.fileId === fileId) { - return removedAttachment; - } - } - - return att; + return deletedAttachments.find((removed) => removed.fileId === fileId) ?? att; });
dougfabris
dismissed stale reviews from aleksandernsilva and KevLehman
via
201cf7a
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/tests/e2e/files-management.spec.ts`:
- Around line 48-54: The test performs an unnecessary click that can trigger a
download and make subsequent deletion flaky: remove the explicit call to
poHomeChannel.tabs.files.getFileByName(TEST_FILE_TXT).click() and rely on
poHomeChannel.tabs.files.deleteFile(TEST_FILE_TXT) which already locates the
file item and opens its "More" menu; update the test in
apps/meteor/tests/e2e/files-management.spec.ts to only call
poHomeChannel.tabs.files.deleteFile(TEST_FILE_TXT) before asserting the file
count and message visibility, ensuring references to getFileByName and
deleteFile remain correct.
🧹 Nitpick comments (3)
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (3)
49-49: Avoidpage.locator()— prefer a semantic locator or page-object accessor.Line 49 uses
page.locator('#main-content'), which violates the project guideline. Consider usingpage.getByRole()or exposing this wait through the page object.Suggested change
- await page.locator('#main-content').waitFor(); + await page.getByRole('main').waitFor();As per coding guidelines: "Avoid using
page.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()."
53-55: Avoid hard assertions inafterEach— a failure here masks the real test failure.If the API call fails or returns a non-200 status (e.g., the room was already deleted by the test), the
expectwill throw and Playwright will report the cleanup error instead of the actual test result. Use a non-throwing check or just fire-and-forget.Suggested change
test.afterEach(async ({ api }) => { - expect((await api.post('/groups.delete', { roomId: encryptedRoomId })).status()).toBe(200); + await api.post('/groups.delete', { roomId: encryptedRoomId }); });
64-64: Extract the repeated encrypted-icon locator into a reusable constant.The selector
.locator('.rcx-icon--name-key')is repeated six times across the file. Storing it as a page-object accessor or a local helper would improve maintainability and align with the project guideline.For example, at the top of the
describeblock:const encryptedIcon = (msg: Locator) => msg.locator('.rcx-icon--name-key');Then use
encryptedIcon(poHomeChannel.content.lastUserMessage)in each assertion.As per coding guidelines: "Store commonly used locators in variables/constants for reuse."
Also applies to: 98-98, 113-113, 128-128, 150-150, 159-159
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
apps/meteor/tests/e2e/page-objects/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Utilize existing page objects pattern from
apps/meteor/tests/e2e/page-objects/
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
apps/meteor/tests/e2e/**/*.{ts,spec.ts}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,spec.ts}: Store commonly used locators in variables/constants for reuse
Follow Page Object Model pattern consistently in Playwright tests
Files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/files-management.spec.ts
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All test files must be created inapps/meteor/tests/e2e/directory
Avoid usingpage.locator()in Playwright tests - always prefer semantic locators such aspage.getByRole(),page.getByLabel(),page.getByText(), orpage.getByTitle()
Usetest.beforeAll()andtest.afterAll()for setup/teardown in Playwright tests
Usetest.step()for complex test scenarios to improve organization in Playwright tests
Group related tests in the same file
Utilize Playwright fixtures (test,page,expect) for consistency in test files
Prefer web-first assertions (toBeVisible,toHaveText, etc.) in Playwright tests
Useexpectmatchers for assertions (toEqual,toContain,toBeTruthy,toHaveLength, etc.) instead ofassertstatements in Playwright tests
Usepage.waitFor()with specific conditions instead of hardcoded timeouts in Playwright tests
Implement proper wait strategies for dynamic content in Playwright tests
Maintain test isolation between test cases in Playwright tests
Ensure clean state for each test execution in Playwright tests
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/files-management.spec.ts
🧠 Learnings (19)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/files-management.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/files-management.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.ts
📚 Learning: 2025-12-16T17:29:40.430Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:40.430Z
Learning: In all page-object files under apps/meteor/tests/e2e/page-objects/, import expect from ../../utils/test (Playwright's async expect) instead of from Jest. Jest's expect is synchronous and incompatible with web-first assertions like toBeVisible, which can cause TypeScript errors.
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Avoid using `page.locator()` in Playwright tests - always prefer semantic locators such as `page.getByRole()`, `page.getByLabel()`, `page.getByText()`, or `page.getByTitle()`
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/toolbar.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Implement proper wait strategies for dynamic content in Playwright tests
Applied to files:
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/page-objects/fragments/files-flextab.tsapps/meteor/tests/e2e/files-management.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts
📚 Learning: 2025-12-16T17:29:45.163Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37834
File: apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-emoji.ts:12-22
Timestamp: 2025-12-16T17:29:45.163Z
Learning: In page object files under `apps/meteor/tests/e2e/page-objects/`, always import `expect` from `../../utils/test` (Playwright's async expect), not from Jest. Jest's `expect` has a synchronous signature and will cause TypeScript errors when used with web-first assertions like `toBeVisible()`.
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.tsapps/meteor/tests/e2e/files-management.spec.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/e2e/files-management.spec.ts
🧬 Code graph analysis (3)
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/files-flextab.ts (1)
FilesFlexTab(7-31)
apps/meteor/tests/e2e/files-management.spec.ts (3)
packages/models/src/index.ts (1)
Users(201-201)apps/meteor/tests/e2e/page-objects/home-channel.ts (1)
HomeChannel(7-141)packages/core-services/src/index.ts (1)
api(55-55)
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (1)
apps/meteor/tests/e2e/page-objects/fragments/menu.ts (1)
MenuOptions(37-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
apps/meteor/tests/e2e/e2e-encryption/e2ee-file-encryption.spec.ts (2)
36-51: Good use of API-backed setup for test isolation. 👍Creating the encrypted room via API in
beforeEachand tearing it down inafterEachkeeps each test fully isolated and avoids brittle UI-driven setup flows. This is well-aligned with the test isolation guidelines.
81-88: New file-deletion step validates the core PR objective — looks good.This step directly tests that deleting a file from the Files list also removes its associated message, which is the central behavior this PR fixes for E2EE rooms.
apps/meteor/tests/e2e/page-objects/fragments/toolbar.ts (1)
14-19: LGTM!Clean integration of
MenuOptionsintoRoomToolbar. The distinction betweenbtnFiles(direct toolbar button) andmenuItemFiles(overflow menu item) is clear and correctly addresses different UI states. Follows the existing page object pattern well.Also applies to: 90-92
apps/meteor/tests/e2e/page-objects/fragments/home-flextab.ts (1)
4-4: LGTM!The
FilesFlexTabintegration follows the same pattern as other flex tab members (members,room,channels, etc.).Also applies to: 29-30, 40-40
apps/meteor/tests/e2e/files-management.spec.ts (1)
29-55: Good use oftest.step()for the multi-phase workflow.The serial describe block with nested steps keeps the upload → verify → download → delete flow organized and readable. Assertions use web-first matchers appropriately.
apps/meteor/tests/e2e/page-objects/fragments/files-flextab.ts (1)
7-31: LGTM!Well-structured page object that follows the existing POM pattern. Uses semantic locators throughout and cleanly encapsulates the file list interactions and delete workflow by composing
MenuMoreandConfirmDeleteModal.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/meteor/tests/e2e/files-management.spec.ts">
<violation number="1" location="apps/meteor/tests/e2e/files-management.spec.ts:49">
P3: Redundant click before `deleteFile()` may cause test flakiness. The `deleteFile()` method already locates the file item and clicks its "More" button internally. This extra click triggers a file download (as shown in the previous test step), which could interfere with the subsequent delete operation by showing a download prompt or causing navigation issues.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
dougfabris
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
dionisio-bot
bot
removed
the
stat: ready to merge
dougfabris
added
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
5 participants
Proposed changes (including videos or screenshots)
On e2ee messages, the link between the message and the file is also being encrypted. This makes the server unable to delete files based on messages and also unable to delete messages based on files.
This PR changes the backend to save the metadata of the encrypted file on the message (just like it does for non-encrypted messages), so that any code that checks for links between files and messages can still find it.
It also changes the message to keep a list of files that have been deleted on the server so that the client may ignore any references to those files on decrypted data.
The only thing being saved to the message is related to the encrypted version of the file, so attributes like name, format and size will not reflect the real file - as the server doesn't have any info about it.
Issue(s)
CORE-1776
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Improvements
Tests
Chores