Skip to content

Skip mark-read when the server tracks none of the loaded messages #6654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gpunto wants to merge 4 commits into
base: develop
Choose a base branch
from
+237 −21
Open

Skip mark-read when the server tracks none of the loaded messages #6654

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f182cc
Skip mark-read when the server tracks none of the loaded messages
gpunto Aug 20, 2026
d363f4a
Pin the randomised silent flag in the gate-dependent mark-read tests
gpunto Aug 20, 2026
0483fa6
Skip mark-read when the only messages are deleted
gpunto Aug 21, 2026
716ff2f
Drop the duplicate deletedAt argument in the nowMessage test helper
gpunto Aug 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions ...kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -1794,6 +1791,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
Comment thread
andremion marked this conversation as resolved.
Outdated

private fun markChannelAsRead() {
val (channelType, channelId) = cid.cidToTypeAndId()
chatClient.markRead(channelType, channelId).enqueue(
Expand Down
140 changes: 125 additions & 15 deletions ...n/io/getstream/chat/android/ui/common/feature/messages/list/MessageListControllerTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
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)
Expand Down Expand Up @@ -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(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.COMPLETED, silent = false)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -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(randomMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS, silent = false)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -416,7 +416,15 @@ 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(
randomMessage(
id = "1",
user = user1,
type = MessageType.ERROR,
syncStatus = SyncStatus.COMPLETED,
silent = false,
),
),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -438,7 +446,13 @@ internal class MessageListControllerTests {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(id = "1", user = user1, type = MessageType.EPHEMERAL, syncStatus = SyncStatus.COMPLETED),
randomMessage(
id = "1",
user = user1,
type = MessageType.EPHEMERAL,
syncStatus = SyncStatus.COMPLETED,
silent = false,
),
),
)
val controller = Fixture(chatClient = chatClient)
Expand All @@ -461,8 +475,89 @@ 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),
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)
.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 {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(
id = "1",
user = user1,
type = MessageType.REGULAR,
syncStatus = SyncStatus.COMPLETED,
silent = false,
shadowed = true,
Comment thread
andremion marked this conversation as resolved.
),
),
)
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)
Expand Down Expand Up @@ -501,8 +596,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)
Expand All @@ -525,7 +620,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()
Expand Down Expand Up @@ -920,14 +1015,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()
Expand All @@ -940,7 +1042,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(
Expand All @@ -949,6 +1056,7 @@ internal class MessageListControllerTests {
syncStatus = SyncStatus.COMPLETED,
deletedAt = null,
deletedForMe = false,
silent = false,
),
)
val channelRead = MutableStateFlow(
Expand Down Expand Up @@ -1834,6 +1942,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,
Expand Down
Loading