feat: Identify call transfer requester on call widget #37207
Conversation
…uested a call transfer.
…dentify-who-transferred
…ansferred' into feat/voipTransferViews
|
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: f317acd The changes in this PR will be included in the next version bump. This PR includes changesets to release 43 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 a transferredBy field through media session state and context, exposes it via MediaCallProvider/Mock, introduces Incoming/Outgoing transfer UI components and stories, and adds English i18n keys and a changeset describing the update. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Engine as Media Session Engine
participant Hook as useMediaSession
participant Provider as MediaCallProvider
participant Widget as MediaCallWidget
participant InView as IncomingCallTransfer
participant OutView as OutgoingCallTransfer
Engine->>Hook: mainCall updates (state, peerInfo, transferredBy)
Hook-->>Provider: session { state, peerInfo, transferredBy, ... }
Provider-->>Widget: context { transferredBy, state, ... }
alt ringing & transferredBy present
Widget->>InView: render(transferredBy, peerInfo)
InView->>Provider: onAccept / onEndCall
else calling & transferredBy present
Widget->>OutView: render(transferredBy, peerInfo)
OutView->>Provider: onEndCall
else no transfer
Widget->>Widget: render standard Incoming/Outgoing call views
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
WalkthroughAdds “transferred by” support to the VoIP widget: extends session/context types with transferredBy, adds i18n strings, updates provider and widget to conditionally render new Incoming/Outgoing Call Transfer views, introduces corresponding components and Storybook stories, and re-exports new views. Includes a changeset for minor version bumps. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Widget as MediaCallWidget
participant Ctx as MediaCallContext
participant Sess as useMediaSession
participant VIn as IncomingCallTransfer
participant VOut as OutgoingCallTransfer
User->>Widget: Open call UI
Widget->>Ctx: useMediaCallContext()
Ctx->>Sess: Session state (state, connectionState, transferredBy, peerInfo)
alt state == "ringing"
alt transferredBy present
Widget->>VIn: Render IncomingCallTransfer
Note right of VIn: Shows "Incoming call transfer"\n"From {{from}}"
else
Widget-->>User: Render IncomingCall
end
else state == "calling"
alt transferredBy present
Widget->>VOut: Render OutgoingCallTransfer
Note right of VOut: Shows "Transferring call"\n"From {{from}}"
else
Widget-->>User: Render OutgoingCall
end
else
Widget-->>User: Render existing states (unchanged)
end
VIn->>Ctx: onAccept() / onEndCall()
VOut->>Ctx: onEndCall()
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 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)
.changeset/heavy-rules-fry.md(1 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/ui-voip/src/v2/MediaCallContext.ts(2 hunks)packages/ui-voip/src/v2/MediaCallProvider.tsx(1 hunks)packages/ui-voip/src/v2/MediaCallWidget.tsx(2 hunks)packages/ui-voip/src/v2/MockedMediaCallProvider.tsx(2 hunks)packages/ui-voip/src/v2/useMediaSession.ts(3 hunks)packages/ui-voip/src/v2/useMediaSessionInstance.ts(1 hunks)packages/ui-voip/src/v2/views/IncomingCallTransfer.stories.tsx(1 hunks)packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx(1 hunks)packages/ui-voip/src/v2/views/OutgoingCallTransfer.stories.tsx(1 hunks)packages/ui-voip/src/v2/views/OutgoingCallTransfer.tsx(1 hunks)packages/ui-voip/src/v2/views/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
packages/ui-voip/src/v2/useMediaSessionInstance.ts (1)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
PeerInfo(22-22)
packages/ui-voip/src/v2/views/OutgoingCallTransfer.tsx (1)
packages/media-signaling/src/lib/Call.ts (1)
transferredBy(83-85)
packages/ui-voip/src/v2/useMediaSession.ts (1)
packages/media-signaling/src/lib/Call.ts (2)
transferredBy(83-85)contact(77-79)
packages/ui-voip/src/v2/MediaCallWidget.tsx (2)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
useMediaCallContext(115-124)packages/media-signaling/src/lib/Call.ts (1)
transferredBy(83-85)
packages/ui-voip/src/v2/views/IncomingCallTransfer.stories.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
packages/ui-voip/src/v2/views/OutgoingCallTransfer.stories.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (1)
packages/media-signaling/src/lib/Call.ts (1)
transferredBy(83-85)
packages/ui-voip/src/v2/MockedMediaCallProvider.tsx (1)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
State(24-24)
🔇 Additional comments (15)
.changeset/heavy-rules-fry.md (1)
1-6: LGTM!The changeset follows the correct format and appropriately marks this as a minor version bump for both affected packages. The description clearly captures the feature addition.
packages/ui-voip/src/v2/MediaCallContext.ts (2)
31-31: LGTM!The
transferredByfield is correctly typed asstring | undefinedand appropriately placed within theMediaCallContextType. This aligns with the PR objective to track who requested the call transfer.
64-64: LGTM!The default value initialization is correct and consistent with other optional fields in the context.
packages/ui-voip/src/v2/MediaCallProvider.tsx (1)
230-230: LGTM!The
transferredByfield is correctly propagated from the session state to the context value, following the established pattern for other session properties.packages/ui-voip/src/v2/views/index.ts (1)
5-6: LGTM!The new exports follow the established pattern and cleanly extend the public API to include the transfer-related views.
packages/ui-voip/src/v2/MediaCallWidget.tsx (3)
2-2: LGTM!The import correctly adds the new transfer-related view components.
5-5: LGTM!Correctly destructures
transferredByfrom the context to enable conditional rendering of transfer views.
17-25: LGTM!The conditional rendering logic is well-structured and follows a clear pattern:
- When
transferredByis present, render the transfer-specific view- Otherwise, fall back to the standard view
This approach ensures transfer scenarios are properly visualized while maintaining backward compatibility.
packages/ui-voip/src/v2/views/OutgoingCallTransfer.stories.tsx (1)
1-30: LGTM!The Storybook configuration is well-structured:
- Translation mocks cover the necessary i18n keys for the transfer UI
- The
MediaCallProviderMockis correctly configured withtransferredBy='Joy'to simulate a transfer scenario- Decorators are properly layered to provide all required contexts
- Story structure follows Storybook best practices
packages/ui-voip/src/v2/MockedMediaCallProvider.tsx (2)
9-17: LGTM!The component signature correctly extends to accept the optional
transferredByprop with appropriate typing and default value.
118-118: LGTM!The
transferredByprop is correctly passed through to the context value, enabling test scenarios for call transfers.packages/ui-voip/src/v2/useMediaSessionInstance.ts (1)
14-15: LGTM!The type changes are appropriate:
- Making
peerInfoexplicitlyPeerInfo | undefined(rather than optional) improves type clarity- Adding
transferredBy: string | undefinedaligns with the feature requirements- Both changes maintain consistency with how
CallSessionrefines these types for active call statespackages/ui-voip/src/v2/useMediaSession.ts (1)
12-12: LGTM! TransferredBy integration is well-implemented.The addition of
transferredByto the session state is correctly initialized, derived fromCallContact, and consistently propagated through both SIP and non-SIP contact paths. The fallback logic (displayName || username || undefined) ensures a meaningful display value.Also applies to: 131-141, 165-165
packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (1)
31-31: Consider error handling for async accept.The
voidoperator intentionally discards the promise returned byonAccept(). If the accept operation can fail, the user won't see any feedback.Verify whether
onAcceptcan throw or reject, and if so, consider adding error handling:onClick={async () => { try { await onAccept(); } catch (error) { console.error('Failed to accept call:', error); // Consider showing user feedback } }}packages/ui-voip/src/v2/views/IncomingCallTransfer.stories.tsx (1)
7-26: Story setup looks solidNice job wiring the translation mock and provider decorator; the story renders with realistic copy and context, so it should mirror the runtime behavior well.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
packages/ui-voip/src/v2/views/IncomingCallTransfer.stories.tsx (1)
16-27: Consider settingstate='ringing'in the story decorator.The
MediaCallProviderMockdefaultsstateto'closed', butIncomingCallTransferis intended to be rendered when the state is'ringing'(based on the MediaCallWidget rendering logic). For a more accurate demonstration of the component in its expected context, consider addingstate='ringing'to the provider props.Apply this diff:
decorators: [ mockedContexts, (Story) => ( - <MediaCallProviderMock transferredBy='Jason'> + <MediaCallProviderMock state='ringing' transferredBy='Jason'> <Story /> </MediaCallProviderMock> ),packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (1)
7-7: Rename component to match filename and export.The component is named
IncomingCallbut the filename and export name isIncomingCallTransfer. This naming inconsistency can cause confusion.Apply this diff:
-const IncomingCall = () => { +const IncomingCallTransfer = () => {And update the export:
-export default IncomingCall; +export default IncomingCallTransfer;packages/ui-voip/src/v2/views/OutgoingCallTransfer.tsx (1)
21-21: Consider using a more specific translation key for the connecting state.The translation key
meteor_status_connectingappears to be a generic status key (possibly from Meteor legacy). For consistency with the new transfer feature, consider using a more specific key likeConnecting_callor similar.
📜 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)
.changeset/heavy-rules-fry.md(1 hunks)packages/i18n/src/locales/en.i18n.json(1 hunks)packages/ui-voip/src/v2/MediaCallContext.ts(2 hunks)packages/ui-voip/src/v2/MediaCallProvider.tsx(1 hunks)packages/ui-voip/src/v2/MediaCallWidget.tsx(2 hunks)packages/ui-voip/src/v2/MockedMediaCallProvider.tsx(2 hunks)packages/ui-voip/src/v2/useMediaSession.ts(3 hunks)packages/ui-voip/src/v2/useMediaSessionInstance.ts(1 hunks)packages/ui-voip/src/v2/views/IncomingCallTransfer.stories.tsx(1 hunks)packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx(1 hunks)packages/ui-voip/src/v2/views/OutgoingCallTransfer.stories.tsx(1 hunks)packages/ui-voip/src/v2/views/OutgoingCallTransfer.tsx(1 hunks)packages/ui-voip/src/v2/views/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
packages/ui-voip/src/v2/views/OutgoingCallTransfer.tsx (1)
packages/media-signaling/src/lib/Call.ts (1)
transferredBy(83-85)
packages/ui-voip/src/v2/views/IncomingCallTransfer.stories.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
packages/ui-voip/src/v2/MockedMediaCallProvider.tsx (1)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
State(24-24)
packages/ui-voip/src/v2/useMediaSessionInstance.ts (1)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
PeerInfo(22-22)
packages/ui-voip/src/v2/useMediaSession.ts (1)
packages/media-signaling/src/lib/Call.ts (2)
transferredBy(83-85)contact(77-79)
packages/ui-voip/src/v2/views/OutgoingCallTransfer.stories.tsx (1)
packages/mock-providers/src/index.ts (1)
mockAppRoot(3-3)
packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (1)
packages/media-signaling/src/lib/Call.ts (1)
transferredBy(83-85)
packages/ui-voip/src/v2/MediaCallWidget.tsx (2)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
useMediaCallContext(115-124)packages/media-signaling/src/lib/Call.ts (1)
transferredBy(83-85)
🔇 Additional comments (4)
packages/ui-voip/src/v2/useMediaSession.ts (1)
131-135: LGTM!The derivation of
transferredBycorrectly handles null/undefined cases with optional chaining and provides a sensible fallback hierarchy (displayName → username → undefined).packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (1)
22-22: VerifytransferredByis always defined when renderingIncomingCallTransfer. Ensure any parent (e.g. MediaCallWidget) only renders it with a non-nulltransferredBy, or add a fallback (e.g.'') to avoid “undefined” showing in the UI.packages/ui-voip/src/v2/MediaCallWidget.tsx (1)
2-2: LGTM! Transfer-aware routing implemented correctly.The conditional rendering based on
transferredBycorrectly routes to the new transfer views (IncomingCallTransferandOutgoingCallTransfer) when a transfer is active. The logic is clear and follows the existing pattern in the component.Also applies to: 5-5, 17-24
packages/ui-voip/src/v2/views/OutgoingCallTransfer.stories.tsx (1)
28-30: LGTM! Storybook story setup is correct.The story correctly wraps the component with mocked providers and supplies the necessary
transferredByprop for testing the transfer scenario.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37207 +/- ##
===========================================
+ Coverage 67.56% 67.62% +0.06%
===========================================
Files 3295 3295
Lines 112652 112597 -55
Branches 20450 20435 -15
===========================================
+ Hits 76110 76143 +33
+ Misses 33869 33784 -85
+ Partials 2673 2670 -3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
pierre-lehnen-rc
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 (1)
packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (1)
19-19: Consider including the ellipsis in the translation key.The ellipsis (
...) is hardcoded outside the translation string, which means it won't be localized if different cultures require different formatting for "in progress" indicators. While this is a minor issue, it's generally better to include all punctuation within translation keys or use a separate key for the formatted version.Consider one of these approaches:
Option 1: Include ellipsis in the translation key:
-<WidgetHeader title={`${t('Transferring_call_incoming')}...`}> +<WidgetHeader title={t('Transferring_call_incoming_ellipsis')}>Option 2: Use i18next interpolation if you need flexibility:
-<WidgetHeader title={`${t('Transferring_call_incoming')}...`}> +<WidgetHeader title={t('Transferring_call_incoming_in_progress')}>
📜 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/MediaCallWidget.stories.tsx(1 hunks)packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx(1 hunks)packages/ui-voip/src/v2/views/OutgoingCallTransfer.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui-voip/src/v2/views/OutgoingCallTransfer.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (2)
packages/ui-voip/src/v2/MediaCallWidget.stories.tsx (1)
IncomingCallTransfer(66-71)packages/media-signaling/src/lib/Call.ts (1)
transferredBy(83-85)
⏰ 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 (1)
packages/ui-voip/src/v2/views/IncomingCallTransfer.tsx (1)
7-40: LGTM! Previous review comments have been addressed.The component is now correctly named
IncomingCallTransferto match the filename, and thetransferredByfield is properly handled with conditional rendering using the&&operator. The overall structure is clean and follows React best practices.
Proposed changes (including videos or screenshots)
Issue(s)
VGA-9
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
Localization
Documentation