feat(llc): migrated callkit incoming into stream_video_push_notification #1044
Conversation
WalkthroughRename CallKit APIs/types to "Ringing", remove the flutter_callkit_incoming dependency, introduce an integrated push/ringing implementation across Dart/Android/iOS (native activities/services/receivers), add EventChannel-based platform wiring and typed RingingEvent models, introduce StreamVideoPushConfiguration, and migrate Android notification image loading from Picasso to Coil. Changes
Sequence Diagram(s)%%{init: {"themeVariables": {"actorBackground":"#E8F0FF","actorBorder":"#7AA2FF","actorText":"#042A5A","noteBackground":"#FFF7E6","noteBorder":"#FFD580"}}}%%
sequenceDiagram
autonumber
actor App
participant StreamVideo as StreamVideo (Dart)
participant PushPlatform as PushPlatform (Method/Event Channel)
participant AndroidNative as Android Native
participant iOSNative as iOS Native
participant OS
App->>StreamVideo: create(pushConfiguration)
StreamVideo->>PushPlatform: init(pushConfiguration)
OS-->>AndroidNative: incoming push
AndroidNative->>PushPlatform: emit RingingEvent(INCOMING, data)
PushPlatform->>StreamVideo: EventChannel emits RingingEvent
StreamVideo->>App: onRingingEvent callbacks
alt User accepts
App->>StreamVideo: startCall / setCallConnected
StreamVideo->>PushPlatform: startCall / setCallConnected
PushPlatform->>AndroidNative: start service / update notification
PushPlatform->>iOSNative: report via CallKit manager
else User declines or timeout
App->>PushPlatform: endCall / hideIncomingCall
PushPlatform->>AndroidNative: clear notification / stop service
PushPlatform->>iOSNative: end call via CallKit manager
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 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 @@
## main #1044 +/- ##
========================================
- Coverage 4.82% 4.82% -0.01%
========================================
Files 591 591
Lines 40402 40410 +8
========================================
Hits 1949 1949
- Misses 38453 38461 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart
Show resolved
Hide resolved
Brazol
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 90
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt (1)
16-38: Potential bitmap recycling issueThe bitmap recycling logic at line 22 recycles the input bitmap when it's different from
squaredBitmap. However, in Coil, the input bitmap is managed by the framework and shouldn't be manually recycled as this could cause crashes if Coil tries to reuse it.Remove the manual recycling of the input bitmap:
- if (squaredBitmap != input) { - input.recycle() - }The
BitmapPoolparameter should be used if you need to recycle bitmaps - Coil will handle the lifecycle properly.dogfooding/lib/app/app_content.dart (1)
49-56: Replace deprecatedobserveCallDeclinedCallKitEventwithobserveCallDeclinedRingingEvent
TheobserveCallDeclinedCallKitEvent()method is marked@Deprecated('Use observeCallDeclinedRingingEvent instead.')and simply forwards to the new API; update the call site indogfooding/lib/app/app_content.dart(line 49) to usestreamVideo.observeCallDeclinedRingingEvent()instead.packages/stream_video_push_notification/ios/Classes/StreamVideoPKDelegateManager.swift (1)
62-76: Avoid naming collisions with Foundation.Data.Using a custom type named Data invites confusion. Consider renaming to CallData (and mirror on Android for parity).
- let data: Data + let data: CallData - data = Data.init(args: configuration.toJSON()) + data = CallData.init(args: configuration.toJSON()) ... - data = Data.init(args: [String: Any]()) + data = CallData.init(args: [String: Any]())If renaming is too broad for this PR, at least qualify Foundation.Data where used elsewhere and document the aliasing.
packages/stream_video/lib/src/stream_video.dart (1)
705-719: Memory leak risk: subscription is not tracked or returned.The subscription created by
onRingingEventis not being tracked or returned, which could lead to memory leaks if the caller doesn't properly dispose of it.Return the subscription so it can be properly managed:
StreamSubscription<RingingEvent>? disposeAfterResolvingRinging({ void Function()? disposingCallback, }) { - return onRingingEvent( + final subscription = onRingingEvent( (event) { if (event is ActionCallAccept || event is ActionCallDecline || event is ActionCallTimeout || event is ActionCallEnded) { disposingCallback?.call(); dispose(); } }, ); + return subscription; }packages/stream_video_push_notification/lib/src/stream_video_push_params.dart (1)
83-98: Merge method has inconsistent null handling.The merge method returns
thiswhenotheris null, but doesn't handle the case where nested Android/iOS params might be null in the current instance while present inother.Improve the merge logic to handle all null cases:
@internal StreamVideoPushParams merge(StreamVideoPushParams? other) { if (other == null) return this; return copyWith( id: other.id, callerName: other.callerName, handle: other.handle, type: other.type, duration: other.duration, extra: other.extra, headers: other.headers, - android: android?.merge(other.android), - ios: ios?.merge(other.ios), + android: android?.merge(other.android) ?? other.android, + ios: ios?.merge(other.ios) ?? other.ios, ); }
♻️ Duplicate comments (1)
packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart (1)
6-6: Centralize json_serializable options in build.yaml (explicitToJson and include_if_null).Agree with the earlier suggestion: set
explicit_to_json: true(and considerinclude_if_null: false) globally to avoid per-class churn.Add to build.yaml:
targets: $default: builders: json_serializable: options: explicit_to_json: true include_if_null: falseAlso applies to: 47-47, 126-126
...eam/video/flutter/stream_video_flutter/service/notification/StreamNotificationBuilderImpl.kt
Show resolved
Hide resolved
...ndroid/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/AppUtils.kt
Show resolved
Hide resolved
...n/io/getstream/video/flutter/stream_video_push_notification/IncomingCallConnectionService.kt
Show resolved
Hide resolved
...n/io/getstream/video/flutter/stream_video_push_notification/IncomingCallConnectionService.kt
Show resolved
Hide resolved
packages/stream_video_push_notification/ios/stream_video_push_notification.podspec
Outdated
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart
Outdated
Show resolved
Hide resolved
packages/stream_video_push_notification/android/src/main/AndroidManifest.xml
Outdated
Show resolved
Hide resolved
...on/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/Call.kt
Show resolved
Hide resolved
...on/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/Call.kt
Outdated
Show resolved
Hide resolved
...on/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/Call.kt
Show resolved
Hide resolved
...on/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/Call.kt
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart
Show resolved
Hide resolved
packages/stream_video/lib/src/push_notification/push_notification_manager.dart
Show resolved
Hide resolved
packages/stream_video/lib/src/push_notification/push_notification_manager.dart
Show resolved
Hide resolved
packages/stream_video/lib/src/push_notification/push_notification_manager.dart
Show resolved
Hide resolved
...io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt
Show resolved
Hide resolved
...io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt
Show resolved
Hide resolved
...io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt
Outdated
Show resolved
Hide resolved
.../io/getstream/video/flutter/stream_video_push_notification/IncomingCallSoundPlayerManager.kt
Outdated
Show resolved
Hide resolved
.../io/getstream/video/flutter/stream_video_push_notification/IncomingCallSoundPlayerManager.kt
Outdated
Show resolved
Hide resolved
.../io/getstream/video/flutter/stream_video_push_notification/IncomingCallSoundPlayerManager.kt
Show resolved
Hide resolved
.../getstream/video/flutter/stream_video_push_notification/StreamVideoPushNotificationPlugin.kt
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 33
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/stream_video/lib/src/push_notification/call_kit_events.dart (4)
137-146: Incorrect docstring for ActionCallConnected.Says “called back” but this is the Connected event.
Apply:
- /// The call data associated with the call that was called back. + /// The call data associated with the call that connected.
190-211: Preserve Dmtf type name via a deprecated alias to ease migration.Many apps may still refer to ActionCallToggleDmtf; provide a typedef alias.
Apply:
class ActionCallToggleDtmf extends RingingEvent { @@ List<Object?> get props => [uuid, digits]; } + +@Deprecated('Use ActionCallToggleDtmf instead.') +typedef ActionCallToggleDmtf = ActionCallToggleDtmf;
248-257: Map in Equatable props: use deep-equality to avoid reference-only comparisons.Equatable will compare Map by identity; prefer deep hash to stabilize ==/hashCode.
Apply:
- @override - List<Object?> get props => [body]; + @override + List<Object?> get props => [ + body == null ? null : const DeepCollectionEquality().hash(body), + ];And ensure the library (push_notification_manager.dart) imports:
import 'package:collection/collection.dart' show DeepCollectionEquality;
301-309: Deep-compare CallData.extraData in Equatable props.Avoid identity-only Map equality which can cause hard-to-repro bugs.
Apply:
callerName, hasVideo, - extraData, + extraData == null ? null : const DeepCollectionEquality().hash(extraData), ];And ensure the library imports:
import 'package:collection/collection.dart' show DeepCollectionEquality;packages/stream_video_push_notification/lib/src/stream_video_push_params.dart (1)
83-98: Deep-merge maps to avoid discarding existing headers/extra.Current merge overwrites entire maps. Merge keys instead.
StreamVideoPushParams merge(StreamVideoPushParams? other) { if (other == null) return this; return copyWith( id: other.id, callerName: other.callerName, handle: other.handle, type: other.type, duration: other.duration, - extra: other.extra, - headers: other.headers, + extra: () { + final result = <String, dynamic>{}; + if (extra != null) result.addAll(extra!); + if (other.extra != null) result.addAll(other.extra!); + return result.isEmpty ? null : result; + }(), + headers: () { + final result = <String, dynamic>{}; + if (headers != null) result.addAll(headers!); + if (other.headers != null) result.addAll(other.headers!); + return result.isEmpty ? null : result; + }(), android: android?.merge(other.android) ?? other.android, ios: ios?.merge(other.ios) ?? other.ios, ); }
♻️ Duplicate comments (23)
packages/stream_video_push_notification/android/src/main/AndroidManifest.xml (1)
2-2: Duplicate POST_NOTIFICATIONS issue resolved.Only a single POST_NOTIFICATIONS declaration remains. Good cleanup.
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt (1)
52-57: Replace deprecated color getter.Use
ContextCompat.getColorfor API 23+.- rippleColor = typedArray.getColor( - R.styleable.ripple_relativeLayout_ripple_color, - resources.getColor(R.color.ripple_main_color) - ) + rippleColor = typedArray.getColor( + R.styleable.ripple_relativeLayout_ripple_color, + ContextCompat.getColor(context, R.color.ripple_main_color) + )Add import:
import androidx.core.content.ContextCompatpackages/stream_video_push_notification/ios/Classes/StringUtils.swift (3)
6-15: Remove hardcoded key/IV and set AES mode explicitly (CBC + PKCS7).Hardcoded secrets + implicit mode are insecure; also be explicit about block mode/padding. Thread key/IV from configuration and prefer per-message random IVs.
- func encrypt( - encryptionKey: String = "xrBixqjjMhHifSDgSJ8O4QJYMZ1UHs45", iv: String = "lmYSgP3vixDAiBzW" - ) -> String { - if let aes = try? AES(key: encryptionKey, iv: iv), - let encrypted = try? aes.encrypt([UInt8](self.utf8)) - { - return encrypted.toHexString() - } - return "" - } + func encrypt(encryptionKey: String, iv: String) -> String { + if let aes = try? AES( + key: Array(encryptionKey.utf8), + blockMode: CBC(iv: Array(iv.utf8)), + padding: .pkcs7 + ), let encrypted = try? aes.encrypt(Array(self.utf8)) { + return encrypted.toHexString() + } + return "" + } @@ - func decrypt( - encryptionKey: String = "xrBixqjjMhHifSDgSJ8O4QJYMZ1UHs45", iv: String = "lmYSgP3vixDAiBzW" - ) -> String { - if let aes = try? AES(key: encryptionKey, iv: iv), - let decrypted = try? aes.decrypt([UInt8](hex: self)) - { - return String(data: Foundation.Data(decrypted), encoding: .utf8) ?? "" - } - return "" - } + func decrypt(encryptionKey: String, iv: String) -> String { + if let aes = try? AES( + key: Array(encryptionKey.utf8), + blockMode: CBC(iv: Array(iv.utf8)), + padding: .pkcs7 + ), let decrypted = try? aes.decrypt([UInt8](hex: self)) { + return String(data: Foundation.Data(decrypted), encoding: .utf8) ?? "" + } + return "" + }Follow-up: If backward compatibility is required, we can keep deprecated wrappers that read defaults from a secure source while you roll out config-based keys. Want a patch?
Also applies to: 17-26
39-49: Deprecate defaulted wrappers and require explicit secrets.Keeping defaults encourages misuse. Mark these as requiring explicit params (or fetch from secure config/Keychain).
- public func encryptHandle( - encryptionKey: String = "xrBixqjjMhHifSDgSJ8O4QJYMZ1UHs45", iv: String = "lmYSgP3vixDAiBzW" - ) -> String { + public func encryptHandle(encryptionKey: String, iv: String) -> String { return self.encrypt(encryptionKey: encryptionKey, iv: iv).toBase64() } @@ - public func decryptHandle( - encryptionKey: String = "xrBixqjjMhHifSDgSJ8O4QJYMZ1UHs45", iv: String = "lmYSgP3vixDAiBzW" - ) -> String { + public func decryptHandle(encryptionKey: String, iv: String) -> String { return self.fromBase64().decrypt(encryptionKey: encryptionKey, iv: iv) }
51-66: Avoid crash on JSON parse; don’t print from library code.This force-unwrap can crash. Parse safely and return an empty map on failure. Also, avoid stdout logging here.
- if let data = self.decryptHandle().data(using: .utf8) { - do { - return try - (JSONSerialization.jsonObject(with: data, options: []) as? [String: Any])! - } catch { - print(error.localizedDescription) - } - } + if let data = self.decryptHandle( + encryptionKey: /* inject key */, iv: /* inject iv */ + ).data(using: .utf8) { + do { + if let json = try JSONSerialization.jsonObject(with: data, options: []) as? [String: Any] { + return json + } + } catch { + // Consider routing to a logger; ignore in release. + } + } return [:]Action: Thread encryptionKey/iv from configuration into this call (or store securely and read here).
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (2)
178-187: Null-safe JSON parsing for extra/type.Avoids cast errors when ‘extra’ is absent or not a Map.
- CallData _callDataFromJson(Map<String, dynamic> json) { - final extraData = json['extra']?.cast<String, dynamic>(); + CallData _callDataFromJson(Map<String, dynamic> json) { + final rawExtra = json['extra']; + final Map<String, dynamic>? extraData = + rawExtra is Map ? Map<String, dynamic>.from(rawExtra) : null; return CallData( uuid: json['id'] as String?, callCid: extraData?['callCid'] as String?, handle: json['handle'] as String?, callerName: json['callerName'] as String?, - hasVideo: json['type'] == 1, + hasVideo: (json['type'] is int) && (json['type'] == 1), extraData: extraData, ); }
190-229: Harden event parsing: guard unknown events and invalid bodies.Prevents stream errors on malformed inputs.
- RingingEvent? _receiveRingingEvent(dynamic data) { - if (data is Map) { - final event = Event.values.firstWhere((e) => e.name == data['event']); - final body = Map<String, dynamic>.from(data['body']); - final callData = _callDataFromJson(body); + RingingEvent? _receiveRingingEvent(dynamic data) { + try { + if (data is! Map) return null; + final eventName = data['event'] as String?; + final event = Event.values.where((e) => e.name == eventName).cast<Event?>().firstOrNull; + if (event == null) { + debugPrint('Unknown ringing event: $eventName'); + return null; + } + final rawBody = data['body']; + if (rawBody is! Map) return null; + final body = Map<String, dynamic>.from(rawBody as Map); + final callData = _callDataFromJson(body); @@ - return switch (event) { + return switch (event) { Event.actionCallIncoming => ActionCallIncoming(data: callData), @@ Event.actionDidUpdateDevicePushTokenVoip => ActionDidUpdateDevicePushTokenVoip( token: body['deviceTokenVoIP'] as String, ), Event.actionCallToggleHold => ActionCallToggleHold( uuid: body['id'] as String, isOnHold: body['isOnHold'] as bool, ), Event.actionCallToggleMute => ActionCallToggleMute( uuid: body['id'] as String, isMuted: body['isMuted'] as bool, ), Event.actionCallToggleDtmf => ActionCallToggleDtmf( uuid: body['id'] as String, digits: body['digits'] as String, ), Event.actionCallToggleGroup => ActionCallToggleGroup( uuid: body['id'] as String, callUUIDToGroupWith: body['callUUIDToGroupWith'] as String, ), Event.actionCallToggleAudioSession => ActionCallToggleAudioSession( isActivate: body['isActivate'] as Bool, ), Event.actionCallCustom => ActionCallCustom(body), }; - } - - return null; + } catch (e, st) { + debugPrint('Error parsing ringing event: $e\n$st'); + return null; + } }Note: If you want
firstOrNull, addpackage:collection/collection.dart. Otherwise, use a safefirstWherewithorElse: () => nullvia a helper.packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart (2)
30-45: merge() logic now preserves headers and platform configs — resolved.This addresses the earlier issue where headers were overwritten and other.android/ios were dropped when current was null. LGTM.
6-6: Consider moving explicitToJson to build.yaml.If you want this behavior repo-wide, set it in build.yaml to reduce per-class annotations.
Also applies to: 53-55, 133-134
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (1)
9-16: sharedInstance can be nil before register — add guards; fix activeCalls return type.Prevent crashes if static APIs are called pre-registration, and align activeCalls signature with the non-optional return.
- @objc public private(set) static var sharedInstance: StreamVideoPushNotificationPlugin! + @objc public private(set) static var sharedInstance: StreamVideoPushNotificationPlugin? @@ - @objc public static func setDevicePushTokenVoIP(deviceToken: String) { - sharedInstance.setDevicePushTokenVoIP(deviceToken: deviceToken) + @objc public static func setDevicePushTokenVoIP(deviceToken: String) { + guard let instance = sharedInstance else { + print("Warning: StreamVideoPushNotificationPlugin not initialized") + return + } + instance.setDevicePushTokenVoIP(deviceToken: deviceToken) //TODO: send event? //ACTION_DID_UPDATE_DEVICE_PUSH_TOKEN_VOIP } @@ - @objc public static func startOutgoingCall( + @objc public static func startOutgoingCall( data: CallData, fromPushKit: Bool ) { - sharedInstance.callKitManager.startCall(data, fromPushKit: fromPushKit) + guard let instance = sharedInstance else { + print("Warning: Plugin not initialized") + return + } + instance.callKitManager.startCall(data, fromPushKit: fromPushKit) } @@ - @objc public static func showIncomingCall( + @objc public static func showIncomingCall( data: CallData, fromPushKit: Bool ) { - sharedInstance.callKitManager.showIncomingCall( - data, fromPushKit: fromPushKit) + guard let instance = sharedInstance else { + print("Warning: Plugin not initialized") + return + } + instance.callKitManager.showIncomingCall(data, fromPushKit: fromPushKit) } @@ - @objc public static func activeCalls() -> [[String: Any]]? { - sharedInstance.callKitManager.activeCalls() + @objc public static func activeCalls() -> [[String: Any]] { + guard let instance = sharedInstance else { + print("Warning: Plugin not initialized") + return [] + } + return instance.callKitManager.activeCalls() }Also applies to: 61-79, 81-83
packages/stream_video_push_notification/ios/Classes/StreamVideoPushConfiguration.swift (1)
46-55: Safe casting fix looks good.Replacing the force-cast on args keys prevents runtime crashes. Nice.
packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart (1)
148-159: Fix iOS silenceEvents API mismatch and race; await and pass boolean flags.The platform Swift handler expects a boolean arg for "silenceEvents" and there’s no separate "unsilenceEvents" method. Current calls pass no args and aren’t awaited, so events aren’t actually silenced and races remain. Use explicit true/false and await (or wrap in a helper).
- await StreamVideoPushNotificationPlatform.instance - .silenceEvents(); + await StreamVideoPushNotificationPlatform.instance + .silenceEvents(true); await endCallByCid(event.callCid.toString()); await Future<void>.delayed(const Duration(milliseconds: 300)); - await StreamVideoPushNotificationPlatform.instance - .unsilenceEvents(); + await StreamVideoPushNotificationPlatform.instance + .silenceEvents(false);Add a scoped helper to avoid repetition and ensure proper finally semantics (place inside the class):
Future<void> _withSilencedEvents(Future<void> Function() action) async { await StreamVideoPushNotificationPlatform.instance.silenceEvents(true); try { await action(); } finally { await StreamVideoPushNotificationPlatform.instance.silenceEvents(false); } }packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (3)
518-521: Don’t call reportOutgoingCall in the incoming-call answer path.This is for outgoing calls only; remove to prevent incorrect CallKit state.
- call.hasConnectDidChange = { [weak self] in - self?.sharedProvider?.reportOutgoingCall( - with: call.uuid, connectedAt: call.connectedData) - } + call.hasConnectDidChange = nil
601-608: Use callUUID (not action.uuid) in timedOutPerforming.action.uuid is the action identifier, not the call id.
- public func provider(_ provider: CXProvider, timedOutPerforming action: CXAction) { - guard let call = self.callController.callWithUUID(uuid: action.uuid) else { + public func provider(_ provider: CXProvider, timedOutPerforming action: CXAction) { + let callUUID = (action as? CXCallAction)?.callUUID + guard let uuid = callUUID, + let call = self.callController.callWithUUID(uuid: uuid) else { action.fail() return } sendEvent(StreamVideoIncomingCallConstants.ACTION_CALL_TIMEOUT, self.data?.toJSON()) action.fulfill() }
619-624: Avoid double-adding outgoing call in didActivate.Call was added in CXStartCallAction; guard before add.
- self.outgoingCall?.startCall(withAudioSession: audioSession) { success in - if success { - self.callController.addCall(self.outgoingCall!) - self.outgoingCall?.startAudio() - } - } + self.outgoingCall?.startCall(withAudioSession: audioSession) { success in + if success, let oc = self.outgoingCall, + self.callController.callWithUUID(uuid: oc.uuid) == nil { + self.callController.addCall(oc) + } + self.outgoingCall?.startAudio() + }packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallSoundPlayerManager.kt (1)
155-158: Don’t swallow exceptions when resolving ringtone URIsLog the failure before falling back. This was flagged earlier.
- } catch (e: Exception) { - // If anything fails, try to return the system default ringtone - return getDefaultRingtoneUri() - } + } catch (e: Exception) { + Log.w("IncomingCallSound", "Failed to resolve ringtone '$fileName', using default", e) + return getDefaultRingtoneUri() + } @@ - } catch (e: Exception) { - // getActualDefaultRingtoneUri can throw an exception on some devices - // for custom ringtones - return getSafeSystemRingtoneUri() - } + } catch (e: Exception) { + // getActualDefaultRingtoneUri can throw on some devices for custom ringtones + Log.w("IncomingCallSound", "Failed to obtain actual default ringtone, using safe fallback", e) + return getSafeSystemRingtoneUri() + }Also applies to: 176-180
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/SharedPreferencesUtils.kt (2)
9-16: Make SharedPreferences init thread-safe and remove shared editorGlobal nullable prefs/editor with re-init on every call is racy and inefficient. Use a single, thread-safe instance and create editors per write.
-private const val INCOMING_CALL_PREFERENCES_FILE_NAME = "stream_video_incoming_call_preferences" -private var prefs: SharedPreferences? = null -private var editor: SharedPreferences.Editor? = null - -private fun initInstance(context: Context) { - prefs = context.getSharedPreferences(INCOMING_CALL_PREFERENCES_FILE_NAME, Context.MODE_PRIVATE) - editor = prefs?.edit() -} +private const val INCOMING_CALL_PREFERENCES_FILE_NAME = "stream_video_incoming_call_preferences" +@Volatile private var INSTANCE: SharedPreferences? = null +private val prefsLock = Any() + +private fun prefs(context: Context): SharedPreferences { + val appCtx = context.applicationContext + return INSTANCE ?: synchronized(prefsLock) { + INSTANCE ?: appCtx.getSharedPreferences( + INCOMING_CALL_PREFERENCES_FILE_NAME, Context.MODE_PRIVATE + ).also { INSTANCE = it } + } +}
46-50: Handle malformed JSON gracefullyJSON parsing can throw; return a safe default and log.
fun getDataActiveCalls(context: Context?): ArrayList<Data> { val json = getString(context, "ACTIVE_CALLS", "[]") - return Utils.getGsonInstance() - .readValue(json, object : TypeReference<ArrayList<Data>>() {}) + return try { + Utils.getGsonInstance() + .readValue(json, object : TypeReference<ArrayList<Data>>() {}) + } catch (e: Exception) { + Log.e("SharedPreferencesUtils", "Failed to parse ACTIVE_CALLS", e) + ArrayList() + } } @@ fun getDataActiveCallsForFlutter(context: Context?): ArrayList<Map<String, Any?>> { val json = getString(context, "ACTIVE_CALLS", "[]") - return Utils.getGsonInstance().readValue(json, object : TypeReference<ArrayList<Map<String, Any?>>>() {}) + return try { + Utils.getGsonInstance() + .readValue(json, object : TypeReference<ArrayList<Map<String, Any?>>>() {}) + } catch (e: Exception) { + Log.e("SharedPreferencesUtils", "Failed to parse ACTIVE_CALLS for Flutter", e) + ArrayList() + } }Also applies to: 52-55
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/Call.kt (3)
14-16: uuid incorrectly sourced from idRead uuid from args["uuid"] or generate one; don’t duplicate id.
@JsonProperty("uuid") -var uuid: String = (args["id"] as? String) ?: "" +var uuid: String = (args["uuid"] as? String) ?: java.util.UUID.randomUUID().toString()Add import if needed:
import java.util.UUID
164-168: Fix unsafe cast in equals and include fast pathAvoid ClassCastException; short-circuit on reference equality.
override fun equals(other: Any?): Boolean { - if (other == null) return false - val e: Data = other as Data - return this.id == e.id + if (this === other) return true + if (other !is Data) return false + return this.id == other.id }
297-301: Safely read Serializable extras/headersDirect cast can crash on unexpected types.
- data.extra = - bundle.getSerializable(IncomingCallConstants.EXTRA_CALL_EXTRA) as HashMap<String, Any?> - data.headers = - bundle.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as HashMap<String, Any?> + data.extra = + (bundle.getSerializable(IncomingCallConstants.EXTRA_CALL_EXTRA) as? HashMap<String, Any?>) + ?: HashMap() + data.headers = + (bundle.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as? HashMap<String, Any?>) + ?: HashMap()packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt (2)
184-185: Use Locale.ROOT in String.format for asset URIsPrevents locale-specific formatting issues. Previously flagged.
- String.format("file:///android_asset/flutter_assets/%s", avatarUrl) + String.format(java.util.Locale.ROOT, "file:///android_asset/flutter_assets/%s", avatarUrl) @@ - avatarUrl = String.format("file:///android_asset/flutter_assets/%s", avatarUrl) + avatarUrl = String.format(java.util.Locale.ROOT, "file:///android_asset/flutter_assets/%s", avatarUrl) @@ - avatarUrl = String.format("file:///android_asset/flutter_assets/%s", avatarUrl) + avatarUrl = String.format(java.util.Locale.ROOT, "file:///android_asset/flutter_assets/%s", avatarUrl)Also applies to: 270-270, 400-400
186-188: Safely cast headers from BundleAvoid ClassCastException when EXTRA_CALL_HEADERS isn’t a HashMap.
- val headers = - data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as HashMap<String, Any?> + val headers = + data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as? HashMap<String, Any?> ?: hashMapOf() @@ - val headers = - data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as HashMap<String, Any?> + val headers = + data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as? HashMap<String, Any?> ?: hashMapOf() @@ - val headers = - data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as HashMap<String, Any?> + val headers = + data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as? HashMap<String, Any?> ?: hashMapOf()Also applies to: 272-274, 402-404
🧹 Nitpick comments (40)
packages/stream_video/lib/src/push_notification/call_kit_events.dart (1)
235-245: Rename isActivate → isActive (add alias getter for BC).“isActivate” is unidiomatic; add a non-breaking alias getter and plan deprecation.
Apply:
/// Indicates whether the audio session is active. final bool isActivate; + + @Deprecated('Use isActive instead.') + bool get isActive => isActivate;packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt (3)
17-21: Drop theisRecycledguard.Coil shouldn’t hand a recycled bitmap to a
Transformation; this check can mask upstream bugs and returning a recycledinputis unsafe.Apply:
- if (input.isRecycled) return input + // input should be valid; no need to guard against recycled bitmaps.
12-14: Confirm Coil major version (key vs cacheKey, BitmapPool presence).This implementation matches Coil 1.x (
key()andBitmapPool). If you’re on Coil 2/3, migrate tooverride val cacheKeyandtransform(input, size)with no pool. This will also simplify future upgrades.References: Coil 2+ removes
BitmapPooland usescacheKey; transform signature istransform(input, size). (coil-kt.github.io)
10-49: Prefer the built-in CircleCropTransformation unless you need custom behavior.It’s well-tested, removes custom bitmap math, and stays current with Coil internals.
Replace usage of
CircleTransformwithcoil.transform.CircleCropTransformationin request builders, and delete this class. (coil-kt.github.io)packages/stream_video_push_notification/android/src/main/AndroidManifest.xml (1)
20-35: IncomingCallActivity launch/runtime tweaks (reduce restarts, improve lockscreen behavior).
- Add showWhenLocked="true" alongside turnScreenOn.
- Widen configChanges to avoid recreation on screenSize/keyboardHidden/uiMode changes.
- Consider singleInstancePerTask (API 31+) instead of deprecated singleInstance.
Apply:
<activity android:name="io.getstream.video.flutter.stream_video_push_notification.IncomingCallActivity" android:taskAffinity="io.getstream.video.INCOMING_CALL_AFFINITY" android:excludeFromRecents="true" android:noHistory="true" android:turnScreenOn="true" - android:configChanges="orientation" + android:showWhenLocked="true" + android:configChanges="orientation|keyboardHidden|screenSize|uiMode" android:exported="true" - android:launchMode="singleInstance" + android:launchMode="singleInstancePerTask" android:allowTaskReparenting="false" android:theme="@style/IncomingCallTheme" android:permission="${applicationId}.PERMISSION_CALL">If min/compile SDKs don’t support singleInstancePerTask, keep singleTask instead.
packages/stream_video_push_notification/android/src/main/res/layout-w600dp-land/activity_incoming_call.xml (3)
25-33: ID name suggests wrong type.
llBackgroundAnimationis aRippleRelativeLayout, not a LinearLayout. Consider renaming torlBackgroundAnimationfor clarity.
33-44: Clarify accessibility of the logo.If decorative, set
contentDescription="@null"andimportantForAccessibility="no". If meaningful, provide a localized description.<ImageView android:id="@+id/ivLogo" @@ - android:visibility="invisible" + android:visibility="invisible" + android:contentDescription="@null" + android:importantForAccessibility="no"
103-110: System window handling.
fitsSystemWindows="true"on the action bar can produce inconsistent insets. Prefer WindowInsets handling in code or dedicated inset padding resources for landscape w600dp.packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt (5)
77-81: Round dimensions correctly.Casting floats truncates. Use
roundToInt()for ripple size.- rippleParams = LayoutParams( - (2 * (rippleRadius)).toInt(), - (2 * (rippleRadius)).toInt() - ) + rippleParams = LayoutParams( + (2 * rippleRadius).roundToInt(), + (2 * rippleRadius).roundToInt() + )Add import:
import kotlin.math.roundToInt
112-121: Non-null context in inner view.
RippleViewdoesn’t need a nullableContext?.- private inner class RippleView(context: Context?) : View(context) { + private inner class RippleView(context: Context) : View(context) {
123-138: Reset child view state on start/stop.Ensure alpha/scale reset between runs to avoid stale visuals.
fun startRippleAnimation() { if (!isRippleAnimationRunning) { for (rippleView in rippleViewList) { - rippleView.visibility = VISIBLE + rippleView.apply { + visibility = VISIBLE + alpha = 1f + scaleX = 1f + scaleY = 1f + } } animatorSet.start() isRippleAnimationRunning = true } } fun stopRippleAnimation() { if (isRippleAnimationRunning) { - animatorSet.cancel() + animatorSet.cancel() + rippleViewList.forEach { + it.alpha = 1f + it.scaleX = 1f + it.scaleY = 1f + } isRippleAnimationRunning = false } }
52-70: Validate attribute inputs.Guard against invalid XML: negative/zero
duration, non-positiveradius/scale.require(rippleDurationTime > 0) { "ripple_duration must be > 0" } require(rippleRadius > 0f) { "ripple_radius must be > 0" } require(rippleScale > 1f) { "ripple_scale must be > 1" }
67-70: Styleable naming consistency.
ripple_relativeLayout_*mixes case; considerrippleRelativeLayout_*orRippleRelativeLayout_*per your conventions.packages/stream_video_push_notification/ios/Classes/StreamVideoIncomingCallConstants.swift (1)
3-3: Naming consistency with Ringing terminology.If the SDK is moving away from “CallKit” naming, consider aligning this doc comment with “Ringing” to avoid mixed terminology.
packages/stream_video_push_notification/ios/Classes/StringUtils.swift (1)
79-82: Base64 detection can misclassify non-UTF8 payloads.If the ciphertext ever decrypts to non-UTF8 (or handle includes bytes outside UTF-8),
fromBase64()will yield empty and this will return false. Consider a stricter base64 check (regex + decode) or carry an explicit flag in payload.packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (1)
41-43: Tighten return types (use Future where no value is used).Improves type safety and API clarity.
- Future showIncomingCall(StreamVideoPushParams params) async { + Future<void> showIncomingCall(StreamVideoPushParams params) async { @@ - Future showMissCallNotification(StreamVideoPushParams params) async { + Future<void> showMissCallNotification(StreamVideoPushParams params) async { @@ - Future hideIncomingCall(StreamVideoPushParams params) async { + Future<void> hideIncomingCall(StreamVideoPushParams params) async { @@ - Future startCall(StreamVideoPushParams params) async { + Future<void> startCall(StreamVideoPushParams params) async { @@ - Future muteCall(String id, {bool isMuted = true}) async { + Future<void> muteCall(String id, {bool isMuted = true}) async { @@ - Future holdCall(String id, {bool isOnHold = true}) async { + Future<void> holdCall(String id, {bool isOnHold = true}) async { @@ - Future endCall(String id) async { + Future<void> endCall(String id) async { @@ - Future setCallConnected(String id) async { + Future<void> setCallConnected(String id) async { @@ - Future endAllCalls() async { + Future<void> endAllCalls() async { @@ - Future silenceEvents() async { + Future<void> silenceEvents() async { @@ - Future unsilenceEvents() async { + Future<void> unsilenceEvents() async {Also applies to: 48-55, 60-66, 72-74, 80-83, 98-101, 107-109, 115-117, 121-123, 129-131, 137-139, 143-151
packages/stream_video_push_notification/ios/Classes/Call.swift (2)
131-131: SwiftLint: add a space after comment slashes.Conforms to
comment_spacing.[derived_from_static_analysis]
- //iOS + // iOS
12-13: Typo/consistency: rename hasConnectDidChange → hasConnectedDidChange.Matches the corresponding “hasConnected” property; reduces confusion.
packages/stream_video_push_notification/lib/src/stream_video_push_params.dart (2)
223-263: Parity check: missing includesCallsInRecents in IOSParams.iOS initializer reads this flag; Dart side can’t set it. If removal wasn’t intentional, add it to IOSParams and mapping.
150-164: Carry Android isImportant/isBot if part of configuration.If
AndroidPushConfigurationcan provide these, mirror them to avoid silent drops.packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart (2)
111-125: Deep-merge nested params in AndroidPushConfiguration.merge.Current merge overwrites nested objects wholesale. If the nested types support merge(), prefer fieldwise merging.
Apply:
AndroidPushConfiguration merge(AndroidPushConfiguration? other) { if (other == null) return this; - return copyWith( - missedCallNotification: other.missedCallNotification, - incomingCallNotification: other.incomingCallNotification, + return copyWith( + missedCallNotification: other.missedCallNotification != null + ? (missedCallNotification?.merge(other.missedCallNotification) ?? other.missedCallNotification) + : missedCallNotification, + incomingCallNotification: other.incomingCallNotification != null + ? (incomingCallNotification?.merge(other.incomingCallNotification) ?? other.incomingCallNotification) + : incomingCallNotification, defaultAvatar: other.defaultAvatar, ringtonePath: other.ringtonePath, incomingCallNotificationChannelName: other.incomingCallNotificationChannelName, missedCallNotificationChannelName: other.missedCallNotificationChannelName, showFullScreenOnLockScreen: other.showFullScreenOnLockScreen, ); }If merge() isn’t available on these types, we can add it or keep current behavior.
2-2: Use relative imports for push params types
Replace the umbrella package import at line 2 inpackages/stream_video_push_notification/lib/src/stream_video_push_configuration.dartwith a direct relative import of the params file, e.g.:import 'stream_video_push_params.dart';This avoids a self-import cycle and speeds up analysis.
packages/stream_video_push_notification/ios/Classes/StreamVideoPKDelegateManager.swift (2)
56-60: Remove unused state parameter.state is not used; drop it from the signature and call site for clarity.
- handleIncomingCall( - streamDict: streamDict, state: UIApplication.shared.applicationState, - completion: completion) + handleIncomingCall(streamDict: streamDict, completion: completion)And update the function:
- func handleIncomingCall( - streamDict: [String: Any]?, state: UIApplication.State, completion: @escaping () -> Void - ) { + func handleIncomingCall(streamDict: [String: Any]?, completion: @escaping () -> Void) {
97-104: Optionally propagate completion to CallKit callback.Calling completion immediately is acceptable, but passing it into showIncomingCall and invoking it after reportNewIncomingCall’s completion reduces races.
I can add a static overload on StreamVideoPushNotificationPlugin to accept a completion and wire it through to StreamVideoCallkitManager if you want.
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (2)
53-56: Remove unnecessary break statements.Swift doesn’t require break at the end of a case.
- case "getDevicePushTokenVoIP": - result(self.getDevicePushTokenVoIP()) - break + case "getDevicePushTokenVoIP": + result(self.getDevicePushTokenVoIP())
63-63: Resolve TODO or downgrade to a tracked task.Either emit the event here or remove the TODO to satisfy SwiftLint.
packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (3)
8-8: Remove redundant nil initialization.Optional vars are nil by default.
- private var sharedProvider: CXProvider? = nil + private var sharedProvider: CXProvider?
146-151: Drop unneeded breaks in switch.Minor SwiftLint cleanup.
- case "number": - typeDefault = CXHandle.HandleType.phoneNumber - break + case "number": + typeDefault = CXHandle.HandleType.phoneNumber case "email": typeDefault = CXHandle.HandleType.emailAddress
124-141: Comment spacing and TODOs.Fix “//” spacing and replace TODOs with tracked tasks or implement them.
Also applies to: 133-136
packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart (4)
407-410: Outdated comment references flutter_callkit_incoming.This code no longer depends on that package; update the comment to describe current behavior/reasoning or remove it.
- // This is a workaround for the issue in flutter_callkit_incoming - // where second CallKit call overrides data in showIncomingCall native method - // and it's not possible to end the call by callCid + // If multiple native ringing calls are stacked with identical metadata, + // ending by callCid may not be sufficient; fall back to endAllCalls.
291-295: Log stream creation: prefer share/multicast over recreating stream per getter.onCallEvent creates a new operator chain each access. If this getter is used widely, share the stream once to avoid duplicated listeners/work.
- Stream<RingingEvent> get onCallEvent { - return RingingEventBroadcaster() - .onEvent - .doOnData((event) => _logger.v(() => '[onCallEvent] event: $event')); - } + late final Stream<RingingEvent> _onCallEvent = + RingingEventBroadcaster().onEvent.doOnData( + (event) => _logger.v(() => '[onCallEvent] event: $event'), + ).share(); + Stream<RingingEvent> get onCallEvent => _onCallEvent;
479-507: Default config: consider externalizing user-facing strings and colors."Incoming Call", "Missed call", "Call back", and hex color are hardcoded. Move into app-localization/config or make them injectable in StreamVideoPushConfiguration.
521-556: RingingEventBroadcaster: guard against double-cancel and late events.
- _stopListenEvent cancels but doesn’t close the controller; events sent between cancel and nulling may be lost silently.
- Consider disposing the controller on last cancel, or track listener count to only tear down when zero.
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (3)
482-482: Fix MARK format for SwiftLint.Use a space after slashes.
- //MARK: - CXProviderDelegate + // MARK: - CXProviderDelegate
15-15: Remove redundant optional initialization.Swift defaults optionals to nil; satisfy SwiftLint.
- private var sharedProvider: CXProvider? = nil + private var sharedProvider: CXProvider?
45-162: Switch cases: multiple unneeded breaks.Clean them up to appease SwiftLint; no behavior change.
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallSoundPlayerManager.kt (2)
132-135: Replace printStackTrace with structured loggingUse Android logging; printing stack traces is noisy and hard to triage.
- } catch (e: Exception) { - e.printStackTrace() - } + } catch (e: Exception) { + Log.w("IncomingCallSound", "Failed to play ringtone", e) + }
22-23: Ensure cross-thread visibility of isPlayingBroadcastReceiver runs on a different thread. Mark isPlaying volatile.
-private var isPlaying: Boolean = false +@Volatile private var isPlaying: Boolean = falsepackages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/SharedPreferencesUtils.kt (1)
41-44: Avoid redundant write before removeremoveAllCalls writes “[]” and then removes the key. Just remove.
-fun removeAllCalls(context: Context?) { - putString(context, "ACTIVE_CALLS", "[]") - remove(context, "ACTIVE_CALLS") -} +fun removeAllCalls(context: Context?) { + if (context == null) return + remove(context, "ACTIVE_CALLS") +}packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt (1)
504-546: Channel property updates on existing channels are often ignoredOnce created, a channel’s importance/sound/vibration can’t be changed programmatically. If you need different behavior, create a new channel ID and migrate.
Do you want a patch that version-bumps channel IDs when desired properties differ from the existing channel?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
packages/stream_video/lib/src/push_notification/call_kit_events.dart(18 hunks)packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt(1 hunks)packages/stream_video_push_notification/android/src/main/AndroidManifest.xml(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/Call.kt(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallActivity.kt(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationService.kt(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallSoundPlayerManager.kt(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/SharedPreferencesUtils.kt(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt(1 hunks)packages/stream_video_push_notification/android/src/main/res/layout-w600dp-land/activity_incoming_call.xml(1 hunks)packages/stream_video_push_notification/android/src/main/res/layout/layout_custom_miss_notification.xml(1 hunks)packages/stream_video_push_notification/android/src/main/res/layout/layout_custom_miss_small_notification.xml(1 hunks)packages/stream_video_push_notification/android/src/main/res/layout/layout_custom_small_ex_notification.xml(1 hunks)packages/stream_video_push_notification/ios/Classes/Call.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoIncomingCallConstants.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoPKDelegateManager.swift(4 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoPushConfiguration.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift(2 hunks)packages/stream_video_push_notification/ios/Classes/StringUtils.swift(1 hunks)packages/stream_video_push_notification/ios/stream_video_push_notification.podspec(1 hunks)packages/stream_video_push_notification/lib/src/stream_video_call_event.dart(1 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart(1 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart(15 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_params.dart(5 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/stream_video_push_notification/ios/stream_video_push_notification.podspec
- packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationService.kt
- packages/stream_video_push_notification/lib/src/stream_video_call_event.dart
- packages/stream_video_push_notification/android/src/main/res/layout/layout_custom_miss_notification.xml
- packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallActivity.kt
- packages/stream_video_push_notification/android/src/main/res/layout/layout_custom_miss_small_notification.xml
- packages/stream_video_push_notification/android/src/main/res/layout/layout_custom_small_ex_notification.xml
🧰 Additional context used
🧬 Code graph analysis (7)
packages/stream_video_push_notification/ios/Classes/StreamVideoPushConfiguration.swift (1)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (1)
configureAudioSession(422-443)
packages/stream_video_push_notification/ios/Classes/Call.swift (4)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (5)
handle(45-162)startCall(248-255)connectedCall(295-304)endCall(283-293)configureAudioSession(422-443)packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (1)
handle(35-59)packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (3)
startCall(15-43)connectedCall(72-86)endCall(63-70)packages/stream_video_push_notification/ios/Classes/StringUtils.swift (1)
encryptHandle(39-43)
packages/stream_video_push_notification/ios/Classes/StreamVideoPKDelegateManager.swift (4)
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/Call.kt (1)
args(6-344)packages/stream_video_push_notification/ios/Classes/Call.swift (1)
toJSON(238-270)packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (3)
handle(45-162)showIncomingCall(174-208)showIncomingCall(210-246)packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (2)
handle(35-59)showIncomingCall(73-79)
packages/stream_video_push_notification/ios/Classes/StringUtils.swift (1)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (1)
getHandleType(363-375)
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (3)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (6)
initCallkitProvider(377-383)handle(45-162)startCall(248-255)showIncomingCall(174-208)showIncomingCall(210-246)activeCalls(306-308)packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (2)
startCall(15-43)activeCalls(100-115)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/StreamVideoPushNotificationPlugin.kt (5)
startCall(125-132)eventSink(382-403)send(390-398)onListen(386-388)onCancel(400-402)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (3)
packages/stream_video_push_notification/ios/Classes/Call.swift (7)
answerCall(99-104)startCall(90-95)endCall(113-115)connectedCall(108-111)getEncryptHandle(272-301)toJSON(238-270)startAudio(117-119)packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (4)
send(97-103)handle(35-59)showIncomingCall(73-79)activeCalls(81-83)packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (14)
startCall(15-43)endCall(63-70)muteCall(45-52)callWithUUID(160-163)holdCall(54-61)connectedCall(72-86)activeCalls(100-115)endAllCalls(88-98)getHandleType(143-155)addCall(165-174)setSharedProvider(11-13)removeAllCalls(183-187)removeCall(176-181)setHold(117-122)
packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (2)
packages/stream_video_push_notification/ios/Classes/Call.swift (5)
startCall(90-95)getEncryptHandle(272-301)endCall(113-115)connectedCall(108-111)toJSON(238-270)packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (8)
startCall(248-255)handle(45-162)muteCall(257-268)holdCall(270-281)endCall(283-293)connectedCall(295-304)endAllCalls(310-313)activeCalls(306-308)
🪛 SwiftLint (0.57.0)
packages/stream_video_push_notification/ios/Classes/Call.swift
[Warning] 131-131: Prefer at least one space after slashes for comments
(comment_spacing)
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift
[Warning] 63-63: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 63-63: TODOs should be resolved (send event? //ACTION_DID_UPDAT...)
(todo)
[Warning] 55-55: Avoid using unneeded break statements
(unneeded_break_in_switch)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift
[Warning] 482-482: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 482-482: MARK comment should be in valid format. e.g. '// MARK: ...' or '// MARK: - ...'
(mark)
[Warning] 15-15: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 284-284: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 296-296: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 57-57: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 60-60: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 71-71: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 86-86: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 98-98: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 113-113: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 124-124: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 139-139: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 142-142: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 146-146: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 155-155: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 158-158: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 320-320: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 325-325: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 330-330: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 335-335: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 340-340: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 368-368: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 450-450: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 453-453: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 456-456: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 459-459: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 462-462: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 465-465: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 468-468: Avoid using unneeded break statements
(unneeded_break_in_switch)
[Warning] 475-475: Avoid using unneeded break statements
(unneeded_break_in_switch)
packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift
[Warning] 129-129: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 133-133: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 135-135: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 8-8: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 133-133: TODOs should be resolved (push notification for Start Ca...)
(todo)
[Warning] 135-135: TODOs should be resolved (push notification for End Call)
(todo)
[Warning] 148-148: Avoid using unneeded break statements
(unneeded_break_in_switch)
🪛 detekt (1.23.8)
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt
[warning] 184-184: String.format("file:///android_asset/flutter_assets/%s", avatarUrl) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
[warning] 270-270: String.format("file:///android_asset/flutter_assets/%s", avatarUrl) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
[warning] 400-400: String.format("file:///android_asset/flutter_assets/%s", avatarUrl) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallSoundPlayerManager.kt
[warning] 155-155: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
[warning] 176-176: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt
[warning] 35-35: This empty block of code can be removed.
(detekt.empty-blocks.EmptySecondaryConstructor)
⏰ 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). (5)
- GitHub Check: stream_video_flutter
- GitHub Check: build
- GitHub Check: analyze
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video
...o/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt
Show resolved
Hide resolved
...o/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt
Show resolved
Hide resolved
...o/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt
Outdated
Show resolved
Hide resolved
packages/stream_video_push_notification/android/src/main/AndroidManifest.xml
Outdated
Show resolved
Hide resolved
packages/stream_video_push_notification/android/src/main/AndroidManifest.xml
Show resolved
Hide resolved
packages/stream_video_push_notification/ios/Classes/StreamVideoIncomingCallConstants.swift
Show resolved
Hide resolved
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (31)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt (2)
25-29: Good fix: no manual recycling of input/intermediate Bitmaps.This addresses prior crash risks with Coil’s ownership.
31-35: Always request ARGB_8888 from the pool and copy density.Ensures transparent corners and avoids stale pixels; copying density preserves expected scaling.
- val config = squaredBitmap.config ?: Bitmap.Config.ARGB_8888 - val output = pool.get(sizeImage, sizeImage, config).apply { - setHasAlpha(true) - eraseColor(Color.TRANSPARENT) - } + val output = pool.get(sizeImage, sizeImage, Bitmap.Config.ARGB_8888).apply { + setHasAlpha(true) + eraseColor(Color.TRANSPARENT) + density = squaredBitmap.density + }packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt (6)
54-57: Replace deprecated getColor with ContextCompat.getColor.Use the non-deprecated API and proper theming.
rippleColor = typedArray.getColor( R.styleable.ripple_relativeLayout_ripple_color, - resources.getColor(R.color.ripple_main_color) + ContextCompat.getColor(context, R.color.ripple_main_color) )Add import:
+import androidx.core.content.ContextCompatAlso applies to: 3-18
72-72: Good fix for ripple_amount=0.Guarding the divisor with
coerceAtLeast(1)avoids divide-by-zero crashes.
89-101: Avoid reflective property names; use View property constants.This avoids reflection and typos, and is the recommended API.
- val scaleXAnimator = ObjectAnimator.ofFloat(rippleView, "ScaleX", 1.0f, rippleScale) + val scaleXAnimator = ObjectAnimator.ofFloat(rippleView, View.SCALE_X, 1.0f, rippleScale) @@ - val scaleYAnimator = ObjectAnimator.ofFloat(rippleView, "ScaleY", 1.0f, rippleScale) + val scaleYAnimator = ObjectAnimator.ofFloat(rippleView, View.SCALE_Y, 1.0f, rippleScale) @@ - val alphaAnimator = ObjectAnimator.ofFloat(rippleView, "Alpha", 1.0f, 0f) + val alphaAnimator = ObjectAnimator.ofFloat(rippleView, View.ALPHA, 1.0f, 0f)
113-116: Resolved: removed unnecessary!!on Paint in onDraw.
paintis already non-null; the current usage is correct.
20-46: Collapse constructors; make Context non-null.Consolidate into a single primary constructor; remove the nullable/empty constructor.
-class RippleRelativeLayout : RelativeLayout { +class RippleRelativeLayout @JvmOverloads constructor( + context: Context, + attrs: AttributeSet? = null, + defStyleAttr: Int = 0 +) : RelativeLayout(context, attrs, defStyleAttr) { @@ - constructor(context: Context?) : super(context) {} - constructor(context: Context, attrs: AttributeSet?) : super(context, attrs) { - init(context, attrs) - } - - constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super( - context, - attrs, - defStyleAttr - ) { - init(context, attrs) - } + init { + init(context, attrs) + } }
30-33: Remove nullable fields and!!; initialize eagerly.Converting to
lateinit/immutable collections removes NPE risk and cleans up call sites.- private var animatorSet: AnimatorSet? = null - private var animatorList: ArrayList<Animator>? = null - private var rippleParams: LayoutParams? = null + private lateinit var animatorSet: AnimatorSet + private val animatorList = mutableListOf<Animator>() + private lateinit var rippleParams: LayoutParams @@ - rippleParams!!.addRule(CENTER_IN_PARENT, TRUE) - animatorSet = AnimatorSet() - animatorSet!!.interpolator = AccelerateDecelerateInterpolator() - animatorList = ArrayList() + rippleParams.addRule(CENTER_IN_PARENT, TRUE) + animatorSet = AnimatorSet().apply { + interpolator = AccelerateDecelerateInterpolator() + } @@ - animatorList!!.add(scaleXAnimator) + animatorList.add(scaleXAnimator) @@ - animatorList!!.add(scaleYAnimator) + animatorList.add(scaleYAnimator) @@ - animatorList!!.add(alphaAnimator) + animatorList.add(alphaAnimator) @@ - animatorSet!!.playTogether(animatorList) + animatorSet.playTogether(animatorList) @@ - animatorSet!!.start() + animatorSet.start() @@ - animatorSet!!.end() + animatorSet.cancel()Also applies to: 81-85, 108-108, 128-128, 135-135
packages/stream_video/lib/src/push_notification/call_kit_events.dart (2)
32-32: Fix grammar in DartDoc (“a incoming/outgoing” → “an incoming/outgoing”).
Improves public API polish.-/// This event is triggered when a incoming call is received. +/// This event is triggered when an incoming call is received. -/// This event is triggered when a outgoing call is started. +/// This event is triggered when an outgoing call is started. -/// This event is triggered when a incoming call is accepted. This can happen -/// when a user clicks on the "Accept" action from a incoming call +/// This event is triggered when an incoming call is accepted. This can happen +/// when a user clicks on the "Accept" action from an incoming call -/// This event is triggered when a incoming call is declined. This can happen -/// when a user clicks on the "Decline" action from a incoming call +/// This event is triggered when an incoming call is declined. This can happen +/// when a user clicks on the "Decline" action from an incoming call -/// This event is triggered when a incoming or outgoing call is ended. This can +/// This event is triggered when an incoming or outgoing call is ended. This can -/// doesn't answer a incoming call within a certain time frame. +/// doesn't answer an incoming call within a certain time frame.Also applies to: 46-46, 60-62, 76-78, 92-94, 107-108
288-291: Soft-migrate nameCaller → callerName: add deprecated alias.
Prevents breakage for apps not yet migrated.class CallData with EquatableMixin { @@ - /// Name of the caller. - final String? callerName; + /// Name of the caller. + final String? callerName; + @Deprecated('Use callerName instead.') + String? get nameCaller => callerName; @@ - callerName, + callerName,Also applies to: 300-309
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (2)
97-106: Main-thread event dispatch LGTM.
Matches Android behavior and avoids threading issues.
81-83: Return type should be non-optional.
Method always returns an array; align signature.-@objc public static func activeCalls() -> [[String: Any]]? { +@objc public static func activeCalls() -> [[String: Any]] {packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (2)
178-188: Harden JSON parsing in _callDataFromJson.
Prevents type errors on malformed payloads.- final extraData = json['extra']?.cast<String, dynamic>(); + final extraData = json['extra'] is Map + ? Map<String, dynamic>.from(json['extra'] as Map) + : null; @@ - hasVideo: json['type'] == 1, + hasVideo: json['type'] != null && json['type'] == 1, extraData: extraData,
190-233: Guard event parsing; avoid throws on unknown events.
Unknown event names or bad bodies currently crash.- RingingEvent? _receiveRingingEvent(dynamic data) { - if (data is Map) { - final event = Event.values.firstWhere((e) => e.name == data['event']); - final body = Map<String, dynamic>.from(data['body']); - final callData = _callDataFromJson(body); - return switch (event) { + RingingEvent? _receiveRingingEvent(dynamic data) { + try { + if (data is Map) { + final eventName = data['event'] as String?; + Event? event; + try { + event = Event.values.firstWhere((e) => e.name == eventName); + } catch (_) { + debugPrint('Unknown event: $eventName'); + return null; + } + final body = Map<String, dynamic>.from(data['body'] as Map); + final callData = _callDataFromJson(body); + return switch (event) { // … existing cases unchanged … - }; - } - return null; + }; + } + } catch (e, st) { + debugPrint('Error parsing ringing event: $e\n$st'); + } + return null; }packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (2)
19-25: UUID guard LGTM.
Prevents crashes on invalid UUIDs.
117-123: hold transaction now requested — good fix.
Addresses previously reported no-op.packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart (2)
148-159: Always unsilence in finally; avoid 300ms races with a helper.If
endCallByCidthrows, events remain silenced. Wrap with try/finally via a small helper.- await StreamVideoPushNotificationPlatform.instance - .silenceEvents(); - await endCallByCid(event.callCid.toString()); - await Future<void>.delayed(const Duration(milliseconds: 300)); - await StreamVideoPushNotificationPlatform.instance - .unsilenceEvents(); + await _withSilencedEvents(() async { + await endCallByCid(event.callCid.toString()); + });Add inside the class:
Future<void> _withSilencedEvents(Future<void> Function() action) async { await StreamVideoPushNotificationPlatform.instance.silenceEvents(); try { await action(); await Future<void>.delayed(const Duration(milliseconds: 300)); } finally { await StreamVideoPushNotificationPlatform.instance.unsilenceEvents(); } }
431-437: Await and use try/finally when silencing around mute.Current calls aren’t awaited and may leave events silenced if
muteCallthrows.- // Silence events to avoid infinite loop - StreamVideoPushNotificationPlatform.instance.silenceEvents(); - await StreamVideoPushNotificationPlatform.instance - .muteCall(call.uuid!, isMuted: isMuted); - StreamVideoPushNotificationPlatform.instance.unsilenceEvents(); + // Silence events to avoid infinite loop + await StreamVideoPushNotificationPlatform.instance.silenceEvents(); + try { + await StreamVideoPushNotificationPlatform.instance + .muteCall(call.uuid!, isMuted: isMuted); + } finally { + await StreamVideoPushNotificationPlatform.instance.unsilenceEvents(); + }packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (9)
610-624: Avoid double-adding outgoing call in didActivate.Guard before
addCall.- self.outgoingCall?.startCall(withAudioSession: audioSession) { success in - if success { - self.callController.addCall(self.outgoingCall!) - self.outgoingCall?.startAudio() - } - } + self.outgoingCall?.startCall(withAudioSession: audioSession) { success in + if success, + let oc = self.outgoingCall, + self.callController.callWithUUID(uuid: oc.uuid) == nil { + self.callController.addCall(oc) + } + self.outgoingCall?.startAudio() + }
490-506: Guardself.datain start-call path; avoid force unwrap crash.- public func provider(_ provider: CXProvider, perform action: CXStartCallAction) { - let call = Call(uuid: action.callUUID, data: self.data!, isOutGoing: true) + public func provider(_ provider: CXProvider, perform action: CXStartCallAction) { + guard let d = self.data else { action.fail(); return } + let call = Call(uuid: action.callUUID, data: d, isOutGoing: true)
518-521: Don’t callreportOutgoingCallfor answered incoming calls.That API is for outgoing; drop the closure.
- call.hasConnectDidChange = { [weak self] in - self?.sharedProvider?.reportOutgoingCall( - with: call.uuid, connectedAt: call.connectedData) - } + call.hasConnectDidChange = nil
601-608: UsecallUUID, notaction.uuid, in timeout handler.
action.uuidis the action id.- public func provider(_ provider: CXProvider, timedOutPerforming action: CXAction) { - guard let call = self.callController.callWithUUID(uuid: action.uuid) else { + public func provider(_ provider: CXProvider, timedOutPerforming action: CXAction) { + let callUUID = (action as? CXCallAction)?.callUUID + guard let uuid = callUUID, + let call = self.callController.callWithUUID(uuid: uuid) else { action.fail() return }
283-294: Guard malformed UUIDs; remove force unwraps in endCall.Prevents crashes and ensures safe teardown.
- @objc public func endCall(_ data: CallData) { - var call: Call? = nil - if self.isFromPushKit { - call = Call(uuid: UUID(uuidString: self.data!.uuid)!, data: data) - self.isFromPushKit = false - self.sendEvent(StreamVideoIncomingCallConstants.ACTION_CALL_ENDED, data.toJSON()) - } else { - call = Call(uuid: UUID(uuidString: data.uuid)!, data: data) - } - self.callController.endCall(call: call!) + @objc public func endCall(_ data: CallData) { + var call: Call? + if self.isFromPushKit { + guard let u = UUID(uuidString: self.data?.uuid ?? "") else { return } + call = Call(uuid: u, data: data) + self.isFromPushKit = false + self.sendEvent(StreamVideoIncomingCallConstants.ACTION_CALL_ENDED, data.toJSON()) + } else { + guard let u = UUID(uuidString: data.uuid) else { return } + call = Call(uuid: u, data: data) + } + if let c = call { self.callController.endCall(call: c) } }
295-304: Same: guard UUIDs in connectedCall.- @objc public func connectedCall(_ data: CallData) { - var call: Call? = nil - if self.isFromPushKit { - call = Call(uuid: UUID(uuidString: self.data!.uuid)!, data: data) - self.isFromPushKit = false - } else { - call = Call(uuid: UUID(uuidString: data.uuid)!, data: data) - } - self.callController.connectedCall(call: call!) + @objc public func connectedCall(_ data: CallData) { + var call: Call? + if self.isFromPushKit { + guard let u = UUID(uuidString: self.data?.uuid ?? "") else { return } + call = Call(uuid: u, data: data) + self.isFromPushKit = false + } else { + guard let u = UUID(uuidString: data.uuid) else { return } + call = Call(uuid: u, data: data) + } + if let c = call { self.callController.connectedCall(call: c) } }
346-353: Avoid force unwrap in timeout scheduler.- func endCallNotExist(_ data: CallData) { - DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(data.duration)) { - let call = self.callController.callWithUUID(uuid: UUID(uuidString: data.uuid)!) + func endCallNotExist(_ data: CallData) { + DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(data.duration)) { + guard let u = UUID(uuidString: data.uuid) else { return } + let call = self.callController.callWithUUID(uuid: u) if call != nil && self.answerCall == nil && self.outgoingCall == nil { self.callEndTimeout(data) } } }
355-361: Guard UUID in callEndTimeout.- func callEndTimeout(_ data: CallData) { - self.saveEndCall(data.uuid, 3) - guard let call = self.callController.callWithUUID(uuid: UUID(uuidString: data.uuid)!) else { + func callEndTimeout(_ data: CallData) { + self.saveEndCall(data.uuid, 3) + guard let u = UUID(uuidString: data.uuid), + let call = self.callController.callWithUUID(uuid: u) else { return } sendEvent(StreamVideoIncomingCallConstants.ACTION_CALL_TIMEOUT, data.toJSON()) }
315-344: Guard UUID once in saveEndCall; reuse it.- public func saveEndCall(_ uuid: String, _ reason: Int) { - switch reason { + public func saveEndCall(_ uuid: String, _ reason: Int) { + guard let u = UUID(uuidString: uuid) else { return } + switch reason { case 1: - self.sharedProvider?.reportCall( - with: UUID(uuidString: uuid)!, endedAt: Date(), reason: CXCallEndedReason.failed) + self.sharedProvider?.reportCall(with: u, endedAt: Date(), reason: .failed) break case 2, 6: - self.sharedProvider?.reportCall( - with: UUID(uuidString: uuid)!, endedAt: Date(), - reason: CXCallEndedReason.remoteEnded) + self.sharedProvider?.reportCall(with: u, endedAt: Date(), reason: .remoteEnded) break case 3: - self.sharedProvider?.reportCall( - with: UUID(uuidString: uuid)!, endedAt: Date(), reason: CXCallEndedReason.unanswered - ) + self.sharedProvider?.reportCall(with: u, endedAt: Date(), reason: .unanswered) break case 4: - self.sharedProvider?.reportCall( - with: UUID(uuidString: uuid)!, endedAt: Date(), - reason: CXCallEndedReason.answeredElsewhere) + self.sharedProvider?.reportCall(with: u, endedAt: Date(), reason: .answeredElsewhere) break case 5: - self.sharedProvider?.reportCall( - with: UUID(uuidString: uuid)!, endedAt: Date(), - reason: CXCallEndedReason.declinedElsewhere) + self.sharedProvider?.reportCall(with: u, endedAt: Date(), reason: .declinedElsewhere) break default: break } }packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt (4)
106-107: Remove DEFAULT_VIBRATE to avoid double vibration (custom player already vibrates).You're already handling vibration via IncomingCallSoundPlayerManager; keeping DEFAULT_VIBRATE causes duplicate patterns.
- notificationBuilder?.setDefaults(NotificationCompat.DEFAULT_VIBRATE) + // Vibration handled by IncomingCallSoundPlayerManager; avoid duplicate vibration + // notificationBuilder?.setDefaults(NotificationCompat.DEFAULT_VIBRATE)
184-185: Use Locale.ROOT with String.format (detekt: ImplicitDefaultLocale).Prevents locale-specific surprises when formatting asset URIs.
import java.util.Date +import java.util.Locale @@ - String.format("file:///android_asset/flutter_assets/%s", avatarUrl) + String.format(Locale.ROOT, "file:///android_asset/flutter_assets/%s", avatarUrl) @@ - avatarUrl = String.format("file:///android_asset/flutter_assets/%s", avatarUrl) + avatarUrl = String.format(Locale.ROOT, "file:///android_asset/flutter_assets/%s", avatarUrl) @@ - avatarUrl = String.format("file:///android_asset/flutter_assets/%s", avatarUrl) + avatarUrl = String.format(Locale.ROOT, "file:///android_asset/flutter_assets/%s", avatarUrl)Also applies to: 270-271, 400-401, 30-31
186-188: Crash risk: unsafe cast of Serializable to HashMap.Direct cast can throw ClassCastException at runtime. Use a safe cast with a default.
- val headers = - data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) as HashMap<String, Any?> + val headers = ( + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) + data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS, HashMap::class.java) + else + @Suppress("DEPRECATION") + data.getSerializable(IncomingCallConstants.EXTRA_CALL_HEADERS) + ) as? HashMap<String, Any?> ?: hashMapOf()Apply in all three places.
Also applies to: 272-274, 402-404
584-587: Potential NPE: avoid passing null Intent to PendingIntent.getActivity.AppUtils.getAppIntent can return null; PendingIntent.getActivity requires a non-null Intent.
private fun getAppPendingIntent(id: Int, data: Bundle): PendingIntent { val intent: Intent? = AppUtils.getAppIntent(context, data = data) - return PendingIntent.getActivity(context, id, intent, getFlagPendingIntent()) + return if (intent != null) { + PendingIntent.getActivity(context, id, intent, getFlagPendingIntent()) + } else { + // Fallback to in-app activity intent + getActivityPendingIntent(id, data) + } }
🧹 Nitpick comments (20)
packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt (2)
16-18: Drop the isRecycled early-return.Coil shouldn’t pass a recycled Bitmap; returning one risks downstream crashes. Safer to remove this guard.
- if (input.isRecycled) return input -
3-3: Avoid wildcard Android graphics import.Keeps ABI/API surface explicit and helps static analysis.
-import android.graphics.* +import android.graphics.Bitmap +import android.graphics.BitmapShader +import android.graphics.Canvas +import android.graphics.Color +import android.graphics.Paint +import android.graphics.Shaderpackages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt (2)
27-27: Remove redundant Paint re-initialization.Initialize once; avoid recreating the Paint in
init.- private var paint: Paint = Paint() + private var paint: Paint = Paint(Paint.ANTI_ALIAS_FLAG) @@ - paint = Paint() - paint.isAntiAlias = true - paint.style = Paint.Style.FILL + paint.style = Paint.Style.FILL paint.color = rippleColorAlso applies to: 73-75
133-138: Prefer cancel() and hide ripples on stop.
end()jumps to end values;cancel()is safer here. Also hide children to avoid lingering views.fun stopRippleAnimation() { if (isRippleAnimationRunning) { - animatorSet!!.end() + animatorSet?.cancel() + for (rippleView in rippleViewList) { + rippleView.visibility = INVISIBLE + } isRippleAnimationRunning = false } }packages/stream_video/lib/src/push_notification/call_kit_events.dart (1)
137-146: Doc fix: “called back” → “connected”.
The event is for connection, not callback.- /// The call data associated with the call that was called back. + /// The call data associated with the call that connected.packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (2)
24-31: Emit device token update event.
Mirror Android and wire ACTION_DID_UPDATE_DEVICE_PUSH_TOKEN_VOIP.-public class StreamVideoPushNotificationPlugin: NSObject, FlutterPlugin { +public class StreamVideoPushNotificationPlugin: NSObject, FlutterPlugin { private let devicePushTokenVoIP = "DevicePushTokenVoIP" + private let eventsHandler: EventCallbackHandler @@ - private var callKitManager: StreamVideoCallkitManager + private var callKitManager: StreamVideoCallkitManager @@ - public init(callKitManager: StreamVideoCallkitManager) { - self.callKitManager = callKitManager + public init(callKitManager: StreamVideoCallkitManager, eventsHandler: EventCallbackHandler) { + self.callKitManager = callKitManager + self.eventsHandler = eventsHandler super.init() } @@ - let eventsHandler = EventCallbackHandler() + let eventsHandler = EventCallbackHandler() @@ - let callKitManager = StreamVideoCallkitManager(eventHandler: eventsHandler) - sharedInstance = StreamVideoPushNotificationPlugin(callKitManager: callKitManager) + let callKitManager = StreamVideoCallkitManager(eventHandler: eventsHandler) + sharedInstance = StreamVideoPushNotificationPlugin(callKitManager: callKitManager, eventsHandler: eventsHandler) @@ @objc public static func setDevicePushTokenVoIP(deviceToken: String) { - guard let instance = sharedInstance else { + guard let instance = sharedInstance else { print("Warning: StreamVideoPushNotificationPlugin not initialized") return } instance.setDevicePushTokenVoIP(deviceToken: deviceToken) - //TODO: send event? //ACTION_DID_UPDATE_DEVICE_PUSH_TOKEN_VOIP + instance.eventsHandler.send( + "actionDidUpdateDevicePushTokenVoip", + ["deviceTokenVoIP": deviceToken] + ) }Also applies to: 61-64, 94-106
55-56: SwiftLint cleanups: remove unneeded break; fix comment spacing.
Minor polish.- result(self.getDevicePushTokenVoIP()) - break + result(self.getDevicePushTokenVoIP()) @@ - //TODO: send event? //ACTION_DID_UPDATE_DEVICE_PUSH_TOKEN_VOIP + // TODO: send event? // ACTION_DID_UPDATE_DEVICE_PUSH_TOKEN_VOIPAlso applies to: 63-64
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (2)
45-46: Nit: “Missed Call” wording.- /// Show Miss Call Notification. + /// Show Missed Call notification.
129-131: Type the activeCalls() result.
Improves callsite ergonomics.- Future<dynamic> activeCalls() async { - return await methodChannel.invokeMethod("activeCalls"); + Future<List<Map<String, dynamic>>> activeCalls() async { + final result = await methodChannel.invokeMethod<List<dynamic>>("activeCalls"); + return (result ?? []).cast<Map>().map((e) => Map<String, dynamic>.from(e as Map)).toList(); }packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (3)
8-8: Remove redundant nil initialization.
Minor SwiftLint cleanup.- private var sharedProvider: CXProvider? = nil + private var sharedProvider: CXProvider?
147-155: Remove unneeded break statements in switch.
Swift doesn’t require them.- case "number": - typeDefault = CXHandle.HandleType.phoneNumber - break + case "number": + typeDefault = CXHandle.HandleType.phoneNumber
128-137: Comment spacing/TODO lint.
Add a space after // and consider tracking TODOs in an issue.- //fail + // fail - //TODO: push notification for Start Call + // TODO: push notification for Start Call - //TODO: push notification for End Call + // TODO: push notification for End CallAlso applies to: 130-130, 134-136
packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart (2)
81-81: Specify local function return type.Use
voidfor clarity.- subscribeToEvents() { + void subscribeToEvents() {
84-91: Await async end-call to keep ordering predictable.Make the handler async and await
endCallByCid.- client.events.on<CoordinatorCallEndedEvent>( - (event) { + client.events.on<CoordinatorCallEndedEvent>( + (event) async { _logger.d( () => '[subscribeToEvents] Call ended event: ${event.callCid}'); - endCallByCid(event.callCid.toString()); + await endCallByCid(event.callCid.toString()); }, ),dogfooding/lib/di/injector.dart (1)
180-182: Minor readability: trailing comma/multiline formattingFormatting this block with trailing commas improves diffs and consistency with Flutter formatting.
- pushConfiguration: const StreamVideoPushConfiguration( - ios: IOSPushConfiguration(iconName: 'IconMask'), - android: AndroidPushConfiguration(defaultAvatar: 'assets/logo.png')), + pushConfiguration: const StreamVideoPushConfiguration( + ios: IOSPushConfiguration(iconName: 'IconMask'), + android: AndroidPushConfiguration( + defaultAvatar: 'assets/logo.png', + ), + ),packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt (5)
521-526: Avoid system vibration if custom player vibrates.If IncomingCallSoundPlayerManager vibrates, disable channel vibration to prevent duplicate patterns on O+.
- vibrationPattern = longArrayOf(0, 1000, 500, 1000, 500) - lightColor = Color.RED - enableLights(true) - enableVibration(true) + lightColor = Color.RED + enableLights(true) + enableVibration(false) // custom vibration handled by player
324-328: Set category for all API levels.On < S, set NotificationCompat.CATEGORY_MISSED_CALL so the system classifies it correctly.
- if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { - builder.setCategory(Notification.CATEGORY_MISSED_CALL) - } - } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + builder.setCategory(Notification.CATEGORY_MISSED_CALL) + } else { + builder.setCategory(NotificationCompat.CATEGORY_MISSED_CALL) + }
433-434: Clarify sound behavior on O+.builder.setSound is ignored on Android O+ (channel controls sound). Either move sound to the channel or gate this call to < O to reduce confusion.
- builder.setSound(missedCallSound) + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { + builder.setSound(missedCallSound) + }
105-105: Remove redundant builder calls (minor cleanup).
- setChannelId is redundant after passing the channel to the Builder constructor.
- priority is set twice; keep one.
- notificationBuilder?.setChannelId(NOTIFICATION_CHANNEL_ID_INCOMING) @@ - notificationBuilder?.setChannelId(NOTIFICATION_CHANNEL_ID_INCOMING) @@ - notificationBuilder?.priority = NotificationCompat.PRIORITY_MAX + notificationBuilder?.priority = NotificationCompat.PRIORITY_MAX @@ - notificationBuilder?.priority = NotificationCompat.PRIORITY_MAXAlso applies to: 141-141, 110-110, 142-142
615-619: Tiny: use the scoped variable in let.Avoid referencing the outer variable inside let.
- getNotificationManager().notify( - it.id, incomingCallNotification.notification - ) + getNotificationManager().notify(it.id, it.notification)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
dogfooding/lib/app/app_content.dart(3 hunks)dogfooding/lib/di/injector.dart(1 hunks)packages/stream_video/lib/src/push_notification/call_kit_events.dart(17 hunks)packages/stream_video_flutter/android/src/main/kotlin/io/getstream/video/flutter/stream_video_flutter/service/notification/image/CircleTransform.kt(1 hunks)packages/stream_video_push_notification/android/src/main/AndroidManifest.xml(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt(1 hunks)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoIncomingCallConstants.swift(1 hunks)packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift(2 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart(15 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/stream_video_push_notification/ios/Classes/StreamVideoIncomingCallConstants.swift
- packages/stream_video_push_notification/android/src/main/AndroidManifest.xml
- dogfooding/lib/app/app_content.dart
🧰 Additional context used
🧬 Code graph analysis (3)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (4)
packages/stream_video_push_notification/ios/Classes/Call.swift (7)
answerCall(99-104)startCall(90-95)endCall(113-115)connectedCall(108-111)getEncryptHandle(272-301)toJSON(238-270)startAudio(117-119)packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (4)
send(97-106)handle(35-59)showIncomingCall(73-79)activeCalls(81-83)packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (13)
startCall(15-43)endCall(63-70)muteCall(45-52)callWithUUID(161-164)holdCall(54-61)connectedCall(72-86)activeCalls(100-115)endAllCalls(88-98)getHandleType(144-156)addCall(166-175)setSharedProvider(11-13)removeAllCalls(184-188)removeCall(177-182)packages/stream_video_push_notification/ios/Classes/StringUtils.swift (1)
getHandleType(68-77)
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift (3)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (6)
initCallkitProvider(377-383)handle(45-162)startCall(248-255)showIncomingCall(174-208)showIncomingCall(210-246)activeCalls(306-308)packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (2)
startCall(15-43)activeCalls(100-115)packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/StreamVideoPushNotificationPlugin.kt (5)
startCall(125-132)eventSink(382-403)send(390-398)onListen(386-388)onCancel(400-402)
packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift (2)
packages/stream_video_push_notification/ios/Classes/StreamVideoCallkitManager.swift (8)
startCall(248-255)handle(45-162)muteCall(257-268)holdCall(270-281)endCall(283-293)connectedCall(295-304)endAllCalls(310-313)activeCalls(306-308)packages/stream_video_push_notification/ios/Classes/Call.swift (5)
startCall(90-95)getEncryptHandle(272-301)endCall(113-115)connectedCall(108-111)toJSON(238-270)
🪛 detekt (1.23.8)
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt
[warning] 184-184: String.format("file:///android_asset/flutter_assets/%s", avatarUrl) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
[warning] 270-270: String.format("file:///android_asset/flutter_assets/%s", avatarUrl) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
[warning] 400-400: String.format("file:///android_asset/flutter_assets/%s", avatarUrl) uses implicitly default locale for string formatting.
(detekt.potential-bugs.ImplicitDefaultLocale)
packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt
[warning] 35-35: This empty block of code can be removed.
(detekt.empty-blocks.EmptySecondaryConstructor)
🪛 SwiftLint (0.57.0)
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift
[Warning] 63-63: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 63-63: TODOs should be resolved (send event? //ACTION_DID_UPDAT...)
(todo)
[Warning] 55-55: Avoid using unneeded break statements
(unneeded_break_in_switch)
packages/stream_video_push_notification/ios/Classes/StreamCallKitCallController.swift
[Warning] 130-130: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 134-134: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 136-136: Prefer at least one space after slashes for comments
(comment_spacing)
[Warning] 8-8: Initializing an optional variable with nil is redundant
(redundant_optional_initialization)
[Warning] 134-134: TODOs should be resolved (push notification for Start Ca...)
(todo)
[Warning] 136-136: TODOs should be resolved (push notification for End Call)
(todo)
[Warning] 149-149: Avoid using unneeded break statements
(unneeded_break_in_switch)
⏰ 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). (7)
- GitHub Check: stream_video
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: build
- GitHub Check: analyze
- GitHub Check: analyze_legacy_version
🔇 Additional comments (4)
packages/stream_video/lib/src/push_notification/call_kit_events.dart (1)
3-4: Deprecation alias looks good.
Typedef provides a clear migration path.dogfooding/lib/di/injector.dart (1)
180-182: defaultAvatar asset path verified The Flutter assetassets/logo.pngis declared in pubspec.yaml and AndroidPushConfiguration supports Flutter asset paths (it resolves them tofile:///android_asset/flutter_assets/...). No changes required.packages/stream_video_push_notification/android/src/main/kotlin/io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt (2)
490-499: Nice: fixed pre-O notification check logic.areNotificationsEnabled() is now respected on all API levels; O+ channel importance is checked conditionally. LGTM.
662-662: Nice: permission result uses value equality (==).The erroneous reference equality (===) is gone. LGTM.
...io/getstream/video/flutter/stream_video_push_notification/IncomingCallNotificationManager.kt
Show resolved
Hide resolved
...in/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt
Show resolved
Hide resolved
...in/io/getstream/video/flutter/stream_video_push_notification/widgets/RippleRelativeLayout.kt
Show resolved
Hide resolved
packages/stream_video_push_notification/ios/Classes/StreamVideoPushNotificationPlugin.swift
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (2)
180-190: Harden JSON parsing in _callDataFromJson.
Avoid unsafe casts; guard nulls.- CallData _callDataFromJson(Map<String, dynamic> json) { - final extraData = json['extra']?.cast<String, dynamic>(); + CallData _callDataFromJson(Map<String, dynamic> json) { + final rawExtra = json['extra']; + final extraData = + rawExtra is Map ? Map<String, dynamic>.from(rawExtra as Map) : null; return CallData( uuid: json['id'] as String?, - callCid: extraData?['callCid'] as String?, + callCid: extraData?['callCid'] as String?, handle: json['handle'] as String?, callerName: json['callerName'] as String?, - hasVideo: json['type'] == 1, + hasVideo: (json['type'] is int && json['type'] == 1) || + (json['hasVideo'] is bool && json['hasVideo'] == true), extraData: extraData, ); }
192-234: Make event parsing crash-safe (unknown events, invalid bodies).
Unknown event names or non-Map bodies will throw today.- RingingEvent? _receiveRingingEvent(dynamic data) { - if (data is Map) { - final event = Event.values.firstWhere((e) => e.name == data['event']); - final body = Map<String, dynamic>.from(data['body']); - final callData = _callDataFromJson(body); - - return switch (event) { + RingingEvent? _receiveRingingEvent(dynamic data) { + try { + if (data is! Map) return null; + final eventName = data['event'] as String?; + if (eventName == null) return null; + final match = Event.values.where((e) => e.name == eventName).toList(); + if (match.isEmpty) { + debugPrint('Unknown RingingEvent: $eventName'); + return null; + } + final rawBody = data['body']; + if (rawBody is! Map) return null; + final body = Map<String, dynamic>.from(rawBody as Map); + final callData = _callDataFromJson(body); + + return switch (match.first) { Event.actionCallIncoming => ActionCallIncoming(data: callData), Event.actionCallStart => ActionCallStart(data: callData), Event.actionCallAccept => ActionCallAccept(data: callData), Event.actionCallDecline => ActionCallDecline(data: callData), Event.actionCallEnded => ActionCallEnded(data: callData), Event.actionCallTimeout => ActionCallTimeout(data: callData), Event.actionCallCallback => ActionCallCallback(data: callData), Event.actionCallConnected => ActionCallConnected(data: callData), Event.actionDidUpdateDevicePushTokenVoip => ActionDidUpdateDevicePushTokenVoip( - token: body['deviceTokenVoIP'] as String, + token: (body['deviceTokenVoIP'] as String?) ?? '', ), Event.actionCallToggleHold => ActionCallToggleHold( - uuid: body['id'] as String, - isOnHold: body['isOnHold'] as bool, + uuid: (body['id'] as String?) ?? '', + isOnHold: (body['isOnHold'] as bool?) ?? false, ), Event.actionCallToggleMute => ActionCallToggleMute( - uuid: body['id'] as String, - isMuted: body['isMuted'] as bool, + uuid: (body['id'] as String?) ?? '', + isMuted: (body['isMuted'] as bool?) ?? false, ), Event.actionCallToggleDtmf => ActionCallToggleDtmf( - uuid: body['id'] as String, - digits: body['digits'] as String, + uuid: (body['id'] as String?) ?? '', + digits: (body['digits'] as String?) ?? '', ), Event.actionCallToggleGroup => ActionCallToggleGroup( - uuid: body['id'] as String, - callUUIDToGroupWith: body['callUUIDToGroupWith'] as String, + uuid: (body['id'] as String?) ?? '', + callUUIDToGroupWith: (body['callUUIDToGroupWith'] as String?) ?? '', ), Event.actionCallToggleAudioSession => ActionCallToggleAudioSession( - isActive: body['isActive'] as bool, + isActive: (body['isActive'] as bool?) ?? false, ), Event.actionCallCustom => ActionCallCustom(body), }; - } - - return null; + } catch (e, st) { + debugPrint('Error parsing RingingEvent: $e\n$st'); + return null; + } }
🧹 Nitpick comments (7)
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (7)
18-24: Use void typed invokeMethod for init.
The call doesn’t use a return value; tighten the type.Future<void> init( Map<String, dynamic> pushConfiguration, ) async { if (!CurrentPlatform.isIos) return; - await methodChannel.invokeMethod<String>('initData', pushConfiguration); + await methodChannel.invokeMethod<void>('initData', pushConfiguration); }
26-33: Type-safety: add generic types to invokeMethod for all void-returning calls.
Prevents dynamic casts and analyzer warnings.- await methodChannel.invokeMethod("ensureFullScreenIntentPermission"); + await methodChannel.invokeMethod<void>("ensureFullScreenIntentPermission"); - await methodChannel.invokeMethod("showIncomingCall", params.toJson()); + await methodChannel.invokeMethod<void>("showIncomingCall", params.toJson()); - await methodChannel.invokeMethod("hideIncomingCall", params.toJson()); + await methodChannel.invokeMethod<void>("hideIncomingCall", params.toJson()); - await methodChannel.invokeMethod("startCall", params.toJson()); + await methodChannel.invokeMethod<void>("startCall", params.toJson()); - await methodChannel.invokeMethod("holdCall", {'id': id, 'isOnHold': isOnHold}); + await methodChannel.invokeMethod<void>("holdCall", {'id': id, 'isOnHold': isOnHold}); - await methodChannel.invokeMethod("endCall", {'id': id}); + await methodChannel.invokeMethod<void>("endCall", {'id': id}); - await methodChannel.invokeMethod("callConnected", {'id': id}); + await methodChannel.invokeMethod<void>("callConnected", {'id': id}); - await methodChannel.invokeMethod("endAllCalls"); + await methodChannel.invokeMethod<void>("endAllCalls"); - return await methodChannel.invokeMethod("silenceEvents", true); + await methodChannel.invokeMethod<void>("silenceEvents", true); - return await methodChannel.invokeMethod("silenceEvents", false); + await methodChannel.invokeMethod<void>("silenceEvents", false); - return await methodChannel.invokeMethod( + await methodChannel.invokeMethod<void>( "requestNotificationPermission", data); - return await methodChannel.invokeMethod("canUseFullScreenIntent"); + await methodChannel.invokeMethod<void>("canUseFullScreenIntent");Also applies to: 38-43, 57-66, 68-75, 94-102, 103-110, 111-118, 119-124, 143-148, 149-154, 155-166, 168-178
45-55: Naming: prefer “Missed” over “Miss”.
Consider a non-breaking alias; keep current method for compatibility./// Show Miss Call Notification. /// Only Android @override Future<void> showMissCallNotification(StreamVideoPushParams params) async { if (!CurrentPlatform.isAndroid) { return; } await methodChannel.invokeMethod( "showMissCallNotification", params.toJson()); } + +// Non-breaking alias (preferred public name) +Future<void> showMissedCallNotification(StreamVideoPushParams params) => + showMissCallNotification(params);
80-83: Annotate return type for muteCall.
Dart best practice: avoid implicit dynamic Future.- Future muteCall(String id, {bool isMuted = true}) async { - await methodChannel - .invokeMethod("muteCall", {'id': id, 'isMuted': isMuted}); - } + Future<void> muteCall(String id, {bool isMuted = true}) async { + await methodChannel.invokeMethod<void>( + "muteCall", + {'id': id, 'isMuted': isMuted}, + ); + }
89-92: Make isMuted type-safe and resilient.
Avoid dynamic cast; coerce non-bool values safely.- Future<bool> isMuted(String id) async { - return (await methodChannel.invokeMethod("isMuted", {'id': id})) as bool? ?? - false; - } + Future<bool> isMuted(String id) async { + final res = + await methodChannel.invokeMethod<Object?>("isMuted", {'id': id}); + if (res is bool) return res; + if (res is num) return res != 0; + if (res is String) return res.toLowerCase() == 'true'; + return false; + }
129-131: Avoid dynamic for activeCalls; return a typed model.
If native returns an array of call maps, convert to CallData using your helper.- Future<dynamic> activeCalls() async { - return await methodChannel.invokeMethod("activeCalls"); - } + Future<List<CallData>> activeCalls() async { + final list = + await methodChannel.invokeMethod<List<dynamic>>("activeCalls"); + final maps = (list ?? const <dynamic>[]) + .whereType<Map>() + .map((m) => Map<String, dynamic>.from(m as Map)) + .toList(); + return maps.map(_callDataFromJson).toList(); + }Please confirm the native return shape (iOS/Android) matches this expectation.
38-41: Doc nits: standardize terminology.
Use “CallKit” (capital K) and consistent phrasing.-/// On iOS, using Callkit. +/// On iOS, using CallKit. -/// On Android, Nothing(only callback event listener). +/// On Android, no-op (emits callbacks only).Also applies to: 69-71, 76-79, 85-88, 94-97, 103-106, 111-114
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart(1 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart(2 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification_platform_interface.dart(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart
- packages/stream_video_push_notification/lib/stream_video_push_notification_platform_interface.dart
⏰ 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). (7)
- GitHub Check: build
- GitHub Check: analyze
- GitHub Check: analyze_legacy_version
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
🔇 Additional comments (3)
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (3)
5-7: LGTM: new imports are consistent with Ringing API.
15-16: LGTM: dedicated EventChannel added.
137-141: API mismatch resolved for getDevicePushTokenVoIP — nice fix.
Platform-gated and returns a String now.
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart
Outdated
Show resolved
Hide resolved
# Conflicts: # dogfooding/lib/app/app_content.dart # packages/stream_video/CHANGELOG.md # packages/stream_video/lib/src/push_notification/call_kit_events.dart # packages/stream_video_flutter/CHANGELOG.md # packages/stream_video_flutter/android/build.gradle # packages/stream_video_flutter/example/lib/stream_video_sdk.dart # packages/stream_video_push_notification/CHANGELOG.md # packages/stream_video_push_notification/android/build.gradle # packages/stream_video_push_notification/ios/stream_video_push_notification.podspec # packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart # packages/stream_video_push_notification/lib/src/stream_video_push_params.dart # packages/stream_video_push_notification/lib/src/stream_video_push_params.g.dart # packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart # packages/stream_video_push_notification/lib/stream_video_push_notification_platform_interface.dart # packages/stream_video_push_notification/pubspec.yaml
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
packages/stream_video/lib/src/stream_video.dart (1)
700-710: Add a runtime guard for event type T to avoid silent mis-subscriptions.Ensure T is a valid RingingEvent subtype before subscribing; log and return if invalid.
StreamSubscription<T>? onRingingEvent<T extends RingingEvent>( void Function(T event)? onEvent, ) { final manager = pushNotificationManager; if (manager == null) { _logger.e(() => '[onRingingEvent] rejected (no manager)'); return null; } + // Guard against invalid event types at runtime to prevent silent mis-subscriptions. + if (onEvent != null && !_isValidRingingEventType<T>()) { + _logger.e(() => '[onRingingEvent] rejected (invalid event type: $T)'); + return null; + } return manager.on<T>(onEvent); }Add helper:
// Place inside StreamVideo (or as a private top-level helper). bool _isValidRingingEventType<T extends RingingEvent>() { return T == RingingEvent || T == ActionCallAccept || T == ActionCallDecline || T == ActionCallTimeout || T == ActionCallEnded; }
🧹 Nitpick comments (2)
packages/stream_video/lib/src/stream_video.dart (2)
712-726: Specify the generic type parameter explicitly.Avoid inference pitfalls by subscribing to the base RingingEvent type explicitly.
- return onRingingEvent( + return onRingingEvent<RingingEvent>( (event) { if (event is ActionCallAccept || event is ActionCallDecline || event is ActionCallTimeout || event is ActionCallEnded) { disposingCallback?.call(); dispose(); } }, );
987-991: Make callerName fallback robust and parse hasVideo strictly.
- Prefer trimmed, non-empty caller display name; fallback to createdByName; finally to createdById.
- Interpret video payload as explicit boolean.
- callerName: (callDisplayName?.isNotEmpty ?? false) - ? callDisplayName - : createdByName, + callerName: (callDisplayName?.trim().isNotEmpty ?? false) + ? callDisplayName!.trim() + : (createdByName?.trim().isNotEmpty ?? false) + ? createdByName!.trim() + : createdById, @@ - callerName: (callDisplayName?.isNotEmpty ?? false) - ? callDisplayName - : createdByName, + callerName: (callDisplayName?.trim().isNotEmpty ?? false) + ? callDisplayName!.trim() + : (createdByName?.trim().isNotEmpty ?? false) + ? createdByName!.trim() + : createdById, - hasVideo: hasVideo != 'false', + hasVideo: (hasVideo?.toLowerCase() == 'true'),Also applies to: 1010-1015
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
dogfooding/lib/app/app_content.dart(2 hunks)dogfooding/lib/di/injector.dart(1 hunks)packages/stream_video/CHANGELOG.md(1 hunks)packages/stream_video/lib/src/push_notification/push_notification_manager.dart(5 hunks)packages/stream_video/lib/src/stream_video.dart(6 hunks)packages/stream_video_flutter/CHANGELOG.md(1 hunks)packages/stream_video_flutter/android/build.gradle(1 hunks)packages/stream_video_flutter/example/lib/app.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/stream_video/lib/src/push_notification/push_notification_manager.dart
- dogfooding/lib/app/app_content.dart
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
packages/stream_video_flutter/CHANGELOG.md
10-10: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
11-11: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
⏰ 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: stream_video_noise_cancellation
- GitHub Check: stream_video
- GitHub Check: stream_video_flutter
- GitHub Check: analyze_legacy_version
- GitHub Check: analyze
- GitHub Check: build
🔇 Additional comments (9)
packages/stream_video/lib/src/stream_video.dart (9)
762-772: Deprecation aliasing looks correct.
773-788: Core Ringing observers wiring LGTM.
790-799: Deprecated accept alias correctly delegates.
801-812: observeCallAcceptRingingEvent implementation LGTM.
814-818: Deprecated decline alias correctly delegates.
819-821: observeCallDeclinedRingingEvent implementation LGTM.
823-827: Deprecated ended alias correctly delegates.
828-830: observeCallEndedRingingEvent implementation LGTM.
890-894: Doc clarifies platform-specific handling; good.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
dogfooding/lib/di/injector.dart (1)
169-172: Verify platform assets for push configuration.
IOSPushConfiguration.iconNameexpects an IconMask asset in the iOS bundle, andAndroidPushConfiguration.defaultAvatarrequiresassets/logo.pngto be declared underflutter.assets. Please confirm both are present; otherwise push UI will fail to load the icons at runtime.#!/bin/bash set -euo pipefail echo "Looking for assets/logo.png in the repo:" fd -a 'logo.png' | rg -n 'assets/logo\.png$' || echo "assets/logo.png not found" echo echo "Checking pubspec.yaml files for assets/logo.png declaration:" fd -a 'pubspec.yaml' | xargs -I{} rg -nC3 'assets/logo\.png' {} echo echo "Searching iOS project for IconMask asset/catalog entries:" fd -a 'IconMask*' ios || true rg -n 'IconMask' -g 'ios/**' || truepackages/stream_video/lib/src/push_notification/call_kit_events.dart (1)
32-32: Grammar issues remain from previous review.The article corrections ("a incoming" → "an incoming", "a outgoing" → "an outgoing") flagged in the previous review have not yet been applied.
Also applies to: 46-46, 60-61, 76-77, 92-92, 106-108
🧹 Nitpick comments (1)
packages/stream_video/lib/src/push_notification/call_kit_events.dart (1)
264-310: Breaking rename: consider adding deprecated getter for smoother migration.The
nameCaller→callerNamerename is a breaking change. A previous review suggested adding a deprecated getter to ease migration, which has not yet been implemented.Apply this diff to add a deprecated alias:
/// Name of the caller. final String? callerName; + + @Deprecated('Use callerName instead.') + String? get nameCaller => callerName;This allows existing code using
nameCallerto continue working with a deprecation warning, providing a gentler migration path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
dogfooding/lib/di/injector.dart(1 hunks)packages/stream_video/lib/src/push_notification/call_kit_events.dart(17 hunks)packages/stream_video_flutter/example/lib/stream_video_sdk.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_video_flutter/example/lib/stream_video_sdk.dart
⏰ 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). (7)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: analyze
- GitHub Check: build
- GitHub Check: analyze_legacy_version
🔇 Additional comments (4)
packages/stream_video/lib/src/push_notification/call_kit_events.dart (4)
3-4: LGTM! Good backward compatibility approach.The deprecated typedef provides a smooth migration path for code using the old
CallKitEventname while guiding users towardRingingEvent.
6-15: LGTM! Base class rename aligns with PR objectives.The rename from
CallKitEventtoRingingEventappropriately reflects the migration to internal ringing functionality and the removal of theflutter_callkit_incomingdependency.
190-209: LGTM! Typo correction in class name.The rename from
ActionCallToggleDmtftoActionCallToggleDtmfcorrectly spells "DTMF" (Dual-Tone Multi-Frequency) and the documentation has been updated accordingly.
232-245: LGTM! Parameter rename improves consistency.The rename from
isActivatetoisActivefollows better naming conventions and is properly reflected in the field, documentation, and props list.
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:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/stream_video_flutter/android/build.gradle(1 hunks)packages/stream_video_push_notification/android/build.gradle(1 hunks)
⏰ 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). (8)
- GitHub Check: analyze_legacy_version
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video
- GitHub Check: build
- GitHub Check: analyze
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart (1)
284-286: Do not register Android tokens as VoIP.
pushProvider.onTokenRefreshruns on both platforms. PassingisVoIP: truehere causes Android FCM tokens to be stored as VoIP tokens server-side, breaking notification routing. Use an iOS-only flag (or provider capability) instead.- () => pushProvider.onTokenRefresh.listen( - (token) => registerDevice(token, true), - ), + () => pushProvider.onTokenRefresh.listen( + (token) => registerDevice( + token, + CurrentPlatform.isIos, + ), + ),
♻️ Duplicate comments (5)
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (2)
194-204: Null‑safe JSON conversion in _callDataFromJsonGuard casts and map conversions; current code can throw if shapes differ.
- CallData _callDataFromJson(Map<String, dynamic> json) { - final extraData = json['extra']?.cast<String, dynamic>(); - return CallData( - uuid: json['id'] as String?, - callCid: extraData?['callCid'] as String?, - handle: json['handle'] as String?, - callerName: json['callerName'] as String?, - hasVideo: json['type'] == 1, - extraData: extraData, - ); - } + CallData _callDataFromJson(Map<String, dynamic> json) { + final extraRaw = json['extra']; + final extraData = (extraRaw is Map) + ? Map<String, dynamic>.from(extraRaw as Map) + : null; + final typeVal = json['type']; + final hasVideo = typeVal is int ? typeVal == 1 : false; + return CallData( + uuid: json['id'] as String?, + callCid: extraData?['callCid'] as String?, + handle: json['handle'] as String?, + callerName: json['callerName'] as String?, + hasVideo: hasVideo, + extraData: extraData, + ); + }
206-246: Harden event parsing with guards and try/catchUnknown event names and mismatched bodies can throw. Parse defensively and log.
- RingingEvent? _receiveRingingEvent(dynamic data) { - if (data is Map) { - final event = Event.values.firstWhere((e) => e.name == data['event']); - final body = Map<String, dynamic>.from(data['body']); - final callData = _callDataFromJson(body); - return switch (event) { + RingingEvent? _receiveRingingEvent(dynamic data) { + try { + if (data is! Map) return null; + final eventName = data['event'] as String?; + final event = Event.values.firstWhere( + (e) => e.name == eventName, + orElse: () => Event.actionCallCustom, + ); + final rawBody = data['body']; + final body = rawBody is Map ? Map<String, dynamic>.from(rawBody) : const <String, dynamic>{}; + final callData = _callDataFromJson(body); + return switch (event) { Event.actionCallIncoming => ActionCallIncoming(data: callData), Event.actionCallStart => ActionCallStart(data: callData), Event.actionCallAccept => ActionCallAccept(data: callData), Event.actionCallDecline => ActionCallDecline(data: callData), Event.actionCallEnded => ActionCallEnded(data: callData), Event.actionCallTimeout => ActionCallTimeout(data: callData), Event.actionCallCallback => ActionCallCallback(data: callData), Event.actionCallConnected => ActionCallConnected(data: callData), - Event.actionDidUpdateDevicePushTokenVoip => + Event.actionDidUpdateDevicePushTokenVoip => ActionDidUpdateDevicePushTokenVoip( - token: body['deviceTokenVoIP'] as String, + token: (body['deviceTokenVoIP'] as String?) ?? '', ), Event.actionCallToggleHold => ActionCallToggleHold( - uuid: body['id'] as String, - isOnHold: body['isOnHold'] as bool, + uuid: (body['id'] as String?) ?? '', + isOnHold: (body['isOnHold'] as bool?) ?? false, ), Event.actionCallToggleMute => ActionCallToggleMute( - uuid: body['id'] as String, - isMuted: body['isMuted'] as bool, + uuid: (body['id'] as String?) ?? '', + isMuted: (body['isMuted'] as bool?) ?? false, ), Event.actionCallToggleDtmf => ActionCallToggleDtmf( - uuid: body['id'] as String, - digits: body['digits'] as String, + uuid: (body['id'] as String?) ?? '', + digits: (body['digits'] as String?) ?? '', ), Event.actionCallToggleGroup => ActionCallToggleGroup( - uuid: body['id'] as String, - callUUIDToGroupWith: body['callUUIDToGroupWith'] as String, + uuid: (body['id'] as String?) ?? '', + callUUIDToGroupWith: (body['callUUIDToGroupWith'] as String?) ?? '', ), Event.actionCallToggleAudioSession => ActionCallToggleAudioSession( - isActive: body['isActive'] as bool, + isActive: (body['isActive'] as bool?) ?? false, ), Event.actionCallCustom => ActionCallCustom(body), }; - } - - return null; + } catch (e, st) { + debugPrint('Error parsing ringing event: $e\n$st\nData: $data'); + return null; + } }packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart (1)
80-83: Standardize VoIP casing and dedupe emissions
- Use consistent casing (prefer VoIP).
- Add
.distinct()to avoid duplicate token emissions.- return RingingEventBroadcaster().onEvent - .whereType<ActionDidUpdateDevicePushTokenVoip>() - .map((event) => event.token); + return RingingEventBroadcaster() + .onEvent + .whereType<ActionDidUpdateDevicePushTokenVoip>() // or ...VoIP after renaming + .map((event) => event.token) + .distinct();packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart (2)
143-147: Guarantee unsilence runs even on failure.If
endCallByCidthrows, the awaitedunsilenceEventsnever executes, leaving the broadcaster permanently silenced and breaking subsequent call control. Wrap the sequence in a try/finally (or reuse a shared helper) sounsilenceEventsalways fires.- await StreamVideoPushNotificationPlatform.instance.silenceEvents(); - await endCallByCid(event.callCid.toString()); - await Future<void>.delayed(const Duration(milliseconds: 300)); - await StreamVideoPushNotificationPlatform.instance - .unsilenceEvents(); + await StreamVideoPushNotificationPlatform.instance.silenceEvents(); + try { + await endCallByCid(event.callCid.toString()); + await Future<void>.delayed(const Duration(milliseconds: 300)); + } finally { + await StreamVideoPushNotificationPlatform.instance + .unsilenceEvents(); + }
456-462: Always unsilence after mute attempts.If
muteCallthrows,unsilenceEventsnever runs and the SDK stays muted. Use a try/finally (or reuse a helper) around the mute call so unsilence happens even on errors.- await StreamVideoPushNotificationPlatform.instance.silenceEvents(); - await StreamVideoPushNotificationPlatform.instance.muteCall( - call.uuid!, - isMuted: isMuted, - ); - await StreamVideoPushNotificationPlatform.instance.unsilenceEvents(); + await StreamVideoPushNotificationPlatform.instance.silenceEvents(); + try { + await StreamVideoPushNotificationPlatform.instance.muteCall( + call.uuid!, + isMuted: isMuted, + ); + } finally { + await StreamVideoPushNotificationPlatform.instance.unsilenceEvents(); + }
🧹 Nitpick comments (8)
packages/stream_video_push_notification/lib/src/stream_video_call_event.dart (1)
30-66: Update the documentation comment for the EventX extension.The current comment implies Dart 2.17.0 compatibility, but this package requires SDK ≥ 3.8.0 and the extension exists to provide custom string constants for platform interop. Replace it with:
/// Provides string constants for Event enum values used in platform interopdogfooding/lib/app/firebase_messaging_handler.dart (2)
40-47: Avoid double reset; make cleanup idempotent
AppInjector.reset()is invoked indisposingCallbackand again after handling the message. This can race or run twice. Make reset single-shot.Apply a guard and reuse it in both places:
// Initialise the video client. final streamVideo = AppInjector.registerStreamVideo( tokenResponse, User(info: credentials.userInfo), prefs.environment, ); - final subscription = streamVideo.observeCallDeclinedRingingEvent(); + final subscription = streamVideo.observeCallDeclinedRingingEvent(); + var _resetDone = false; + void _safeReset() { + if (_resetDone) return; + _resetDone = true; + AppInjector.reset(); + } streamVideo.disposeAfterResolvingRinging( disposingCallback: () { - subscription?.cancel(); - AppInjector.reset(); + subscription?.cancel(); + _safeReset(); }, ); // Handle the message. await handleRemoteMessage(message); ... - return AppInjector.reset(); + return _safeReset();Please confirm
AppInjector.reset()is idempotent; otherwise, use this guard.Also applies to: 56-58
40-45: Minor: make the subscription non-nullableIf
observeCallDeclinedRingingEvent()never returns null, declare a non-nullable type and drop?.cancel().- final subscription = streamVideo.observeCallDeclinedRingingEvent(); + final StreamSubscription subscription = + streamVideo.observeCallDeclinedRingingEvent(); ... - subscription?.cancel(); + subscription.cancel();packages/stream_video_push_notification/lib/stream_video_push_notification_platform_interface.dart (1)
40-44: Tighten API: non-null events and typed permission request
Prefer
Stream<RingingEvent>to avoid nullable events at the interface. If parsing can fail, filter nulls in the method-channel impl.
requestNotificationPermission(dynamic data)should use a small typed model or aMap<String, Object?>for clarity.Change signature to
Stream<RingingEvent> get onEvent;and ensure the method-channel uses.where((e) => e != null).cast<RingingEvent>().Replace
dynamic datawith a typed DTO orMap<String, Object?> data.Also applies to: 140-144
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart (3)
35-38: Emit non-null eventsFilter nulls before exposing to callers.
- Stream<RingingEvent?> get onEvent => - eventChannel.receiveBroadcastStream().map(_receiveRingingEvent); + Stream<RingingEvent?> get onEvent => eventChannel + .receiveBroadcastStream() + .map(_receiveRingingEvent) + .where((e) => e != null) + .cast<RingingEvent>();
94-97: Use typed invokeMethod for isMutedAvoid dynamic cast; request a bool from the platform.
- return (await methodChannel.invokeMethod('isMuted', {'id': id})) as bool? ?? - false; + final muted = + await methodChannel.invokeMethod<bool>('isMuted', {'id': id}); + return muted ?? false;
144-148: Type the VoIP token platform callRequest a
Stringfrom the channel for safety.- final token = await methodChannel.invokeMethod('getDevicePushTokenVoIP'); + final token = + await methodChannel.invokeMethod<String>('getDevicePushTokenVoIP'); return token ?? '';packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart (1)
69-73: VoIP token retrieval is fine; minor nitReturning null for empty tokens is good. Optionally trim whitespace to be safe.
- if (token.isEmpty) return null; + if (token.trim().isEmpty) return null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
dogfooding/lib/app/firebase_messaging_handler.dart(1 hunks)packages/stream_video/lib/src/sorting/call_participant_sorting_presets.dart(0 hunks)packages/stream_video_flutter/pubspec.yaml(1 hunks)packages/stream_video_push_notification/lib/src/stream_video_call_event.dart(1 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart(1 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_notification.dart(16 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_params.dart(5 hunks)packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart(2 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification.dart(1 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart(2 hunks)packages/stream_video_push_notification/lib/stream_video_push_notification_platform_interface.dart(3 hunks)
💤 Files with no reviewable changes (1)
- packages/stream_video/lib/src/sorting/call_participant_sorting_presets.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_video_push_notification/lib/src/stream_video_push_configuration.dart
⏰ 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: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video
- GitHub Check: stream_video_flutter
- GitHub Check: analyze
- GitHub Check: build
🔇 Additional comments (4)
packages/stream_video_flutter/pubspec.yaml (1)
30-30: Dev tooling bump looks good.Upgrading the dev dependency to
alchemist ^0.12.1aligns with the rest of the migration; no issues spotted from the pubspec side.packages/stream_video_push_notification/lib/src/stream_video_call_event.dart (1)
12-28: LGTM!The
Eventenum is well-defined with clear, descriptive names following a consistent pattern. It comprehensively covers the call lifecycle events and user actions needed for the push notification system.packages/stream_video_push_notification/lib/stream_video_push_notification.dart (1)
13-15: Exports/visibility LGTMExposing
stream_video_push_configuration.dartand hidingRingingEventBroadcaster/StreamTokenProvideraligns the public surface with the new Ringing API.packages/stream_video_push_notification/lib/src/stream_video_push_provider.dart (1)
15-22: APN provider constructor LGTMDefaulting to the VoIP token stream provider and setting
isVoIP = trueis correct.
packages/stream_video_push_notification/lib/src/stream_video_call_event.dart
Show resolved
Hide resolved
packages/stream_video_push_notification/lib/stream_video_push_notification_method_channel.dart
Show resolved
Hide resolved
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:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/actions/pana/action.yml(2 hunks)packages/stream_video_push_notification/pubspec.yaml(1 hunks)
⏰ 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). (7)
- GitHub Check: analyze_legacy_version
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video
- GitHub Check: build
- GitHub Check: analyze
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/pana/action.yml (1)
61-64: Fix brittle score parsing and invocation; ensure reliable numeric compare.Current parsing assigns a literal to SCORE and relies on arithmetic re-evaluation. Also, calling
panaassumes PATH contains pub-cache/bin. Useflutter pub global run pana, parse safely, and compare numerically.Apply:
- PANA=$(pana . --no-warning); PANA_SCORE=$(echo $PANA | sed -n "s/.*Points: \([0-9]*\)\/\([0-9]*\)./\1\/\2/p") - echo "Score: $PANA_SCORE" - IFS='/'; read -a SCORE_ARR <<< "$PANA_SCORE"; SCORE=SCORE_ARR[0]; - if (( $SCORE < ${{inputs.min_score}} )); then echo "The minimum score of ${{inputs.min_score}} was not met!"; exit 1; fi + set -eo pipefail + PANA_OUTPUT=$(flutter pub global run pana . --no-warning) + PANA_SCORE=$(sed -n 's/.*Points: \([0-9]\+\)\/\([0-9]\+\).*/\1\/\2/p' <<< "$PANA_OUTPUT") + echo "Score: $PANA_SCORE" + IFS=/ read -r SCORE MAX <<< "$PANA_SCORE" + if (( SCORE < ${{ inputs.min_score }} )); then + echo "The minimum score of ${{ inputs.min_score }} was not met!" + exit 1 + fi
♻️ Duplicate comments (1)
.github/actions/pana/action.yml (1)
27-43: Past WD issue resolved by explicit targets.Explicit
-i packages/.../pubspec.yamleliminates the earlier working-directory bug. No action needed.
🧹 Nitpick comments (1)
.github/actions/pana/action.yml (1)
52-56: Optional: handle emptypana_version.If
inputs.pana_versionis empty, the trailing arg is unnecessary. Consider defaulting or omitting.Example:
- run: flutter pub global activate pana ${{inputs.pana_version}} + run: | + if [[ -n "${{ inputs.pana_version }}" ]]; then + flutter pub global activate pana "${{ inputs.pana_version }}" + else + flutter pub global activate pana + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/actions/pana/action.yml(2 hunks)
⏰ 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: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
- GitHub Check: analyze
- GitHub Check: build
resolves FLU-37
resolves FLU-220
docs: https://github.com/GetStream/docs-content/pull/571
This PR removes the
flutter_callkit_incomingdependency and moves the logic of handling CallKit and ringing notifications tostream_video_push_notificationpackage.Summary by CodeRabbit
Breaking Changes
New Features
Android
iOS