fix(llc, push): some fixes to ringing flow #1027
Conversation
WalkthroughA guard clause was added to prevent reconnect attempts during an ongoing join operation in call logic. Caller display name selection was improved to avoid empty names in notifications. An unused import was removed, and a dependency was pinned to a specific version. iOS notification logic now skips empty display names. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant Call
participant Logger
App->>Call: Attempt _reconnect()
Call->>Call: Check if _callJoinLock is held
alt Lock is held
Call->>Logger: Log warning
Call-->>App: Return early, skip reconnect
else Lock is not held
Call->>Call: Proceed with reconnect logic
end
sequenceDiagram
participant Notification
participant Logic
Notification->>Logic: Prepare caller display name
Logic->>Logic: Check if callDisplayName is non-empty
alt callDisplayName is non-empty
Logic-->>Notification: Use callDisplayName
else
Logic->>Logic: Use createdByName if non-empty, else default text
Logic-->>Notification: Use fallback name
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/stream_video_push_notification/pubspec.yaml (1)
19-19: Exact version pin may impede future patch fixesLocking
flutter_callkit_incomingto2.5.2forfeits automatic bug-/security-fix uptake (e.g., 2.5.x). Consider caret (^2.5.2) and rely onpubspec.lockfor reproducibility unless a strict pin is required for compatibility.packages/stream_video/lib/src/stream_video.dart (2)
909-912: Trim whitespace to avoid “blank-but-non-empty” caller names
String.isNotEmptytreats" "(spaces) as valid content, so the current guard can still surface a visually empty caller name.
A more robust check would trim the string first:- nameCaller: (callDisplayName?.isNotEmpty ?? false) - ? callDisplayName - : createdByName, + nameCaller: (callDisplayName?.trim().isNotEmpty ?? false) + ? callDisplayName!.trim() + : createdByName,This prevents notifications that show only whitespace.
932-935: DRY: extract the display-name fallback into a local variableThe same ternary expression appears twice (missed & incoming). Computing it once improves readability and removes duplication:
+ final callerName = (callDisplayName?.trim().isNotEmpty ?? false) + ? callDisplayName!.trim() + : createdByName; + ... - nameCaller: (callDisplayName?.isNotEmpty ?? false) - ? callDisplayName - : createdByName, + nameCaller: callerName,(Apply similarly in the missed-call branch above.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/stream_video/lib/src/call/call.dart(1 hunks)packages/stream_video/lib/src/call/state/mixins/state_coordinator_mixin.dart(0 hunks)packages/stream_video/lib/src/stream_video.dart(2 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoPKDelegateManager.swift(1 hunks)packages/stream_video_push_notification/pubspec.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- packages/stream_video/lib/src/call/state/mixins/state_coordinator_mixin.dart
🔇 Additional comments (1)
packages/stream_video_push_notification/ios/Classes/StreamVideoPKDelegateManager.swift (1)
96-103: Caller-name fallback logic looks good – empty strings are now skipped, preventing blank CallKit titles. No further issues spotted.
| if (_callJoinLock.inLock) { | ||
| _logger.w(() => '[_reconnect] skipping reconnect (join in progress)'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Broken property: _callJoinLock.inLock does not exist
Lock exposes locked, not inLock. The code won’t compile.
- if (_callJoinLock.inLock) {
+ if (_callJoinLock.locked) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (_callJoinLock.inLock) { | |
| _logger.w(() => '[_reconnect] skipping reconnect (join in progress)'); | |
| return; | |
| } | |
| if (_callJoinLock.locked) { | |
| _logger.w(() => '[_reconnect] skipping reconnect (join in progress)'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In packages/stream_video/lib/src/call/call.dart around lines 1236 to 1239, the
property `_callJoinLock.inLock` is incorrect because the `Lock` class uses
`locked` to indicate if it is locked. Replace `_callJoinLock.inLock` with
`_callJoinLock.locked` to fix the compilation error.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1027 +/- ##
========================================
+ Coverage 4.24% 4.40% +0.16%
========================================
Files 574 574
Lines 38526 38589 +63
========================================
+ Hits 1634 1700 +66
+ Misses 36892 36889 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
Bug Fixes
Chores