proccessing notifications so they correctly alert on changes and only display upon new messages

core/src/main/kotlin/app/dapk/st/core/extensions/MapExtensions.kt

@@ -0,0 +1,3 @@
+package app.dapk.st.core.extensions
+
+fun <K, V> Map<K, V>?.containsKey(key: K) = this?.containsKey(key) ?: false

features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationFactory.kt

@@ -24,14 +24,16 @@ class NotificationFactory(
     private val intentFactory: IntentFactory,
 ) {
 
+    private val shouldAlwaysAlertDms = true
+
     private fun RoomEvent.toNotifiableContent(): String = when (this) {
         is RoomEvent.Image -> "\uD83D\uDCF7"
         is RoomEvent.Message -> this.content
         is RoomEvent.Reply -> this.message.toNotifiableContent()
     }
 
-    suspend fun createNotifications(events: Map<RoomOverview, List<RoomEvent>>, onlyContainsRemovals: Boolean, roomsWithNewEvents: Set<RoomId>): Notifications {
-        val notifications = events.map { (roomOverview, events) ->
+    suspend fun createNotifications(allUnread: Map<RoomOverview, List<RoomEvent>>, roomsWithNewEvents: Set<RoomId>): Notifications {
+        val notifications = allUnread.map { (roomOverview, events) ->
             val messageEvents = events.map {
                 when (it) {
                     is RoomEvent.Image -> Notifiable(content = it.toNotifiableContent(), it.utcTimestamp, it.author)
@@ -46,14 +48,15 @@ class NotificationFactory(
         }
 
         val summaryNotification = if (notifications.filterIsInstance<NotificationDelegate.Room>().isNotEmpty()) {
-            createSummary(notifications, onlyContainsRemovals)
+            val isAlerting = notifications.any { it is NotificationDelegate.Room && it.isAlerting }
+            createSummary(notifications, isAlerting = isAlerting)
         } else {
             null
         }
         return Notifications(summaryNotification, notifications)
     }
 
-    private fun createSummary(notifications: List<NotificationDelegate>, onlyContainsRemovals: Boolean): Notification {
+    private fun createSummary(notifications: List<NotificationDelegate>, isAlerting: Boolean): Notification {
         val summaryInboxStyle = Notification.InboxStyle().also { style ->
             notifications.forEach {
                 when (it) {
@@ -79,7 +82,7 @@ class NotificationFactory(
 
         return builder()
             .setStyle(summaryInboxStyle)
-            .setOnlyAlertOnce(onlyContainsRemovals)
+            .setOnlyAlertOnce(!isAlerting)
             .setSmallIcon(R.drawable.ic_notification_small_icon)
             .setCategory(Notification.CATEGORY_MESSAGE)
             .setGroupSummary(true)
@@ -145,12 +148,17 @@ class NotificationFactory(
             PendingIntent.FLAG_ONE_SHOT or PendingIntent.FLAG_IMMUTABLE
         )
 
+        val shouldAlertMoreThanOnce = when {
+            roomOverview.isDm() -> roomsWithNewEvents.contains(roomOverview.roomId) && shouldAlwaysAlertDms
+            else -> false
+        }
+
         return NotificationDelegate.Room(
             builder()
                 .setWhen(sortedEvents.last().utcTimestamp)
                 .setShowWhen(true)
                 .setGroup(GROUP_ID)
-                .setOnlyAlertOnce(roomOverview.isGroup || !roomsWithNewEvents.contains(roomOverview.roomId))
+                .setOnlyAlertOnce(!shouldAlertMoreThanOnce)
                 .setContentIntent(openRoomIntent)
                 .setStyle(messageStyle)
                 .setCategory(Notification.CATEGORY_MESSAGE)
@@ -168,8 +176,8 @@ class NotificationFactory(
             roomId = roomOverview.roomId,
             summary = events.last().content,
             messageCount = events.size,
+            isAlerting = shouldAlertMoreThanOnce
         )
-
     }
 
     private fun builder() = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
@@ -180,6 +188,8 @@ class NotificationFactory(
 
 }
 
+private fun RoomOverview.isDm() = !this.isGroup
+
 data class Notifications(val summaryNotification: Notification?, val delegates: List<NotificationDelegate>)
 
 data class Notifiable(val content: String, val utcTimestamp: Long, val author: RoomMember)

features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationRenderer.kt

@@ -19,9 +19,9 @@ class NotificationRenderer(
     private val notificationFactory: NotificationFactory,
 ) {
 
-    suspend fun render(result: Map<RoomOverview, List<RoomEvent>>, removedRooms: Set<RoomId>, onlyContainsRemovals: Boolean, roomsWithNewEvents: Set<RoomId>) {
+    suspend fun render(allUnread: Map<RoomOverview, List<RoomEvent>>, removedRooms: Set<RoomId>, roomsWithNewEvents: Set<RoomId>) {
         removedRooms.forEach { notificationManager.cancel(it.value, MESSAGE_NOTIFICATION_ID) }
-        val notifications = notificationFactory.createNotifications(result, onlyContainsRemovals, roomsWithNewEvents)
+        val notifications = notificationFactory.createNotifications(allUnread, roomsWithNewEvents)
 
         withContext(Dispatchers.Main) {
             notifications.summaryNotification.ifNull {
@@ -29,6 +29,7 @@ class NotificationRenderer(
                 notificationManager.cancel(SUMMARY_NOTIFICATION_ID)
             }
 
+            val onlyContainsRemovals = removedRooms.isNotEmpty() && roomsWithNewEvents.isEmpty()
             notifications.delegates.forEach {
                 when (it) {
                     is NotificationDelegate.DismissRoom -> notificationManager.cancel(it.roomId.value, MESSAGE_NOTIFICATION_ID)
@@ -42,8 +43,10 @@ class NotificationRenderer(
             }
 
             notifications.summaryNotification?.let {
-                log(AppLogTag.NOTIFICATION, "notifying summary")
-                notificationManager.notify(SUMMARY_NOTIFICATION_ID, it)
+                if (notifications.delegates.filterIsInstance<NotificationDelegate.Room>().isNotEmpty()) {
+                    log(AppLogTag.NOTIFICATION, "notifying summary")
+                    notificationManager.notify(SUMMARY_NOTIFICATION_ID, it)
+                }
             }
         }
     }
@@ -51,6 +54,6 @@ class NotificationRenderer(
 }
 
 sealed interface NotificationDelegate {
-    data class Room(val notification: Notification, val roomId: RoomId, val summary: String, val messageCount: Int) : NotificationDelegate
+    data class Room(val notification: Notification, val roomId: RoomId, val summary: String, val messageCount: Int, val isAlerting: Boolean) : NotificationDelegate
     data class DismissRoom(val roomId: RoomId) : NotificationDelegate
 }

features/notifications/src/main/kotlin/app/dapk/st/notifications/NotificationsUseCase.kt

@@ -2,8 +2,10 @@ package app.dapk.st.notifications
 
 import app.dapk.st.core.AppLogTag.NOTIFICATION
 import app.dapk.st.core.log
+import app.dapk.st.matrix.common.EventId
 import app.dapk.st.matrix.common.RoomId
 import app.dapk.st.matrix.sync.RoomEvent
+import app.dapk.st.matrix.sync.RoomOverview
 import app.dapk.st.matrix.sync.RoomStore
 import kotlinx.coroutines.flow.*
 
@@ -14,35 +16,64 @@ class NotificationsUseCase(
 ) {
 
     private val inferredCurrentNotifications = mutableMapOf<RoomId, List<RoomEvent>>()
+    private var previousUnreadEvents: Map<RoomId, List<EventId>>? = null
 
     init {
         notificationChannels.initChannels()
     }
 
+    data class NotificationDiff(
+        val unchanged: Map<RoomId, List<EventId>>,
+        val changedOrNew: Map<RoomId, List<EventId>>,
+        val removed: Map<RoomId, List<EventId>>
+    )
+
     suspend fun listenForNotificationChanges() {
         roomStore.observeUnread()
+            .map { each ->
+                val allUnreadIds = each.toIds()
+                val notificationDiff = calculateDiff(allUnreadIds, previousUnreadEvents)
+                previousUnreadEvents = allUnreadIds
+                each to notificationDiff
+            }
             .skipFirst()
-            .onEach { result ->
-                log(NOTIFICATION, "unread changed - render notifications")
-
-                val changes = result.mapKeys { it.key.roomId }
-
-                val asRooms = changes.keys
-                val removedRooms = inferredCurrentNotifications.keys - asRooms
-
-                val roomsWithNewEvents = changes.filter {
-                    inferredCurrentNotifications[it.key]?.map { it.eventId } != it.value.map { it.eventId }
-                }.keys
-
-                val onlyContainsRemovals =
-                    inferredCurrentNotifications.filterKeys { !removedRooms.contains(it) } == changes.filterKeys { !removedRooms.contains(it) }
-                inferredCurrentNotifications.clear()
-                inferredCurrentNotifications.putAll(changes)
-
-                notificationRenderer.render(result, removedRooms, onlyContainsRemovals, roomsWithNewEvents)
+            .onEach { (each, diff) ->
+                when {
+                    diff.changedOrNew.isEmpty() && diff.removed.isEmpty() -> {
+                        log(NOTIFICATION, "Ignoring unread change due to no renderable changes")
+                    }
+                    inferredCurrentNotifications.isEmpty() && diff.removed.isNotEmpty() -> {
+                        log(NOTIFICATION, "Ignoring unread change due to no currently showing messages and changes are all messages marked as read")
+                    }
+                    else -> renderUnreadChange(each, diff)
+                }
             }
             .collect()
     }
 
+    private fun calculateDiff(allUnread: Map<RoomId, List<EventId>>, previousUnread: Map<RoomId, List<EventId>>?): NotificationDiff {
+        val unchanged = previousUnread?.filter { allUnread.containsKey(it.key) && it.value == allUnread[it.key] } ?: emptyMap()
+        val changedOrNew = allUnread.filterNot { unchanged.containsKey(it.key) }
+        val removed = previousUnread?.filter { !unchanged.containsKey(it.key) } ?: emptyMap()
+        return NotificationDiff(unchanged, changedOrNew, removed)
+    }
+
+    private suspend fun renderUnreadChange(allUnread: Map<RoomOverview, List<RoomEvent>>, diff: NotificationDiff) {
+        log(NOTIFICATION, "unread changed - render notifications")
+        inferredCurrentNotifications.clear()
+        inferredCurrentNotifications.putAll(allUnread.mapKeys { it.key.roomId })
+
+        notificationRenderer.render(
+            allUnread = allUnread,
+            removedRooms = diff.removed.keys,
+            roomsWithNewEvents = diff.changedOrNew.keys
+        )
+    }
+
     private fun <T> Flow<T>.skipFirst() = drop(1)
 }
+
+private fun List<RoomEvent>.toEventIds() = this.map { it.eventId }
+private fun Map<RoomOverview, List<RoomEvent>>.toIds() = this
+    .mapValues { it.value.toEventIds() }
+    .mapKeys { it.key.roomId }