test: Enhance end-to-end tests for DMs #37813
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…irect message scenarios
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. WalkthroughAdded TODO comments related to federation in add-users/invite code, changed error handling in the leave command catch path to rethrow Meteor.Error, and reorganized/added end-to-end tests for DM vs Channel contexts including a new "kick" DM test. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (15)📓 Common learnings📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-12-10T21:00:54.909ZApplied to files:
📚 Learning: 2025-10-28T16:53:42.761ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
📚 Learning: 2025-11-24T17:08:17.065ZApplied to files:
🧬 Code graph analysis (1)apps/meteor/tests/end-to-end/api/methods.ts (3)
⏰ 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). (3)
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 |
…ndling in user invitation
sampaiodiego
previously approved these changes
Dec 15, 2025
sampaiodiego
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/meteor/tests/end-to-end/api/commands.ts (1)
524-595: DM kick/leave command coverage looks good;user2fixture is redundantThe new DM tests for
kickandleavecorrectly exercise the/commands.runbehavior in direct-message rooms (400 +success: false+ stringerror) and use isolated setup/teardown.In the
"leave"suite,user2is created and later deleted but never participates in the scenario; you can safely drop that user to reduce test setup cost if desired.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/meteor/app/lib/server/methods/addUsersToRoom.ts(1 hunks)apps/meteor/app/slashcommands-invite/server/server.ts(1 hunks)apps/meteor/app/slashcommands-leave/server/leave.ts(1 hunks)apps/meteor/tests/end-to-end/api/commands.ts(1 hunks)apps/meteor/tests/end-to-end/api/methods.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/app/slashcommands-invite/server/server.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsapps/meteor/tests/end-to-end/api/methods.tsapps/meteor/tests/end-to-end/api/commands.tsapps/meteor/app/slashcommands-leave/server/leave.ts
🧠 Learnings (18)
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
apps/meteor/app/slashcommands-invite/server/server.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-12-02T22:23:49.593Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37654
File: apps/meteor/client/hooks/useAppSlashCommands.ts:32-38
Timestamp: 2025-12-02T22:23:49.593Z
Learning: In apps/meteor/client/hooks/useAppSlashCommands.ts, the `data?.forEach((command) => slashCommands.add(command))` call during render is intentional. The query is configured with `structuralSharing: false` to prevent React Query from keeping stable data references, and `slashCommands.add` is idempotent, so executing on every render is acceptable and ensures the command registry stays current.
Applied to files:
apps/meteor/app/slashcommands-invite/server/server.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/app/slashcommands-invite/server/server.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsapps/meteor/tests/end-to-end/api/methods.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: UserBridge.doGetUserRoomIds in packages/apps-engine/src/server/bridges/UserBridge.ts has a bug where it implicitly returns undefined when the app lacks read permission (missing return statement in the else case of the permission check).
Applied to files:
apps/meteor/app/slashcommands-invite/server/server.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/app/slashcommands-invite/server/server.tsapps/meteor/app/lib/server/methods/addUsersToRoom.tsapps/meteor/app/slashcommands-leave/server/leave.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/slashcommands-invite/server/server.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Applied to files:
apps/meteor/app/slashcommands-invite/server/server.tsapps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
apps/meteor/app/lib/server/methods/addUsersToRoom.ts
📚 Learning: 2025-12-09T20:01:07.355Z
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
Applied to files:
apps/meteor/app/lib/server/methods/addUsersToRoom.tsapps/meteor/app/slashcommands-leave/server/leave.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/end-to-end/api/methods.tsapps/meteor/tests/end-to-end/api/commands.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/end-to-end/api/methods.tsapps/meteor/tests/end-to-end/api/commands.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/end-to-end/api/methods.tsapps/meteor/tests/end-to-end/api/commands.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/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/end-to-end/api/methods.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/end-to-end/api/methods.tsapps/meteor/tests/end-to-end/api/commands.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.ts
🧬 Code graph analysis (2)
apps/meteor/tests/end-to-end/api/methods.ts (3)
apps/meteor/tests/data/users.helper.ts (2)
TestUser(10-10)createUser(45-78)packages/core-typings/src/IUser.ts (1)
IUser(186-256)apps/meteor/tests/data/api-data.ts (3)
request(10-10)methodCall(50-52)credentials(39-42)
apps/meteor/app/slashcommands-leave/server/leave.ts (1)
apps/meteor/app/livechat/server/methods/changeLivechatStatus.ts (1)
Meteor(18-72)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/app/slashcommands-invite/server/server.ts (1)
96-104: Non-functional TODO comment onlyThis hunk just adds documentation around existing behavior; no runtime or API change. Nothing to block here.
apps/meteor/app/lib/server/methods/addUsersToRoom.ts (1)
55-60: Clarifying comment aligns with current behaviorOnly the explanatory comment was updated; the direct-room guard and thrown error remain unchanged and consistent with the tests.
Comment on lines
+21
to
30
| } catch (error: any) { | ||
| if (typeof error.error !== 'string') { | ||
| throw error; | ||
| } | ||
| const user = await Users.findOneById(userId); | ||
| void api.broadcast('notify.ephemeralMessage', userId, message.rid, { | ||
| msg: i18n.t(error, { lng: user?.language || settings.get('Language') || 'en' }), | ||
| }); | ||
| throw new Meteor.Error(error.error, error.message); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the leave.ts file and examine its full context
find . -path "*/slashcommands-leave/server/leave.ts" -type fRepository: RocketChat/Rocket.Chat
Length of output: 119
🏁 Script executed:
# Examine the full content of the leave.ts file with line numbers
cat -n "apps/meteor/app/slashcommands-leave/server/leave.ts"Repository: RocketChat/Rocket.Chat
Length of output: 1838
🏁 Script executed:
# Check what i18n.t expects - examine i18n module signature
fd -t f "i18n\\.ts$" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 207
🏁 Script executed:
# Search for how i18n.t is used elsewhere in the codebase (passing object vs string)
rg "i18n\.t\(" --max-count 10 -A 2 -B 1Repository: RocketChat/Rocket.Chat
Length of output: 50378
🏁 Script executed:
# Check if Meteor is imported or used as global in similar files
rg "import.*Meteor" apps/meteor/app/slashcommands-leave/server/ || echo "No Meteor import found"Repository: RocketChat/Rocket.Chat
Length of output: 88
🏁 Script executed:
# Check how error handling is typically done in this codebase
rg "catch.*error.*:" apps/meteor/app/slashcommands-leave/ -A 5 | head -30Repository: RocketChat/Rocket.Chat
Length of output: 581
🏁 Script executed:
# Check the error structure more closely - see how leaveRoomMethod throws errors
rg "leaveRoomMethod" --max-count 5 -B 2 -A 10 | head -50Repository: RocketChat/Rocket.Chat
Length of output: 3977
🏁 Script executed:
# Check how other catch blocks handle similar error patterns
rg "catch.*error.*any" apps/meteor/app/slashcommands-invite/server/server.ts -A 15 | head -40Repository: RocketChat/Rocket.Chat
Length of output: 833
Change i18n.t(error, …) to i18n.t(error.error, …) on line 27
The code validates that error.error is a string (line 22), but then passes the entire error object to i18n.t() instead of the error code. This will fail to translate because i18n.t() expects a string translation key, not an object. Use error.error to pass the actual error code string, matching the string key validation on line 22 and consistent with the error code used in Meteor.Error on line 29.
Optionally, import Meteor from 'meteor/meteor' for explicitness; it currently works as a global but being explicit improves type safety.
🤖 Prompt for AI Agents
In apps/meteor/app/slashcommands-leave/server/leave.ts around lines 21 to 30,
the catch block validates that error.error is a string but incorrectly calls
i18n.t(error, ...) with the whole error object; change the call to
i18n.t(error.error, { lng: ... }) so the translation key is the string code, and
keep the rest of the broadcast and Meteor.Error construction unchanged;
optionally add an explicit import for Meteor from 'meteor/meteor' at the top for
clearer typing.
Comment on lines
3105
to
3253
| describe('Direct Message', () => { | ||
| let thirdUser: TestUser<IUser>; | ||
| before(async () => { | ||
| guestUser = await createUser({ roles: ['user'] }); | ||
| thirdUser = await createUser({ roles: ['user'] }); | ||
| user = await createUser(); | ||
| room = ( | ||
| await createRoom({ | ||
| type: 'd', | ||
| name: `direct.test.${Date.now()}-${Math.random()}`, | ||
| username: guestUser.username, | ||
| }) | ||
| ).body.channel; | ||
| createdRooms.push(room); | ||
| }); | ||
| after(() => | ||
| Promise.all([ | ||
| ...createdRooms.map((r) => deleteRoom({ type: 'd', roomId: r._id })), | ||
| deleteUser(user), | ||
| deleteUser(guestUser), | ||
| deleteUser(thirdUser), | ||
| ]), | ||
| ); | ||
|
|
||
| it('should add a single user to a room', (done) => { | ||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentials) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid: room._id, users: [user.username] }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| }) | ||
| .then(() => { | ||
| void request | ||
| .get(api('channels.members')) | ||
| .set(credentials) | ||
| .query({ | ||
| roomId: room._id, | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| expect(res.body).to.have.property('members').and.to.be.an('array'); | ||
| expect(res.body.members).to.have.lengthOf(2); | ||
| }) | ||
| .end(done); | ||
| }) | ||
| .catch(done); | ||
| it('should fail when trying to add a user to a direct message room', (done) => { | ||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentials) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid: room._id, users: [thirdUser.username] }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', false); | ||
| expect(res.body).to.have.property('error').that.is.a('string'); | ||
| expect(res.body.error).to.equal('error-cant-invite-for-direct-room'); | ||
| }) | ||
| .end(done); | ||
| }); | ||
| }); | ||
|
|
||
| it('should not add guest users to more rooms than defined in the license', async function () { | ||
| // TODO this is not the right way to do it. We're doing this way for now just because we have separate CI jobs for EE and CE, | ||
| // ideally we should have a single CI job that adds a license and runs both CE and EE tests. | ||
| if (!process.env.IS_EE) { | ||
| this.skip(); | ||
| } | ||
| const promises = []; | ||
| for (let i = 0; i < maxRoomsPerGuest; i++) { | ||
| promises.push( | ||
| createRoom({ | ||
| describe('Channel', () => { | ||
| before(async () => { | ||
| guestUser = await createUser({ roles: ['guest'] }); | ||
| user = await createUser(); | ||
| room = ( | ||
| await createRoom({ | ||
| type: 'c', | ||
| name: `channel.test.${Date.now()}-${Math.random()}`, | ||
| members: [guestUser.username], | ||
| }), | ||
| ); | ||
| } | ||
| createdRooms = [...createdRooms, ...(await Promise.all(promises)).map((res) => res.body.channel)]; | ||
| }) | ||
| ).body.channel; | ||
| createdRooms.push(room); | ||
| }); | ||
| after(() => | ||
| Promise.all([...createdRooms.map((r) => deleteRoom({ type: 'c', roomId: r._id })), deleteUser(user), deleteUser(guestUser)]), | ||
| ); | ||
|
|
||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentials) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid: room._id, users: [guestUser.username] }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| const parsedBody = JSON.parse(res.body.message); | ||
| expect(parsedBody).to.have.property('error'); | ||
| expect(parsedBody.error).to.have.property('error', 'error-max-rooms-per-guest-reached'); | ||
| }); | ||
| it('should fail if not logged in', (done) => { | ||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(401) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('status', 'error'); | ||
| expect(res.body).to.have.property('message'); | ||
| }) | ||
| .end(done); | ||
| }); | ||
|
|
||
| it('should add a single user to a room', (done) => { | ||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentials) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid: room._id, users: [user.username] }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| }) | ||
| .then(() => { | ||
| void request | ||
| .get(api('channels.members')) | ||
| .set(credentials) | ||
| .query({ | ||
| roomId: room._id, | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| expect(res.body).to.have.property('members').and.to.be.an('array'); | ||
| expect(res.body.members).to.have.lengthOf(2); | ||
| }) | ||
| .end(done); | ||
| }) | ||
| .catch(done); | ||
| }); | ||
|
|
||
| it('should not add guest users to more rooms than defined in the license', async function () { | ||
| // TODO this is not the right way to do it. We're doing this way for now just because we have separate CI jobs for EE and CE, | ||
| // ideally we should have a single CI job that adds a license and runs both CE and EE tests. | ||
| if (!process.env.IS_EE) { | ||
| this.skip(); | ||
| } | ||
| const promises = []; | ||
| for (let i = 0; i < maxRoomsPerGuest; i++) { | ||
| promises.push( | ||
| createRoom({ | ||
| type: 'c', | ||
| name: `channel.test.${Date.now()}-${Math.random()}`, | ||
| members: [guestUser.username], | ||
| }), | ||
| ); | ||
| } | ||
| createdRooms = [...createdRooms, ...(await Promise.all(promises)).map((res) => res.body.channel)]; | ||
|
|
||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentials) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid: room._id, users: [guestUser.username] }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| const parsedBody = JSON.parse(res.body.message); | ||
| expect(parsedBody).to.have.property('error'); | ||
| expect(parsedBody.error).to.have.property('error', 'error-max-rooms-per-guest-reached'); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "methods.ts" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 414
🏁 Script executed:
git ls-files | grep -E "tests/(end-to-end|e2e).*api.*methods" | head -20Repository: RocketChat/Rocket.Chat
Length of output: 238
🏁 Script executed:
wc -l ./apps/meteor/tests/end-to-end/api/methods.tsRepository: RocketChat/Rocket.Chat
Length of output: 116
🏁 Script executed:
sed -n '3105,3253p' ./apps/meteor/tests/end-to-end/api/methods.tsRepository: RocketChat/Rocket.Chat
Length of output: 4606
🏁 Script executed:
sed -n '3090,3120p' ./apps/meteor/tests/end-to-end/api/methods.tsRepository: RocketChat/Rocket.Chat
Length of output: 887
🏁 Script executed:
# Check for other instances of `void request` without await/return in async tests
grep -n "void request" ./apps/meteor/tests/end-to-end/api/methods.ts | head -20Repository: RocketChat/Rocket.Chat
Length of output: 461
🏁 Script executed:
# Also check the specific test patterns - look for async tests with void request chains
rg -A 5 "it\('.*', async.*\)" ./apps/meteor/tests/end-to-end/api/methods.ts | head -40Repository: RocketChat/Rocket.Chat
Length of output: 1277
🏁 Script executed:
# Look at the specific license test more carefully to see if it has any awaits or returns
sed -n '3219,3245p' ./apps/meteor/tests/end-to-end/api/methods.tsRepository: RocketChat/Rocket.Chat
Length of output: 806
🏁 Script executed:
# Search for async tests with void request (not using .end(done)) - potential issue pattern
rg -B 2 "it\('.*', async" ./apps/meteor/tests/end-to-end/api/methods.ts | rg -A 10 "it\('.*', async" | grep -A 8 "void request" | head -40Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Get full license test to see what pattern it follows
sed -n '3216,3250p' ./apps/meteor/tests/end-to-end/api/methods.tsRepository: RocketChat/Rocket.Chat
Length of output: 1338
🏁 Script executed:
# Check if after() calls actually remove from array or just delete from API
sed -n '3118,3127p' ./apps/meteor/tests/end-to-end/api/methods.tsRepository: RocketChat/Rocket.Chat
Length of output: 302
🏁 Script executed:
# Verify the parent suite's after() to understand cleanup hierarchy
sed -n '3100,3110p' ./apps/meteor/tests/end-to-end/api/methods.tsRepository: RocketChat/Rocket.Chat
Length of output: 419
🏁 Script executed:
# Verify deleteRoom function signature to confirm it validates room type
rg -A 3 "deleteRoom.*=" ./apps/meteor/tests/end-to-end/api/methods.ts | head -20Repository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Search for deleteRoom import or definition
head -100 ./apps/meteor/tests/end-to-end/api/methods.ts | grep -E "import|deleteRoom"Repository: RocketChat/Rocket.Chat
Length of output: 1181
🏁 Script executed:
# Also check if deleteRoom function validates the type parameter
rg "deleteRoom" ./apps/meteor/tests/end-to-end/api/methods.ts -B 2 -A 2 | head -30Repository: RocketChat/Rocket.Chat
Length of output: 986
🏁 Script executed:
# Check the deleteRoom helper implementation to understand its behavior
find . -type f -name "rooms.helper.ts" | head -5Repository: RocketChat/Rocket.Chat
Length of output: 106
🏁 Script executed:
# Verify the deleteRoom function and its type handling
grep -A 20 "export.*deleteRoom" ./apps/meteor/tests/data/rooms.helper.ts | head -30Repository: RocketChat/Rocket.Chat
Length of output: 817
🏁 Script executed:
# Check the actionRoom function to understand type handling
grep -B 5 -A 25 "const actionRoom" ./apps/meteor/tests/data/rooms.helper.tsRepository: RocketChat/Rocket.Chat
Length of output: 48
🏁 Script executed:
# Search for actionRoom definition
rg "actionRoom.*=" ./apps/meteor/tests/data/rooms.helper.ts -A 20 | head -50Repository: RocketChat/Rocket.Chat
Length of output: 708
🏁 Script executed:
# Also check what ActionRoomParams includes
rg "ActionRoomParams" ./apps/meteor/tests/data/rooms.helper.ts -B 5 -A 5Repository: RocketChat/Rocket.Chat
Length of output: 1059
Fix shared createdRooms teardown and await the license-limit request
Two major issues in the [@addUsersToRoom] block:
- Shared
createdRoomsbetween DM and Channel suites causes type mismatch in teardown
createdRooms is declared once at the describe level. The DM suite pushes a room with type: 'd' and its after() deletes it as such. The Channel suite then pushes a room with type: 'c' to the same array. When Channel's after() runs, it attempts to delete all entries as type: 'c', which means it tries to call channels.delete on the DM room. This API call fails because the DM room uses the im endpoint.
Use a DM-local array instead:
describe('Direct Message', () => {
let thirdUser: TestUser<IUser>;
+ const dmRooms: IRoom[] = [];
before(async () => {
...
createdRooms.push(room);
+ dmRooms.push(room);
});
after(() =>
Promise.all([
- ...createdRooms.map((r) => deleteRoom({ type: 'd', roomId: r._id })),
+ ...dmRooms.map((r) => deleteRoom({ type: 'd', roomId: r._id })),
deleteUser(user),
deleteUser(guestUser),
deleteUser(thirdUser),
]),
);Leave createdRooms for the Channel suite only.
- License-limit test does not await the final HTTP request
The "should not add guest users to more rooms than defined in the license" test is declared async function but starts a supertest chain with void, preventing Mocha from waiting for the HTTP expectations:
-void request
+await request
.post(methodCall('addUsersToRoom'))
...These fixes ensure proper cleanup and prevent test flakiness.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/meteor/tests/end-to-end/api/methods.ts around lines 3105 to 3253, the DM
and Channel suites share the same createdRooms array causing teardown to call
the wrong delete endpoint for DM rooms, and the license-limit test starts a
supertest chain with void so Mocha doesn't await it; fix by creating a separate
local array (e.g., dmCreatedRooms) inside the Direct Message describe, push only
DM rooms into that and update its after() to delete with type 'd' while leaving
the outer createdRooms for Channel tests and their 'c' deletes, and change the
license-limit test to return or await the supertest promise (remove void and
await or return the chain) so the test runner waits for the HTTP expectations to
complete.
Contributor
📦 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/15 22:28", "11/16 01:28", "11/17 23:50", "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 13:56", "12/15 14:53 (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]
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]
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]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 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]
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]
line "queue-worker-service" [0.14, 0.14, 0.14, 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]
line "rocketchat" [0.36, 0.36, 0.35, 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.35]
line "stream-hub-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]
Statistics (last 21 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Mon, 15 Dec 2025 14:53:11 GMT |
ggazzo
force-pushed
the
chore/extra-dms
branch
from
5f95bb4 to
ca2ee4d
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/meteor/tests/end-to-end/api/methods.ts (2)
3099-3104: Isolate DM vs Channel room cleanup instead of sharingcreatedRooms
createdRoomsis shared between the Direct Message and Channel suites but eachafter()assumes a different room type ('d'vs'c'). Once.onlyis removed, this will cause one suite to try to delete rooms of the wrong type (e.g., calling the channel delete endpoint for a DM room) and can make teardown fail or flake. This was already flagged in a previous review and is still present. Based on learnings, test state should not be shared between suites.Use a DM-local array and keep
createdRoomsfor channels only; also avoid reassigning the array in the license test:describe('[@addUsersToRoom]', () => { let guestUser: TestUser<IUser>; let user: TestUser<IUser>; let room: IRoom; - let createdRooms: IRoom[] = []; + const createdRooms: IRoom[] = []; + const dmRooms: IRoom[] = []; @@ before(async () => { @@ room = ( await createRoom({ type: 'd', username: guestUser.username, }) ).body.room; - createdRooms.push(room); + dmRooms.push(room); }); after(() => Promise.all([ - ...createdRooms.map((r) => deleteRoom({ type: 'd', roomId: r._id })), + ...dmRooms.map((r) => deleteRoom({ type: 'd', roomId: r._id })), deleteUser(user), deleteUser(guestUser), deleteUser(thirdUser), ]), ); @@ - createdRooms = [...createdRooms, ...(await Promise.all(promises)).map((res) => res.body.channel)]; + createdRooms.push(...(await Promise.all(promises)).map((res) => res.body.channel));Also applies to: 3117-3121, 3160-3164, 3230-3230
3214-3251: Await the license-limit request so Mocha waits for assertionsIn
"should not add guest users to more rooms than defined in the license", the test isasyncbut the supertest chain is started withvoid requestand never awaited or returned. Mocha will resolve the test as soon as the function body finishes, so HTTP assertions can be skipped or run after the test has already passed, potentially as unhandled rejections. This was already raised previously and remains unfixed.Await the supertest promise (or return it); with an
asynctest,awaitis simplest:- void request + await request .post(methodCall('addUsersToRoom')) .set(credentials) .send({ message: JSON.stringify({ method: 'addUsersToRoom', params: [{ rid: room._id, users: [guestUser.username] }], id: 'id', msg: 'method', }), }) .expect('Content-Type', 'application/json') .expect(200) .expect((res) => { expect(res.body).to.have.property('success', true); const parsedBody = JSON.parse(res.body.message); expect(parsedBody).to.have.property('error'); expect(parsedBody.error).to.have.property('error', 'error-max-rooms-per-guest-reached'); });#!/bin/bash # Show the updated license-limit test to confirm the request is awaited rg -n "should not add guest users to more rooms than defined in the license" apps/meteor/tests/end-to-end/api/methods.ts -n -C8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/tests/end-to-end/api/methods.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/end-to-end/api/methods.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:07.355Z
Learning: In Rocket.Chat's federation invite handling (ee/packages/federation-matrix/src/FederationMatrix.ts), when a user rejects an invite via federationSDK.rejectInvite(), the subscription cleanup happens automatically through an event-driven flow: Matrix emits a leave event back, which is processed by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, and that function calls Room.performUserRemoval() to clean up the subscription. No explicit cleanup is needed in the reject branch of handleInvite() because the leave event handler takes care of it.
<!-- </add_learning>
📚 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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.ts
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/tests/end-to-end/api/methods.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/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.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/end-to-end/api/methods.ts
🧬 Code graph analysis (1)
apps/meteor/tests/end-to-end/api/methods.ts (3)
apps/meteor/tests/data/users.helper.ts (2)
TestUser(10-10)createUser(45-78)packages/core-typings/src/IUser.ts (1)
IUser(186-256)apps/meteor/tests/data/api-data.ts (3)
request(10-10)methodCall(50-52)credentials(39-42)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
Comment on lines
3105
to
3147
| describe.only('Direct Message', () => { | ||
| let thirdUser: TestUser<IUser>; | ||
| before(async () => { | ||
| guestUser = await createUser({ roles: ['user'] }); | ||
| thirdUser = await createUser({ roles: ['user'] }); | ||
| user = await createUser(); | ||
| room = ( | ||
| await createRoom({ | ||
| type: 'd', | ||
| username: guestUser.username, | ||
| }) | ||
| ).body.room; | ||
| createdRooms.push(room); | ||
| }); | ||
| after(() => | ||
| Promise.all([ | ||
| ...createdRooms.map((r) => deleteRoom({ type: 'd', roomId: r._id })), | ||
| deleteUser(user), | ||
| deleteUser(guestUser), | ||
| deleteUser(thirdUser), | ||
| ]), | ||
| ); | ||
|
|
||
| it('should add a single user to a room', (done) => { | ||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentials) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid: room._id, users: [user.username] }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| }) | ||
| .then(() => { | ||
| void request | ||
| .get(api('channels.members')) | ||
| .set(credentials) | ||
| .query({ | ||
| roomId: room._id, | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('success', true); | ||
| expect(res.body).to.have.property('members').and.to.be.an('array'); | ||
| expect(res.body.members).to.have.lengthOf(2); | ||
| }) | ||
| .end(done); | ||
| }) | ||
| .catch(done); | ||
| it('should fail when trying to add a user to a direct message room', (done) => { | ||
| void request | ||
| .post(methodCall('addUsersToRoom')) | ||
| .set(credentials) | ||
| .send({ | ||
| message: JSON.stringify({ | ||
| method: 'addUsersToRoom', | ||
| params: [{ rid: room._id, users: [thirdUser.username] }], | ||
| id: 'id', | ||
| msg: 'method', | ||
| }), | ||
| }) | ||
| .expect('Content-Type', 'application/json') | ||
| .expect(200) | ||
| .expect((res) => { | ||
| expect(res.body).to.have.property('message').that.is.an('string'); | ||
| expect(res.body.message).to.include('error-cant-invite-for-direct-room'); | ||
| }) | ||
| .end(done); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and check for describe.only
rg -n "describe\.only" apps/meteor/tests/end-to-end/api/methods.ts || echo "No describe.only found"Repository: RocketChat/Rocket.Chat
Length of output: 112
🏁 Script executed:
# Also check if the file exists at the expected path
fd "methods.ts" apps/meteor/tests/end-to-end/ || echo "File not found at that path"Repository: RocketChat/Rocket.Chat
Length of output: 109
🏁 Script executed:
# Check the actual content around lines 3105-3150 to verify the snippet
if [ -f "apps/meteor/tests/end-to-end/api/methods.ts" ]; then
sed -n '3105,3150p' apps/meteor/tests/end-to-end/api/methods.ts | head -50
else
echo "File not found"
fiRepository: RocketChat/Rocket.Chat
Length of output: 1338
Remove describe.only to prevent skipping other tests in the suite
The describe.only at line 3105 will cause Mocha to run only the "Direct Message" block and skip all other tests in this file, which is incorrect for production and CI environments.
Change it to a normal describe:
- describe.only('Direct Message', () => {
+ describe('Direct Message', () => {🤖 Prompt for AI Agents
In apps/meteor/tests/end-to-end/api/methods.ts around lines 3105 to 3147, the
test block is using describe.only which causes Mocha to run only this suite and
skip others; change describe.only('Direct Message', ...) to describe('Direct
Message', ...) (i.e., remove the .only) so all tests run normally in CI and
local runs.
…nstead of failure
ggazzo
force-pushed
the
chore/extra-dms
branch
from
ca2ee4d to
ff1c945
Compare
gaolin1
pushed a commit
to gaolin1/medsense.webchat
that referenced
this pull request
Jan 6, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://rocketchat.atlassian.net/browse/FB-146
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.