[WIP] Improve Foreground Service orchestration when transitioning fromn outgoing call to active call#1624
Draft
rahul-lohra wants to merge 8 commits intodevelopfrom
Draft
Conversation
Introduces `OutgoingCallType` (Video, Audio, Unknown) to `StreamCallActivity.callIntent`. This allows specifying whether an outgoing call should start with audio-only or with video enabled. The permission check for starting a call is now aware of the outgoing call type to request the correct permissions (camera and/or microphone) before the call is created.
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
rahul-lohra
changed the title
rahul-lohra
changed the title
WalkthroughThe PR introduces a typed Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as DirectCallJoinScreen
participant Nav as DogfoodingNavHost
participant Activity as StreamCallActivity
participant PermMgr as PermissionManager
participant CallSvc as CallService
User->>UI: Click Audio/Video Call
activate UI
UI->>UI: Determine OutgoingCallType
UI->>Nav: navigateToDirectCall(cid, members, joinAndRing, OutgoingCallType)
deactivate UI
activate Nav
Nav->>Activity: callIntent(..., outgoingCallType)
deactivate Nav
Activity->>Activity: Extract OutgoingCallType from intent
activate Activity
Activity->>Activity: Compute outgoing capabilities (Audio/Video)
Activity->>PermMgr: hasRequiredCallPermissions(call, capabilities)
activate PermMgr
PermMgr->>PermMgr: Check RECORD_AUDIO & CAMERA perms
PermMgr-->>Activity: Boolean result
deactivate PermMgr
alt Permissions Granted
Activity->>CallSvc: Create call with Trigger.OutgoingCall
activate CallSvc
CallSvc->>CallSvc: Show outgoing notification
CallSvc->>CallSvc: Start foreground service
CallSvc-->>Activity: Call created
deactivate CallSvc
else Permissions Denied
Activity->>Activity: No-op (TODO)
end
deactivate Activity
sequenceDiagram
participant Service as CallService
participant Intent as Intent Extra
participant Trigger as Trigger Type
participant Notification as NotificationManager
Service->>Intent: Read EXTRA_OUTGOING_CALL_TYPE
activate Service
Intent-->>Service: String value (e.g., "video")
Service->>Trigger: toTrigger(stringValue)
activate Trigger
Trigger-->>Service: Trigger.OutgoingCall
deactivate Trigger
Service->>Notification: handleNotification(Trigger.OutgoingCall)
activate Notification
Notification->>Notification: Match on Trigger type
Notification->>Notification: Show appropriate notification
deactivate Notification
deactivate Service
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/permission/PermissionManager.kt (2)
1-33:⚠️ Potential issue | 🟡 MinorDuplicate license header block.
Lines 1–15 and 19–33 contain two separate license headers. The second block (with the 2014-2022 date) should be removed.
Proposed fix
package io.getstream.video.android.ui.common.permission -/* - * Copyright (c) 2014-2022 Stream.io Inc. All rights reserved. - * - * Licensed under the Stream License; - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://github.com/GetStream/stream-video-android/blob/main/LICENSE - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - import android.Manifest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/permission/PermissionManager.kt` around lines 1 - 33, Remove the duplicated license header in PermissionManager.kt by deleting the second block (the 2014-2022 license) that appears after the first header (2014-2026); keep only the topmost license comment and ensure no other code/comments are altered so the remaining file header and package declaration remain intact.
101-127:⚠️ Potential issue | 🔴 CriticalThe
outgoingCallCapabilitiesparameter is ineffective—ringingStateis notOutgoingwhenhasRequiredCallPermissionsis invoked.In
StreamCallActivity.create(), this function is called beforecall.create(...), butRingingState.Outgoingis only set insideCall.create()after the server responds. The check at line 114 will never be true, andoutgoingCallCapabilitieswill always be empty, silently falling through to thecall.get()path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/permission/PermissionManager.kt` around lines 101 - 127, hasRequiredCallPermissions currently ignores the outgoingCallCapabilities because RingingState.Outgoing isn't set yet; fix it in hasRequiredCallPermissions by using the outgoingCallCapabilities parameter as the first fallback when ownCapabilities is empty (i.e., if call.state.ownCapabilities.value is empty and outgoingCallCapabilities.isNotEmpty() then set capabilities = outgoingCallCapabilities) before calling call.get() or checking ringingState; update references in hasRequiredCallPermissions and keep the existing call.get() failure behavior.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceLauncher.kt (1)
1-33:⚠️ Potential issue | 🟡 MinorDuplicate license header block.
Same as in
PermissionManager.kt— lines 1–15 and 19–33 contain duplicate license headers (with different years: 2026 vs 2024). Remove the second block.🤖 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/internal/service/ServiceLauncher.kt` around lines 1 - 33, Remove the duplicated license header comment block that appears above the package declaration in ServiceLauncher.kt (the second redundant /* ... */ block with the 2014-2024 header) so only a single, correct license header remains; locate the duplicate by finding the package line "package io.getstream.video.android.core.notifications.internal.service" and delete the extra comment block immediately preceding it.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/utils/AndroidUtils.kt (1)
183-186:⚠️ Potential issue | 🟡 MinorStale KDoc – update
@param triggerdescription.The parameter description still references the removed
TRIGGER_*string constants. It should referenceCallService.Companion.Triggerenum values instead.📝 Proposed fix
- * `@param` trigger The trigger that started the service: [TRIGGER_ONGOING_CALL], [TRIGGER_OUTGOING_CALL], [TRIGGER_INCOMING_CALL], [TRIGGER_SHARE_SCREEN] + * `@param` trigger The trigger that started the service: [CallService.Companion.Trigger.OnGoingCall], [CallService.Companion.Trigger.OutgoingCall], [CallService.Companion.Trigger.IncomingCall], [CallService.Companion.Trigger.ShareScreen]🤖 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/utils/AndroidUtils.kt` around lines 183 - 186, Update the stale KDoc for the parameter named "trigger" to reference the enum values on CallService.Companion.Trigger instead of the removed TRIGGER_* string constants; locate the KDoc for the function that documents parameters notificationId, notification and trigger in AndroidUtils.kt and replace the `@param` trigger description with a brief mention of the CallService.Companion.Trigger enum (e.g., list or refer to its enum members) so the documentation points to the correct type and values.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceNotificationRetriever.kt (1)
40-42:⚠️ Potential issue | 🟡 MinorStale KDoc – update
@param triggerdescription.The example references
CallService.TRIGGER_INCOMING_CALLetc., which are removed string constants. Update to reference theCallService.Companion.Triggerenum values.📝 Proposed fix
- * eg. [CallService.TRIGGER_INCOMING_CALL], [CallService.TRIGGER_ONGOING_CALL], [CallService.TRIGGER_OUTGOING_CALL] + * e.g. [CallService.Companion.Trigger.IncomingCall], [CallService.Companion.Trigger.OnGoingCall], [CallService.Companion.Trigger.OutgoingCall]🤖 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/internal/service/ServiceNotificationRetriever.kt` around lines 40 - 42, Update the stale KDoc for the parameter 'trigger' in ServiceNotificationRetriever (in ServiceNotificationRetriever.kt) so it references the CallService.Companion.Trigger enum values instead of the removed string constants; specifically change the example and description to mention CallService.Companion.Trigger.INCOMING_CALL, .ONGOING_CALL, .OUTGOING_CALL (or the actual enum member names) and ensure the text explains that 'trigger' is a stringified/enum-backed trigger coming from CallService.Companion.Trigger.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.kt (1)
581-610:⚠️ Potential issue | 🟡 MinorRemove or scope the no-op
serviceTriggercollector.The new
collectLatestblock has an empty body and is launched incall.scope, which can outlive the service. Consider removing it until there’s real handling, or scope it toserviceScopeto keep teardown deterministic.As per coding guidelines: Ensure cleanup/teardown paths handle cancellation and failure (important for sockets, queues, retries).Suggested cleanup (remove until implemented)
- call.scope.launch { - call.state.serviceTrigger.collectLatest { - /** - * TODO Rahul, what to write - * Notifications are already updated via notification update-triggers - */ - } - }🤖 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/internal/service/CallService.kt` around lines 581 - 610, The empty collector launched with call.scope.launch over call.state.serviceTrigger is a no-op that may outlive the service; either remove this collectLatest block until real handling is implemented, or change its coroutine scope to the service-bound serviceScope so it is cancelled with the service; locate the collectLatest call in CallService.kt (the call.scope.launch { call.state.serviceTrigger.collectLatest { ... } } block) and either delete it or replace call.scope with serviceScope and ensure proper cancellation/teardown consistent with other observers such as CallServiceNotificationUpdateObserver and the enableCallNotificationUpdates branch.
🧹 Nitpick comments (6)
demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt (2)
142-147: Misleading parameter nameisVideofor typeOutgoingCallType.The parameter name
isVideoimplies aBoolean, but the type isOutgoingCallType. This is inconsistent with the same callback inDirectCallJoinScreen(line 75) where it's correctly namedoutgoingCallType.Proposed fix
onStartCallClick: ( cid: StreamCallId, membersList: String, joinAndRing: Boolean, - isVideo: StreamCallActivity.Companion.OutgoingCallType, + outgoingCallType: StreamCallActivity.Companion.OutgoingCallType, ) -> Unit,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt` around lines 142 - 147, Rename the misleading parameter name isVideo in the onStartCallClick callback to match the correct descriptive name outgoingCallType (type StreamCallActivity.Companion.OutgoingCallType) so the signature aligns with the other DirectCallJoinScreen declaration; update any call sites using onStartCallClick to pass and destructure/outgoingCallType accordingly to avoid treating a non-Boolean as a Boolean.
206-213: Remove commented-out code.Line 207 has a commented-out
StreamCallId("default", ...)line that looks like a debugging artifact.Proposed fix
onStartCallClick( StreamCallId("audio_call", UUID.randomUUID().toString()), -// StreamCallId("default", UUID.randomUUID().toString()), users🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt` around lines 206 - 213, Remove the leftover commented-out debug line containing StreamCallId("default", ...) in DirectCallJoinScreen.kt so only the intended StreamCallId("audio_call", UUID.randomUUID().toString()) is used; locate the StreamCallId construction in the outgoing call invocation inside the DirectCallJoinScreen component (around the block building the call parameters and passing callerJoinsFirst and OutgoingCallType.Audio) and delete the commented line to clean up the code.stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt (2)
149-165: Consider using a Kotlinenum classinstead of asealed classforOutgoingCallType.
OutgoingCallTypehas a fixed set of singleton variants with anamestring, atoString(), and a companion mapper — this is exactly whatenum classprovides out of the box (withvalueOf()andentries). The sealed class approach is more boilerplate for no additional flexibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt` around lines 149 - 165, Replace the sealed class OutgoingCallType with a Kotlin enum class to reduce boilerplate: define enum class OutgoingCallType(val name: String) with entries Video, Audio, Unknown and remove the manual toString(); replace the companion's toOutgoingCallType(name: String) with a lookup using values().firstOrNull { it.name == name } ?: OutgoingCallType.Unknown (or use valueOf with safe handling) and update any usages of OutgoingCallType.Video/Audio/Unknown and the toOutgoingCallType call sites accordingly.
864-890:create()readsoutgoingCallTypefrom the activity intent — implicit coupling.The public
create()method (inherited fromActivityCallOperations) readsEXTRA_OUTGOING_CALL_TYPEdirectly fromthis.intent. This makescreate()silently depend on the activity's intent state, which is fragile — subclasses or different call flows might not set this extra. Consider passingoutgoingCallTypeas an explicit parameter or extracting it once inonIntentActionand passing it down.Additionally, when
outgoingCallTypeisUnknown, theelse -> {}at line 888 silently skips capability setup without logging, so no capabilities are added and the call falls through tocall.get().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt` around lines 864 - 890, The create() method in StreamCallActivity currently reads EXTRA_OUTGOING_CALL_TYPE from this.intent (implicit coupling); instead, extract and normalize the outgoingCallType once (e.g., in onIntentAction) and pass it explicitly into create() (or store it in a clearly-named property set by onIntentAction) so create() no longer depends on intent state; also handle the OutgoingCallType.Unknown branch in the capability-building logic (outgoingCallCapabilities / OwnCapability) by adding a default capability set or at minimum logging a warning before falling through so missing extras are visible.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/utils/AndroidUtils.kt (1)
204-209: Redundantwhenbranches – simplify to a single condition.
OnGoingCall,OutgoingCall,IncomingCall, andelseall evaluate toforegroundServiceType; onlyShareScreendiffers. The three explicit arms add no value and will silently drift if newTriggervalues are introduced.♻️ Proposed simplification
- when (trigger) { - CallService.Companion.Trigger.OnGoingCall -> foregroundServiceType - CallService.Companion.Trigger.OutgoingCall, CallService.Companion.Trigger.IncomingCall -> foregroundServiceType - CallService.Companion.Trigger.ShareScreen -> ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION - else -> foregroundServiceType - }, + when (trigger) { + CallService.Companion.Trigger.ShareScreen -> ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION + else -> foregroundServiceType + },🤖 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/utils/AndroidUtils.kt` around lines 204 - 209, The when-expression in AndroidUtils.kt that checks CallService.Companion.Trigger (OnGoingCall, OutgoingCall, IncomingCall, else) is redundant because all cases except ShareScreen return foregroundServiceType; simplify it to a single conditional that returns ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION only when trigger == CallService.Companion.Trigger.ShareScreen and returns foregroundServiceType for all other Trigger values (use either an if/else or a two-branch when with ShareScreen and an else) to avoid duplicated branches and accidental drift.stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.kt (1)
235-266: Consider delayingupdateCallStateuntil after permission checks.Right now the trigger is emitted even when
verifyPermissionsfails and the service stops immediately. Moving the update below the permission gate keepsserviceTriggerconsistent with actual service start.Suggested adjustment
- updateCallState(params) maybeHandleMediaIntent(intent, params.callId) val notificationId = serviceNotificationRetriever.getNotificationId( params.trigger, params.streamVideo, params.callId, ) @@ if (params.trigger != Trigger.IncomingCall) { if (!verifyPermissions( params.streamVideo, call, params.callId, params.trigger, ) ) { stopServiceGracefully() return START_NOT_STICKY } } + + updateCallState(params) val (notification, _) = getNotificationPair( params.trigger, params.streamVideo, params.callId,🤖 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/internal/service/CallService.kt` around lines 235 - 266, Move the call state update so the serviceTrigger isn't emitted when permissions fail: only call updateCallState(params) after the permission gate succeeds (i.e., after verifyPermissions(...) returns true) or immediately when params.trigger == Trigger.IncomingCall (since incoming calls don't need permissions); keep maybeHandleMediaIntent(params, params.callId) and notificationId retrieval as-is but ensure updateCallState is invoked after the verifyPermissions check (and before promoteToFgServiceIfNoActiveCall where the service is promoted) and remove the original early updateCallState(params) call.
🤖 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/CallState.kt`:
- Around line 727-730: Add an explicit StateFlow type annotation to the exposed
property `serviceTrigger`: change the declaration of `serviceTrigger` so it is
typed as StateFlow<CallService.Companion.Trigger> that wraps the backing
MutableStateFlow `_serviceTrigger` (i.e., make `serviceTrigger` a StateFlow of
CallService.Companion.Trigger and assign `_serviceTrigger.asStateFlow()` to it);
this keeps the API consistent with other exposed StateFlow properties like
`_pinnedParticipants`.
In
`@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt`:
- Around line 858-861: Remove the developer-facing TODO block in
StreamCallActivity (the comment referencing call-related permission checks
before starting the call service) — either implement the permission check in the
relevant method (e.g., where startCallService or similar call-start logic exists
in class StreamCallActivity) or delete/replace the note with a tracked task
reference (issue/PR ID) and a clear comment stating the resolution; do not leave
informal discussion-style TODOs in the code.
- Around line 896-913: In StreamCallActivity, when hasCallPermission is false
the else branch is currently silent; instead call the provided onError callback
with a descriptive exception (e.g., SecurityException or IllegalStateException)
so callers (like onIntentAction) can handle the failure; update the else branch
that surrounds the call.create(...) / result.onOutcome(...) logic to invoke
onError(Throwable("Missing call permission: user must grant CALL/RECORD
permission")) (or similar), referencing the hasCallPermission check, the
call.create(...) flow and the existing onError parameter to locate where to add
this.
---
Outside diff comments:
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.kt`:
- Around line 581-610: The empty collector launched with call.scope.launch over
call.state.serviceTrigger is a no-op that may outlive the service; either remove
this collectLatest block until real handling is implemented, or change its
coroutine scope to the service-bound serviceScope so it is cancelled with the
service; locate the collectLatest call in CallService.kt (the call.scope.launch
{ call.state.serviceTrigger.collectLatest { ... } } block) and either delete it
or replace call.scope with serviceScope and ensure proper cancellation/teardown
consistent with other observers such as CallServiceNotificationUpdateObserver
and the enableCallNotificationUpdates branch.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceLauncher.kt`:
- Around line 1-33: Remove the duplicated license header comment block that
appears above the package declaration in ServiceLauncher.kt (the second
redundant /* ... */ block with the 2014-2024 header) so only a single, correct
license header remains; locate the duplicate by finding the package line
"package io.getstream.video.android.core.notifications.internal.service" and
delete the extra comment block immediately preceding it.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceNotificationRetriever.kt`:
- Around line 40-42: Update the stale KDoc for the parameter 'trigger' in
ServiceNotificationRetriever (in ServiceNotificationRetriever.kt) so it
references the CallService.Companion.Trigger enum values instead of the removed
string constants; specifically change the example and description to mention
CallService.Companion.Trigger.INCOMING_CALL, .ONGOING_CALL, .OUTGOING_CALL (or
the actual enum member names) and ensure the text explains that 'trigger' is a
stringified/enum-backed trigger coming from CallService.Companion.Trigger.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/utils/AndroidUtils.kt`:
- Around line 183-186: Update the stale KDoc for the parameter named "trigger"
to reference the enum values on CallService.Companion.Trigger instead of the
removed TRIGGER_* string constants; locate the KDoc for the function that
documents parameters notificationId, notification and trigger in AndroidUtils.kt
and replace the `@param` trigger description with a brief mention of the
CallService.Companion.Trigger enum (e.g., list or refer to its enum members) so
the documentation points to the correct type and values.
In
`@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/permission/PermissionManager.kt`:
- Around line 1-33: Remove the duplicated license header in PermissionManager.kt
by deleting the second block (the 2014-2022 license) that appears after the
first header (2014-2026); keep only the topmost license comment and ensure no
other code/comments are altered so the remaining file header and package
declaration remain intact.
- Around line 101-127: hasRequiredCallPermissions currently ignores the
outgoingCallCapabilities because RingingState.Outgoing isn't set yet; fix it in
hasRequiredCallPermissions by using the outgoingCallCapabilities parameter as
the first fallback when ownCapabilities is empty (i.e., if
call.state.ownCapabilities.value is empty and
outgoingCallCapabilities.isNotEmpty() then set capabilities =
outgoingCallCapabilities) before calling call.get() or checking ringingState;
update references in hasRequiredCallPermissions and keep the existing call.get()
failure behavior.
---
Nitpick comments:
In
`@demo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.kt`:
- Around line 142-147: Rename the misleading parameter name isVideo in the
onStartCallClick callback to match the correct descriptive name outgoingCallType
(type StreamCallActivity.Companion.OutgoingCallType) so the signature aligns
with the other DirectCallJoinScreen declaration; update any call sites using
onStartCallClick to pass and destructure/outgoingCallType accordingly to avoid
treating a non-Boolean as a Boolean.
- Around line 206-213: Remove the leftover commented-out debug line containing
StreamCallId("default", ...) in DirectCallJoinScreen.kt so only the intended
StreamCallId("audio_call", UUID.randomUUID().toString()) is used; locate the
StreamCallId construction in the outgoing call invocation inside the
DirectCallJoinScreen component (around the block building the call parameters
and passing callerJoinsFirst and OutgoingCallType.Audio) and delete the
commented line to clean up the code.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.kt`:
- Around line 235-266: Move the call state update so the serviceTrigger isn't
emitted when permissions fail: only call updateCallState(params) after the
permission gate succeeds (i.e., after verifyPermissions(...) returns true) or
immediately when params.trigger == Trigger.IncomingCall (since incoming calls
don't need permissions); keep maybeHandleMediaIntent(params, params.callId) and
notificationId retrieval as-is but ensure updateCallState is invoked after the
verifyPermissions check (and before promoteToFgServiceIfNoActiveCall where the
service is promoted) and remove the original early updateCallState(params) call.
In
`@stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/utils/AndroidUtils.kt`:
- Around line 204-209: The when-expression in AndroidUtils.kt that checks
CallService.Companion.Trigger (OnGoingCall, OutgoingCall, IncomingCall, else) is
redundant because all cases except ShareScreen return foregroundServiceType;
simplify it to a single conditional that returns
ServiceInfo.FOREGROUND_SERVICE_TYPE_MEDIA_PROJECTION only when trigger ==
CallService.Companion.Trigger.ShareScreen and returns foregroundServiceType for
all other Trigger values (use either an if/else or a two-branch when with
ShareScreen and an else) to avoid duplicated branches and accidental drift.
In
`@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt`:
- Around line 149-165: Replace the sealed class OutgoingCallType with a Kotlin
enum class to reduce boilerplate: define enum class OutgoingCallType(val name:
String) with entries Video, Audio, Unknown and remove the manual toString();
replace the companion's toOutgoingCallType(name: String) with a lookup using
values().firstOrNull { it.name == name } ?: OutgoingCallType.Unknown (or use
valueOf with safe handling) and update any usages of
OutgoingCallType.Video/Audio/Unknown and the toOutgoingCallType call sites
accordingly.
- Around line 864-890: The create() method in StreamCallActivity currently reads
EXTRA_OUTGOING_CALL_TYPE from this.intent (implicit coupling); instead, extract
and normalize the outgoingCallType once (e.g., in onIntentAction) and pass it
explicitly into create() (or store it in a clearly-named property set by
onIntentAction) so create() no longer depends on intent state; also handle the
OutgoingCallType.Unknown branch in the capability-building logic
(outgoingCallCapabilities / OwnCapability) by adding a default capability set or
at minimum logging a warning before falling through so missing extras are
visible.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
demo-app/src/main/kotlin/io/getstream/video/android/ui/DogfoodingNavHost.ktdemo-app/src/main/kotlin/io/getstream/video/android/ui/outgoing/DirectCallJoinScreen.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ClientState.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/IncomingCallPresenter.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceIntentBuilder.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceLauncher.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceNotificationRetriever.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/ServiceParam.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/models/CallIntentParams.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/observers/CallServiceNotificationUpdateObserver.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/permissions/ForegroundServicePermissionManager.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/screenshare/StreamScreenShareService.ktstream-video-android-core/src/main/kotlin/io/getstream/video/android/core/utils/AndroidUtils.ktstream-video-android-ui-core/api/stream-video-android-ui-core.apistream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.ktstream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/permission/PermissionManager.kt
Comment on lines
727
to
730
| private val _serviceTrigger = | ||
| MutableStateFlow<CallService.Companion.Trigger>(CallService.Companion.Trigger.None) | ||
| internal val serviceTrigger = _serviceTrigger.asStateFlow() | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -e kt -e java | xargs grep -l "class CallService" | head -5Repository: GetStream/stream-video-android
Length of output: 730
🏁 Script executed:
grep -n "enum\|sealed\|object Trigger\|case.*None" stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.kt | head -50Repository: GetStream/stream-video-android
Length of output: 146
🏁 Script executed:
sed -n '117,150p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/notifications/internal/service/CallService.ktRepository: GetStream/stream-video-android
Length of output: 1608
🏁 Script executed:
sed -n '727,732p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.ktRepository: GetStream/stream-video-android
Length of output: 346
🏁 Script executed:
# Check if there are similar patterns in CallState.kt with explicit StateFlow type annotations
grep -n "internal val.*= .*asStateFlow()" stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt | head -10Repository: GetStream/stream-video-android
Length of output: 141
🏁 Script executed:
# Check other internal StateFlow properties in CallState.kt to see the pattern
grep -n "internal val.*=" stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt | grep -i "stateflow\|flow" | head -15Repository: GetStream/stream-video-android
Length of output: 836
🏁 Script executed:
sed -n '313,316p' stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.ktRepository: GetStream/stream-video-android
Length of output: 314
Add explicit StateFlow type annotation to serviceTrigger for consistency.
Trigger.None is correctly defined in the enum. However, serviceTrigger should have an explicit type annotation like other exposed StateFlow properties in this file (see line 313: internal val _pinnedParticipants: StateFlow<...>). Update line 729 to:
internal val serviceTrigger: StateFlow<CallService.Companion.Trigger> = _serviceTrigger.asStateFlow()🤖 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/CallState.kt`
around lines 727 - 730, Add an explicit StateFlow type annotation to the exposed
property `serviceTrigger`: change the declaration of `serviceTrigger` so it is
typed as StateFlow<CallService.Companion.Trigger> that wraps the backing
MutableStateFlow `_serviceTrigger` (i.e., make `serviceTrigger` a StateFlow of
CallService.Companion.Trigger and assign `_serviceTrigger.asStateFlow()` to it);
this keeps the API consistent with other exposed StateFlow properties like
`_pinnedParticipants`.
Comment on lines
858
to
861
| /** | ||
| * Todo Rahul we need to check call-related permission here - as this is going to start the callservice | ||
| * Is this the right place | ||
| */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove TODO comments before merging.
Lines 858–861 contain a developer-facing TODO that reads like internal discussion notes. These should be resolved or removed before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt`
around lines 858 - 861, Remove the developer-facing TODO block in
StreamCallActivity (the comment referencing call-related permission checks
before starting the call service) — either implement the permission check in the
relevant method (e.g., where startCallService or similar call-start logic exists
in class StreamCallActivity) or delete/replace the note with a tracked task
reference (issue/PR ID) and a clear comment stating the resolution; do not leave
informal discussion-style TODOs in the code.
Comment on lines
896
to
913
| ) | ||
| result.onOutcome(call, onSuccess, onError) | ||
| if (hasCallPermission) { | ||
| val instance = StreamVideo.instance() | ||
| val result = call.create( | ||
| // List of all users, containing the caller also | ||
| memberIds = members + instance.userId, | ||
| // If other users will get push notification. | ||
| ring = ring, | ||
| ) | ||
| result.onOutcome(call, onSuccess, onError) | ||
| } else { | ||
| /** | ||
| * TODO Rahul, maybe just do nothing, at this moment we should render get permission from setting | ||
| * Once we have the permission from setting, we re-open the our active - restarts the processing of | ||
| * last intent. | ||
| */ | ||
| } | ||
| } |
There was a problem hiding this comment.
Silent failure when permission check fails — onError is not invoked.
When hasCallPermission is false, the else branch (lines 906–912) does nothing. The onError callback passed to create() is never invoked, and the caller (e.g., onIntentAction) has no way to know the operation failed. This can leave the user staring at a loading screen indefinitely.
At minimum, invoke onError with a descriptive exception so the activity can react.
Proposed fix
if (hasCallPermission) {
val instance = StreamVideo.instance()
val result = call.create(
// List of all users, containing the caller also
memberIds = members + instance.userId,
// If other users will get push notification.
ring = ring,
)
result.onOutcome(call, onSuccess, onError)
} else {
- /**
- * TODO Rahul, maybe just do nothing, at this moment we should render get permission from setting
- * Once we have the permission from setting, we re-open the our active - restarts the processing of
- * last intent.
- */
+ onError?.invoke(
+ StreamCallActivityException(
+ call,
+ "Required call permissions (camera/microphone) are not granted.",
+ null,
+ ),
+ )
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@stream-video-android-ui-core/src/main/kotlin/io/getstream/video/android/ui/common/StreamCallActivity.kt`
around lines 896 - 913, In StreamCallActivity, when hasCallPermission is false
the else branch is currently silent; instead call the provided onError callback
with a descriptive exception (e.g., SecurityException or IllegalStateException)
so callers (like onIntentAction) can handle the failure; update the else branch
that surrounds the call.create(...) / result.onOutcome(...) logic to invoke
onError(Throwable("Missing call permission: user must grant CALL/RECORD
permission")) (or similar), referencing the hasCallPermission check, the
call.create(...) flow and the existing onError parameter to locate where to add
this.
Contributor
SDK Size Comparison 📏
|
rahul-lohra
changed the title
rahul-lohra
changed the title
…ll-service-exception # Conflicts: # stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/CallState.kt
…o-active-call-service-exception # Conflicts: # stream-video-android-core/src/main/kotlin/io/getstream/video/android/core/ClientState.kt
…ions should be asked from the UI layer
|
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
1 participant
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.
This is WIP, please do not review
Goal
Previously, during the transition from an outgoing call to an active call, the Foreground Service (FGS) was started twice.
When initiating an outgoing call, we started the FGS with a minimal foreground service permission type. Upon transitioning to an active call, we then restarted the FGS with the full foreground service permission types (camera/microphone).
However, the transition from an outgoing call to an active call can occur while the app is in the background, which caused an exception because restarting a Foreground Service is not allowed when the app is in the background.
Solution -
To resolve this, we now start the Foreground Service only once, at the time of initiating the outgoing call, after all required permissions are granted.
By starting the FGS upfront with all necessary foreground service permission types, we eliminate the need to restart the service during the state transition, thereby avoiding the background execution exception.
Implementation
Key changes
StreamCallActivitycall.get()StreamCallActivity- we need a way to know if we are making a video outgoing call or audio outgoing callStreamCallActivityexplicitly about thisSo we created a param to pass through intent
🎨 UI Changes
Add relevant screenshots
Add relevant videos
Testing
Explain how this change can be tested (or why it can't be tested)
Provide a patch below if it is necessary for testing
Provide the patch summary here
Summary by CodeRabbit
New Features
Refactor