ensuring that we removing the summary group before removing individua…

…l notifications - adds some comments to explain the positioning
element-hq · Oct 11, 2021 · 1c35109 · 1c35109
1 parent ee12c15
commit 1c35109
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 7 deletions.
diff --git a/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt b/vector/src/main/java/im/vector/app/features/notifications/NotificationRenderer.kt
@@ -65,9 +65,20 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces
                     useCompleteNotificationFormat = useCompleteNotificationFormat
             )
 
+            // Remove summary first to avoid briefly displaying it after dismissing the last notification
+            when (summaryNotification) {
+                SummaryNotification.Removed -> {
+                    Timber.d("Removing summary notification")
+                    notificationDisplayer.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID)
+                }
+            }
+
             roomNotifications.forEach { wrapper ->
                 when (wrapper) {
-                    is RoomNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.roomId, ROOM_MESSAGES_NOTIFICATION_ID)
+                    is RoomNotification.Removed -> {
+                        Timber.d("Removing room messages notification ${wrapper.roomId}")
+                        notificationDisplayer.cancelNotificationMessage(wrapper.roomId, ROOM_MESSAGES_NOTIFICATION_ID)
+                    }
                     is RoomNotification.Message -> if (useCompleteNotificationFormat) {
                         Timber.d("Updating room messages notification ${wrapper.meta.roomId}")
                         notificationDisplayer.showNotificationMessage(wrapper.meta.roomId, ROOM_MESSAGES_NOTIFICATION_ID, wrapper.notification)
@@ -77,7 +88,10 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces
 
             invitationNotifications.forEach { wrapper ->
                 when (wrapper) {
-                    is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_INVITATION_NOTIFICATION_ID)
+                    is OneShotNotification.Removed -> {
+                        Timber.d("Removing invitation notification ${wrapper.key}")
+                        notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_INVITATION_NOTIFICATION_ID)
+                    }
                     is OneShotNotification.Append  -> if (useCompleteNotificationFormat) {
                         Timber.d("Updating invitation notification ${wrapper.meta.key}")
                         notificationDisplayer.showNotificationMessage(wrapper.meta.key, ROOM_INVITATION_NOTIFICATION_ID, wrapper.notification)
@@ -87,19 +101,19 @@ class NotificationRenderer @Inject constructor(private val notifiableEventProces
 
             simpleNotifications.forEach { wrapper ->
                 when (wrapper) {
-                    is OneShotNotification.Removed -> notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_EVENT_NOTIFICATION_ID)
+                    is OneShotNotification.Removed -> {
+                        Timber.d("Removing simple notification ${wrapper.key}")
+                        notificationDisplayer.cancelNotificationMessage(wrapper.key, ROOM_EVENT_NOTIFICATION_ID)
+                    }
                     is OneShotNotification.Append  -> if (useCompleteNotificationFormat) {
                         Timber.d("Updating simple notification ${wrapper.meta.key}")
                         notificationDisplayer.showNotificationMessage(wrapper.meta.key, ROOM_EVENT_NOTIFICATION_ID, wrapper.notification)
                     }
                 }
             }
 
+            // Update summary last to avoid briefly displaying it before other notifications
             when (summaryNotification) {
-                SummaryNotification.Removed   -> {
-                    Timber.d("Removing summary notification")
-                    notificationDisplayer.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID)
-                }
                 is SummaryNotification.Update -> {
                     Timber.d("Updating summary notification")
                     notificationDisplayer.showNotificationMessage(null, SUMMARY_NOTIFICATION_ID, summaryNotification.notification)

diff --git a/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt b/vector/src/test/java/im/vector/app/features/notifications/NotificationRendererTest.kt
@@ -63,6 +63,18 @@ class NotificationRendererTest {
         notificationDisplayer.verifyNoOtherInteractions()
     }
 
+    @Test
+    fun `given last room message group notification is removed when rendering then remove the summary and then remove message notification`() {
+        givenNotifications(roomNotifications = listOf(RoomNotification.Removed(A_ROOM_ID)), summaryNotification = A_REMOVE_SUMMARY_NOTIFICATION)
+
+        renderEventsAsNotifications()
+
+        notificationDisplayer.verifyInOrder {
+            cancelNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID)
+            cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_MESSAGES_NOTIFICATION_ID)
+        }
+    }
+
     @Test
     fun `given a room message group notification is removed when rendering then remove the message notification and update summary`() {
         givenNotifications(roomNotifications = listOf(RoomNotification.Removed(A_ROOM_ID)))
@@ -90,6 +102,18 @@ class NotificationRendererTest {
         }
     }
 
+    @Test
+    fun `given last simple notification is removed when rendering then remove the summary and then remove simple notification`() {
+        givenNotifications(simpleNotifications = listOf(OneShotNotification.Removed(AN_EVENT_ID)), summaryNotification = A_REMOVE_SUMMARY_NOTIFICATION)
+
+        renderEventsAsNotifications()
+
+        notificationDisplayer.verifyInOrder {
+            cancelNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID)
+            cancelNotificationMessage(tag = AN_EVENT_ID, NotificationDrawerManager.ROOM_EVENT_NOTIFICATION_ID)
+        }
+    }
+
     @Test
     fun `given a simple notification is removed when rendering then remove the simple notification and update summary`() {
         givenNotifications(simpleNotifications = listOf(OneShotNotification.Removed(AN_EVENT_ID)))
@@ -117,6 +141,18 @@ class NotificationRendererTest {
         }
     }
 
+    @Test
+    fun `given last invitation notification is removed when rendering then remove the summary and then remove invitation notification`() {
+        givenNotifications(invitationNotifications = listOf(OneShotNotification.Removed(A_ROOM_ID)), summaryNotification = A_REMOVE_SUMMARY_NOTIFICATION)
+
+        renderEventsAsNotifications()
+
+        notificationDisplayer.verifyInOrder {
+            cancelNotificationMessage(tag = null, NotificationDrawerManager.SUMMARY_NOTIFICATION_ID)
+            cancelNotificationMessage(tag = A_ROOM_ID, NotificationDrawerManager.ROOM_INVITATION_NOTIFICATION_ID)
+        }
+    }
+
     @Test
     fun `given an invitation notification is removed when rendering then remove the invitation notification and update summary`() {
         givenNotifications(invitationNotifications = listOf(OneShotNotification.Removed(A_ROOM_ID)))