diff --git a/stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListController.kt b/stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListController.kt index 126c1cab412..9c50cdcc68c 100644 --- a/stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListController.kt +++ b/stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListController.kt @@ -1758,13 +1758,10 @@ public class MessageListController( val messageText = message?.text logger.d { "[markLastMessageRead] cid: $cid, msgId($isInThread): $messageId, msgText: \"$messageText\"" } - // The server keeps our own local-only messages out of its read state, so marking read - // without any other message makes it emit message.read with no last_read_message_id. + // Marking read with nothing the server tracks makes it emit message.read with no + // last_read_message_id. val currentUserId = clientState.user.value?.id - val hasServerSideMessage = messageItems.any { item -> - !(item.message.isMine(currentUserId) && item.message.isLocalOnly()) - } - if (!hasServerSideMessage) { + if (messageItems.none { it.message.isInServerReadState(currentUserId) }) { logger.v { "[markLastMessageRead] cid: $cid; rejected[$isInThread] (no server-side message)" } return } @@ -1794,6 +1791,12 @@ public class MessageListController( } } + // The server keeps our own local-only messages out of its channel read state, along with + // silent, shadowed and deleted ones, so none of them can resolve a mark-read call. Deleted for + // the current user only does not count: the message is still there for everyone else. + private fun Message.isInServerReadState(currentUserId: String?): Boolean = + !(isMine(currentUserId) && isLocalOnly()) && !silent && !shadowed && deletedAt == null + private fun markChannelAsRead() { val (channelType, channelId) = cid.cidToTypeAndId() chatClient.markRead(channelType, channelId).enqueue( diff --git a/stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListControllerTests.kt b/stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListControllerTests.kt index 2edb257c3a6..4c26c49c43d 100644 --- a/stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListControllerTests.kt +++ b/stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListControllerTests.kt @@ -343,9 +343,9 @@ internal class MessageListControllerTests { fun `When repetitive markLastMessageRead calls appear only single API call should be sent`() = runTest { val chatClient: ChatClient = mock() val messages = arrayListOf( - randomMessage(id = "1", syncStatus = SyncStatus.COMPLETED), - randomMessage(id = "2", syncStatus = SyncStatus.COMPLETED), - randomMessage(id = "3", syncStatus = SyncStatus.COMPLETED), + gateMessage(id = "1"), + gateMessage(id = "2"), + gateMessage(id = "3"), ) val messagesState = MutableStateFlow(messages) val controller = Fixture(chatClient = chatClient) @@ -373,7 +373,7 @@ internal class MessageListControllerTests { fun `When current user's last message is COMPLETED markLastMessageRead should invoke markRead`() = runTest { val chatClient: ChatClient = mock() val messagesState = MutableStateFlow( - listOf(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.COMPLETED)), + listOf(gateMessage(id = "1", user = user1)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -393,7 +393,7 @@ internal class MessageListControllerTests { fun `When current user's last message is not COMPLETED markLastMessageRead should not invoke markRead`() = runTest { val chatClient: ChatClient = mock() val messagesState = MutableStateFlow( - listOf(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS)), + listOf(gateMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -416,7 +416,7 @@ internal class MessageListControllerTests { // as COMPLETED, while the server keeps it out of its read state. val chatClient: ChatClient = mock() val messagesState = MutableStateFlow( - listOf(randomMessage(id = "1", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED)), + listOf(gateMessage(id = "1", user = user1, type = MessageType.ERROR)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -437,9 +437,7 @@ internal class MessageListControllerTests { runTest { val chatClient: ChatClient = mock() val messagesState = MutableStateFlow( - listOf( - randomMessage(id = "1", user = user1, type = MessageType.EPHEMERAL, syncStatus = SyncStatus.COMPLETED), - ), + listOf(gateMessage(id = "1", user = user1, type = MessageType.EPHEMERAL)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -461,8 +459,117 @@ internal class MessageListControllerTests { val chatClient: ChatClient = mock() val messagesState = MutableStateFlow( listOf( - randomMessage(id = "1", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED), - randomMessage(id = "2", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED), + gateMessage(id = "1", user = user2), + gateMessage(id = "2", user = user1, type = MessageType.ERROR), + ), + ) + val controller = Fixture(chatClient = chatClient) + .givenCurrentUser() + .givenChannelQuery() + .givenMarkRead() + .givenChannelState(messagesState = messagesState) + .get() + + controller.markLastMessageRead() + delay(1000) + + verify(chatClient, times(1)).markRead(eq(CHANNEL_TYPE), eq(CHANNEL_ID)) + controller.lastSeenMessageId `should be equal to` "2" + } + + @Test + fun `When the channel holds only a silent message markLastMessageRead should not invoke markRead`() = runTest { + // A silent message does not mark a channel unread, so the server has nothing to resolve. + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf(gateMessage(id = "1", user = user2, silent = true)), + ) + val controller = Fixture(chatClient = chatClient) + .givenCurrentUser() + .givenChannelQuery() + .givenMarkRead() + .givenChannelState(messagesState = messagesState) + .get() + + controller.markLastMessageRead() + delay(1000) + + verify(chatClient, times(0)).markRead(any(), any()) + controller.lastSeenMessageId.shouldBeNull() + } + + @Test + fun `When the channel holds only a shadowed message markLastMessageRead should not invoke markRead`() = runTest { + // Defensive only: the server clears shadowed for the author, and ChannelStateImpl drops + // other users' shadowed messages, so this state does not reach the list today. + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf(gateMessage(id = "1", user = user1, shadowed = true)), + ) + val controller = Fixture(chatClient = chatClient) + .givenCurrentUser() + .givenChannelQuery() + .givenMarkRead() + .givenChannelState(messagesState = messagesState) + .get() + + controller.markLastMessageRead() + delay(1000) + + verify(chatClient, times(0)).markRead(any(), any()) + controller.lastSeenMessageId.shouldBeNull() + } + + @Test + fun `When the channel holds only a deleted message markLastMessageRead should not invoke markRead`() = runTest { + // The server drops a deleted message from its read state, so it has nothing to resolve. + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf(gateMessage(id = "1", user = user2, deletedAt = randomDate())), + ) + val controller = Fixture(chatClient = chatClient) + .givenCurrentUser() + .givenChannelQuery() + .givenMarkRead() + .givenChannelState(messagesState = messagesState) + .get() + + controller.markLastMessageRead() + delay(1000) + + verify(chatClient, times(0)).markRead(any(), any()) + controller.lastSeenMessageId.shouldBeNull() + } + + @Test + fun `When a message is deleted for the current user only markLastMessageRead should invoke markRead`() = runTest { + // Deleted for me leaves the message in place for everyone else, so the server can still + // resolve the read state. + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf(gateMessage(id = "1", user = user2, deletedForMe = true)), + ) + val controller = Fixture(chatClient = chatClient) + .givenCurrentUser() + .givenChannelQuery() + .givenMarkRead() + .givenChannelState(messagesState = messagesState) + .get() + + controller.markLastMessageRead() + delay(1000) + + verify(chatClient, times(1)).markRead(eq(CHANNEL_TYPE), eq(CHANNEL_ID)) + controller.lastSeenMessageId `should be equal to` "1" + } + + @Test + fun `When a silent message follows a tracked one markLastMessageRead should invoke markRead`() = runTest { + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf( + gateMessage(id = "1", user = user2), + gateMessage(id = "2", user = user2, silent = true), ), ) val controller = Fixture(chatClient = chatClient) @@ -501,8 +608,8 @@ internal class MessageListControllerTests { val chatClient: ChatClient = mock() val messagesState = MutableStateFlow( listOf( - randomMessage(id = "1", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED), - randomMessage(id = "2", user = user1, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED), + gateMessage(id = "1", user = user1, type = MessageType.ERROR), + gateMessage(id = "2", user = user1), ), ) val controller = Fixture(chatClient = chatClient) @@ -525,7 +632,7 @@ internal class MessageListControllerTests { // class default — the gate must not block them on that. val chatClient: ChatClient = mock() val messagesState = MutableStateFlow( - listOf(randomMessage(id = "1", user = user2, syncStatus = SyncStatus.IN_PROGRESS)), + listOf(gateMessage(id = "1", user = user2, syncStatus = SyncStatus.IN_PROGRESS)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -920,14 +1027,21 @@ internal class MessageListControllerTests { controller.unreadLabelState.value `should be equal to` null messagesState.value = listOf( - randomMessage(id = "last_read_message_id", user = user1, deletedAt = null, deletedForMe = false), - randomMessage(id = "unread_1", user = user2, deletedAt = null, deletedForMe = false), + randomMessage( + id = "last_read_message_id", + user = user1, + deletedAt = null, + deletedForMe = false, + silent = false, + ), + randomMessage(id = "unread_1", user = user2, deletedAt = null, deletedForMe = false, silent = false), randomMessage( id = "unread_2", user = user2, syncStatus = SyncStatus.COMPLETED, deletedAt = null, deletedForMe = false, + silent = false, ), ) controller.markLastMessageRead() @@ -940,7 +1054,12 @@ internal class MessageListControllerTests { fun `Keep unread label, when marking read zeroes the read state`() = runTest { val chatClient: ChatClient = mock() - val lastReadMessage = randomMessage(id = "last_read_message_id", deletedAt = null, deletedForMe = false) + val lastReadMessage = randomMessage( + id = "last_read_message_id", + deletedAt = null, + deletedForMe = false, + silent = false, + ) val messages = listOf( lastReadMessage, randomMessage( @@ -949,6 +1068,7 @@ internal class MessageListControllerTests { syncStatus = SyncStatus.COMPLETED, deletedAt = null, deletedForMe = false, + silent = false, ), ) val channelRead = MutableStateFlow( @@ -1822,6 +1942,31 @@ internal class MessageListControllerTests { @OptIn(ExperimentalCoroutinesApi::class) private fun nowDate() = Date(testCoroutines.dispatcher.scheduler.currentTime) + /** + * A message for the mark-read gate, tracked by the server's read state unless a field is + * overridden. [randomMessage] randomises `silent`, `deletedAt` and `deletedForMe`, so every + * field the gate reads is pinned here and each test overrides only the one it covers. + */ + private fun gateMessage( + id: String = randomString(), + user: User = randomUser(), + type: String = MessageType.REGULAR, + syncStatus: SyncStatus = SyncStatus.COMPLETED, + silent: Boolean = false, + shadowed: Boolean = false, + deletedAt: Date? = null, + deletedForMe: Boolean = false, + ) = randomMessage( + id = id, + user = user, + type = type, + syncStatus = syncStatus, + silent = silent, + shadowed = shadowed, + deletedAt = deletedAt, + deletedForMe = deletedForMe, + ) + private fun nowMessage( author: User, type: String, @@ -1834,6 +1979,8 @@ internal class MessageListControllerTests { type = type, text = text, syncStatus = syncStatus, + // randomMessage randomises silent and deletedAt, which the mark-read gate keys on. + silent = false, createdAt = nowDate, updatedAt = nowDate, deletedAt = null,