Fixed incoming call notification not waking the device on locked screen#1639
Merged
aleksandar-apostolov merged 4 commits intodevelopfrom Apr 1, 2026
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
...n/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.kt
Show resolved
Hide resolved
...n/io/getstream/video/android/core/notifications/handlers/StreamDefaultNotificationHandler.kt
Outdated
Show resolved
Hide resolved
Contributor
SDK Size Comparison 📏
|
WalkthroughA parameterized notification API is introduced, allowing notification handlers to differentiate setup notifications based on trigger type and call ID. The zero-argument Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.kt (1)
186-188: Add KDoc for the new publicStreamSettingUpCallNotificationProvidercontract.This is a public API surface and should document
triggerexpectations and valid values to prevent mismatched implementations.As per coding guidelines: "Use KDoc (
/** ... */) for public APIs and complex subsystems; link to Stream docs when relevant".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.kt` around lines 186 - 188, Add KDoc to the public interface StreamSettingUpCallNotificationProvider and its method getSettingUpCallNotification: use /** ... */ above the interface and above getSettingUpCallNotification to describe the purpose of the contract, the expected semantics of the trigger parameter (enumerate or list valid trigger string values and whether they are case-sensitive), the meaning of callId (StreamCallId), the returned Notification behavior (when null is allowed), and link to the relevant Stream docs for notification triggers; ensure the KDoc covers visibility as a public API and any threading/usage notes so implementers won’t misinterpret trigger expectations.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamDefaultNotificationHandler.kt (1)
556-559: Extract duplicated incoming-notification builder logic to reduce drift risk.This branch duplicates core incoming notification construction logic already present in the file. Reusing a shared internal builder will reduce behavioral divergence in future changes.
Also applies to: 580-606
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamDefaultNotificationHandler.kt` around lines 556 - 559, The duplicated incoming notification construction logic should be extracted into a shared internal builder function and both callers should delegate to it to avoid drift; create a private helper (e.g., buildIncomingCallNotification or getIncomingCallNotificationInternal) that encapsulates the common steps (notification channel, pending intent, actions, content/title, extras, and notification builder configuration) and replace the duplicated blocks at the current comment region and lines ~580-606 to call that helper, ensuring the helper accepts necessary parameters (callId, callerName, isVideo, context/notificationManager, and any callbacks) so both existing functions reuse the same implementation.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/DefaultNotificationHandler.kt (1)
287-290: Consider honoringtriggerfor incoming setup notifications here as well.This overload currently ignores
trigger/callIdand always falls back to the generic setup notification. For apps still usingDefaultNotificationHandler, incoming lock-screen wake behavior may not match the new trigger-aware path.♻️ Suggested direction
override fun getSettingUpCallNotification( trigger: String, callId: StreamCallId, -): Notification? = getSettingUpCallNotification() +): Notification? { + return when (trigger) { + io.getstream.video.android.core.notifications.internal.service.CallService.TRIGGER_INCOMING_CALL -> + getRingingCallNotification( + ringingState = RingingState.Incoming(), + callId = callId, + callDisplayName = null, + shouldHaveContentIntent = true, + payload = emptyMap(), + ) ?: getSettingUpCallNotification() + else -> getSettingUpCallNotification() + } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/DefaultNotificationHandler.kt` around lines 287 - 290, The override of getSettingUpCallNotification(trigger: String, callId: StreamCallId) currently ignores trigger and callId and always returns the generic getSettingUpCallNotification(); update it to honor trigger and callId by routing to the trigger-aware creation path (e.g., when trigger indicates an incoming/lock-screen wake, call the existing incoming-call notification builder such as the incoming call notification method used elsewhere in DefaultNotificationHandler, passing callId and trigger), and only fall back to getSettingUpCallNotification() when no trigger-specific behavior applies; ensure you reference and reuse the same helper/creator methods inside DefaultNotificationHandler so trigger-aware behavior matches the new path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamDefaultNotificationHandler.kt`:
- Around line 574-595: The notification builder uses fullScreenPendingIntent
(from intentResolver.searchIncomingCallPendingIntent) without null-checking;
since searchIncomingCallPendingIntent can return null, update
StreamDefaultNotificationHandler where fullScreenPendingIntent is obtained to
guard against null and avoid calling setFullScreenIntent/setContentIntent with a
null PendingIntent: explicitly check fullScreenPendingIntent != null before
calling setFullScreenIntent(fullScreenPendingIntent, true) and
setContentIntent(fullScreenPendingIntent), and if null, fall back to a safe
behavior such as omitting full-screen/content intents or creating a conservative
content intent (e.g., a generic tap intent) before calling
ensureChannelAndBuildNotification so ensureChannelAndBuildNotification and the
notification build path never receive a null PendingIntent.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.kt`:
- Around line 355-359: The `@Deprecated` annotation on the no-arg
getSettingUpCallNotification() should not include a ReplaceWith suggesting
StreamSettingUpCallNotificationProvider.getSettingUpCallNotification(trigger,callId)
because the replacement requires parameters not available at call sites; update
the `@Deprecated` on getSettingUpCallNotification to be message-only (remove the
replaceWith argument) so it points callers to
StreamSettingUpCallNotificationProvider.getSettingUpCallNotification without
providing a broken quick-fix.
- Line 235: StreamNotificationProvider now inherits
StreamSettingUpCallNotificationProvider which declares
getSettingUpCallNotification(trigger: String, callId: StreamCallId) and breaks
existing implementations that only override the deprecated no-arg
getSettingUpCallNotification(); add a default implementation of the two-arg
method inside the StreamNotificationProvider interface that delegates to the
zero-arg deprecated getSettingUpCallNotification() so existing custom providers
continue to compile (refer to StreamNotificationProvider,
StreamSettingUpCallNotificationProvider, getSettingUpCallNotification(trigger:
String, callId: StreamCallId), and the deprecated
getSettingUpCallNotification()).
---
Nitpick comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/DefaultNotificationHandler.kt`:
- Around line 287-290: The override of getSettingUpCallNotification(trigger:
String, callId: StreamCallId) currently ignores trigger and callId and always
returns the generic getSettingUpCallNotification(); update it to honor trigger
and callId by routing to the trigger-aware creation path (e.g., when trigger
indicates an incoming/lock-screen wake, call the existing incoming-call
notification builder such as the incoming call notification method used
elsewhere in DefaultNotificationHandler, passing callId and trigger), and only
fall back to getSettingUpCallNotification() when no trigger-specific behavior
applies; ensure you reference and reuse the same helper/creator methods inside
DefaultNotificationHandler so trigger-aware behavior matches the new path.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamDefaultNotificationHandler.kt`:
- Around line 556-559: The duplicated incoming notification construction logic
should be extracted into a shared internal builder function and both callers
should delegate to it to avoid drift; create a private helper (e.g.,
buildIncomingCallNotification or getIncomingCallNotificationInternal) that
encapsulates the common steps (notification channel, pending intent, actions,
content/title, extras, and notification builder configuration) and replace the
duplicated blocks at the current comment region and lines ~580-606 to call that
helper, ensuring the helper accepts necessary parameters (callId, callerName,
isVideo, context/notificationManager, and any callbacks) so both existing
functions reuse the same implementation.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.kt`:
- Around line 186-188: Add KDoc to the public interface
StreamSettingUpCallNotificationProvider and its method
getSettingUpCallNotification: use /** ... */ above the interface and above
getSettingUpCallNotification to describe the purpose of the contract, the
expected semantics of the trigger parameter (enumerate or list valid trigger
string values and whether they are case-sensitive), the meaning of callId
(StreamCallId), the returned Notification behavior (when null is allowed), and
link to the relevant Stream docs for notification triggers; ensure the KDoc
covers visibility as a public API and any threading/usage notes so implementers
won’t misinterpret trigger expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 56590115-144a-4a64-9aa3-81fbe6b1dcee
📒 Files selected for processing (6)
stream-video-android-core/api/stream-video-android-core.apistream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/DefaultNotificationHandler.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamDefaultNotificationHandler.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/NoOpNotificationHandler.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.kt
...n/io/getstream/video/android/core/notifications/handlers/StreamDefaultNotificationHandler.kt
Outdated
Show resolved
Hide resolved
...n/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.kt
Show resolved
Hide resolved
...n/kotlin/io/getstream/video/android/core/notifications/handlers/StreamNotificationHandler.kt
Show resolved
Hide resolved
rahul-lohra
changed the title
rahul-lohra
changed the title
aleksandar-apostolov
approved these changes
Apr 1, 2026
aleksandar-apostolov
changed the title
aleksandar-apostolov
changed the title
aleksandar-apostolov
deleted the
bugfix/rahullohra/incoming-call-lock-screen-notification
branch
|
Collaborator
|
🚀 Available in v1.21.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Fix locked screen incoming-call wake up notification behaviour
Implementation
When we launch a temporary notification for an incoming call, it should include all the properties of the standard incoming call notification.
🎨 UI Changes
None
Testing
Summary by CodeRabbit
New Features
Deprecations