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

Merged
gpunto merged 5 commits into from
Aug 24, 2026
+173 −23
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 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
485f7d7
Pin every gate field in the mark-read tests through one helper
gpunto Aug 24, 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
15 changes: 9 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,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(
Expand Down
181 changes: 164 additions & 17 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),
gateMessage(id = "1"),
gateMessage(id = "2"),
gateMessage(id = "3"),
)
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(gateMessage(id = "1", user = user1)),
)
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(gateMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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()
Expand Down Expand Up @@ -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()
Expand All @@ -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(
Expand All @@ -949,6 +1068,7 @@ internal class MessageListControllerTests {
syncStatus = SyncStatus.COMPLETED,
deletedAt = null,
deletedForMe = false,
silent = false,
),
)
val channelRead = MutableStateFlow(
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Loading