-
Notifications
You must be signed in to change notification settings - Fork 44
chore(llc): update callkit_incoming dependency #1034
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
Conversation
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant CallEvent
participant Extension
participant CallKitEvent
CallEvent->>Extension: toCallKitEvent()
alt Event.actionCallConnected
Extension->>CallKitEvent: ActionCallConnected(data)
else Other events
Extension->>CallKitEvent: Other CallKitEvent
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
⏰ 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)
✨ Finishing Touches🧪 Generate unit tests
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 (
|
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: 1
🧹 Nitpick comments (1)
packages/stream_video/lib/src/push_notification/call_kit_events.dart (1)
134-143
: Fix the documentation comment.The class structure correctly follows the established pattern, but the documentation comment on line 138 appears to be copied from
ActionCallCallback
and is incorrect.- /// The call data associated with the call that was called back. + /// The call data associated with the call that was connected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/stream_video/lib/src/push_notification/call_kit_events.dart
(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/StreamVideoPushNotificationPlugin.kt
(2 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
(1 hunks)packages/stream_video_push_notification/pubspec.yaml
(1 hunks)
🔇 Additional comments (3)
packages/stream_video_push_notification/pubspec.yaml (1)
19-19
: Dependency version verification passedVersion 2.5.5 of
flutter_callkit_incoming
exists on pub.dev and is currently the latest release. Its Dart (>=2.18.0 <4.0.0) and Flutter (>=1.20.0) constraints align with our project’s requirements. No further action needed.packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/StreamVideoPushNotificationPlugin.kt (2)
16-16
: LGTM! Import added for shared instance access.The import enables access to the shared
CallkitNotificationManager
instance fromFlutterCallkitIncomingPlugin
.
31-31
: Excellent improvement in resource management.The updated initialization logic correctly prioritizes reusing an existing
CallkitNotificationManager
instance fromFlutterCallkitIncomingPlugin
before creating a new one. This prevents resource duplication and potential conflicts between plugin instances.
@@ -557,6 +557,7 @@ extension on CallEvent { | |||
Event.actionCallEnded => ActionCallEnded(data: toCallData()), | |||
Event.actionCallTimeout => ActionCallTimeout(data: toCallData()), | |||
Event.actionCallCallback => ActionCallCallback(data: toCallData()), | |||
Event.actionCallConnected => ActionCallConnected(data: toCallData()), |
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.
💡 Verification agent
🧩 Analysis chain
LGTM! Event mapping follows established pattern.
The addition of Event.actionCallConnected => ActionCallConnected(data: toCallData())
is consistent with the existing event mapping pattern and correctly extracts call data.
🏁 Script executed:
#!/bin/bash
# Description: Verify ActionCallConnected class is defined and imported
# Search for ActionCallConnected class definition
ast-grep --pattern 'class ActionCallConnected extends CallKitEvent {
$$$
}'
# Check if it's imported in the current file
rg -A 2 -B 2 "ActionCallConnected" packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
Length of output: 514
🏁 Script executed:
#!/bin/bash
# Search for definition of ActionCallConnected across the repository
rg -n "class ActionCallConnected"
# Search for any references to ActionCallConnected to locate its definition context
rg -n "ActionCallConnected"
Length of output: 749
🏁 Script executed:
#!/bin/bash
# Verify that ActionCallConnected (and CallKitEvent) are imported in the Push Notification file
rg -n "import .*call_kit_events" packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
# Show the first 30 lines to confirm imports
head -n 30 packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
Length of output: 1393
🏁 Script executed:
#!/bin/bash
# Show exports in the main entrypoint of stream_video package
head -n 200 packages/stream_video/lib/stream_video.dart
# Specifically look for exporting call_kit_events
rg -n "export .*call_kit_events" packages/stream_video/lib/stream_video.dart
Length of output: 2138
Add missing import for CallKit events
It looks like the new ActionCallConnected
(and the other ActionCall*
classes) are defined in
package:stream_video/src/push_notification/call_kit_events.dart
but that file isn’t imported. Without it the switch case won’t compile.
Please update packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
to include:
import 'package:stream_video_push_notification/stream_video_push_notification_platform_interface.dart';
+import 'package:stream_video/src/push_notification/call_kit_events.dart';
import 'stream_video_push_params.dart';
🤖 Prompt for AI Agents
In
packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
at line 560, the switch case uses ActionCallConnected and other ActionCall*
classes which are defined in
package:stream_video/src/push_notification/call_kit_events.dart but this file is
not imported. To fix this, add an import statement for call_kit_events.dart at
the top of the file to ensure these classes are recognized and the code compiles
correctly.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1034 +/- ##
========================================
- Coverage 4.63% 4.63% -0.01%
========================================
Files 574 574
Lines 38635 38638 +3
========================================
Hits 1789 1789
- Misses 36846 36849 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary by CodeRabbit
flutter_callkit_incoming
package, including Android 14 fixes for ringing notifications and lock screen handling.