Skip to content

Commit f0bc26d

Browse files
test(chat): cover the unread marker boundary latch
Extracts the latch condition from buildChatItems into the pure findFirstUnreadMessageId (no behavior change) and covers it: the marker position is only derived when a message at or below lastReadMessage is visible, windows floating entirely above the boundary and temporary messages with negative ids yield no marker, and a deleted or expired boundary message is handled by any older visible message. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
1 parent 04723d2 commit f0bc26d

2 files changed

Lines changed: 91 additions & 18 deletions

File tree

app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,22 +1134,10 @@ class ChatViewModel @AssistedInject constructor(
11341134

11351135
return buildList {
11361136
if (firstUnreadMessageId == null && lastReadMessage > 0) {
1137-
// Latch the marker position only when the visible window provably reaches back to
1138-
// the unread boundary, i.e. a message at or below lastReadMessage is visible.
1139-
// Without that proof the oldest visible message may still be far above the true
1140-
// first unread message (e.g. after a capped fetch of only the newest messages) and
1141-
// the marker would be latched in the middle of the unread messages. Temporary
1142-
// messages carry negative ids and don't count as proof.
1143-
val unreadBoundaryIsVisible = uiMessages.any { it.id in 1..lastReadMessage }
1144-
if (unreadBoundaryIsVisible) {
1145-
firstUnreadMessageId =
1146-
uiMessages.firstOrNull {
1147-
it.id > lastReadMessage
1148-
}?.id
1149-
Log.d(TAG, "reversedMessages.size = ${uiMessages.size}")
1150-
Log.d(TAG, "firstUnreadMessageId = $firstUnreadMessageId")
1151-
Log.d(TAG, "conversation.lastReadMessage = $lastReadMessage")
1152-
}
1137+
firstUnreadMessageId = findFirstUnreadMessageId(uiMessages, lastReadMessage)
1138+
Log.d(TAG, "reversedMessages.size = ${uiMessages.size}")
1139+
Log.d(TAG, "firstUnreadMessageId = $firstUnreadMessageId")
1140+
Log.d(TAG, "conversation.lastReadMessage = $lastReadMessage")
11531141
}
11541142

11551143
for (uiMessage in uiMessages) {
@@ -2408,6 +2396,25 @@ class ChatViewModel @AssistedInject constructor(
24082396

24092397
companion object {
24102398
private val TAG = ChatViewModel::class.java.simpleName
2399+
2400+
/**
2401+
* Returns the id of the first unread message, or null when it cannot be determined (yet).
2402+
*
2403+
* The position is only trustworthy when the visible window provably reaches back to the
2404+
* unread boundary, i.e. a message at or below [lastReadMessage] is visible. Without that
2405+
* proof the oldest visible message may still be far above the true first unread message
2406+
* (e.g. after a capped fetch of only the newest messages) and a marker latched onto it
2407+
* would sit in the middle of the unread messages. Temporary messages carry negative ids
2408+
* and don't count as proof.
2409+
*/
2410+
internal fun findFirstUnreadMessageId(uiMessages: List<ChatMessageUi>, lastReadMessage: Int): Int? {
2411+
val unreadBoundaryIsVisible = uiMessages.any { it.id in 1..lastReadMessage }
2412+
if (!unreadBoundaryIsVisible) {
2413+
return null
2414+
}
2415+
return uiMessages.firstOrNull { it.id > lastReadMessage }?.id
2416+
}
2417+
24112418
const val JOIN_ROOM_RETRY_COUNT: Long = 3
24122419
const val HTTP_CODE_OK: Int = 200
24132420
private const val CONVERSATION_AND_USER_FLOW_SHARING_TIMEOUT_MS = 5_000L

app/src/test/java/com/nextcloud/talk/chat/viewmodels/ChatViewModelTest.kt

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,15 @@
77

88
package com.nextcloud.talk.chat.viewmodels
99

10+
import com.nextcloud.talk.chat.ui.model.ChatMessageUi
11+
import com.nextcloud.talk.chat.ui.model.MessageStatusIcon
12+
import com.nextcloud.talk.chat.ui.model.MessageTypeContent
13+
import org.junit.Assert.assertEquals
1014
import org.junit.Assert.assertFalse
15+
import org.junit.Assert.assertNull
1116
import org.junit.Assert.assertTrue
1217
import org.junit.Test
18+
import java.time.LocalDate
1319

1420
class ChatViewModelTest {
1521

@@ -31,15 +37,15 @@ class ChatViewModelTest {
3137
@Test
3238
fun `isPlausibleLastReadMessageId returns true within the buffer above the newest known message id`() {
3339
assertTrue(
34-
ChatViewModel.isPlausibleLastReadMessageId(messageId = 158761 + 2000, newestKnownRealMessageId = 158761L)
40+
ChatViewModel.isPlausibleLastReadMessageId(messageId = 158761 + 10_000, newestKnownRealMessageId = 158761L)
3541
)
3642
}
3743

3844
@Test
3945
fun `isPlausibleLastReadMessageId returns false just beyond the buffer above the newest known message id`() {
4046
assertFalse(
4147
ChatViewModel.isPlausibleLastReadMessageId(
42-
messageId = 158761 + 2000 + 1,
48+
messageId = 158761 + 10_000 + 1,
4349
newestKnownRealMessageId = 158761L
4450
)
4551
)
@@ -51,4 +57,64 @@ class ChatViewModelTest {
5157
ChatViewModel.isPlausibleLastReadMessageId(messageId = 1_963_726_147, newestKnownRealMessageId = 158761L)
5258
)
5359
}
60+
61+
// The unread marker latch: the marker position must only be derived from the visible window
62+
// when the window provably reaches back to the unread boundary — otherwise a window of
63+
// only-unread messages (e.g. after a capped fetch of the newest messages) would place the
64+
// marker in the middle of the unread messages.
65+
66+
@Test
67+
fun `marker is placed at the first message above the boundary when the boundary is visible`() {
68+
val messages = listOf(uiMessage(39), uiMessage(40), uiMessage(41), uiMessage(42))
69+
70+
assertEquals(41, ChatViewModel.findFirstUnreadMessageId(messages, lastReadMessage = 40))
71+
}
72+
73+
@Test
74+
fun `marker is placed correctly when the boundary message itself is missing`() {
75+
// the last read message may have been deleted or expired — an older read message is
76+
// equally valid proof that the window reaches the boundary
77+
val messages = listOf(uiMessage(38), uiMessage(41), uiMessage(42))
78+
79+
assertEquals(41, ChatViewModel.findFirstUnreadMessageId(messages, lastReadMessage = 40))
80+
}
81+
82+
@Test
83+
fun `no marker is placed when the window floats entirely above the boundary`() {
84+
val messages = listOf(uiMessage(141), uiMessage(142), uiMessage(143))
85+
86+
assertNull(ChatViewModel.findFirstUnreadMessageId(messages, lastReadMessage = 40))
87+
}
88+
89+
@Test
90+
fun `temporary messages with negative ids are no proof of the boundary`() {
91+
val messages = listOf(uiMessage(-5), uiMessage(141), uiMessage(142))
92+
93+
assertNull(ChatViewModel.findFirstUnreadMessageId(messages, lastReadMessage = 40))
94+
}
95+
96+
@Test
97+
fun `no marker is placed when everything is read`() {
98+
val messages = listOf(uiMessage(38), uiMessage(39), uiMessage(40))
99+
100+
assertNull(ChatViewModel.findFirstUnreadMessageId(messages, lastReadMessage = 40))
101+
}
102+
103+
private fun uiMessage(id: Int): ChatMessageUi =
104+
ChatMessageUi(
105+
id = id,
106+
message = "message $id",
107+
renderMarkdown = false,
108+
actorDisplayName = "Other User",
109+
isThread = false,
110+
threadTitle = "",
111+
threadReplies = 0,
112+
incoming = true,
113+
isDeleted = false,
114+
avatarUrl = null,
115+
statusIcon = MessageStatusIcon.SENT,
116+
timestamp = id.toLong(),
117+
date = LocalDate.of(2026, 8, 12),
118+
content = MessageTypeContent.RegularText
119+
)
54120
}

0 commit comments

Comments
 (0)