-
Notifications
You must be signed in to change notification settings - Fork 13k
feat: Added invitation information to user info contextual bar #37712
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 |
|
WalkthroughAdds invitation metadata and invite-state handling across room member and user-info UI: introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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 |
|
@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: 1
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/hooks/useMembersList.ts (1)
113-146: The endpoint consolidation is incomplete and will fail for direct messages.The code hardcodes
/v1/rooms.membersOrderedByRolefor all room types, but this endpoint only supports public channels ('c') and private groups ('p'). Direct messages ('d') require/v1/im.membersinstead. Add conditional endpoint selection: use/v1/im.memberswhenroomType === 'd', otherwise use/v1/rooms.membersOrderedByRole.
🧹 Nitpick comments (5)
packages/i18n/src/locales/en.i18n.json (2)
2676-2676: Align key casing with existing Invitation_ pattern*Nearby keys use Title_Case segments (e.g., Invitation_HTML, Invitation_Subject). Recommend renaming to Invitation_Date for consistency. Based on learnings, only en.i18n.json needs updates.
- "Invitation_date": "Invitation date", + "Invitation_Date": "Invitation date",Please update all call sites accordingly (grep for "Invitation_date").
4479-4479: Consistent casing for action labelUse Title_Case segments to match common convention; suggest Revoke_Invitation.
- "Revoke_invitation": "Revoke invitation", + "Revoke_Invitation": "Revoke invitation",Verify and update usages of "Revoke_invitation" after the rename.
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx (1)
1-1: Tightenusertyping instead of| {}and keep subscription wiringThe new
RoomMemberimport andsubscription={user.subscription}wiring are consistent with the updatedRoomMembershape and look good.However, the current type
user: Pick<RoomMember, 'federated' | 'username' | 'name' | '_id' | 'freeSwitchExtension' | 'subscription'> | {};is type‑unsafe: accessing
user.username,user._id, etc. on a{} | Pick<...>union will fail TypeScript and doesn’t reflect the actual runtime contract (we only ever renderRoomMembersItemwhen we have a real member).You can make this safer and simpler by modeling
useras optional and dropping the{}union, relying on the early return:- type RoomMembersRowProps = { - user: Pick<RoomMember, 'federated' | 'username' | 'name' | '_id' | 'freeSwitchExtension' | 'subscription'> | {}; + type RoomMembersRowProps = { + user?: Pick<RoomMember, 'federated' | 'username' | 'name' | '_id' | 'freeSwitchExtension' | 'subscription'>; @@ -const RoomMembersRow = ({ user, data: { onClickView, rid }, index, reload, useRealName }: RoomMembersRowProps): ReactElement => { - if (!user?._id) { +const RoomMembersRow = ({ user, data: { onClickView, rid }, index, reload, useRealName }: RoomMembersRowProps): ReactElement => { + if (!user) { return <RoomMembersItem.Skeleton />; }This keeps the runtime behavior but restores type safety for all subsequent
user.*property accesses.Also applies to: 6-9, 24-35
apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts (1)
53-60: Check UX for invited non‑members now thatremoveUseris shown whenisInvitedThe new
isInvitedparameter and its propagation:
- Into
useRemoveUserAction(user, rid, reload, isInvited), and- Into
userinfoActionsvia...((isMember || isInvited) && removeUser && { removeUser }), plus the updated depsare all consistent and correctly wired.
One behavioral point to verify: for a user who is invited but not yet a member (
isInvited === true,isMember === false), the action set will now include:
addUservia!isMember && addUser, andremoveUser(labelled “Revoke invitation”) via(isMember || isInvited) && removeUser.If the intended UX is to only allow revoking the invitation in that state (and not also “Add user”), you could tighten the add condition:
- ...(!isMember && addUser && { addUser }), + ...(!isMember && !isInvited && addUser && { addUser }),Otherwise, if both actions are intentionally available, the current implementation is fine.
Also applies to: 62-69, 79-80, 86-100, 102-119
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (1)
56-63: Optional: avoid storing event objects inshowButtonstateThe new
data-invitationdateattribute and passingisInvited/reloadintoUserActionslook correct and align with the updated invitation flow.Currently,
handleMenuEventforwards the React event object directly intosetShowButton(becausesetShowButtonis used as the handler), soshowButtonwill hold the last event object rather than a boolean. Since you only rely on its truthiness to toggle between the kebab icon and<UserActions />, you can make this state explicitly boolean and avoid retaining event objects:- const [showButton, setShowButton] = useState(); + const [showButton, setShowButton] = useState(false); @@ - const handleMenuEvent = { - [isReduceMotionEnabled ? 'onMouseEnter' : 'onTransitionEnd']: setShowButton, - }; + const handleMenuEvent = { + [isReduceMotionEnabled ? 'onMouseEnter' : 'onTransitionEnd']: () => setShowButton(true), + };This keeps the behavior the same while improving typing and avoiding unnecessary event retention.
Also applies to: 71-81
📜 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 (13)
apps/meteor/client/components/UserInfo/UserInfo.stories.tsx(1 hunks)apps/meteor/client/components/UserInfo/UserInfo.tsx(3 hunks)apps/meteor/client/views/hooks/useMembersList.ts(3 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx(5 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx(2 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsx(3 hunks)apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsx(2 hunks)apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsx(2 hunks)apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx(2 hunks)apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts(5 hunks)packages/i18n/src/locales/en.i18n.json(2 hunks)packages/rest-typings/src/v1/rooms.ts(2 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/components/UserInfo/UserInfo.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsxapps/meteor/client/components/UserInfo/UserInfo.stories.tsxapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsxpackages/rest-typings/src/v1/rooms.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.tsapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx
🧠 Learnings (6)
📓 Common learnings
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.
📚 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/hooks/useUserInfoActions/actions/useRemoveUserAction.tsxpackages/rest-typings/src/v1/rooms.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.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:
packages/rest-typings/src/v1/rooms.tsapps/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:
packages/rest-typings/src/v1/rooms.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.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:
packages/rest-typings/src/v1/rooms.tsapps/meteor/client/views/hooks/useMembersList.tsapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.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
🧬 Code graph analysis (8)
apps/meteor/client/components/UserInfo/UserInfo.tsx (1)
apps/meteor/client/lib/utils/timeAgo.ts (1)
timeAgo(12-26)
packages/rest-typings/src/v1/rooms.ts (1)
packages/core-typings/src/ISubscription.ts (1)
SubscriptionStatus(10-10)
apps/meteor/client/views/hooks/useMembersList.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-256)packages/core-typings/src/ISubscription.ts (1)
SubscriptionStatus(10-10)
apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts (1)
apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx (1)
useRemoveUserAction(24-141)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (3)
apps/meteor/client/views/hooks/useMembersList.ts (1)
RoomMember(17-23)packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)packages/ui-kit/src/index.ts (1)
Option(2-2)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsx (1)
packages/core-typings/src/IUser.ts (1)
IUser(186-256)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx (1)
apps/meteor/client/views/hooks/useMembersList.ts (1)
RoomMember(17-23)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx (1)
apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts (1)
useUserInfoActions(62-153)
🔇 Additional comments (14)
apps/meteor/client/components/UserInfo/UserInfo.tsx (3)
53-53: LGTM! Clean type addition.The optional
invitationDateprop follows the existing pattern for optional string fields in the component.
78-78: LGTM! Proper destructuring.The
invitationDateprop is correctly destructured and matches the type definition.
211-216: LGTM! Clean implementation following existing patterns.The invitation date field rendering follows the same pattern as other optional fields in the component:
- Conditional rendering with
&&operator- Consistent use of
InfoPanelField,InfoPanelLabel, andInfoPanelText- Proper use of the
timeAgofunction for date formattingThe placement between custom fields and
createdAtis logical, grouping invitation-related information appropriately.apps/meteor/client/components/UserInfo/UserInfo.stories.tsx (1)
49-52: LGTM!The new story variant provides appropriate test coverage for the invitation feature with a clear example of the
invitationDateprop.apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx (1)
114-138: LGTM!The refactoring to separate content computation from the action option construction is clean. The memos correctly include all dependencies (
invited,room?.teamMain,tfor content; androomCanRemove,userCanRemove,removeUserOptionAction,contentfor the final option).apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx (1)
8-32: LGTM!The component correctly threads the
isInvitedflag through touseUserInfoActionsand appropriately setsisMember: !isInvitedto distinguish between invited and active members.apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsx (2)
27-34: LGTM!The addition of
invitationDateto the props is consistent with the invitation feature flow.
117-123: LGTM!The component correctly passes
invitationDatetoUserInfoand derivesisInvitedusingBoolean(invitationDate)forUserInfoActions, maintaining a clear data flow for the invitation state.packages/rest-typings/src/v1/rooms.ts (2)
1-11: LGTM!The addition of
SubscriptionStatusto the imports is necessary for the type refinement below.
895-899: LGTM!The type refinement from
stringtoSubscriptionStatusimproves type safety and aligns with the core-typings definition. This ensures that only valid subscription status values are used throughout the codebase.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsx (2)
96-106: LGTM!The conditional rendering correctly checks for
state.user?.idand passes bothuidandinvitationDatetoUserInfoWithData, enabling the invitation information display.
51-82: No action needed. The dataset attribute naming is correct and consistent.
RoomMembersItem.tsxsetsdata-useridanddata-invitationdateattributes, which the browser's dataset API automatically converts todataset.useridanddataset.invitationdatewhen accessed inRoomMembersWithData.tsx. This is standard HTML/JavaScript behavior and requires no changes.Likely an incorrect or invalid review comment.
apps/meteor/client/views/hooks/useMembersList.ts (1)
17-23: LGTM!The
RoomMembertype extension to includefederated,freeSwitchExtension, and the newsubscriptionfield withstatusandcreatedAtproperly supports the invitation feature and aligns with the type updates inpackages/rest-typings/src/v1/rooms.ts.apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsx (1)
13-42: TheisInvitedprop is correctly threaded through touseUserInfoActions.The
size: 2parameter is the default value defined in the hook (not a change from 3). UserCardWithData.tsx is the only location that explicitly overrides this tosize: 3. No action required.
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
Show resolved
Hide resolved
e8af0cb to
f26796b
Compare
c420eb2 to
36e006f
Compare
3f2f57e to
0703b45
Compare
36e006f to
65ca437
Compare
0703b45 to
fff9239
Compare
fddb5b3 to
cb7d33a
Compare
fff9239 to
a6b0b80
Compare
3591e8f to
78f37b5
Compare
aef1327 to
7e82fb1
Compare
7e82fb1 to
239cd6f
Compare
239cd6f to
0e0465d
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37712 +/- ##
===========================================
- Coverage 67.70% 67.69% -0.02%
===========================================
Files 3475 3475
Lines 113860 113871 +11
Branches 20943 20947 +4
===========================================
- Hits 77093 77084 -9
- Misses 34586 34601 +15
- Partials 2181 2186 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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
🧹 Nitpick comments (1)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsx (1)
23-29: Story data aligned with new RoomMember shape; consider consistency across storiesAdding
roles: ['user']here keeps the Default story in sync with the expanded RoomMember type and should help prevent type errors or missing-role UI paths during development.For consistency and to better exercise role‑aware UI, you might also add a
rolesfield to the member object inWithABACRoom.args.members(even if the prop is optional), e.g.roles: ['user']or a role set that reflects the ABAC scenario.
📜 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/components/UserInfo/__snapshots__/UserInfo.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
apps/meteor/client/components/UserInfo/UserInfo.stories.tsx(1 hunks)apps/meteor/client/components/UserInfo/UserInfo.tsx(3 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/RoomMembersActions.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx(5 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx(2 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsx(3 hunks)apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsx(2 hunks)apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsx(2 hunks)apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx(2 hunks)apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts
- apps/meteor/client/components/UserInfo/UserInfo.tsx
- apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsx
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersRow.tsx
- apps/meteor/client/components/UserInfo/UserInfo.stories.tsx
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx
- apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.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/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.stories.tsxapps/meteor/client/views/hooks/useMembersList.ts
🧠 Learnings (7)
📓 Common learnings
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.
📚 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.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsxapps/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/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/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/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/hooks/useMembersList.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/room/contextualBar/RoomMembers/RoomMembers.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxapps/meteor/client/views/hooks/useMembersList.ts
📚 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:
apps/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: 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/RoomMembersItem.tsx
🧬 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)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (2)
apps/meteor/client/views/hooks/useMembersList.ts (1)
RoomMember(23-28)packages/core-typings/src/IRoom.ts (1)
IRoom(21-95)
apps/meteor/client/views/hooks/useMembersList.ts (2)
packages/core-typings/src/IUser.ts (1)
IUser(186-256)packages/core-typings/src/ISubscription.ts (1)
ISubscription(11-79)
🔇 Additional comments (8)
apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx (2)
24-29: LGTM on the function signature update.The optional
invitedparameter is correctly placed at the end of the parameter list, maintaining backward compatibility with existing callers.
126-138: LGTM on the refactored memoization.The early return pattern improves readability, and the dependencies array correctly includes all referenced values:
roomCanRemove,userCanRemove,removeUserOptionAction, andcontent.apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsx (2)
16-16: LGTM: Invitation state propagation looks correct.The
isInvitedprop is correctly added to the component signature and properly threaded through touseUserInfoActions.Also applies to: 20-20, 37-37
35-35: Thesizeparameter change is intentional and appropriate.The
sizeparameter controls how many user action buttons display inline versus in the overflow menu. UserInfoActions usessize: 2(matching the hook's default), showing 2 action buttons inline in the contextual bar sidebar. This differs from UserCardWithData which usessize: 3for the popup context. The change reflects appropriate layout constraints for the component's context and is unrelated to the invitation feature changes.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembers.tsx (1)
1-1: LGTM: Consistent type migration to centralizedRoomMember.The transition from
RoomMemberUser[]toRoomMember[]is implemented consistently throughout the file, with proper imports and type declarations. This aligns with the centralized type definition inuseMembersList.ts.Also applies to: 25-25, 36-36, 93-96
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (2)
45-46: LGTM: TypeScript safety is correctly handled.The optional chaining on
subscription?.ts(line 46) properly handles the case wheresubscriptionmay be undefined, ensuring type safety. The invitation state derivation is correct and safe.
24-29: LGTM: Invitation data integration is well-structured.The component correctly:
- Accepts
subscriptionthrough props derived fromRoomMember- Computes
isInvitedbased on subscription status- Propagates
isInvitedtoUserActions- Exposes
invitationDatevia data attribute for UI consumptionAlso applies to: 39-42, 73-81
apps/meteor/client/views/hooks/useMembersList.ts (1)
1-1: LGTM: RoomMember type expansion supports invitation tracking.The enhanced
RoomMembertype correctly:
- Wraps in
Serialized<>for API response data handling- Adds
subscriptionfield withstatusandtsfor invitation state tracking- Includes
federatedandfreeSwitchExtensionfromIUserfor complete member data- Maintains backward compatibility with optional fields
This provides the foundation for invitation information display throughout the UI.
Also applies to: 23-28
apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx
Show resolved
Hide resolved
c94ca69 to
365e2ab
Compare
2a7e07c to
8fcbe66
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
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/room/contextualBar/RoomMembers/RoomMembersItem.tsx (1)
32-44: Fix duplicate parametersubscription.The parameter
subscriptionis declared twice (lines 40 and 43), which is a syntax error. Remove the duplicate declaration on line 43.🔎 Proposed fix
const RoomMembersItem = ({ _id, name, username, federated, freeSwitchExtension, onClickView, rid, subscription, reload, useRealName, - subscription, }: RoomMembersItemProps): ReactElement => {
♻️ Duplicate comments (1)
apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx (1)
101-111: Modal body text inconsistency for invitation context.The
confirmText(line 104) correctly switches to "Revoke_invitation" wheninvitedis true, but the modal body (line 109) still displays the generic "The_user_will_be_removed_from_s" message. This creates a UX mismatch.🔎 Proposed fix
setModal( <GenericModal variant='danger' confirmText={invited ? t('Revoke_invitation') : t('Yes_remove_user')} onClose={closeModal} onCancel={closeModal} onConfirm={(): Promise<void> => handleRemoveFromRoom(rid, uid)} > - {t('The_user_will_be_removed_from_s', roomName)} + {invited ? t('The_invitation_will_be_revoked') : t('The_user_will_be_removed_from_s', roomName)} </GenericModal>, );Note: This requires adding the
The_invitation_will_be_revokedi18n key.
🧹 Nitpick comments (1)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (1)
73-77: Use optional chaining for consistency and TypeScript safety.While the conditional on line 73 checks
subscription?.status === 'INVITED', TypeScript may not narrow the type inside JSX blocks. Usesubscription?.tson line 75 for consistency with line 48 and to avoid potential type errors under strict null checks.🔎 Proposed fix
{subscription?.status === 'INVITED' && ( <OptionColumn> - <InvitationBadge mbs={2} size='x20' invitationDate={subscription.ts} /> + <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/components/UserInfo/__snapshots__/UserInfo.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
apps/meteor/client/components/UserInfo/UserInfo.stories.tsx(1 hunks)apps/meteor/client/components/UserInfo/UserInfo.tsx(3 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx(1 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx(3 hunks)apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsx(3 hunks)apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsx(2 hunks)apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsx(2 hunks)apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx(3 hunks)apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts(5 hunks)packages/i18n/src/locales/en.i18n.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/meteor/client/components/UserInfo/UserInfo.stories.tsx
- apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoActions.tsx
- packages/i18n/src/locales/en.i18n.json
- apps/meteor/client/components/UserInfo/UserInfo.tsx
- apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts
🧰 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/RoomMembersActions.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsxapps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
🧠 Learnings (5)
📚 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/RoomMembersActions.tsxapps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.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/room/contextualBar/RoomMembers/RoomMembersWithData.tsxapps/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 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/RoomMembersWithData.tsxapps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.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:
apps/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: 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/RoomMembersItem.tsx
🧬 Code graph analysis (3)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx (1)
apps/meteor/client/views/room/hooks/useUserInfoActions/useUserInfoActions.ts (1)
useUserInfoActions(62-153)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsx (1)
packages/core-typings/src/IUser.ts (1)
IUser(187-259)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (1)
packages/ui-kit/src/index.ts (1)
Option(2-2)
🪛 Biome (2.1.2)
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx
[error] 43-43: Duplicate parameter name.
The parameter overrides a preceding parameter by using the same name.
(lint/suspicious/noDuplicateParameters)
[error] 43-43: Shouldn't redeclare 'subscription'. Consider to delete it or rename it.
'subscription' is defined here:
(lint/suspicious/noRedeclare)
⏰ 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 (7)
apps/meteor/client/views/room/hooks/useUserInfoActions/actions/useRemoveUserAction.tsx (3)
24-29: LGTM!The optional
invitedparameter is a clean, non-breaking addition to the hook signature that enables invitation-specific behavior.
114-124: LGTM!The
contentmemo correctly prioritizes the invitation case, then team context, with a sensible fallback for regular rooms. Dependencies are properly specified.
126-138: LGTM!The
removeUserOptionmemo cleanly separates permission checks from content determination. The early return for missing permissions is appropriate, and the action object structure is well-typed.apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersActions.tsx (1)
10-10: LGTM: Invitation state properly threaded through.The
isInvitedprop is correctly added to the type, destructured, and passed touseUserInfoActions. The logicisMember: !isInvitedappropriately reflects that invited users are not yet members.Also applies to: 20-20, 30-31
apps/meteor/client/views/room/contextualBar/UserInfo/UserInfoWithData.tsx (1)
31-31: LGTM: Invitation date propagated correctly.The
invitationDateprop is properly added to the type signature, accepted by the component, and passed through toUserInfo. DerivingisInvited={Boolean(invitationDate)}forUserInfoActionsis a clean approach.Also applies to: 36-36, 117-123
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersWithData.tsx (1)
51-54: LGTM: State refactored to group invitation data.The state structure change from a flat
userIdto a nesteduser: { id, invitationDate }cleanly groups related data. The dataset attributeinvitationdate(lowercase) is consistently read and propagated toUserInfoWithData.Also applies to: 76-82, 96-106
apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsx (1)
47-48: LGTM: Invitation state correctly derived and propagated.The
isInvitedflag andinvitationDateare correctly derived fromsubscriptionusing optional chaining. Thedata-invitationdateattribute andisInvitedprop are properly passed to enable invitation-aware UI and actions.Also applies to: 58-65, 80-88
…tChat#37712) Co-authored-by: Diego Sampaio <chinello@gmail.com>
Proposed changes (including videos or screenshots)
Issue(s)
FB-66
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Localization
✏️ Tip: You can customize this high-level summary in your review settings.