fix: add team leave flow to room menu action#38448
fix: add team leave flow to room menu action#38448sezallagwal wants to merge 4 commits intoRocketChat:developfrom
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
|
|
WalkthroughAdds team-aware leave behavior: Changes
Sequence DiagramsequenceDiagram
participant User
participant LeaveAction as Leave Action Handler
participant LeaveTeamModal as LeaveTeam Modal
participant TeamsAPI as Teams API
participant UI as UI/Navigation
User->>LeaveAction: Trigger leave (includes teamId + teamMain)
alt Team leave flow
LeaveAction->>LeaveTeamModal: Open modal with teamId
User->>LeaveTeamModal: Confirm leave
LeaveTeamModal->>TeamsAPI: POST teams.leave (teamId, rooms?)
TeamsAPI-->>LeaveTeamModal: Success
LeaveTeamModal->>UI: Show success toast
LeaveTeamModal->>UI: Navigate home (if active room)
LeaveTeamModal->>UI: Close legacy room
LeaveTeamModal->>LeaveTeamModal: Close/cleanup modal
else Per-room flow
LeaveAction->>UI: Execute existing per-room leave flow (modal, endpoint, toast, cleanup)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (3)📚 Learning: 2025-10-28T16:53:42.761ZApplied to files:
📚 Learning: 2025-09-25T09:59:26.461ZApplied to files:
📚 Learning: 2025-09-25T09:59:26.461ZApplied to files:
🧬 Code graph analysis (1)apps/meteor/client/hooks/useRoomMenuActions.ts (2)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
sezallagwal
changed the title
dougfabris
added
the
contrib: valid
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.ts (1)
3-3: 🛠️ Refactor suggestion | 🟠 MajorMerge
useUserRoominto the existing@rocket.chat/ui-contextsimport on line 3.
useUserRoomis imported separately on line 14, but line 3 already imports from the same package. Consolidate to avoid a duplicate import source.-import { usePermission, useRouter, useSetting, useUserSubscription } from '@rocket.chat/ui-contexts'; +import { usePermission, useRouter, useSetting, useUserRoom, useUserSubscription } from '@rocket.chat/ui-contexts'; ... -import { useUserRoom } from '@rocket.chat/ui-contexts';Also applies to: 14-14
🧹 Nitpick comments (2)
apps/meteor/client/hooks/useRoomMenuActions.spec.ts (2)
45-53: Inconsistent indentation: mixed spaces and tabs.Lines 48 and 65 use spaces for indentation on
.withRoom(...), while all other chained calls use tabs. This should be consistent.renderHook(() => useRoomMenuActions(mockHookProps), { wrapper: mockAppRoot() .withSubscription({ ...mockSubscription, rid: 'room1' }) - .withRoom(roomWithTeam) + .withRoom(roomWithTeam) .withPermission('leave-c')
41-76: Consider resetting mocks between tests to prevent state leakage.
useLeaveRoomActionis ajest.fn()whose call history persists across tests. The second test's assertions don't depend on it, but adding abeforeEach(() => jest.clearAllMocks())is a good practice to keep tests isolated.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/meteor/client/hooks/useRoomMenuActions.spec.tsapps/meteor/client/hooks/useRoomMenuActions.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.spec.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/hooks/useRoomMenuActions.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/client/hooks/useRoomMenuActions.spec.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.spec.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/hooks/useRoomMenuActions.spec.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.spec.ts
🧠 Learnings (16)
📚 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/client/hooks/useRoomMenuActions.spec.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/hooks/useRoomMenuActions.spec.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/hooks/useRoomMenuActions.spec.tsapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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 tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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 : Group related tests in the same file
Applied to files:
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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} : Store commonly used locators in variables/constants for reuse
Applied to files:
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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 : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.spec.ts
📚 Learning: 2026-01-19T18:08:13.423Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38169
File: packages/ui-client/src/hooks/useGoToDirectMessage.ts:20-27
Timestamp: 2026-01-19T18:08:13.423Z
Learning: In React hooks, including custom hooks like useUserSubscriptionByName, must always be called unconditionally and in the same order on every render, per the Rules of Hooks. Passing empty strings or fallback values to hooks is the correct approach when dealing with potentially undefined values, rather than conditionally calling the hook based on whether the value exists.
Applied to files:
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.spec.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/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/views/navigation/sidepanel/hooks/useRoomMenuActions.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/client/views/navigation/sidepanel/hooks/useRoomMenuActions.ts
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.
Applied to files:
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.ts
🧬 Code graph analysis (2)
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.spec.ts (1)
apps/meteor/tests/mocks/data.ts (2)
createFakeSubscription(97-124)createFakeRoom(47-62)
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.ts (2)
packages/ui-contexts/src/index.ts (1)
useUserRoom(87-87)apps/meteor/client/hooks/menuActions/useLeaveRoom.tsx (1)
useLeaveRoomAction(30-95)
⏰ 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). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (3)
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.ts (1)
40-41: LGTM — team context plumbing looks correct.
useUserRoom(rid)is called unconditionally, and optional chaining onroom?.teamId/room?.teamMainsafely handles the case where the room is not yet loaded. The values flow correctly intouseLeaveRoomAction.Also applies to: 61-61
apps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.spec.ts (1)
5-8: LGTM — room context correctly supplied to all test cases.The addition of
createFakeRoomand.withRoom(mockRoom)ensuresuseUserRoom(rid)returns valid data in all test wrappers, aligning with the newroomdependency in the hook.Also applies to: 45-45, 63-63, 81-81, 95-95
apps/meteor/client/hooks/useRoomMenuActions.spec.ts (1)
11-29: No mock needed —useToggleNotificationActionis not imported byuseRoomMenuActions.The implementation file does not import
useToggleNotificationAction, so the missing mock is not an issue. The spec file already mocks all the hooks that the implementation actually uses.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
3 participants
Addresses TODO in
useLeaveRoom.tsxto consider team leaving when leaving rooms.When leaving a team main room via the sidebar menu, users now see the
LeaveTeammodal instead of a simple warning, allowing them to select which team channels to also leave.Changes
apps/meteor/client/hooks/menuActions/useLeaveRoom.tsxteamIdandteamMainprops toLeaveRoomPropsLeaveTeammodal for team main roomsapps/meteor/client/hooks/useRoomMenuActions.tsteamId/teamMainfromRoomsstore touseLeaveRoomActionapps/meteor/client/views/navigation/sidepanel/hooks/useRoomMenuActions.tsVerification
yarn lint→ ✅ passesSummary by CodeRabbit
New Features
Improvements
Tests