Skip to content

Skip mark-read when the only messages are deleted #6658

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 3 commits into from
Aug 24, 2026
+91 −53
Merged

Skip mark-read when the only messages are deleted #6658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b99997
Skip mark-read when the only messages are deleted
gpunto Aug 21, 2026
573cd24
Drop the duplicate deletedAt argument in the nowMessage test helper
gpunto Aug 21, 2026
79fb018
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
7 changes: 4 additions & 3 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 @@ -1746,10 +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.
// 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
!(isMine(currentUserId) && isLocalOnly()) && !silent && !shadowed && deletedAt == null

private fun markChannelAsRead() {
val (channelType, channelId) = cid.cidToTypeAndId()
Expand Down
137 changes: 87 additions & 50 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 @@ -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, silent = false),
randomMessage(id = "2", syncStatus = SyncStatus.COMPLETED, silent = false),
randomMessage(id = "3", syncStatus = SyncStatus.COMPLETED, silent = false),
gateMessage(id = "1"),
gateMessage(id = "2"),
gateMessage(id = "3"),
)
val messagesState = MutableStateFlow(messages)
val controller = Fixture(chatClient = chatClient)
Expand Down Expand Up @@ -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, silent = false)),
listOf(gateMessage(id = "1", user = user1)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -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, silent = false)),
listOf(gateMessage(id = "1", user = user1, syncStatus = SyncStatus.IN_PROGRESS)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -424,15 +424,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,
silent = false,
),
),
listOf(gateMessage(id = "1", user = user1, type = MessageType.ERROR)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -453,15 +445,7 @@ internal class MessageListControllerTests {
runTest {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
randomMessage(
id = "1",
user = user1,
type = MessageType.EPHEMERAL,
syncStatus = SyncStatus.COMPLETED,
silent = false,
),
),
listOf(gateMessage(id = "1", user = user1, type = MessageType.EPHEMERAL)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -483,8 +467,8 @@ internal class MessageListControllerTests {
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 = user1, type = MessageType.ERROR, syncStatus = SyncStatus.COMPLETED, silent = false),
gateMessage(id = "1", user = user2),
gateMessage(id = "2", user = user1, type = MessageType.ERROR),
),
)
val controller = Fixture(chatClient = chatClient)
Expand All @@ -506,15 +490,7 @@ internal class MessageListControllerTests {
// 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,
),
),
listOf(gateMessage(id = "1", user = user2, silent = true)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -532,18 +508,11 @@ internal class MessageListControllerTests {

@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(
randomMessage(
id = "1",
user = user1,
type = MessageType.REGULAR,
syncStatus = SyncStatus.COMPLETED,
silent = false,
shadowed = true,
),
),
listOf(gateMessage(id = "1", user = user1, shadowed = true)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand All @@ -559,13 +528,56 @@ internal class MessageListControllerTests {
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(
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),
gateMessage(id = "1", user = user2),
gateMessage(id = "2", user = user2, silent = true),
),
)
val controller = Fixture(chatClient = chatClient)
Expand Down Expand Up @@ -604,8 +616,8 @@ internal class MessageListControllerTests {
val chatClient: ChatClient = mock()
val messagesState = MutableStateFlow(
listOf(
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),
gateMessage(id = "1", user = user1, type = MessageType.ERROR),
gateMessage(id = "2", user = user1),
),
)
val controller = Fixture(chatClient = chatClient)
Expand All @@ -628,7 +640,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, silent = false)),
listOf(gateMessage(id = "1", user = user2, syncStatus = SyncStatus.IN_PROGRESS)),
)
val controller = Fixture(chatClient = chatClient)
.givenCurrentUser()
Expand Down Expand Up @@ -1557,6 +1569,31 @@ internal class MessageListControllerTests {

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 @@ -1569,7 +1606,7 @@ internal class MessageListControllerTests {
type = type,
text = text,
syncStatus = syncStatus,
// randomMessage randomises silent, which the mark-read gate keys on.
// randomMessage randomises silent and deletedAt, which the mark-read gate keys on.
silent = false,
createdAt = nowDate,
updatedAt = nowDate,
Expand Down
Loading