Merge pull request #6190 from element-hq/feature/bma/fallbackNotificationCleanup

Fallback notification cleanup

Author: bmarty

features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt

@@ -1238,7 +1238,7 @@ class MessagesPresenterTest {
     }
 
     @Test
-    fun `present - shows a "world_readable" icon if the room is encrypted and history is world_readable`() = runTest {
+    fun `present - shows a 'world_readable' icon if the room is encrypted and history is world_readable`() = runTest {
         val presenter = createMessagesPresenter(
             joinedRoom = FakeJoinedRoom(
                 baseRoom = FakeBaseRoom(

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
 
@@ -71,7 +79,10 @@ class DefaultNotificationDrawerManager(
     private fun onAppNavigationStateChange(navigationState: NavigationState) {
         when (navigationState) {
             NavigationState.Root -> {}
-            is NavigationState.Session -> {}
+            is NavigationState.Session -> {
+                // Cleanup the fallback notification
+                clearFallbackForSession(navigationState.sessionId)
+            }
             is NavigationState.Room -> {
                 // Cleanup notification for current room
                 clearMessagesForRoom(
@@ -94,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<NotifiableEvent>) {
-        val eventsToNotify = notifiableEvents.filter { !it.shouldIgnoreEventInRoom(appNavigationStateService.appNavigationState.value) }
+        val eventsToNotify = notifiableEvents.filter { !appNavigationStateService.appNavigationState.value.shouldIgnoreEvent(it) }
         renderEvents(eventsToNotify)
     }
 
@@ -121,6 +129,17 @@ class DefaultNotificationDrawerManager(
             .forEach { notificationDisplayer.cancelNotification(it.tag, it.id) }
     }
 
+    /**
+     * Remove the fallback notification for the session.
+     */
+    fun clearFallbackForSession(sessionId: SessionId) {
+        notificationDisplayer.cancelNotification(
+            DefaultNotificationDataFactory.FALLBACK_NOTIFICATION_TAG,
+            NotificationIdProvider.getFallbackNotificationId(sessionId),
+        )
+        clearSummaryNotificationIfNeeded(sessionId)
+    }
+
     /**
      * Should be called when the application is currently opened and showing timeline for the given [roomId].
      * Used to ignore events related to that room (no need to display notification) and clean any existing notification on this room.
@@ -192,3 +211,30 @@ class DefaultNotificationDrawerManager(
         }
     }
 }
+
+/**
+ * Used to check if a notifiableEvent should be ignored based on the current application navigation state.
+ */
+private fun AppNavigationState.shouldIgnoreEvent(event: NotifiableEvent): Boolean {
+    if (!isInForeground) return false
+    return navigationState.currentSessionId() == event.sessionId &&
+        when (event) {
+            is NotifiableRingingCallEvent -> {
+                // Never ignore ringing call notifications
+                // Note that NotifiableRingingCallEvent are not handled by DefaultNotificationDrawerManager
+                false
+            }
+            is FallbackNotifiableEvent -> {
+                // Ignore if the room list is currently displayed
+                navigationState is NavigationState.Session
+            }
+            is InviteNotifiableEvent,
+            is SimpleNotifiableEvent -> {
+                event.roomId == navigationState.currentRoomId()
+            }
+            is NotifiableMessageEvent -> {
+                event.roomId == navigationState.currentRoomId() &&
+                    event.threadId == navigationState.currentThreadId()
+            }
+        }
+}

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationDataFactory.kt

@@ -144,12 +144,12 @@ class DefaultNotificationDataFactory(
             .getFallbackNotification(notificationAccountParams.user.userId)
             ?.notification
         val notification = notificationCreator.createFallbackNotification(
-            existingNotification,
-            notificationAccountParams,
-            fallback,
+            existingNotification = existingNotification,
+            notificationAccountParams = notificationAccountParams,
+            fallbackNotifiableEvents = fallback,
         )
         return OneShotNotification(
-            tag = "FALLBACK",
+            tag = FALLBACK_NOTIFICATION_TAG,
             notification = notification,
             isNoisy = false,
             timestamp = fallback.first().timestamp
@@ -174,6 +174,10 @@ class DefaultNotificationDataFactory(
             )
         }
     }
+
+    companion object {
+        const val FALLBACK_NOTIFICATION_TAG = "FALLBACK"
+    }
 }
 
 data class RoomNotification(

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()
-            }
-        }
-    }
-}