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.

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

Ensure that the SDK is syncing during an incoming call so that the app can cancel the notification #3931

Merged
merged 1 commit into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion libraries/push/impl/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import extension.setupAnvil
* Please see LICENSE in the repository root for full details.
*/
plugins {
id("io.element.android-library")
id("io.element.android-compose-library")
alias(libs.plugins.kotlin.serialization)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,16 @@ class DefaultPushHandler @Inject constructor(
when (resolvedPushEvent) {
null -> Timber.tag(loggerTag.value).w("Unable to get a notification data")
is ResolvedPushEvent.Event -> {
when (resolvedPushEvent.notifiableEvent) {
is NotifiableRingingCallEvent -> handleRingingCallEvent(resolvedPushEvent.notifiableEvent)
when (val notifiableEvent = resolvedPushEvent.notifiableEvent) {
is NotifiableRingingCallEvent -> {
onNotifiableEventReceived.onNotifiableEventReceived(notifiableEvent)
handleRingingCallEvent(notifiableEvent)
}
else -> {
val userPushStore = userPushStoreFactory.getOrCreate(userId)
val areNotificationsEnabled = userPushStore.getNotificationEnabledForDevice().first()
if (areNotificationsEnabled) {
onNotifiableEventReceived.onNotifiableEventReceived(resolvedPushEvent.notifiableEvent)
onNotifiableEventReceived.onNotifiableEventReceived(notifiableEvent)
} else {
Timber.tag(loggerTag.value).i("Notification are disabled for this device, ignore push.")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.push.impl.notifications.DefaultNotificationDrawerManager
import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent
import io.element.android.libraries.push.impl.notifications.model.NotifiableRingingCallEvent
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import javax.inject.Inject
Expand All @@ -28,7 +29,9 @@
override fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) {
coroutineScope.launch {
launch { syncOnNotifiableEvent(notifiableEvent) }
defaultNotificationDrawerManager.onNotifiableEventReceived(notifiableEvent)
if (notifiableEvent !is NotifiableRingingCallEvent) {
defaultNotificationDrawerManager.onNotifiableEventReceived(notifiableEvent)

Check warning on line 33 in libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt

View check run for this annotation

Codecov / codecov/patch

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/OnNotifiableEventReceived.kt#L33

Added line #L33 was not covered by tests
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.element.android.libraries.matrix.api.sync.SyncService
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent
import io.element.android.libraries.push.impl.notifications.model.NotifiableRingingCallEvent
import io.element.android.services.appnavstate.api.AppForegroundStateService
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.withContext
Expand All @@ -34,7 +35,8 @@
private var syncCounter = AtomicInteger(0)

suspend operator fun invoke(notifiableEvent: NotifiableEvent) = withContext(dispatchers.io) {
if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush)) {
val isRingingCallEvent = notifiableEvent is NotifiableRingingCallEvent
if (!featureFlagService.isFeatureEnabled(FeatureFlags.SyncOnPush) && !isRingingCallEvent) {
return@withContext
}
val client = matrixClientProvider.getOrRestore(notifiableEvent.sessionId).getOrNull() ?: return@withContext
Expand All @@ -45,12 +47,28 @@
if (!appForegroundStateService.isInForeground.value) {
val syncService = client.syncService()
syncService.startSyncIfNeeded()
room.waitsUntilEventIsKnown(eventId = notifiableEvent.eventId, timeout = 10.seconds)
if (isRingingCallEvent) {
room.waitsUntilUserIsInTheCall(timeout = 60.seconds)
} else {
room.waitsUntilEventIsKnown(eventId = notifiableEvent.eventId, timeout = 10.seconds)
}
syncService.stopSyncIfNeeded()
}
}
}

/**
* User can be in the call if they answer using another session.
* If the user does not join the call, the timeout will be reached.
*/
private suspend fun MatrixRoom.waitsUntilUserIsInTheCall(timeout: Duration) {
withTimeoutOrNull(timeout) {
roomInfoFlow.first {
sessionId in it.activeRoomCallParticipants

Check warning on line 67 in libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt

View check run for this annotation

Codecov / codecov/patch

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/push/SyncOnNotifiableEvent.kt#L67

Added line #L67 was not covered by tests
}
}
}

private suspend fun MatrixRoom.waitsUntilEventIsKnown(eventId: EventId, timeout: Duration) {
withTimeoutOrNull(timeout) {
liveTimeline.timelineItems.first { timelineItems ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ class DefaultPushHandlerTest {
)
val handleIncomingCallLambda = lambdaRecorder<CallType.RoomCall, EventId, UserId, String?, String?, String?, String, Unit> { _, _, _, _, _, _, _ -> }
val elementCallEntryPoint = FakeElementCallEntryPoint(handleIncomingCallResult = handleIncomingCallLambda)
val onNotifiableEventReceived = lambdaRecorder<NotifiableEvent, Unit> {}
val defaultPushHandler = createDefaultPushHandler(
elementCallEntryPoint = elementCallEntryPoint,
notifiableEventResult = { _, _, _ ->
Expand All @@ -249,10 +250,11 @@ class DefaultPushHandlerTest {
pushClientSecret = FakePushClientSecret(
getUserIdFromSecretResult = { A_USER_ID }
),
onNotifiableEventReceived = onNotifiableEventReceived,
)
defaultPushHandler.handle(aPushData)

handleIncomingCallLambda.assertions().isCalledOnce()
onNotifiableEventReceived.assertions().isCalledOnce()
}

@Test
Expand Down Expand Up @@ -310,7 +312,7 @@ class DefaultPushHandlerTest {
)
defaultPushHandler.handle(aPushData)
handleIncomingCallLambda.assertions().isCalledOnce()
onNotifiableEventReceived.assertions().isNeverCalled()
onNotifiableEventReceived.assertions().isCalledOnce()
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
import io.element.android.libraries.matrix.test.sync.FakeSyncService
import io.element.android.libraries.matrix.test.timeline.FakeTimeline
import io.element.android.libraries.matrix.test.timeline.anEventTimelineItem
import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiableCallEvent
import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiableMessageEvent
import io.element.android.services.appnavstate.test.FakeAppForegroundStateService
import io.element.android.tests.testutils.lambda.assert
Expand Down Expand Up @@ -60,6 +61,7 @@ class SyncOnNotifiableEventTest {
}

private val notifiableEvent = aNotifiableMessageEvent()
private val incomingCallNotifiableEvent = aNotifiableCallEvent()

@Test
fun `when feature flag is disabled, nothing happens`() = runTest {
Expand All @@ -72,15 +74,38 @@ class SyncOnNotifiableEventTest {
assert(subscribeToSyncLambda).isNeverCalled()
}

@Test
fun `when feature flag is enabled, a ringing call starts and stops the sync`() = runTest {
val sut = createSyncOnNotifiableEvent(client = client, isAppInForeground = false, isSyncOnPushEnabled = true)

sut(incomingCallNotifiableEvent)

assert(startSyncLambda).isCalledOnce()
assert(stopSyncLambda).isCalledOnce()
assert(subscribeToSyncLambda).isCalledOnce()
}

@Test
fun `when feature flag is disabled, a ringing call starts and stops the sync`() = runTest {
val sut = createSyncOnNotifiableEvent(client = client, isAppInForeground = false, isSyncOnPushEnabled = false)

sut(incomingCallNotifiableEvent)

assert(startSyncLambda).isCalledOnce()
assert(stopSyncLambda).isCalledOnce()
assert(subscribeToSyncLambda).isCalledOnce()
}

@Test
fun `when feature flag is enabled and app is in foreground, sync is not started`() = runTest {
val sut = createSyncOnNotifiableEvent(client = client, isAppInForeground = true, isSyncOnPushEnabled = true)

sut(notifiableEvent)
sut(incomingCallNotifiableEvent)

assert(startSyncLambda).isNeverCalled()
assert(stopSyncLambda).isNeverCalled()
assert(subscribeToSyncLambda).isCalledOnce()
assert(subscribeToSyncLambda).isCalledExactly(2)
}

@Test
Expand Down
Loading