-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
base: feat/invites
Are you sure you want to change the base?
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 |
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR introduces support for displaying invited member status in the room members list. It extracts the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
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
| "Message_list": "Message list", | ||
| "Message_pinning": "Message pinning", | ||
| "Message_removed": "message removed", | ||
| "Message_request": "Message request", |
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.
🧩 Analysis chain
🏁 Script executed:
# Search for usages of the new i18n key
rg -n '"Message_request"' --glob '!**/dist/**' --glob '!**/build/**'Repository: RocketChat/Rocket.Chat
Length of output: 149
🏁 Script executed:
# Search for InvitationBadge component and invitation-related code
fd -i 'InvitationBadge' --glob '!**/dist/**' --glob '!**/build/**'Repository: RocketChat/Rocket.Chat
Length of output: 223
🏁 Script executed:
# Look for invitation-related i18n keys to compare naming patterns
rg -n '"Invite' packages/i18n/src/locales/en.i18n.json -A 1 -B 1Repository: RocketChat/Rocket.Chat
Length of output: 1384
🏁 Script executed:
# Search for recent changes referencing message request or invitation badge
rg -n 'invitation.*badge|invited.*member' apps meteor packages -i --glob '!**/dist/**' --glob '!**/build/**' | head -30Repository: RocketChat/Rocket.Chat
Length of output: 1008
🏁 Script executed:
cat -n apps/meteor/client/views/room/contextualBar/RoomMembers/badges/InvitationBadge.tsxRepository: RocketChat/Rocket.Chat
Length of output: 601
🏁 Script executed:
# Check the context around where InvitationBadge is used
sed -n '15,70p' apps/meteor/client/views/room/contextualBar/RoomMembers/RoomMembersItem.tsxRepository: RocketChat/Rocket.Chat
Length of output: 2067
🏁 Script executed:
# Search more broadly for where Message_request might be used
rg -n 'Message_request' --type json --type tsx --type ts --type jsx --type jsRepository: RocketChat/Rocket.Chat
Length of output: 93
Use a label that accurately describes the invitation status; "Message request" doesn't match the feature.
The InvitationBadge component uses t('Message_request') for both title and aria-label of a mail icon shown when a room member has status === 'INVITED'. The label "Message request" is semantically mismatched with an invitation status badge. Use a clearer label like "Invited" or "Invitation pending" to accurately reflect the displayed state and improve accessibility.
🤖 Prompt for AI Agents
In packages/i18n/src/locales/en.i18n.json around line 3451, the label
"Message_request" is used for the InvitationBadge title/aria-label but does not
reflect an invitation state; change the key/value to a clear invitation label
(for example update key/value to "Invitation_pending": "Invitation pending" or
"Invited": "Invited") and then update the InvitationBadge usage to call the new
i18n key so the title and aria-label accurately describe the INVITED status for
accessibility.
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 @@
## feat/invites #37643 +/- ##
================================================
- Coverage 54.20% 53.96% -0.25%
================================================
Files 2639 2640 +1
Lines 50047 50051 +4
Branches 11200 11201 +1
================================================
- Hits 27129 27010 -119
- Misses 20774 20907 +133
+ Partials 2144 2134 -10
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
60e7fc1 to
04782f5
Compare
bc5722b to
9c7345e
Compare
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
Tests
✏️ Tip: You can customize this high-level summary in your review settings.