diff --git a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/message/MessageUtils.kt b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/message/MessageUtils.kt index cbd8c8189e2..e11e2a97195 100644 --- a/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/message/MessageUtils.kt +++ b/stream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/message/MessageUtils.kt @@ -209,9 +209,9 @@ public fun Message.isModerationError(currentUserId: String?): Boolean = isMine(c (isError() && isModerationBounce()) /** - * @return If the message exists only on this client and is not persisted server-side: it is - * still in flight, failed to send, or has a type the server never persists (ephemeral previews, - * error messages such as rejected or moderation-bounced sends). + * @return If the message is not part of the server's message list: it is unsynced, or has a type + * the server leaves out of message queries and read state (ephemeral previews, error messages such + * as rejected or moderation-bounced sends). */ @InternalStreamChatApi public fun Message.isLocalOnly(): Boolean = 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 50e0a8ec4c8..d97dbce3aea 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 @@ -33,6 +33,7 @@ import io.getstream.chat.android.client.utils.message.isDeleted import io.getstream.chat.android.client.utils.message.isError import io.getstream.chat.android.client.utils.message.isGiphy import io.getstream.chat.android.client.utils.message.isLocalOnly +import io.getstream.chat.android.client.utils.message.isMine import io.getstream.chat.android.client.utils.message.isModerationBounce import io.getstream.chat.android.client.utils.message.isModerationError import io.getstream.chat.android.client.utils.message.isSystem @@ -1717,19 +1718,17 @@ public class MessageListController( * Marks that the last message in the list as read. This also sets the unread count to 0. */ private fun markLastMessageReadInternal() { - val itemState = messagesState.messageItems.lastOrNull { messageItem -> - messageItem is HasMessageListItemState - } as? HasMessageListItemState - val message = itemState?.message + val messageItems = messagesState.messageItems.filterIsInstance() + val message = messageItems.lastOrNull()?.message val messageId = message?.id val messageText = message?.text logger.d { "[markLastMessageRead] cid: $cid, msgId($isInThread): $messageId, msgText: \"$messageText\"" } - // Marking read while our own local-only message is at the bottom makes a channel the - // server sees as empty 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 - if (message != null && message.user.id == currentUserId && message.isLocalOnly()) { - logger.v { "[markLastMessageRead] cid: $cid; rejected[$isInThread] (own local-only): $messageId" } + if (messageItems.none { it.message.isInServerReadState(currentUserId) }) { + logger.v { "[markLastMessageRead] cid: $cid; rejected[$isInThread] (no server-side message)" } return } @@ -1747,6 +1746,11 @@ public class MessageListController( } } + // The server keeps our own local-only messages out of its read state, and silent and shadowed + // ones from anyone, so none of them can resolve a mark-read call. + private fun Message.isInServerReadState(currentUserId: String?): Boolean = + !(isMine(currentUserId) && isLocalOnly()) && !silent && !shadowed + 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 5c82bf203fa..3a2c4ecf0c2 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 @@ -351,9 +351,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), + randomMessage(id = "1", syncStatus = SyncStatus.COMPLETED, silent = false), + randomMessage(id = "2", syncStatus = SyncStatus.COMPLETED, silent = false), + randomMessage(id = "3", syncStatus = SyncStatus.COMPLETED, silent = false), ) val messagesState = MutableStateFlow(messages) val controller = Fixture(chatClient = chatClient) @@ -381,7 +381,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(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.COMPLETED, silent = false)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -401,7 +401,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(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS, silent = false)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -418,16 +418,20 @@ internal class MessageListControllerTests { } @Test - fun `When current user's last message is a rejected error echo markLastMessageRead should not invoke markRead`() = + fun `When the channel holds only our own rejected error echo markLastMessageRead should not invoke markRead`() = runTest { // A send into a frozen channel returns 201 with a type "error" echo which is stored - // as COMPLETED, but the server never persists it. The peer message makes sure the - // error one is the item the gate sees, and not filtered out. + // as COMPLETED, while the server keeps it out of its read state. 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), + randomMessage( + id = "1", + user = user1, + type = MessageType.ERROR, + syncStatus = SyncStatus.COMPLETED, + silent = false, + ), ), ) val controller = Fixture(chatClient = chatClient) @@ -445,12 +449,71 @@ internal class MessageListControllerTests { } @Test - fun `When current user's last message is ephemeral markLastMessageRead should not invoke markRead`() = runTest { + fun `When the channel holds only our own ephemeral message markLastMessageRead should not invoke markRead`() = + runTest { + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf( + randomMessage( + id = "1", + user = user1, + type = MessageType.EPHEMERAL, + syncStatus = SyncStatus.COMPLETED, + silent = false, + ), + ), + ) + 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 server-side message precedes our own error echo markLastMessageRead should invoke markRead`() = runTest { + // The server has messages of its own to mark read, so it resolves the read state itself. 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.EPHEMERAL, syncStatus = SyncStatus.COMPLETED), + randomMessage(id = "1", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = false), + randomMessage(id = "2", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED, silent = false), + ), + ) + 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( + randomMessage( + id = "1", + user = user2, + type = MessageType.REGULAR, + syncStatus = SyncStatus.COMPLETED, + silent = true, + ), ), ) val controller = Fixture(chatClient = chatClient) @@ -467,6 +530,58 @@ internal class MessageListControllerTests { controller.lastSeenMessageId.shouldBeNull() } + @Test + fun `When the channel holds only a shadowed message markLastMessageRead should not invoke markRead`() = runTest { + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf( + randomMessage( + id = "1", + user = user1, + type = MessageType.REGULAR, + syncStatus = SyncStatus.COMPLETED, + silent = false, + 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 a silent message follows a tracked one markLastMessageRead should invoke markRead`() = runTest { + val chatClient: ChatClient = mock() + val messagesState = MutableStateFlow( + listOf( + randomMessage(id = "1", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = false), + randomMessage(id = "2", user = user2, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = 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` "2" + } + @Test fun `When the message list is empty markLastMessageRead should not invoke markRead`() = runTest { val chatClient: ChatClient = mock() @@ -489,8 +604,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), + randomMessage(id = "1", user = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED, silent = false), + randomMessage(id = "2", user = user1, type = MessageType.REGULAR, syncStatus = SyncStatus.COMPLETED, silent = false), ), ) val controller = Fixture(chatClient = chatClient) @@ -513,7 +628,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(randomMessage(id = "1", user = user2, syncStatus = SyncStatus.IN_PROGRESS, silent = false)), ) val controller = Fixture(chatClient = chatClient) .givenCurrentUser() @@ -1454,6 +1569,8 @@ internal class MessageListControllerTests { type = type, text = text, syncStatus = syncStatus, + // randomMessage randomises silent, which the mark-read gate keys on. + silent = false, createdAt = nowDate, updatedAt = nowDate, deletedAt = null,