Skip to content

Commit 58d503f

Browse files
bmartybmarty
authored
Mark room as fully read when user goes back to the room list. (#2687)
* Remove not helping warning. * Add and improve tests * Send the `m.fully_read` read marker when the user navigates back to the room list, to mark the room as read.
1 parent 390e5cf commit 58d503f

File tree

10 files changed

+232
-56
lines changed

10 files changed

+232
-56
lines changed

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/MarkAsFullyRead.kt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
package io.element.android.features.messages.impl.timeline
9+
10+
import com.squareup.anvil.annotations.ContributesBinding
11+
import io.element.android.libraries.di.SessionScope
12+
import io.element.android.libraries.matrix.api.MatrixClient
13+
import io.element.android.libraries.matrix.api.core.RoomId
14+
import io.element.android.libraries.matrix.api.timeline.ReceiptType
15+
import kotlinx.coroutines.launch
16+
import timber.log.Timber
17+
import javax.inject.Inject
18+
19+
interface MarkAsFullyRead {
20+
operator fun invoke(roomId: RoomId)
21+
}
22+
23+
@ContributesBinding(SessionScope::class)
24+
class DefaultMarkAsFullyRead @Inject constructor(
25+
private val matrixClient: MatrixClient,
26+
) : MarkAsFullyRead {
27+
override fun invoke(roomId: RoomId) {
28+
matrixClient.sessionCoroutineScope.launch {
29+
matrixClient.getRoom(roomId)?.use { room ->
30+
room.markAsRead(receiptType = ReceiptType.FULLY_READ)
31+
.onFailure {
32+
Timber.e("Failed to mark room $roomId as fully read", it)
33+
}
34+
}
35+
}
36+
}
37+
}

‎features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package io.element.android.features.messages.impl.timeline
99

1010
import androidx.compose.runtime.Composable
11+
import androidx.compose.runtime.DisposableEffect
1112
import androidx.compose.runtime.LaunchedEffect
1213
import androidx.compose.runtime.MutableState
1314
import androidx.compose.runtime.collectAsState
@@ -76,6 +77,7 @@ class TimelinePresenter @AssistedInject constructor(
7677
private val resolveVerifiedUserSendFailurePresenter: Presenter<ResolveVerifiedUserSendFailureState>,
7778
private val typingNotificationPresenter: Presenter<TypingNotificationState>,
7879
private val roomCallStatePresenter: Presenter<RoomCallState>,
80+
private val markAsFullyRead: MarkAsFullyRead,
7981
) : Presenter<TimelineState> {
8082
@AssistedFactory
8183
interface Factory {
@@ -177,6 +179,12 @@ class TimelinePresenter @AssistedInject constructor(
177179
}
178180
}
179181

182+
DisposableEffect(Unit) {
183+
onDispose {
184+
markAsFullyRead(room.roomId)
185+
}
186+
}
187+
180188
LaunchedEffect(Unit) {
181189
timelineItemsFactory.timelineItems
182190
.onEach { newTimelineItems ->

‎features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/MessagesPresenterTest.kt‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,10 @@ class MessagesPresenterTest {
139139
canRedactOtherResult = { Result.success(true) },
140140
canUserJoinCallResult = { Result.success(true) },
141141
canUserPinUnpinResult = { Result.success(true) },
142+
markAsReadResult = { lambdaError() }
142143
),
143144
typingNoticeResult = { Result.success(Unit) },
144145
)
145-
assertThat(room.baseRoom.markAsReadCalls).isEmpty()
146146
val presenter = createMessagesPresenter(joinedRoom = room)
147147
presenter.testWithLifecycleOwner {
148148
runCurrent()
@@ -848,12 +848,12 @@ class MessagesPresenterTest {
848848
fun `present - permission to redact other`() = runTest {
849849
val joinedRoom = FakeJoinedRoom(
850850
baseRoom = FakeBaseRoom(
851-
canRedactOtherResult = { Result.success(true) },
852-
canUserSendMessageResult = { _, _ -> Result.success(true) },
853-
canRedactOwnResult = { Result.success(false) },
854-
canUserJoinCallResult = { Result.success(true) },
855-
canUserPinUnpinResult = { Result.success(true) },
856-
),
851+
canRedactOtherResult = { Result.success(true) },
852+
canUserSendMessageResult = { _, _ -> Result.success(true) },
853+
canRedactOwnResult = { Result.success(false) },
854+
canUserJoinCallResult = { Result.success(true) },
855+
canUserPinUnpinResult = { Result.success(true) },
856+
),
857857
typingNoticeResult = { Result.success(Unit) },
858858
)
859859
val presenter = createMessagesPresenter(joinedRoom = joinedRoom)
@@ -897,12 +897,12 @@ class MessagesPresenterTest {
897897
val timeline = FakeTimeline()
898898
val room = FakeJoinedRoom(
899899
baseRoom = FakeBaseRoom(
900-
canUserSendMessageResult = { _, _ -> Result.success(true) },
901-
canRedactOwnResult = { Result.success(true) },
902-
canRedactOtherResult = { Result.success(true) },
903-
canUserJoinCallResult = { Result.success(true) },
904-
canUserPinUnpinResult = { Result.success(true) },
905-
),
900+
canUserSendMessageResult = { _, _ -> Result.success(true) },
901+
canRedactOwnResult = { Result.success(true) },
902+
canRedactOtherResult = { Result.success(true) },
903+
canUserJoinCallResult = { Result.success(true) },
904+
canUserPinUnpinResult = { Result.success(true) },
905+
),
906906
liveTimeline = timeline,
907907
typingNoticeResult = { Result.success(Unit) },
908908
)
@@ -937,12 +937,12 @@ class MessagesPresenterTest {
937937
val analyticsService = FakeAnalyticsService()
938938
val room = FakeJoinedRoom(
939939
baseRoom = FakeBaseRoom(
940-
canUserSendMessageResult = { _, _ -> Result.success(true) },
941-
canRedactOwnResult = { Result.success(true) },
942-
canRedactOtherResult = { Result.success(true) },
943-
canUserJoinCallResult = { Result.success(true) },
944-
canUserPinUnpinResult = { Result.success(true) },
945-
),
940+
canUserSendMessageResult = { _, _ -> Result.success(true) },
941+
canRedactOwnResult = { Result.success(true) },
942+
canRedactOtherResult = { Result.success(true) },
943+
canUserJoinCallResult = { Result.success(true) },
944+
canUserPinUnpinResult = { Result.success(true) },
945+
),
946946
liveTimeline = timeline,
947947
typingNoticeResult = { Result.success(Unit) },
948948
)
@@ -1096,12 +1096,12 @@ class MessagesPresenterTest {
10961096
}
10971097
val room = FakeJoinedRoom(
10981098
baseRoom = FakeBaseRoom(
1099-
canUserSendMessageResult = { _, _ -> Result.success(true) },
1100-
canRedactOwnResult = { Result.success(true) },
1101-
canRedactOtherResult = { Result.success(true) },
1102-
canUserJoinCallResult = { Result.success(true) },
1103-
canUserPinUnpinResult = { Result.success(true) },
1104-
),
1099+
canUserSendMessageResult = { _, _ -> Result.success(true) },
1100+
canRedactOwnResult = { Result.success(true) },
1101+
canRedactOtherResult = { Result.success(true) },
1102+
canUserJoinCallResult = { Result.success(true) },
1103+
canUserPinUnpinResult = { Result.success(true) },
1104+
),
11051105
liveTimeline = timeline,
11061106
typingNoticeResult = { Result.success(Unit) },
11071107
)
@@ -1134,16 +1134,16 @@ class MessagesPresenterTest {
11341134
fun `present - when room is encrypted and a DM, the DM user's identity state is fetched onResume`() = runTest {
11351135
val room = FakeJoinedRoom(
11361136
baseRoom = FakeBaseRoom(
1137-
sessionId = A_SESSION_ID,
1138-
canUserSendMessageResult = { _, _ -> Result.success(true) },
1139-
canRedactOwnResult = { Result.success(true) },
1140-
canRedactOtherResult = { Result.success(true) },
1141-
canUserJoinCallResult = { Result.success(true) },
1142-
canUserPinUnpinResult = { Result.success(true) },
1143-
initialRoomInfo = aRoomInfo(isDirect = true, isEncrypted = true)
1144-
).apply {
1145-
givenRoomMembersState(RoomMembersState.Ready(persistentListOf(aRoomMember(userId = A_SESSION_ID), aRoomMember(userId = A_USER_ID_2))))
1146-
},
1137+
sessionId = A_SESSION_ID,
1138+
canUserSendMessageResult = { _, _ -> Result.success(true) },
1139+
canRedactOwnResult = { Result.success(true) },
1140+
canRedactOtherResult = { Result.success(true) },
1141+
canUserJoinCallResult = { Result.success(true) },
1142+
canUserPinUnpinResult = { Result.success(true) },
1143+
initialRoomInfo = aRoomInfo(isDirect = true, isEncrypted = true)
1144+
).apply {
1145+
givenRoomMembersState(RoomMembersState.Ready(persistentListOf(aRoomMember(userId = A_SESSION_ID), aRoomMember(userId = A_USER_ID_2))))
1146+
},
11471147
typingNoticeResult = { Result.success(Unit) },
11481148
)
11491149
val encryptionService = FakeEncryptionService(getUserIdentityResult = { Result.success(IdentityState.Verified) })

‎features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/DefaultMarkAsFullyReadTest.kt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
@file:OptIn(ExperimentalCoroutinesApi::class)
9+
10+
package io.element.android.features.messages.impl.timeline
11+
12+
import io.element.android.libraries.matrix.api.timeline.ReceiptType
13+
import io.element.android.libraries.matrix.test.A_ROOM_ID
14+
import io.element.android.libraries.matrix.test.FakeMatrixClient
15+
import io.element.android.libraries.matrix.test.room.FakeBaseRoom
16+
import io.element.android.tests.testutils.lambda.lambdaRecorder
17+
import io.element.android.tests.testutils.lambda.value
18+
import kotlinx.coroutines.ExperimentalCoroutinesApi
19+
import kotlinx.coroutines.test.runCurrent
20+
import kotlinx.coroutines.test.runTest
21+
import org.junit.Test
22+
23+
class DefaultMarkAsFullyReadTest {
24+
@Test
25+
fun `When room is not found, then no exception is thrown`() = runTest {
26+
val markAsFullyRead = DefaultMarkAsFullyRead(
27+
FakeMatrixClient(
28+
sessionCoroutineScope = backgroundScope,
29+
).apply {
30+
givenGetRoomResult(A_ROOM_ID, null)
31+
}
32+
)
33+
markAsFullyRead.invoke(A_ROOM_ID)
34+
runCurrent()
35+
}
36+
37+
@Test
38+
fun `When room is found, the expected method is invoked`() = runTest {
39+
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
40+
val baseRoom = FakeBaseRoom(
41+
markAsReadResult = markAsReadResult
42+
)
43+
val markAsFullyRead = DefaultMarkAsFullyRead(
44+
FakeMatrixClient(
45+
sessionCoroutineScope = backgroundScope,
46+
).apply {
47+
givenGetRoomResult(A_ROOM_ID, baseRoom)
48+
}
49+
)
50+
markAsFullyRead.invoke(A_ROOM_ID)
51+
runCurrent()
52+
markAsReadResult.assertions().isCalledOnce().with(value(ReceiptType.FULLY_READ))
53+
baseRoom.assertDestroyed()
54+
}
55+
}

‎features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/FakeMarkAsFullyRead.kt‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/*
2+
* Copyright 2025 New Vector Ltd.
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
5+
* Please see LICENSE files in the repository root for full details.
6+
*/
7+
8+
package io.element.android.features.messages.impl.timeline
9+
10+
import io.element.android.libraries.matrix.api.core.RoomId
11+
import io.element.android.tests.testutils.lambda.lambdaError
12+
13+
class FakeMarkAsFullyRead(
14+
private val invokeResult: (RoomId) -> Unit = { lambdaError() }
15+
) : MarkAsFullyRead {
16+
override fun invoke(roomId: RoomId) {
17+
invokeResult(roomId)
18+
}
19+
}

‎features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenterTest.kt‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import io.element.android.features.poll.test.actions.FakeEndPollAction
2929
import io.element.android.features.poll.test.actions.FakeSendPollResponseAction
3030
import io.element.android.features.roomcall.api.aStandByCallState
3131
import io.element.android.libraries.matrix.api.core.EventId
32+
import io.element.android.libraries.matrix.api.core.RoomId
3233
import io.element.android.libraries.matrix.api.core.UniqueId
3334
import io.element.android.libraries.matrix.api.room.RoomMembersState
3435
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
@@ -40,6 +41,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.Receipt
4041
import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem
4142
import io.element.android.libraries.matrix.test.AN_EVENT_ID
4243
import io.element.android.libraries.matrix.test.AN_EVENT_ID_2
44+
import io.element.android.libraries.matrix.test.A_ROOM_ID
4345
import io.element.android.libraries.matrix.test.A_UNIQUE_ID
4446
import io.element.android.libraries.matrix.test.A_UNIQUE_ID_2
4547
import io.element.android.libraries.matrix.test.A_USER_ID
@@ -58,6 +60,7 @@ import io.element.android.tests.testutils.lambda.any
5860
import io.element.android.tests.testutils.lambda.assert
5961
import io.element.android.tests.testutils.lambda.lambdaRecorder
6062
import io.element.android.tests.testutils.lambda.value
63+
import io.element.android.tests.testutils.test
6164
import io.element.android.tests.testutils.testCoroutineDispatchers
6265
import kotlinx.collections.immutable.persistentListOf
6366
import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -76,7 +79,9 @@ import java.util.Date
7679
import kotlin.time.Duration
7780
import kotlin.time.Duration.Companion.seconds
7881

79-
@OptIn(ExperimentalCoroutinesApi::class) class TimelinePresenterTest {
82+
@Suppress("LargeClass")
83+
@OptIn(ExperimentalCoroutinesApi::class)
84+
class TimelinePresenterTest {
8085
@get:Rule
8186
val warmUpRule = WarmUpRule()
8287

@@ -119,21 +124,42 @@ import kotlin.time.Duration.Companion.seconds
119124
}
120125
}
121126

122-
@OptIn(ExperimentalCoroutinesApi::class)
123127
@Test
124-
fun `present - on scroll finished mark a room as read if the first visible index is 0`() = runTest(StandardTestDispatcher()) {
128+
fun `present - on scroll finished mark a room as read if the first visible index is 0 - read private`() {
129+
`present - on scroll finished mark a room as read if the first visible index is 0`(
130+
isSendPublicReadReceiptsEnabled = false,
131+
expectedReceiptType = ReceiptType.READ_PRIVATE,
132+
)
133+
}
134+
135+
@Test
136+
fun `present - on scroll finished mark a room as read if the first visible index is 0 - read`() {
137+
`present - on scroll finished mark a room as read if the first visible index is 0`(
138+
isSendPublicReadReceiptsEnabled = true,
139+
expectedReceiptType = ReceiptType.READ,
140+
)
141+
}
142+
143+
private fun `present - on scroll finished mark a room as read if the first visible index is 0`(
144+
isSendPublicReadReceiptsEnabled: Boolean,
145+
expectedReceiptType: ReceiptType,
146+
) = runTest(StandardTestDispatcher()) {
125147
val timeline = FakeTimeline(
126148
timelineItems = flowOf(
127149
listOf(
128150
MatrixTimelineItem.Event(A_UNIQUE_ID, anEventTimelineItem())
129151
)
130152
)
131153
)
154+
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
132155
val room = FakeJoinedRoom(
133156
liveTimeline = timeline,
134-
baseRoom = FakeBaseRoom(canUserSendMessageResult = { _, _ -> Result.success(true) })
157+
baseRoom = FakeBaseRoom(
158+
canUserSendMessageResult = { _, _ -> Result.success(true) },
159+
markAsReadResult = markAsReadResult,
160+
)
135161
)
136-
val sessionPreferencesStore = InMemorySessionPreferencesStore(isSendPublicReadReceiptsEnabled = false)
162+
val sessionPreferencesStore = InMemorySessionPreferencesStore(isSendPublicReadReceiptsEnabled = isSendPublicReadReceiptsEnabled)
137163
val presenter = createTimelinePresenter(
138164
timeline = timeline,
139165
room = room,
@@ -145,11 +171,32 @@ import kotlin.time.Duration.Companion.seconds
145171
val initialState = awaitFirstItem()
146172
initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
147173
runCurrent()
148-
assertThat(room.baseRoom.markAsReadCalls).isNotEmpty()
174+
assert(markAsReadResult)
175+
.isCalledOnce()
176+
.with(value(expectedReceiptType))
149177
cancelAndIgnoreRemainingEvents()
150178
}
151179
}
152180

181+
@Test
182+
fun `present - once presenter is disposed, room is marked as fully read`() = runTest {
183+
val invokeResult = lambdaRecorder<RoomId, Unit> { }
184+
val presenter = createTimelinePresenter(
185+
room = FakeJoinedRoom(
186+
baseRoom = FakeBaseRoom(
187+
canUserSendMessageResult = { _, _ -> Result.success(true) },
188+
)
189+
),
190+
markAsFullyRead = FakeMarkAsFullyRead(
191+
invokeResult = invokeResult,
192+
)
193+
)
194+
presenter.test {
195+
awaitFirstItem()
196+
}
197+
invokeResult.assertions().isCalledOnce().with(value(A_ROOM_ID))
198+
}
199+
153200
@Test
154201
fun `present - on scroll finished send read receipt if an event is before the index`() = runTest {
155202
val sendReadReceiptsLambda = lambdaRecorder { _: EventId, _: ReceiptType ->
@@ -674,6 +721,7 @@ import kotlin.time.Duration.Companion.seconds
674721
sendPollResponseAction: SendPollResponseAction = FakeSendPollResponseAction(),
675722
sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(),
676723
timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
724+
markAsFullyRead: MarkAsFullyRead = FakeMarkAsFullyRead(),
677725
): TimelinePresenter {
678726
return TimelinePresenter(
679727
timelineItemsFactoryCreator = aTimelineItemsFactoryCreator(),
@@ -690,6 +738,7 @@ import kotlin.time.Duration.Companion.seconds
690738
resolveVerifiedUserSendFailurePresenter = { aResolveVerifiedUserSendFailureState() },
691739
typingNotificationPresenter = { aTypingNotificationState() },
692740
roomCallStatePresenter = { aStandByCallState() },
741+
markAsFullyRead = markAsFullyRead,
693742
)
694743
}
695744
}

0 commit comments

Comments
 (0)