ignore push for a thread if it's currently visible to user

changelog.d/7634.bugfix

@@ -0,0 +1 @@
+Push notification for thread message is now shown correctly when user observes rooms main timeline

vector/src/main/java/im/vector/app/core/pushers/VectorPushHandler.kt

@@ -28,6 +28,7 @@ import im.vector.app.core.network.WifiDetector
 import im.vector.app.core.pushers.model.PushData
 import im.vector.app.core.resources.BuildMeta
 import im.vector.app.features.notifications.NotifiableEventResolver
+import im.vector.app.features.notifications.NotifiableMessageEvent
 import im.vector.app.features.notifications.NotificationActionIds
 import im.vector.app.features.notifications.NotificationDrawerManager
 import im.vector.app.features.settings.VectorDataStore
@@ -142,11 +143,6 @@ class VectorPushHandler @Inject constructor(
         pushData.roomId ?: return
         pushData.eventId ?: return
 
-        // If the room is currently displayed, we will not show a notification, so no need to get the Event faster
-        if (notificationDrawerManager.shouldIgnoreMessageEventInRoom(pushData.roomId)) {
-            return
-        }
-
         if (wifiDetector.isConnectedToWifi().not()) {
             Timber.tag(loggerTag.value).d("No WiFi network, do not get Event")
             return
@@ -157,6 +153,13 @@ class VectorPushHandler @Inject constructor(
 
         val resolvedEvent = notifiableEventResolver.resolveInMemoryEvent(session, event, canBeReplaced = true)
 
+        if (resolvedEvent is NotifiableMessageEvent) {
+            // If the room is currently displayed, we will not show a notification, so no need to get the Event faster
+            if (notificationDrawerManager.shouldIgnoreMessageEventInRoom(resolvedEvent)) {
+                return
+            }
+        }
+
         resolvedEvent
                 ?.also { Timber.tag(loggerTag.value).d("Fast lane: notify drawer") }
                 ?.let {

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

@@ -972,6 +972,7 @@ class TimelineFragment :
     override fun onResume() {
         super.onResume()
         notificationDrawerManager.setCurrentRoom(timelineArgs.roomId)
+        notificationDrawerManager.setCurrentThread(timelineArgs.threadTimelineArgs?.rootThreadEventId)
         roomDetailPendingActionStore.data?.let { handlePendingAction(it) }
         roomDetailPendingActionStore.data = null
     }
@@ -991,6 +992,7 @@ class TimelineFragment :
     override fun onPause() {
         super.onPause()
         notificationDrawerManager.setCurrentRoom(null)
+        notificationDrawerManager.setCurrentThread(null)
     }
 
     private val emojiActivityResultLauncher = registerStartForActivityResult { activityResult ->

vector/src/main/java/im/vector/app/features/notifications/NotifiableEventProcessor.kt

@@ -30,13 +30,13 @@ class NotifiableEventProcessor @Inject constructor(
         private val autoAcceptInvites: AutoAcceptInvites
 ) {
 
-    fun process(queuedEvents: List<NotifiableEvent>, currentRoomId: String?, renderedEvents: ProcessedEvents): ProcessedEvents {
+    fun process(queuedEvents: List<NotifiableEvent>, currentRoomId: String?, currentThreadId: String?, renderedEvents: ProcessedEvents): ProcessedEvents {
         val processedEvents = queuedEvents.map {
             val type = when (it) {
                 is InviteNotifiableEvent -> if (autoAcceptInvites.hideInvites) REMOVE else KEEP
                 is NotifiableMessageEvent -> when {
-                    shouldIgnoreMessageEventInRoom(currentRoomId, it.roomId) -> REMOVE
-                            .also { Timber.d("notification message removed due to currently viewing the same room") }
+                    it.shouldIgnoreMessageEventInRoom(currentRoomId, currentThreadId) -> REMOVE
+                            .also { Timber.d("notification message removed due to currently viewing the same room or thread") }
                     outdatedDetector.isMessageOutdated(it) -> REMOVE
                             .also { Timber.d("notification message removed due to being read") }
                     else -> KEEP
@@ -55,8 +55,4 @@ class NotifiableEventProcessor @Inject constructor(
 
         return removedEventsDiff + processedEvents
     }
-
-    private fun shouldIgnoreMessageEventInRoom(currentRoomId: String?, roomId: String?): Boolean {
-        return currentRoomId != null && roomId == currentRoomId
-    }
 }

vector/src/main/java/im/vector/app/features/notifications/NotifiableEventResolver.kt

@@ -32,6 +32,7 @@ import org.matrix.android.sdk.api.session.crypto.MXCryptoError
 import org.matrix.android.sdk.api.session.crypto.model.OlmDecryptionResult
 import org.matrix.android.sdk.api.session.events.model.Event
 import org.matrix.android.sdk.api.session.events.model.EventType
+import org.matrix.android.sdk.api.session.events.model.getRootThreadEventId
 import org.matrix.android.sdk.api.session.events.model.isEdition
 import org.matrix.android.sdk.api.session.events.model.isImageMessage
 import org.matrix.android.sdk.api.session.events.model.supportsNotification
@@ -133,7 +134,7 @@ class NotifiableEventResolver @Inject constructor(
         }
     }
 
-    private suspend fun resolveMessageEvent(event: TimelineEvent, session: Session, canBeReplaced: Boolean, isNoisy: Boolean): NotifiableEvent? {
+    private suspend fun resolveMessageEvent(event: TimelineEvent, session: Session, canBeReplaced: Boolean, isNoisy: Boolean): NotifiableMessageEvent? {
         // The event only contains an eventId, and roomId (type is m.room.*) , we need to get the displayable content (names, avatar, text, etc...)
         val room = session.getRoom(event.root.roomId!! /*roomID cannot be null*/)
 
@@ -155,6 +156,7 @@ class NotifiableEventResolver @Inject constructor(
                     body = body.toString(),
                     imageUriString = event.fetchImageIfPresent(session)?.toString(),
                     roomId = event.root.roomId!!,
+                    threadId = event.root.getRootThreadEventId(),
                     roomName = roomName,
                     matrixID = session.myUserId
             )
@@ -178,6 +180,7 @@ class NotifiableEventResolver @Inject constructor(
                             body = body,
                             imageUriString = event.fetchImageIfPresent(session)?.toString(),
                             roomId = event.root.roomId!!,
+                            threadId = event.root.getRootThreadEventId(),
                             roomName = roomName,
                             roomIsDirect = room.roomSummary()?.isDirect ?: false,
                             roomAvatarPath = session.contentUrlResolver()

vector/src/main/java/im/vector/app/features/notifications/NotifiableMessageEvent.kt

@@ -31,6 +31,7 @@ data class NotifiableMessageEvent(
         // NotSerializableException when persisting this to storage
         val imageUriString: String?,
         val roomId: String,
+        val threadId: String?,
         val roomName: String?,
         val roomIsDirect: Boolean = false,
         val roomAvatarPath: String? = null,
@@ -51,3 +52,10 @@ data class NotifiableMessageEvent(
     val imageUri: Uri?
         get() = imageUriString?.let { Uri.parse(it) }
 }
+
+fun NotifiableMessageEvent.shouldIgnoreMessageEventInRoom(currentRoomId: String?, currentThreadId: String?): Boolean {
+    return when (currentRoomId) {
+        null -> false
+        else -> roomId == currentRoomId && threadId == currentThreadId
+    }
+}

vector/src/main/java/im/vector/app/features/notifications/NotificationBroadcastReceiver.kt

@@ -148,6 +148,7 @@ class NotificationBroadcastReceiver : BroadcastReceiver() {
                 body = message,
                 imageUriString = null,
                 roomId = room.roomId,
+                threadId = null, // needs to be changed: https://github.com/vector-im/element-android/issues/7475
                 roomName = room.roomSummary()?.displayName ?: room.roomId,
                 roomIsDirect = room.roomSummary()?.isDirect == true,
                 outGoingMessage = true,

vector/src/main/java/im/vector/app/features/notifications/NotificationDrawerManager.kt

@@ -63,6 +63,7 @@ class NotificationDrawerManager @Inject constructor(
     private val notificationState by lazy { createInitialNotificationState() }
     private val avatarSize = context.resources.getDimensionPixelSize(R.dimen.profile_avatar_size)
     private var currentRoomId: String? = null
+    private var currentThreadId: String? = null
     private val firstThrottler = FirstThrottler(200)
 
     private var useCompleteNotificationFormat = vectorPreferences.useCompleteNotificationFormat()
@@ -123,6 +124,22 @@ class NotificationDrawerManager @Inject constructor(
         }
     }
 
+    /**
+     * Should be called when the application is currently opened and showing timeline for the given threadId.
+     * Used to ignore events related to that thread (no need to display notification) and clean any existing notification on this room.
+     */
+    fun setCurrentThread(threadId: String?) {
+        updateEvents {
+            val hasChanged = threadId != currentThreadId
+            currentThreadId = threadId
+            currentRoomId?.let { roomId ->
+                if (hasChanged && threadId != null) {
+                    it.clearMessagesForThread(roomId, threadId)
+                }
+            }
+        }
+    }
+
     fun notificationStyleChanged() {
         updateEvents {
             val newSettings = vectorPreferences.useCompleteNotificationFormat()
@@ -164,7 +181,7 @@ class NotificationDrawerManager @Inject constructor(
     private fun refreshNotificationDrawerBg() {
         Timber.v("refreshNotificationDrawerBg()")
         val eventsToRender = notificationState.updateQueuedEvents(this) { queuedEvents, renderedEvents ->
-            notifiableEventProcessor.process(queuedEvents.rawEvents(), currentRoomId, renderedEvents).also {
+            notifiableEventProcessor.process(queuedEvents.rawEvents(), currentRoomId, currentThreadId, renderedEvents).also {
                 queuedEvents.clearAndAdd(it.onlyKeptEvents())
             }
         }
@@ -198,8 +215,8 @@ class NotificationDrawerManager @Inject constructor(
         notificationRenderer.render(session.myUserId, myUserDisplayName, myUserAvatarUrl, useCompleteNotificationFormat, eventsToRender)
     }
 
-    fun shouldIgnoreMessageEventInRoom(roomId: String?): Boolean {
-        return currentRoomId != null && roomId == currentRoomId
+    fun shouldIgnoreMessageEventInRoom(resolvedEvent: NotifiableMessageEvent): Boolean {
+        return resolvedEvent.shouldIgnoreMessageEventInRoom(currentRoomId, currentThreadId)
     }
 
     companion object {

vector/src/main/java/im/vector/app/features/notifications/NotificationEventQueue.kt

@@ -122,15 +122,20 @@ data class NotificationEventQueue(
     }
 
     fun clearMemberShipNotificationForRoom(roomId: String) {
-        Timber.v("clearMemberShipOfRoom $roomId")
+        Timber.d("clearMemberShipOfRoom $roomId")
         queue.removeAll { it is InviteNotifiableEvent && it.roomId == roomId }
     }
 
     fun clearMessagesForRoom(roomId: String) {
-        Timber.v("clearMessageEventOfRoom $roomId")
+        Timber.d("clearMessageEventOfRoom $roomId")
         queue.removeAll { it is NotifiableMessageEvent && it.roomId == roomId }
     }
 
+    fun clearMessagesForThread(roomId: String, threadId: String) {
+        Timber.d("clearMessageEventOfThread $roomId, $threadId")
+        queue.removeAll { it is NotifiableMessageEvent && it.roomId == roomId && it.threadId == threadId }
+    }
+
     fun rawEvents(): List<NotifiableEvent> = queue
 }
 

vector/src/test/java/im/vector/app/features/notifications/NotifiableEventProcessorTest.kt

@@ -27,6 +27,7 @@ import org.junit.Test
 import org.matrix.android.sdk.api.session.events.model.EventType
 
 private val NOT_VIEWING_A_ROOM: String? = null
+private val NOT_VIEWING_A_THREAD: String? = null
 
 class NotifiableEventProcessorTest {
 
@@ -42,7 +43,7 @@ class NotifiableEventProcessorTest {
                 aSimpleNotifiableEvent(eventId = "event-2")
         )
 
-        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
+        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, currentThreadId = NOT_VIEWING_A_THREAD, renderedEvents = emptyList())
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.KEEP to events[0],
@@ -54,7 +55,7 @@ class NotifiableEventProcessorTest {
     fun `given redacted simple event when processing then remove redaction event`() {
         val events = listOf(aSimpleNotifiableEvent(eventId = "event-1", type = EventType.REDACTION))
 
-        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
+        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, currentThreadId = NOT_VIEWING_A_THREAD, renderedEvents = emptyList())
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.REMOVE to events[0]
@@ -69,7 +70,7 @@ class NotifiableEventProcessorTest {
                 anInviteNotifiableEvent(roomId = "room-2")
         )
 
-        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
+        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, currentThreadId = NOT_VIEWING_A_THREAD, renderedEvents = emptyList())
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.REMOVE to events[0],
@@ -85,7 +86,7 @@ class NotifiableEventProcessorTest {
                 anInviteNotifiableEvent(roomId = "room-2")
         )
 
-        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
+        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, currentThreadId = NOT_VIEWING_A_THREAD, renderedEvents = emptyList())
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.KEEP to events[0],
@@ -98,7 +99,7 @@ class NotifiableEventProcessorTest {
         val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1"))
         outdatedDetector.givenEventIsOutOfDate(events[0])
 
-        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
+        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, currentThreadId = NOT_VIEWING_A_THREAD, renderedEvents = emptyList())
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.REMOVE to events[0],
@@ -110,24 +111,59 @@ class NotifiableEventProcessorTest {
         val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1"))
         outdatedDetector.givenEventIsInDate(events[0])
 
-        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
+        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, currentThreadId = NOT_VIEWING_A_THREAD, renderedEvents = emptyList())
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.KEEP to events[0],
         )
     }
 
     @Test
-    fun `given viewing the same room as message event when processing then removes message`() {
-        val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1"))
+    fun `given viewing the same room main timeline when processing main timeline message event then removes message`() {
+        val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1", threadId = null))
+
+        val result = eventProcessor.process(events, currentRoomId = "room-1", currentThreadId = null, renderedEvents = emptyList())
+
+        result shouldBeEqualTo listOfProcessedEvents(
+                Type.REMOVE to events[0],
+        )
+    }
 
-        val result = eventProcessor.process(events, currentRoomId = "room-1", renderedEvents = emptyList())
+    @Test
+    fun `given viewing the same thread timeline when processing thread message event then removes message`() {
+        val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1", threadId = "thread-1"))
+
+        val result = eventProcessor.process(events, currentRoomId = "room-1", currentThreadId = "thread-1", renderedEvents = emptyList())
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.REMOVE to events[0],
         )
     }
 
+    @Test
+    fun `given viewing main timeline of the same room when processing thread timeline message event then keep message`() {
+        val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1", threadId = "thread-1"))
+        outdatedDetector.givenEventIsInDate(events[0])
+
+        val result = eventProcessor.process(events, currentRoomId = "room-1", currentThreadId = null, renderedEvents = emptyList())
+
+        result shouldBeEqualTo listOfProcessedEvents(
+                Type.KEEP to events[0],
+        )
+    }
+
+    @Test
+    fun `given viewing thread timeline of the same room when processing main timeline message event then keep message`() {
+        val events = listOf(aNotifiableMessageEvent(eventId = "event-1", roomId = "room-1"))
+        outdatedDetector.givenEventIsInDate(events[0])
+
+        val result = eventProcessor.process(events, currentRoomId = "room-1", currentThreadId = "thread-1", renderedEvents = emptyList())
+
+        result shouldBeEqualTo listOfProcessedEvents(
+                Type.KEEP to events[0],
+        )
+    }
+
     @Test
     fun `given events are different to rendered events when processing then removes difference`() {
         val events = listOf(aSimpleNotifiableEvent(eventId = "event-1"))
@@ -136,7 +172,7 @@ class NotifiableEventProcessorTest {
                 ProcessedEvent(Type.KEEP, anInviteNotifiableEvent(roomId = "event-2"))
         )
 
-        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, renderedEvents = renderedEvents)
+        val result = eventProcessor.process(events, currentRoomId = NOT_VIEWING_A_ROOM, currentThreadId = NOT_VIEWING_A_THREAD, renderedEvents = renderedEvents)
 
         result shouldBeEqualTo listOfProcessedEvents(
                 Type.REMOVE to renderedEvents[1].event,

vector/src/test/java/im/vector/app/test/fixtures/NotifiableEventFixture.kt

@@ -63,6 +63,7 @@ fun anInviteNotifiableEvent(
 fun aNotifiableMessageEvent(
         eventId: String = "a-message-event-id",
         roomId: String = "a-message-room-id",
+        threadId: String? = null,
         isRedacted: Boolean = false
 ) = NotifiableMessageEvent(
         eventId = eventId,
@@ -73,6 +74,7 @@ fun aNotifiableMessageEvent(
         senderId = "sending-id",
         body = "message-body",
         roomId = roomId,
+        threadId = threadId,
         roomName = "room-name",
         roomIsDirect = false,
         canBeReplaced = false,