diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index 751af01da8c..b38f04e5d9c 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -137,6 +137,9 @@ class OfflineFirstChatRepository @Inject constructor( private val _isLoadingFlow: MutableStateFlow = MutableStateFlow(false) private var newXChatLastCommonRead: Int? = null + + // written on the main thread by handleOnPause/handleOnResume, read from fetch and polling coroutines + @Volatile private var itIsPaused = false lateinit var internalConversationId: String @@ -363,6 +366,10 @@ class OfflineFirstChatRepository @Inject constructor( * [ChatMessageSyncer.tryCloseBacklog]'s round budget and just burns requests before falling * back anyway. * + * Server push notifications are only cleared while the chat is on screen: when the chat is + * paused (background, another screen) the request carries markNotificationsAsRead=0, so the + * fetch does not dismiss notifications for messages the user has not seen yet. + * * @return `true` if at least one new message was received and persisted. */ override suspend fun fetchNewMessages(): Boolean { @@ -381,6 +388,7 @@ class OfflineFirstChatRepository @Inject constructor( fromMessageId = lastHttpSyncedMessageId, limit = 200, lastCommonRead = newXChatLastCommonRead, + markNotificationsAsRead = !itIsPaused, events = syncEvents ) return outcome.persistedNewMessages diff --git a/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt b/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt index 12e3fa3780c..b32c18b34fa 100644 --- a/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt +++ b/app/src/test/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepositoryTest.kt @@ -51,6 +51,7 @@ import org.junit.Before import org.junit.Test import org.mockito.kotlin.any import org.mockito.kotlin.argumentCaptor +import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.eq import org.mockito.kotlin.doSuspendableAnswer import org.mockito.kotlin.mock @@ -212,6 +213,22 @@ class OfflineFirstChatRepositoryTest { verifyBlocking(network, never()) { pullChatMessages(any(), any(), any()) } } + @Test + fun `fetchNewMessages keeps notifications only while the chat is paused`() = + runTest { + givenHttpSyncedAnchor() + repository.fetchNewMessages() + assertFalse(lastRequestFieldMap().containsKey("markNotificationsAsRead")) + + repository.handleOnPause() + repository.fetchNewMessages() + assertEquals(0, lastRequestFieldMap()["markNotificationsAsRead"]) + + repository.handleOnResume() + repository.fetchNewMessages() + assertFalse(lastRequestFieldMap().containsKey("markNotificationsAsRead")) + } + @Test fun `addTemporaryMessage upserts a pending local echo and emits it on success`() = runTest { @@ -307,6 +324,22 @@ class OfflineFirstChatRepositoryTest { .thenReturn(flowOf(block)) } + private fun lastRequestFieldMap(): HashMap { + val fieldMapCaptor = argumentCaptor>() + verifyBlocking(network, atLeastOnce()) { pullChatMessages(any(), any(), fieldMapCaptor.capture()) } + assertEquals(51, fieldMapCaptor.lastValue["lastKnownMessageId"]) // insurance request, not the initial load + return fieldMapCaptor.lastValue + } + + /** Records an HTTP sync anchor via the initial load, so fetchNewMessages does not skip. */ + private suspend fun givenHttpSyncedAnchor() { + givenLatestBlock(block(oldest = 10, newest = 50)) + repository.updateConversation(conversation(lastReadMessage = 40, unreadMessages = 5)) + wheneverBlocking { network.pullChatMessages(any(), any(), any()) } + .thenReturn(Response.success(overall(message(51)))) + repository.loadInitialMessages(Bundle()) + } + private fun singleRequestFieldMap(): HashMap { val fieldMapCaptor = argumentCaptor>() verifyBlocking(network) { pullChatMessages(eq(CREDENTIALS), eq(CHAT_URL), fieldMapCaptor.capture()) }