From 48fae51ef004fe44ca7cdbb725de9e8aa4a6ebf0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Thu, 12 Feb 2026 10:46:20 +0100 Subject: [PATCH] Ignore fallback notification when the room list is rendered. Add more tests. --- .../DefaultNotificationDrawerManager.kt | 44 ++- .../model/NotifiableMessageEvent.kt | 25 -- .../DefaultNotificationDrawerManagerTest.kt | 266 +++++++++++++++++- .../fixtures/NotifiableEventFixture.kt | 6 +- .../appnavstate/test/AppNavStateFixture.kt | 9 + 5 files changed, 316 insertions(+), 34 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt index 575206de10..a0e6193d99 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt @@ -22,12 +22,20 @@ import io.element.android.libraries.matrix.ui.media.ImageLoaderHolder import io.element.android.libraries.push.api.notifications.NotificationCleaner import io.element.android.libraries.push.api.notifications.NotificationIdProvider import io.element.android.libraries.push.impl.notifications.factories.NotificationCreator +import io.element.android.libraries.push.impl.notifications.model.FallbackNotifiableEvent +import io.element.android.libraries.push.impl.notifications.model.InviteNotifiableEvent import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent -import io.element.android.libraries.push.impl.notifications.model.shouldIgnoreEventInRoom +import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent +import io.element.android.libraries.push.impl.notifications.model.NotifiableRingingCallEvent +import io.element.android.libraries.push.impl.notifications.model.SimpleNotifiableEvent import io.element.android.libraries.sessionstorage.api.observer.SessionListener import io.element.android.libraries.sessionstorage.api.observer.SessionObserver +import io.element.android.services.appnavstate.api.AppNavigationState import io.element.android.services.appnavstate.api.AppNavigationStateService import io.element.android.services.appnavstate.api.NavigationState +import io.element.android.services.appnavstate.api.currentRoomId +import io.element.android.services.appnavstate.api.currentSessionId +import io.element.android.services.appnavstate.api.currentThreadId import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -97,14 +105,11 @@ class DefaultNotificationDrawerManager( * Events might be grouped and there might not be one notification per event! */ suspend fun onNotifiableEventReceived(notifiableEvent: NotifiableEvent) { - if (notifiableEvent.shouldIgnoreEventInRoom(appNavigationStateService.appNavigationState.value)) { - return - } - renderEvents(listOf(notifiableEvent)) + onNotifiableEventsReceived(listOf(notifiableEvent)) } suspend fun onNotifiableEventsReceived(notifiableEvents: List) { - val eventsToNotify = notifiableEvents.filter { !it.shouldIgnoreEventInRoom(appNavigationStateService.appNavigationState.value) } + val eventsToNotify = notifiableEvents.filter { !it.shouldIgnoreRegardingApplicationState(appNavigationStateService.appNavigationState.value) } renderEvents(eventsToNotify) } @@ -206,3 +211,30 @@ class DefaultNotificationDrawerManager( } } } + +/** + * Used to check if a notification should be ignored based on the current application navigation state. + */ +private fun NotifiableEvent.shouldIgnoreRegardingApplicationState(appNavigationState: AppNavigationState): Boolean { + if (!appNavigationState.isInForeground) return false + return appNavigationState.navigationState.currentSessionId() == sessionId && + when (this) { + is NotifiableRingingCallEvent -> { + // Never ignore ringing call notifications + // Note that NotifiableRingingCallEvent are not handled by DefaultNotificationDrawerManage + false + } + is FallbackNotifiableEvent -> { + // Ignore if the room list is currently displayed + appNavigationState.navigationState is NavigationState.Session + } + is InviteNotifiableEvent, + is SimpleNotifiableEvent -> { + roomId == appNavigationState.navigationState.currentRoomId() + } + is NotifiableMessageEvent -> { + roomId == appNavigationState.navigationState.currentRoomId() && + threadId == appNavigationState.navigationState.currentThreadId() + } + } +} diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt index 07f7f8b3c7..1b29527cac 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt @@ -15,10 +15,6 @@ import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.core.ThreadId import io.element.android.libraries.matrix.api.core.UserId import io.element.android.libraries.matrix.api.timeline.item.event.EventType -import io.element.android.services.appnavstate.api.AppNavigationState -import io.element.android.services.appnavstate.api.currentRoomId -import io.element.android.services.appnavstate.api.currentSessionId -import io.element.android.services.appnavstate.api.currentThreadId data class NotifiableMessageEvent( override val sessionId: SessionId, @@ -56,24 +52,3 @@ data class NotifiableMessageEvent( val imageUri: Uri? get() = imageUriString?.toUri() } - -/** - * Used to check if a notification should be ignored based on the current app and navigation state. - */ -fun NotifiableEvent.shouldIgnoreEventInRoom(appNavigationState: AppNavigationState): Boolean { - val currentSessionId = appNavigationState.navigationState.currentSessionId() ?: return false - return when (val currentRoomId = appNavigationState.navigationState.currentRoomId()) { - null -> false - else -> { - // Never ignore ringing call notifications - if (this is NotifiableRingingCallEvent) { - false - } else { - appNavigationState.isInForeground && - sessionId == currentSessionId && - roomId == currentRoomId && - (this as? NotifiableMessageEvent)?.threadId == appNavigationState.navigationState.currentThreadId() - } - } - } -} diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManagerTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManagerTest.kt index ced45ca192..b080c77a2d 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManagerTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManagerTest.kt @@ -15,8 +15,11 @@ import io.element.android.features.enterprise.api.EnterpriseService import io.element.android.features.enterprise.test.FakeEnterpriseService import io.element.android.libraries.matrix.test.AN_EVENT_ID import io.element.android.libraries.matrix.test.A_ROOM_ID +import io.element.android.libraries.matrix.test.A_ROOM_ID_2 import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.matrix.test.A_SESSION_ID_2 import io.element.android.libraries.matrix.test.A_THREAD_ID +import io.element.android.libraries.matrix.test.A_THREAD_ID_2 import io.element.android.libraries.matrix.test.FakeMatrixClient import io.element.android.libraries.matrix.test.FakeMatrixClientProvider import io.element.android.libraries.matrix.ui.components.aMatrixUser @@ -28,7 +31,11 @@ import io.element.android.libraries.push.impl.notifications.fake.FakeNotificatio import io.element.android.libraries.push.impl.notifications.fake.FakeNotificationDisplayer import io.element.android.libraries.push.impl.notifications.fake.FakeRoomGroupMessageCreator import io.element.android.libraries.push.impl.notifications.fake.FakeSummaryGroupMessageCreator +import io.element.android.libraries.push.impl.notifications.fixtures.aFallbackNotifiableEvent import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiableMessageEvent +import io.element.android.libraries.push.impl.notifications.fixtures.aSimpleNotifiableEvent +import io.element.android.libraries.push.impl.notifications.fixtures.anInviteNotifiableEvent +import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent import io.element.android.libraries.sessionstorage.api.SessionStore import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import io.element.android.libraries.sessionstorage.test.InMemorySessionStore @@ -37,6 +44,7 @@ import io.element.android.services.appnavstate.api.AppNavigationState import io.element.android.services.appnavstate.api.AppNavigationStateService import io.element.android.services.appnavstate.test.FakeAppNavigationStateService import io.element.android.services.appnavstate.test.aNavigationState +import io.element.android.services.appnavstate.test.anAppNavigationState import io.element.android.tests.testutils.lambda.any import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value @@ -231,6 +239,262 @@ class DefaultNotificationDrawerManagerTest { listOf(value(null), value(summaryId)), ) } + + @Test + fun `when the application is in background, all events trigger a notification`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID), + isInForeground = false, + ), + notifiableEvents = listOf( + aFallbackNotifiableEvent(sessionId = A_SESSION_ID), + aFallbackNotifiableEvent(sessionId = A_SESSION_ID_2), + anInviteNotifiableEvent(sessionId = A_SESSION_ID), + anInviteNotifiableEvent(sessionId = A_SESSION_ID_2), + aSimpleNotifiableEvent(sessionId = A_SESSION_ID), + aSimpleNotifiableEvent(sessionId = A_SESSION_ID_2), + aNotifiableMessageEvent(sessionId = A_SESSION_ID), + aNotifiableMessageEvent(sessionId = A_SESSION_ID_2), + aNotifiableMessageEvent(sessionId = A_SESSION_ID, threadId = A_THREAD_ID), + aNotifiableMessageEvent(sessionId = A_SESSION_ID_2, threadId = A_THREAD_ID_2), + ), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 2, + ) + + @Test + fun `fallback event is ignored when the room list is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID), + ), + notifiableEvents = listOf(aFallbackNotifiableEvent(sessionId = A_SESSION_ID)), + shouldEmitNotification = false, + ) + + @Test + fun `fallback event is not ignored when a room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID), + ), + notifiableEvents = listOf(aFallbackNotifiableEvent(sessionId = A_SESSION_ID)), + shouldEmitNotification = true, + ) + + @Test + fun `fallback event for other session is not ignored when the room list is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID_2), + ), + notifiableEvents = listOf(aFallbackNotifiableEvent(sessionId = A_SESSION_ID)), + shouldEmitNotification = true, + ) + + @Test + fun `invite notifiable event is emits a notification when the room list is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID), + ), + notifiableEvents = listOf(anInviteNotifiableEvent(sessionId = A_SESSION_ID)), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `invite notifiable event does not emit a notification when the same room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID), + ), + notifiableEvents = listOf( + anInviteNotifiableEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + ) + ), + shouldEmitNotification = false, + ) + + @Test + fun `invite notifiable event emits a notification when another room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID_2), + ), + notifiableEvents = listOf( + anInviteNotifiableEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + ) + ), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `simple notifiable event is emits a notification when the room list is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID), + ), + notifiableEvents = listOf(aSimpleNotifiableEvent(sessionId = A_SESSION_ID)), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `simple notifiable event does not emit a notification when the same room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID), + ), + notifiableEvents = listOf( + aSimpleNotifiableEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + ) + ), + shouldEmitNotification = false, + ) + + @Test + fun `simple notifiable event emits a notification when another room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID_2), + ), + notifiableEvents = listOf( + aSimpleNotifiableEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + ) + ), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `notifiable event is emits a notification when the room list is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID), + ), + notifiableEvents = listOf(aNotifiableMessageEvent(sessionId = A_SESSION_ID)), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `notifiable event does not emit a notification when the same room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID), + ), + notifiableEvents = listOf( + aNotifiableMessageEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + ) + ), + shouldEmitNotification = false, + ) + + @Test + fun `notifiable event for a thread emits a notification when the same room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID), + ), + notifiableEvents = listOf( + aNotifiableMessageEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + threadId = A_THREAD_ID, + ) + ), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `notifiable event for a thread does not emit a notification when the same thread is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID, threadId = A_THREAD_ID), + ), + notifiableEvents = listOf( + aNotifiableMessageEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + threadId = A_THREAD_ID, + ) + ), + shouldEmitNotification = false, + ) + + @Test + fun `notifiable event for a thread emits a notification when another thread is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID, threadId = A_THREAD_ID_2), + ), + notifiableEvents = listOf( + aNotifiableMessageEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + threadId = A_THREAD_ID, + ) + ), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `notifiable event for a thread emits a notification when a thread of another room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID_2, threadId = A_THREAD_ID_2), + ), + notifiableEvents = listOf( + aNotifiableMessageEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + threadId = A_THREAD_ID, + ) + ), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + @Test + fun `notifiable event emits a notification when another room is displayed`() = testOnNotifiableEventReceived( + appNavigationState = anAppNavigationState( + navigationState = aNavigationState(sessionId = A_SESSION_ID, roomId = A_ROOM_ID_2), + ), + notifiableEvents = listOf( + aNotifiableMessageEvent( + sessionId = A_SESSION_ID, + roomId = A_ROOM_ID, + ) + ), + shouldEmitNotification = true, + extraInvocationsForNotificationSummary = 1, + ) + + private fun testOnNotifiableEventReceived( + appNavigationState: AppNavigationState, + notifiableEvents: List, + shouldEmitNotification: Boolean, + extraInvocationsForNotificationSummary: Int = 0, + ) = runTest { + val showNotificationResult = lambdaRecorder { _, _, _ -> + true + } + val defaultNotificationDrawerManager = createDefaultNotificationDrawerManager( + appNavigationStateService = FakeAppNavigationStateService( + initialAppNavigationState = appNavigationState, + ), + notificationDisplayer = FakeNotificationDisplayer( + showNotificationResult = showNotificationResult, + ) + ) + defaultNotificationDrawerManager.onNotifiableEventsReceived(notifiableEvents) + showNotificationResult.assertions().isCalledExactly( + if (shouldEmitNotification) { + notifiableEvents.size + extraInvocationsForNotificationSummary + } else { + 0 + } + ) + } } fun TestScope.createDefaultNotificationDrawerManager( @@ -248,7 +512,7 @@ fun TestScope.createDefaultNotificationDrawerManager( return DefaultNotificationDrawerManager( notificationDisplayer = notificationDisplayer, notificationRenderer = notificationRenderer ?: NotificationRenderer( - notificationDisplayer = FakeNotificationDisplayer(), + notificationDisplayer = notificationDisplayer, notificationDataFactory = DefaultNotificationDataFactory( notificationCreator = FakeNotificationCreator(), roomGroupMessageCreator = roomGroupMessageCreator, diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fixtures/NotifiableEventFixture.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fixtures/NotifiableEventFixture.kt index 9b7929b6a0..0ab39d5180 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fixtures/NotifiableEventFixture.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fixtures/NotifiableEventFixture.kt @@ -144,8 +144,10 @@ fun aNotifiableCallEvent( rtcNotificationType = rtcNotificationType, ) -fun aFallbackNotifiableEvent() = FallbackNotifiableEvent( - sessionId = A_SESSION_ID, +fun aFallbackNotifiableEvent( + sessionId: SessionId = A_SESSION_ID, +) = FallbackNotifiableEvent( + sessionId = sessionId, roomId = A_ROOM_ID, eventId = AN_EVENT_ID, editedEventId = null, diff --git a/services/appnavstate/test/src/main/kotlin/io/element/android/services/appnavstate/test/AppNavStateFixture.kt b/services/appnavstate/test/src/main/kotlin/io/element/android/services/appnavstate/test/AppNavStateFixture.kt index 7860320eee..e8338cb09c 100644 --- a/services/appnavstate/test/src/main/kotlin/io/element/android/services/appnavstate/test/AppNavStateFixture.kt +++ b/services/appnavstate/test/src/main/kotlin/io/element/android/services/appnavstate/test/AppNavStateFixture.kt @@ -11,6 +11,7 @@ package io.element.android.services.appnavstate.test import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.matrix.api.core.ThreadId +import io.element.android.services.appnavstate.api.AppNavigationState import io.element.android.services.appnavstate.api.NavigationState const val A_SESSION_OWNER = "aSessionOwner" @@ -35,3 +36,11 @@ fun aNavigationState( } return NavigationState.Thread(A_THREAD_OWNER, threadId, room) } + +fun anAppNavigationState( + navigationState: NavigationState = aNavigationState(), + isInForeground: Boolean = true, +) = AppNavigationState( + navigationState = navigationState, + isInForeground = isInForeground, +)