-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: Added invitation badge to room members list #37643
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 |
🦋 Changeset detectedLatest commit: 99bf882 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 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 |
WalkthroughAdds per-member Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (UI / Storybook)
participant Server as API Server (/v1/im.members)
participant DB as Subscriptions Collection
participant Members as Members Retrieval Logic
Client->>Server: GET /v1/im.members?roomId=...
Server->>Members: fetch members for roomId
Members-->>Server: members[] (user objects)
Server->>DB: find subscriptions by roomId & userIds
DB-->>Server: subscriptions[] (subscription docs)
Server->>Server: merge subscriptions into members by userId
Server-->>Client: 200 OK { members: [ { ..., subscription: { _id, status, ts, roles } }, ... ] }
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/meteor/client/views/room/contextualBar/RoomMembers/types.ts (1)
1-6: SharedRoomMemberUsertype looks correct; consider reusingIUser['roles']to avoid duplicationThe alias cleanly centralizes the member shape and the
status?: UserStatus | SubscriptionStatusunion fits the invited-member use case.You can make the
rolestyping more robust (and drop theIRoleimport) by reusing the existingIUser.rolesdefinition instead of duplicatingIRole['_id'][]:-import type { IUser, IRole, SubscriptionStatus, UserStatus } from '@rocket.chat/core-typings'; - -export type RoomMemberUser = Pick<IUser, 'username' | '_id' | 'name' | 'freeSwitchExtension' | 'federated'> & { - roles?: IRole['_id'][]; - status?: UserStatus | SubscriptionStatus; -}; +import type { IUser, SubscriptionStatus, UserStatus } from '@rocket.chat/core-typings'; + +export type RoomMemberUser = Pick<IUser, 'username' | '_id' | 'name' | 'freeSwitchExtension' | 'federated'> & + Partial<Pick<IUser, 'roles'>> & { + status?: UserStatus | SubscriptionStatus; + };apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx (1)
96-96: ClarifyisABACRoomconfiguration.Setting
isABACRoom: truein the invited member story appears unrelated to the invitation feature. This should likely befalseto isolate the invitation badge functionality, unless ABAC rooms have specific invitation behavior.Consider applying this diff if ABAC is not relevant to the invitation scenario:
reload: action('reload'), - isABACRoom: true, + isABACRoom: false, };
📜 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 ignored due to path filters (1)
apps/meteor/client/views/room/contextualBar/RoomMembers/__snapshots__/RoomMembers.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx(2 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx(3 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx(2 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/badges/InvitationBadge.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/types.ts(1 hunks)packages/i18n/src/locales/en.i18n.json(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/client/views/room/contextualBar/RoomMembers/types.tsapps/meteor/client/views/room/contextualBar/RoomMembers/badges/InvitationBadge.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx
🧠 Learnings (7)
📚 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/room/contextualBar/RoomMembers/types.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx
📚 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/room/contextualBar/RoomMembers/types.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx
📚 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/room/contextualBar/RoomMembers/types.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx
📚 Learning: 2025-11-17T15:07:13.273Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37398
File: packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx:357-363
Timestamp: 2025-11-17T15:07:13.273Z
Learning: In packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx, IconElement is a presentational, non-actionable element that does not require wrapping in AppIdProvider, similar to plain_text and mrkdwn renderers. Only actionable elements (those with actions, actionId, or interactive behavior) should be wrapped in AppIdProvider.
Applied to files:
apps/meteor/client/views/room/contextualBar/RoomMembers/badges/InvitationBadge.tsx
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/i18n/src/locales/en.i18n.json
📚 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/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx
📚 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/room/contextualBar/RoomMembers/RoomMembers.tsx
🧬 Code graph analysis (2)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (2)
apps/meteor/client/views/room/contextualBar/RoomMembers/types.ts (1)
RoomMemberUser(3-6)packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx (1)
apps/meteor/client/views/room/contextualBar/RoomMembers/types.ts (1)
RoomMemberUser(3-6)
🔇 Additional comments (4)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx (1)
1-41: LGTM! Type refactoring is clean.The migration from local type definition to centralized
RoomMemberUsertype improves maintainability, and the newstatusprop is correctly wired toRoomMembersItem.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx (1)
1-12: LGTM! Type centralization is consistent.Moving the
RoomMemberUsertype to a dedicated types module improves code organization and reusability.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (1)
19-20: LGTM! Invitation badge integration is well-structured.The component correctly:
- Uses the centralized
RoomMemberUsertype- Conditionally renders
InvitationBadgefor invited members- Wraps the badge in
OptionColumnfor proper layoutThe string literal comparison
status === 'INVITED'should be verified to match the enum definition (see verification comment in the stories file).Also applies to: 25-30, 37-37, 63-67
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx (1)
82-82: Remove this comment—'INVITED' is a valid SubscriptionStatus value.SubscriptionStatus is defined as
type SubscriptionStatus = 'INVITED'in packages/core-typings/src/ISubscription.ts. The status value in the story correctly uses this type.
apps/meteor/client/views/room/contextualBar/RoomMembers/badges/InvitationBadge.tsx
Outdated
Show resolved
Hide resolved
02ef424 to
432d3b1
Compare
432d3b1 to
2b81d46
Compare
b2cc5c0 to
0d14b19
Compare
cd472aa to
8316778
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37643 +/- ##
===========================================
- Coverage 67.71% 67.69% -0.02%
===========================================
Files 3475 3475
Lines 113861 113860 -1
Branches 20942 20943 +1
===========================================
- Hits 77097 77080 -17
- Misses 34579 34592 +13
- Partials 2185 2188 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dba8733 to
85183c9
Compare
85183c9 to
aa7354e
Compare
bc5722b to
9c7345e
Compare
c420eb2 to
36e006f
Compare
9c7345e to
b243f28
Compare
36e006f to
65ca437
Compare
b243f28 to
2fe6480
Compare
fddb5b3 to
cb7d33a
Compare
3591e8f to
78f37b5
Compare
2fe6480 to
ffa1368
Compare
8b4ed2f to
2e911d6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx (1)
101-101: Clarify the necessity ofisABACRoom: truefor this story.The
WithInvitedMemberstory demonstrates invitation status, but settingisABACRoom: trueappears unrelated to the invitation feature. Is this flag necessary, or was it copied from theWithABACRoomstory?apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (1)
63-67: Consider using a constant for the status comparison.The conditional rendering logic is correct, but the string literal
'INVITED'could be replaced with a constant or enum value for better type safety and maintainability.🔎 Suggested refactor using a constant
If a
SubscriptionStatusconstant/enum is available, consider:-{subscription?.status === 'INVITED' && ( +{subscription?.status === SubscriptionStatus.INVITED && ( <OptionColumn> <InvitationBadge mbs={2} size='x20' invitationDate={subscription.ts} /> </OptionColumn> )}
📜 Review details
Configuration used: Organization 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 ignored due to path filters (1)
apps/meteor/client/views/room/contextualBar/RoomMembers/__snapshots__/RoomMembers.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
.changeset/nasty-moons-speak.md(1 hunks)apps/meteor/client/views/hooks/useMembersList.ts(2 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx(4 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx(4 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx
- .changeset/nasty-moons-speak.md
🧰 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/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsxapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
🧠 Learnings (5)
📚 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/room/contextualBar/RoomMembers/RoomMembers.stories.tsxapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
📚 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/room/contextualBar/RoomMembers/RoomMembers.stories.tsxapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
📚 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/client/views/hooks/useMembersList.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/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
📚 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/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
🧬 Code graph analysis (1)
apps/meteor/client/views/hooks/useMembersList.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)
⏰ 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). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/views/hooks/useMembersList.ts (1)
23-28: LGTM! Type definition properly extended.The
RoomMembertype is appropriately wrapped inSerialized<>for API data and includes the optionalsubscriptionfield. The use ofPick<ISubscription, ...>correctly selects only the necessary subscription fields.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx (1)
85-85: No changes needed. The string literal'INVITED'is already type-safe and the correct way to satisfy theSubscriptionStatustype in core-typings.SubscriptionStatusis a type alias with only one valid value ('INVITED'), not an enum, so there is no constant to import. The subscription object is properly typed without modification.Likely an incorrect or invalid review comment.
2e911d6 to
5f4b7b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization 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 ignored due to path filters (1)
apps/meteor/client/views/room/contextualBar/RoomMembers/__snapshots__/RoomMembers.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
.changeset/nasty-moons-speak.md(1 hunks)apps/meteor/app/api/server/v1/im.ts(1 hunks)apps/meteor/client/views/hooks/useMembersList.ts(2 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx(4 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx(4 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx(2 hunks)packages/rest-typings/src/v1/dm/im.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/nasty-moons-speak.md
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx
🧰 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/client/views/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/app/api/server/v1/im.tspackages/rest-typings/src/v1/dm/im.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx
🧠 Learnings (6)
📚 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/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/app/api/server/v1/im.tsapps/meteor/client/views/hooks/useMembersList.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/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/app/api/server/v1/im.tspackages/rest-typings/src/v1/dm/im.tsapps/meteor/client/views/hooks/useMembersList.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/room/contextualBar/RoomMembers/RoomMembers.tsxpackages/rest-typings/src/v1/dm/im.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx
📚 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/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/dm/im.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/client/views/hooks/useMembersList.ts
🧬 Code graph analysis (3)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx (1)
apps/meteor/client/views/hooks/useMembersList.ts (1)
RoomMember(23-28)
packages/rest-typings/src/v1/dm/im.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)
apps/meteor/client/views/hooks/useMembersList.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)
🔇 Additional comments (4)
apps/meteor/app/api/server/v1/im.ts (1)
419-432: LGTM! Subscription augmentation logic is correct.The implementation correctly fetches subscriptions for room members and merges them into the response. The use of
Array.findappropriately handles cases where a subscription may not exist for a member, resulting inundefined, which aligns with the optional subscription field in the type definitions.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx (1)
1-1: LGTM! Type updates are consistent.The transition from
RoomMemberUserto the importedRoomMembertype correctly reflects the new data structure that includes optional subscription information. The changes are purely type-level with no behavioral modifications.Also applies to: 25-25, 36-36, 93-96
apps/meteor/client/views/hooks/useMembersList.ts (1)
1-1: LGTM! Type definitions are well-structured.The
RoomMembertype correctly:
- Uses
Serialized<>wrapper for proper date handling- Marks
subscriptionas optional with?, aligning with the discussion in past review comments- Includes the necessary subscription fields for invitation status tracking
The type structure is consistent across the client-side codebase.
Also applies to: 23-28, 30-35
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx (1)
85-85: 'INVITED' is the valid SubscriptionStatus value.The
SubscriptionStatustype is defined as'INVITED'in the codebase (packages/core-typings/src/ISubscription.ts), making it the only valid value forISubscription.status.
| isTeam: false, | ||
| isDirect: false, | ||
| reload: action('reload'), | ||
| isABACRoom: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect prop value: isABACRoom should be false.
The WithInvitedMember story demonstrates invitation functionality, not ABAC room behavior. Setting isABACRoom: true appears to be a copy-paste error from the WithABACRoom story and will disable the invite button.
🔎 Proposed fix
- isABACRoom: true,
+ isABACRoom: false,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| isABACRoom: true, | |
| isABACRoom: false, |
🤖 Prompt for AI Agents
In
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx
around line 101, the WithInvitedMember story incorrectly sets isABACRoom: true
which disables invite behavior; change this prop to isABACRoom: false so the
invite button and invitation flow work as intended. Update the story prop value
accordingly and run the story to verify the invite button is enabled.
| members: Pick<IUser, '_id' | 'status' | 'name' | 'username' | 'utcOffset'> & | ||
| { subscription: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'> }[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix type syntax and make subscription optional.
Two issues:
-
Array bracket placement error: The
[]is positioned incorrectly, causing TypeScript to interpretsubscriptionas an array property rather than an array of member objects. The type should wrap the entire intersection in parentheses. -
Missing optional marker: Based on the past review discussion,
subscriptionshould be optional since this type is shared between endpoints where not all have subscription support yet.
🔎 Proposed fix
- members: Pick<IUser, '_id' | 'status' | 'name' | 'username' | 'utcOffset'> &
- { subscription: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'> }[];
+ members: (Pick<IUser, '_id' | 'status' | 'name' | 'username' | 'utcOffset'> & {
+ subscription?: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>;
+ })[];🤖 Prompt for AI Agents
In packages/rest-typings/src/v1/dm/im.ts around lines 52-53, the member type
currently places the array brackets incorrectly and marks subscription as
required; wrap the entire intersection in parentheses so the array applies to
the whole member shape (i.e. (A & B)[]), and make the subscription property
optional by adding a ? to its key so the type becomes (Pick<IUser, '...'> & {
subscription?: Pick<ISubscription, '...'> })[] — update the line accordingly.
23da6ed to
99bf882
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/rest-typings/src/v1/dm/im.ts (1)
52-54: Make subscription optional to support endpoints without invitation support.As discussed in the previous review, the
subscriptionfield should be optional since this type is shared between/v1/im.membersand/v1/rooms.membersOrderedByRole, where the latter doesn't have invitation support yet.🔎 Proposed fix
members: (Pick<IUser, '_id' | 'status' | 'name' | 'username' | 'utcOffset'> & { - subscription: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; + subscription?: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; })[];apps/meteor/client/views/hooks/useMembersList.ts (1)
23-27: Make subscription optional to align with REST API types.The
subscriptionfield should be optional to match the REST API type definition. Since the type is shared between endpoints where not all have subscription support, making this field optional ensures type consistency across the stack.🔎 Proposed fix
export type RoomMember = Serialized< Pick<IUser, 'username' | '_id' | 'name' | 'status' | 'federated' | 'freeSwitchExtension'> & { roles?: IRole['_id'][] } & { - subscription: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; + subscription?: Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>; } >;
📜 Review details
Configuration used: Organization 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)
.changeset/nasty-moons-speak.md(1 hunks)apps/meteor/app/api/server/v1/im.ts(1 hunks)apps/meteor/client/views/hooks/useMembersList.ts(2 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx(2 hunks)packages/rest-typings/src/v1/dm/im.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx
🧰 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:
packages/rest-typings/src/v1/dm/im.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/app/api/server/v1/im.ts
🧠 Learnings (7)
📓 Common learnings
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>
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.
📚 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:
.changeset/nasty-moons-speak.mdpackages/rest-typings/src/v1/dm/im.tsapps/meteor/client/views/hooks/useMembersList.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:
.changeset/nasty-moons-speak.md
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/dm/im.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:
packages/rest-typings/src/v1/dm/im.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/app/api/server/v1/im.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/client/views/hooks/useMembersList.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/hooks/useMembersList.tsapps/meteor/app/api/server/v1/im.ts
🧬 Code graph analysis (2)
packages/rest-typings/src/v1/dm/im.ts (1)
packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)
apps/meteor/client/views/hooks/useMembersList.ts (2)
packages/apps/src/AppsEngine.ts (1)
IRole(15-15)packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-81)
⏰ 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). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
.changeset/nasty-moons-speak.md (1)
1-5: LGTM!The changeset is properly formatted with the correct version bump (patch) and an accurate description of the feature.
| const membersWithSubscriptionInfo = members.map((member) => { | ||
| const sub = subs.find((sub) => sub.u._id === member._id); | ||
|
|
||
| const { u: _u, ...subscription } = sub || {}; | ||
|
|
||
| return { | ||
| ...member, | ||
| subscription, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix type safety issue with subscription fallback.
When a subscription is not found, the current code creates an empty object {} instead of undefined. This results in a subscription field that doesn't match the expected type Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'>, potentially causing runtime errors when components try to access properties like subscription._id or subscription.status.
🔎 Proposed fix
const membersWithSubscriptionInfo = members.map((member) => {
const sub = subs.find((sub) => sub.u._id === member._id);
- const { u: _u, ...subscription } = sub || {};
-
- return {
- ...member,
- subscription,
- };
+ if (!sub) {
+ return member;
+ }
+
+ const { u: _u, ...subscription } = sub;
+
+ return {
+ ...member,
+ subscription,
+ };
});🤖 Prompt for AI Agents
In apps/meteor/app/api/server/v1/im.ts around lines 426 to 435, the mapping
creates an empty object when no subscription is found which breaks the expected
Pick<ISubscription, '_id' | 'status' | 'ts' | 'roles'> type; update the map so
that when subs.find(...) returns undefined you set subscription to undefined
(not {}), and when a sub exists explicitly pick only the allowed fields (or
construct the Pick) before returning — i.e. guard the destructure against
undefined and assign subscription = undefined when absent, or subscription = {
_id: sub._id, status: sub.status, ts: sub.ts, roles: sub.roles } when present.
Co-authored-by: Diego Sampaio <chinello@gmail.com>
Proposed changes (including videos or screenshots)
This pull request adds a visual badge for invited users in the room members contextual. The changes also add a new story and snapshot test for rooms with invited members.
Issue(s)
FB-63
Steps to test or reproduce
ℹ️ Currently invitation requests are only available for federated rooms.
ℹ️ You'll need two workspaces with federation enabled (I'll refer to them as
ws-aandws-b)ws-a)ws-bto the room (ws-a)Further comments
Summary by CodeRabbit
New Features
Storybook
Refactor
API & Types
✏️ Tip: You can customize this high-level summary in your review settings.