fix: Filter out current peer and users without extension when sip is forced #37320
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 9a0661d 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 SIP-routing-aware filtering to voice call peer autocomplete: extracts peer extension, conditionally augments the autocomplete query to exclude the current call peer and users without extensions when forced SIP routing is enabled, and includes a changeset documenting the fix. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as MediaCallProvider
participant Settings as useSetting
participant Session as useMediaSession
participant API as usersAutoCompleteEndpoint
UI->>Settings: read VoIP_TeamCollab_SIP_Integration_For_Internal_Calls
UI->>Session: getExtensionFromPeerInfo(session.peerInfo)
alt forceSIPRouting && peerExtension exists
UI->>API: query(users, conditions: { extension: peerExtension, excludeCurrentPeer, requireExtension })
else not forced or no extension
UI->>API: query(users, standard params)
end
API-->>UI: autocomplete results (already filtered by backend when conditions provided)
UI->>UI: present filtered options (ensures current peer excluded)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ 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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37320 +/- ##
===========================================
- Coverage 68.11% 68.07% -0.05%
===========================================
Files 3364 3364
Lines 115749 115751 +2
Branches 20907 20864 -43
===========================================
- Hits 78847 78800 -47
- Misses 34212 34259 +47
- Partials 2690 2692 +2
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: 0
🧹 Nitpick comments (1)
.changeset/great-apes-pay.md (1)
1-5: Consider clarifying the changeset description.The description states filtering occurs "in case of forced SIP routing," but the implementation has two distinct conditions:
- Current peer filtered by extension whenever the peer has an extension (not just when SIP routing is forced)
- Users without extensions filtered only when SIP routing is forced
For precision, consider:
"Fixes an issue in the voice call peer selection field (on the widget and transfer modal) by filtering out the current peer (by extension when available) and users without assigned extensions (when SIP routing is forced)."
📜 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 (3)
.changeset/great-apes-pay.md(1 hunks)packages/ui-voip/src/v2/MediaCallProvider.tsx(4 hunks)packages/ui-voip/src/v2/useMediaSession.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/ui-voip/src/v2/useMediaSession.ts (1)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
PeerInfo(22-22)
packages/ui-voip/src/v2/MediaCallProvider.tsx (2)
packages/ui-contexts/src/index.ts (1)
useSetting(69-69)packages/ui-voip/src/v2/useMediaSession.ts (1)
getExtensionFromPeerInfo(36-46)
🔇 Additional comments (4)
packages/ui-voip/src/v2/useMediaSession.ts (1)
36-46: LGTM! Clean helper function for extension extraction.The function correctly extracts extension information from
PeerInfowith appropriate priority:callerId(internal user extension) beforenumber(external SIP contact). The type guards and return logic are sound.packages/ui-voip/src/v2/MediaCallProvider.tsx (3)
12-12: LGTM! Required imports for the new filtering logic.The new imports are correctly utilized:
useSettingfor the SIP routing configuration (line 49) andgetExtensionFromPeerInfofor extracting extension identifiers (line 208).Also applies to: 22-22
49-49: LGTM! Correctly retrieves the SIP routing configuration.The setting
VoIP_TeamCollab_SIP_Integration_For_Internal_Callsappropriately controls when SIP-extension-based filtering is enforced.
206-239: Verify: Peer extension filtering applied even when SIP routing is not forced.The code filters the current peer by extension whenever
peerExtensionis available, regardless of theforceSIPRoutingsetting (line 211). The logic uses an OR condition that causes the peer to be excluded in this scenario, which appears broader than the stated PR objective of filtering "when SIP is forced."This might be intentional defensive coding to prevent peer self-transfer in all cases (reinforced by excluding
peerUsernamein the exceptions array), but no tests or documentation explain this rationale. Please confirm this behavior aligns with requirements.
tassoevan
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
dionisio-bot
bot
removed
the
stat: ready to merge
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (2)
packages/ui-voip/src/v2/MediaCallProvider.tsx(4 hunks)packages/ui-voip/src/v2/useMediaSession.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ui-voip/src/v2/MediaCallProvider.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
packages/ui-voip/src/v2/useMediaSession.ts (1)
packages/ui-voip/src/v2/MediaCallContext.ts (1)
PeerInfo(24-24)
⏰ 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
tassoevan
added
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
This additionally fixes an issue where if SIP routing is forced, it would be possible to transfer the call to the current peer.
Issue(s)
VGA-51
Steps to test or reproduce
Further comments
Summary by CodeRabbit