ignore push for a thread if it's currently visible to user (#7641)

This commit is contained in:
Nikita Fedrunov 2022-11-28 17:29:30 +01:00 committed by GitHub
parent 49199bd5e2
commit 46fc0ac563
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 102 additions and 28 deletions

1
changelog.d/7634.bugfix Normal file
View file

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

View file

@ -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 {

View file

@ -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 ->

View file

@ -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
}
}

View file

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

View file

@ -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
}
}

View file

@ -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,

View file

@ -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 {

View file

@ -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
}

View file

@ -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,7 +111,7 @@ 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],
@ -118,16 +119,51 @@ class NotifiableEventProcessorTest {
}
@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", renderedEvents = emptyList())
val result = eventProcessor.process(events, currentRoomId = "room-1", currentThreadId = null, renderedEvents = emptyList())
result shouldBeEqualTo listOfProcessedEvents(
Type.REMOVE to events[0],
)
}
@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,

View file

@ -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,