feat: Add user status on voice call widget#37217
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: b2eb827 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 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 user presence (status) propagation and rendering across the VoIP peer selection flow: presence is fetched via a presence hook, added to peer/autocomplete payloads and context, and rendered as StatusBullet UI next to user names in autocomplete and peer info components. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Caller UI
participant Auto as PeerAutocomplete
participant Provider as MediaCallProvider / Context
participant Presence as useUserPresence
participant UI as StatusBullet Renderer
User->>Auto: Open/select peer
Auto->>Provider: Request autocomplete options
Provider->>Presence: Read presence for users
Presence-->>Provider: Return statuses
Provider-->>Auto: Return options including status
Auto->>UI: Render option with StatusBullet(status)
UI-->>User: Display presence indicator
Note over User,Auto: Call/Transfer actions remain enabled (non-blocking)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37217 +/- ##
===========================================
- Coverage 67.79% 65.34% -2.46%
===========================================
Files 3345 3045 -300
Lines 114470 109490 -4980
Branches 20716 19601 -1115
===========================================
- Hits 77605 71545 -6060
- Misses 34178 35714 +1536
+ Partials 2687 2231 -456
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/ui-voip/src/v2/MockedMediaCallProvider.tsx (2)
8-8: Addstatusfield to mock data for consistency.The initial
peerInfoincludesstatus: UserStatus.ONLINE(line 25), but themyDataarray items don't include a status field. This creates an inconsistency where selecting different peers in the mock won't populate the status.Apply this diff:
-const myData: any[] = Array.from({ length: 100 }, (_, i) => ({ value: `user-${i}`, label: `User ${i}`, identifier: `000${i}`, avatarUrl })); +const myData: any[] = Array.from({ length: 100 }, (_, i) => ({ + value: `user-${i}`, + label: `User ${i}`, + identifier: `000${i}`, + avatarUrl, + status: UserStatus.ONLINE, +}));
61-74: IncludestatusingetPeerInforeturn value.The
getPeerInfofunction returns aPeerInfoobject without thestatusfield, which is inconsistent with the initial state (line 25) and the real implementation inMediaCallProvider.tsx.Apply this diff:
const getPeerInfo = (id: string) => { const peerInfo = myData.find((item) => item.value === id); if (!peerInfo) { return Promise.resolve(undefined); } return Promise.resolve({ displayName: peerInfo.label, userId: peerInfo.value, avatarUrl: peerInfo.avatarUrl, username: peerInfo.identifier, callerId: peerInfo.value, + status: peerInfo.status, }); };packages/ui-voip/src/v2/MediaCallContext.ts (1)
192-197: Remove unsafe type cast.The cast
as UserStatusat line 196 is unsafe iflocalInfo.statuscontains an invalid value. This cast will become unnecessary oncePeerAutocompleteOptions.statusis properly typed asUserStatusinstead ofstring(as suggested in PeerAutocomplete.tsx).After fixing the type in
PeerAutocomplete.tsx, update this code:onSelectPeer({ userId: localInfo.value, displayName: localInfo.label, avatarUrl: localInfo.avatarUrl, - status: localInfo.status as UserStatus, + status: localInfo.status, });
📜 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)
packages/ui-voip/src/v2/__snapshots__/MediaCallWidget.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
.changeset/itchy-books-report.md(1 hunks)packages/ui-voip/src/v2/MediaCallContext.ts(3 hunks)packages/ui-voip/src/v2/MediaCallProvider.tsx(3 hunks)packages/ui-voip/src/v2/MockedMediaCallProvider.tsx(2 hunks)packages/ui-voip/src/v2/components/PeerAutocomplete.tsx(3 hunks)packages/ui-voip/src/v2/components/PeerInfo/InternalUser.tsx(1 hunks)packages/ui-voip/src/v2/components/PeerInfo/PeerInfo.stories.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/ui-voip/src/v2/components/PeerInfo/InternalUser.tsx (1)
packages/ui-voip/src/v2/components/PeerInfo/PeerInfo.stories.tsx (1)
InternalUser(12-31)
packages/ui-voip/src/v2/MediaCallProvider.tsx (1)
packages/ui-contexts/src/index.ts (1)
useUserPresence(96-96)
⏰ 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). (6)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
15-15: LGTM!The addition of
status?: UserStatustoInternalPeerInfoproperly extends the type to support presence status throughout the call flow..changeset/itchy-books-report.md (1)
1-5: LGTM!The changeset properly documents the new presence indicator feature as a minor version bump.
packages/ui-voip/src/v2/components/PeerInfo/PeerInfo.stories.tsx (1)
1-1: LGTM!The story properly demonstrates the new
statusprop withUserStatus.ONLINE, providing visual documentation of the presence indicator feature.Also applies to: 17-17
packages/ui-voip/src/v2/components/PeerInfo/InternalUser.tsx (1)
1-2: LGTM!The component properly integrates the status indicator:
- Accepts
status?: UserStatusin props- Conditionally renders
StatusBulletwhen status is provided- Layout updated to display status inline with the display name using flex row
- Appropriate spacing with
mis={4}between the bullet and nameAlso applies to: 6-6, 11-11, 16-19
packages/ui-voip/src/v2/MediaCallProvider.tsx (1)
221-221: The review comment is incorrect—the code is already type-safe.The
user.statusvalue is already properly typed through the API response chain:
- The
/v1/users.autocompleteendpoint response is typed via rest-typings asRequired<Pick<IUser, '_id' | 'name' | 'username' | 'nickname' | 'status' | 'avatarETag'>>[]- The
IUser.statusfield is typed asUserStatus(the enum defined inpackages/core-typings/src/UserStatus.ts)- TypeScript enforces this type at compile time, so
user.statusis guaranteed to be a validUserStatusenum valueNo additional type guards or runtime validation are needed. The existing type definitions already ensure type safety.
Likely an incorrect or invalid review comment.
gabriellsh
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/ui-voip/src/v2/useMediaSession.ts (3)
1-3: Tidy imports after reducer cleanupIf you remove payload.status above, UserStatus becomes unused here. Drop it to satisfy linters.
-import { UserStatus } from '@rocket.chat/core-typings'; import { MediaSignalingSession, CallState, CallRole } from '@rocket.chat/media-signaling'; -import { useUserAvatarPath, useUserPresence } from '@rocket.chat/ui-contexts'; +import { useUserAvatarPath, useUserPresence } from '@rocket.chat/ui-contexts';
303-308: Gate presence to user peers and avoid name shadowingPass userId separately, rename local var, and only inject status for user peers. Prevents accidental SIP presence and clarifies intent.
- const status = useUserPresence(mediaSession.peerInfo && 'userId' in mediaSession.peerInfo ? mediaSession.peerInfo.userId : undefined); - - const peerInfo = useMemo(() => { - return mediaSession.peerInfo ? { ...mediaSession.peerInfo, status: status?.status } : undefined; - }, [mediaSession.peerInfo, status]); + const userId = mediaSession.peerInfo && 'userId' in mediaSession.peerInfo ? mediaSession.peerInfo.userId : undefined; + const presence = useUserPresence(userId); + + const peerInfo = useMemo(() => { + if (!mediaSession.peerInfo) { + return undefined; + } + if ('userId' in mediaSession.peerInfo) { + return { ...mediaSession.peerInfo, status: presence?.status }; + } + return mediaSession.peerInfo; + }, [mediaSession.peerInfo, presence?.status]);Please confirm that useUserPresence(undefined) is a no-op and does not return the current user’s presence.
311-314: Avoid broad 'as MediaSession' assertionLet TS check the shape; assign to a typed const and return it.
- ...mediaSession, - peerInfo, - ...cbs, - } as MediaSession; + ...mediaSession, + peerInfo, + ...cbs, + } as const satisfies MediaSession; // if TS 4.9+; otherwise: + // const result: MediaSession = { ...mediaSession, peerInfo, ...cbs }; + // return result;If your TS version < 4.9, use the commented variant with an explicit const result: MediaSession.
📜 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)
packages/ui-voip/src/v2/__snapshots__/MediaCallWidget.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
packages/ui-voip/src/v2/MediaCallProvider.tsx(1 hunks)packages/ui-voip/src/v2/components/PeerAutocomplete.tsx(3 hunks)packages/ui-voip/src/v2/useMediaSession.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ui-voip/src/v2/MediaCallProvider.tsx
- packages/ui-voip/src/v2/components/PeerAutocomplete.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui-voip/src/v2/useMediaSession.ts (2)
packages/ui-voip/src/v2/useMediaSessionInstance.ts (1)
SessionInfo(31-31)packages/ui-contexts/src/index.ts (1)
useUserPresence(96-96)
⏰ 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
3 participants
Proposed changes (including videos or screenshots)
Issue(s)
VGA-17
Steps to test or reproduce
Further comments
Summary by CodeRabbit