Sulkta-OSS/element-x-ada
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix: make sure we ignore notifications for open rooms (#867)

Browse source 
* Make sure we ignore notifications for open rooms
- Listen to process lifecycle changes in `AppForegroundStateService`. Use initializers to reliable create it.
- Merge `AppNavigationState` with `AppForegroundState`. Renamed the previous `AppNavigationState` to `NavigationState`, created a new `AppNavigationState` which contains both the navigation state and the foreground state.
This commit is contained in:
Jorge Martin Espinosa 2023-07-17 17:02:06 +02:00 • committed by GitHub
parent 004b86b05d
commit 9247cd765a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
26 changed files with 552 additions and 246 deletions
Download patch file Download diff file Expand all files Collapse all files

38
libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotificationDrawerManager.kt
View file

@ -32,9 +32,7 @@ import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.push.api.notifications.NotificationDrawerManager
import io.element.android.libraries.push.api.store.PushDataStore
import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent
import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent
import io.element.android.libraries.push.impl.notifications.model.shouldIgnoreEventInRoom
import io.element.android.services.appnavstate.api.AppNavigationState
import io.element.android.services.appnavstate.api.NavigationState
import io.element.android.services.appnavstate.api.AppNavigationStateService
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
@ -65,7 +63,6 @@ class DefaultNotificationDrawerManager @Inject constructor(
* Lazily initializes the NotificationState as we rely on having a current session in order to fetch the persisted queue of events.
*/
private val notificationState by lazy { createInitialNotificationState() }
private var currentAppNavigationState: AppNavigationState? = null
private val firstThrottler = FirstThrottler(200)
// TODO EAx add a setting per user for this
@ -74,26 +71,25 @@ class DefaultNotificationDrawerManager @Inject constructor(
init {
// Observe application state
coroutineScope.launch {
appNavigationStateService.appNavigationStateFlow
.collect { onAppNavigationStateChange(it) }
appNavigationStateService.appNavigationState
.collect { onAppNavigationStateChange(it.navigationState) }
}
}
private fun onAppNavigationStateChange(appNavigationState: AppNavigationState) {
currentAppNavigationState = appNavigationState
when (appNavigationState) {
AppNavigationState.Root -> {}
is AppNavigationState.Session -> {}
is AppNavigationState.Space -> {}
is AppNavigationState.Room -> {
private fun onAppNavigationStateChange(navigationState: NavigationState) {
when (navigationState) {
NavigationState.Root -> {}
is NavigationState.Session -> {}
is NavigationState.Space -> {}
is NavigationState.Room -> {
// Cleanup notification for current room
clearMessagesForRoom(appNavigationState.parentSpace.parentSession.sessionId, appNavigationState.roomId)
clearMessagesForRoom(navigationState.parentSpace.parentSession.sessionId, navigationState.roomId)
}
is AppNavigationState.Thread -> {
is NavigationState.Thread -> {
onEnteringThread(
appNavigationState.parentRoom.parentSpace.parentSession.sessionId,
appNavigationState.parentRoom.roomId,
appNavigationState.threadId
navigationState.parentRoom.parentSpace.parentSession.sessionId,
navigationState.parentRoom.roomId,
navigationState.threadId
)
}
}
@ -225,7 +221,7 @@ class DefaultNotificationDrawerManager @Inject constructor(
private suspend fun refreshNotificationDrawerBg() {
Timber.v("refreshNotificationDrawerBg()")
val eventsToRender = notificationState.updateQueuedEvents(this) { queuedEvents, renderedEvents ->
notifiableEventProcessor.process(queuedEvents.rawEvents(), currentAppNavigationState, renderedEvents).also {
notifiableEventProcessor.process(queuedEvents.rawEvents(), renderedEvents).also {
queuedEvents.clearAndAdd(it.onlyKeptEvents())
}
}
@ -275,8 +271,4 @@ class DefaultNotificationDrawerManager @Inject constructor(
notificationRenderer.render(currentUser, useCompleteNotificationFormat, notifiableEvents)
}
}
fun shouldIgnoreMessageEventInRoom(resolvedEvent: NotifiableMessageEvent): Boolean {
return resolvedEvent.shouldIgnoreEventInRoom(currentAppNavigationState)
}
}

9
libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventProcessor.kt
View file

@ -23,7 +23,7 @@ import io.element.android.libraries.push.impl.notifications.model.NotifiableEven
import io.element.android.libraries.push.impl.notifications.model.NotifiableMessageEvent
import io.element.android.libraries.push.impl.notifications.model.SimpleNotifiableEvent
import io.element.android.libraries.push.impl.notifications.model.shouldIgnoreEventInRoom
import io.element.android.services.appnavstate.api.AppNavigationState
import io.element.android.services.appnavstate.api.AppNavigationStateService
import timber.log.Timber
import javax.inject.Inject
@ -31,18 +31,19 @@ private typealias ProcessedEvents = List<ProcessedEvent<NotifiableEvent>>
class NotifiableEventProcessor @Inject constructor(
private val outdatedDetector: OutdatedEventDetector,
private val appNavigationStateService: AppNavigationStateService,
) {
fun process(
queuedEvents: List<NotifiableEvent>,
appNavigationState: AppNavigationState?,
renderedEvents: ProcessedEvents,
): ProcessedEvents {
val appState = appNavigationStateService.appNavigationState.value
val processedEvents = queuedEvents.map {
val type = when (it) {
is InviteNotifiableEvent -> ProcessedEvent.Type.KEEP
is NotifiableMessageEvent -> when {
it.shouldIgnoreEventInRoom(appNavigationState) -> {
it.shouldIgnoreEventInRoom(appState) -> {
ProcessedEvent.Type.REMOVE
.also { Timber.d("notification message removed due to currently viewing the same room or thread") }
}
@ -55,7 +56,7 @@ class NotifiableEventProcessor @Inject constructor(
else -> ProcessedEvent.Type.KEEP
}
is FallbackNotifiableEvent -> when {
it.shouldIgnoreEventInRoom(appNavigationState) -> {
it.shouldIgnoreEventInRoom(appState) -> {
ProcessedEvent.Type.REMOVE
.also { Timber.d("notification fallback removed due to currently viewing the same room or thread") }
}

17
libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt
View file

@ -16,8 +16,6 @@
package io.element.android.libraries.push.impl.notifications.model
import android.net.Uri
import androidx.lifecycle.Lifecycle
import androidx.lifecycle.ProcessLifecycleOwner
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
@ -69,18 +67,13 @@ data class NotifiableMessageEvent(
/**
* 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?.currentSessionId() ?: return false
return when (val currentRoomId = appNavigationState.currentRoomId()) {
fun NotifiableEvent.shouldIgnoreEventInRoom(appNavigationState: AppNavigationState): Boolean {
val currentSessionId = appNavigationState.navigationState.currentSessionId() ?: return false
return when (val currentRoomId = appNavigationState.navigationState.currentRoomId()) {
null -> false
else -> isAppInForeground
else -> appNavigationState.isInForeground
&& sessionId == currentSessionId
&& roomId == currentRoomId
&& (this as? NotifiableMessageEvent)?.threadId == appNavigationState.currentThreadId()
&& (this as? NotifiableMessageEvent)?.threadId == appNavigationState.navigationState.currentThreadId()
}
}
private val isAppInForeground: Boolean
get() = ProcessLifecycleOwner.get().lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)

54
libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventProcessorTest.kt
View file

@ -30,17 +30,20 @@ import io.element.android.libraries.push.impl.notifications.fixtures.aNotifiable
import io.element.android.libraries.push.impl.notifications.fixtures.aSimpleNotifiableEvent
import io.element.android.libraries.push.impl.notifications.fixtures.anInviteNotifiableEvent
import io.element.android.libraries.push.impl.notifications.model.NotifiableEvent
import io.element.android.services.appnavstate.test.anAppNavigationState
import io.element.android.services.appnavstate.api.NavigationState
import io.element.android.services.appnavstate.api.AppNavigationState
import io.element.android.services.appnavstate.test.FakeAppNavigationStateService
import io.element.android.services.appnavstate.test.aNavigationState
import kotlinx.coroutines.flow.MutableStateFlow
import org.junit.Test
private val NOT_VIEWING_A_ROOM = anAppNavigationState()
private val VIEWING_A_ROOM = anAppNavigationState(A_SESSION_ID, A_SPACE_ID, A_ROOM_ID)
private val VIEWING_A_THREAD = anAppNavigationState(A_SESSION_ID, A_SPACE_ID, A_ROOM_ID, A_THREAD_ID)
private val NOT_VIEWING_A_ROOM = aNavigationState()
private val VIEWING_A_ROOM = aNavigationState(A_SESSION_ID, A_SPACE_ID, A_ROOM_ID)
private val VIEWING_A_THREAD = aNavigationState(A_SESSION_ID, A_SPACE_ID, A_ROOM_ID, A_THREAD_ID)
class NotifiableEventProcessorTest {
private val outdatedDetector = FakeOutdatedEventDetector()
private val eventProcessor = NotifiableEventProcessor(outdatedDetector.instance)
@Test
fun `given simple events when processing then keep simple events`() {
@ -48,8 +51,9 @@ class NotifiableEventProcessorTest {
aSimpleNotifiableEvent(eventId = AN_EVENT_ID),
aSimpleNotifiableEvent(eventId = AN_EVENT_ID_2)
)
val eventProcessor = createProcessor(navigationState = NOT_VIEWING_A_ROOM)
val result = eventProcessor.process(events, appNavigationState = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -62,8 +66,9 @@ class NotifiableEventProcessorTest {
@Test
fun `given redacted simple event when processing then remove redaction event`() {
val events = listOf(aSimpleNotifiableEvent(eventId = AN_EVENT_ID, type = EventType.REDACTION))
val eventProcessor = createProcessor(navigationState = NOT_VIEWING_A_ROOM)
val result = eventProcessor.process(events, appNavigationState = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -78,8 +83,9 @@ class NotifiableEventProcessorTest {
anInviteNotifiableEvent(roomId = A_ROOM_ID),
anInviteNotifiableEvent(roomId = A_ROOM_ID_2)
)
val eventProcessor = createProcessor(navigationState = NOT_VIEWING_A_ROOM)
val result = eventProcessor.process(events, appNavigationState = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -94,7 +100,9 @@ class NotifiableEventProcessorTest {
val events = listOf(aNotifiableMessageEvent(eventId = AN_EVENT_ID, roomId = A_ROOM_ID))
outdatedDetector.givenEventIsOutOfDate(events[0])
val result = eventProcessor.process(events, appNavigationState = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
val eventProcessor = createProcessor(navigationState = NOT_VIEWING_A_ROOM)
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -107,8 +115,9 @@ class NotifiableEventProcessorTest {
fun `given in date message event when processing then keep message event`() {
val events = listOf(aNotifiableMessageEvent(eventId = AN_EVENT_ID, roomId = A_ROOM_ID))
outdatedDetector.givenEventIsInDate(events[0])
val eventProcessor = createProcessor(navigationState = NOT_VIEWING_A_ROOM)
val result = eventProcessor.process(events, appNavigationState = NOT_VIEWING_A_ROOM, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -121,8 +130,9 @@ class NotifiableEventProcessorTest {
fun `given viewing the same room main timeline when processing main timeline message event then removes message`() {
val events = listOf(aNotifiableMessageEvent(eventId = AN_EVENT_ID, roomId = A_ROOM_ID, threadId = null))
events.forEach { outdatedDetector.givenEventIsOutOfDate(it) }
val eventProcessor = createProcessor(isInForeground = true, navigationState = VIEWING_A_ROOM)
val result = eventProcessor.process(events, VIEWING_A_ROOM, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -135,8 +145,9 @@ class NotifiableEventProcessorTest {
fun `given viewing the same thread timeline when processing thread message event then removes message`() {
val events = listOf(aNotifiableMessageEvent(eventId = AN_EVENT_ID, roomId = A_ROOM_ID, threadId = A_THREAD_ID))
events.forEach { outdatedDetector.givenEventIsOutOfDate(it) }
val eventProcessor = createProcessor(isInForeground = true, navigationState = VIEWING_A_THREAD)
val result = eventProcessor.process(events, VIEWING_A_THREAD, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -149,8 +160,9 @@ class NotifiableEventProcessorTest {
fun `given viewing main timeline of the same room when processing thread timeline message event then keep message`() {
val events = listOf(aNotifiableMessageEvent(eventId = AN_EVENT_ID, roomId = A_ROOM_ID, threadId = A_THREAD_ID))
outdatedDetector.givenEventIsInDate(events[0])
val eventProcessor = createProcessor(isInForeground = true, navigationState = VIEWING_A_ROOM)
val result = eventProcessor.process(events, VIEWING_A_ROOM, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -163,8 +175,9 @@ class NotifiableEventProcessorTest {
fun `given viewing thread timeline of the same room when processing main timeline message event then keep message`() {
val events = listOf(aNotifiableMessageEvent(eventId = AN_EVENT_ID, roomId = A_ROOM_ID))
outdatedDetector.givenEventIsInDate(events[0])
val eventProcessor = createProcessor(isInForeground = true, navigationState = VIEWING_A_THREAD)
val result = eventProcessor.process(events, VIEWING_A_THREAD, renderedEvents = emptyList())
val result = eventProcessor.process(events, renderedEvents = emptyList())
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -180,8 +193,9 @@ class NotifiableEventProcessorTest {
ProcessedEvent(ProcessedEvent.Type.KEEP, events[0]),
ProcessedEvent(ProcessedEvent.Type.KEEP, anInviteNotifiableEvent(eventId = AN_EVENT_ID_2))
)
val eventProcessor = createProcessor(navigationState = NOT_VIEWING_A_ROOM)
val result = eventProcessor.process(events, appNavigationState = NOT_VIEWING_A_ROOM, renderedEvents = renderedEvents)
val result = eventProcessor.process(events, renderedEvents = renderedEvents)
assertThat(result).isEqualTo(
listOfProcessedEvents(
@ -194,4 +208,14 @@ class NotifiableEventProcessorTest {
private fun listOfProcessedEvents(vararg event: Pair<ProcessedEvent.Type, NotifiableEvent>) = event.map {
ProcessedEvent(it.first, it.second)
}
private fun createProcessor(
isInForeground: Boolean = false,
navigationState: NavigationState
): NotifiableEventProcessor {
return NotifiableEventProcessor(
outdatedDetector.instance,
FakeAppNavigationStateService(MutableStateFlow(AppNavigationState(navigationState, isInForeground))),
)
}
}