Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cancel ringing call notification on call cancellation #3047

Merged
merged 3 commits into from
Jul 19, 2024
Merged

Cancel ringing call notification on call cancellation #3047

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9cc62bd
Cancel ringing call notification on call cancellation
jmartinesp Jun 18, 2024
0813fe9
Improve implementation, add some comments to clarify how it works.
jmartinesp Jul 18, 2024
d68e5e4
Make sure the call timeout job is cancelled
jmartinesp Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions ...all/impl/src/main/kotlin/io/element/android/features/call/impl/utils/ActiveCallManager.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,25 @@ import io.element.android.features.call.impl.notifications.CallNotificationData
import io.element.android.features.call.impl.notifications.RingingCallNotificationCreator
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.matrix.api.MatrixClientProvider
import io.element.android.libraries.push.api.notifications.ForegroundServiceType
import io.element.android.libraries.push.api.notifications.NotificationIdProvider
import io.element.android.libraries.push.api.notifications.OnMissedCallNotificationHandler
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.Job
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.drop
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject
Expand Down Expand Up @@ -79,11 +90,16 @@ class DefaultActiveCallManager @Inject constructor(
private val onMissedCallNotificationHandler: OnMissedCallNotificationHandler,
private val ringingCallNotificationCreator: RingingCallNotificationCreator,
private val notificationManagerCompat: NotificationManagerCompat,
private val matrixClientProvider: MatrixClientProvider,
) : ActiveCallManager {
private var timedOutCallJob: Job? = null

override val activeCall = MutableStateFlow<ActiveCall?>(null)

init {
observeRingingCall()
}

override fun registerIncomingCall(notificationData: CallNotificationData) {
if (activeCall.value != null) {
displayMissedCallNotification(notificationData)
Expand Down Expand Up @@ -173,6 +189,34 @@ class DefaultActiveCallManager @Inject constructor(
)
}
}

@OptIn(ExperimentalCoroutinesApi::class)
private fun observeRingingCall() {
// This will observe ringing calls and ensure they're terminated if the room call is cancelled
activeCall
.filterNotNull()
.filter { it.callState is CallState.Ringing && it.callType is CallType.RoomCall }
.flatMapLatest { activeCall ->
val callType = activeCall.callType as CallType.RoomCall
// Get a flow of updated `hasRoomCall` values for the room
matrixClientProvider.getOrRestore(callType.sessionId).getOrNull()
?.getRoom(callType.roomId)
?.roomInfoFlow
?.map { it.hasRoomCall }
?: flowOf()
}
// We only want to check if the room active call status changes
.distinctUntilChanged()
// Skip the first one, we're not interested in it (if the check below passes, it had to be active anyway)
.drop(1)
Copy link
Member

@bmarty bmarty Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit scared about this drop. If it is not mandatory, can we just remove it?
Or maybe replace by

            // We only want to check if the room active call status changes
            .distinctUntilChanged()
            .filter { roomHasActiveCall -> !roomHasActiveCall }
            .onEach {
                // The call was cancelled
                incomingCallTimedOut()
            }
            .launchIn(coroutineScope)

(Unit tests are still passing with this)

Copy link
Member Author

@jmartinesp jmartinesp Jul 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC we can't, because in a real life situation this flow will start before we get the room info, and by default the roomHasActiveCall will be false, instantly cancelling any incoming call as a result.

We could replace it by some operator that takes pairs of events instead so we can compare hadCall == true && newValue == false, but that's technically doing exactly the same as the drop as it wouldn't trigger until there are 2 elements in the flow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

.onEach { roomHasActiveCall ->
if (!roomHasActiveCall) {
// The call was cancelled
incomingCallTimedOut()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to add timedOutCallJob?.cancel() too, even if it should not change anything.

}
}
.launchIn(coroutineScope)
}
}

/**
Expand Down
Loading
Loading