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.
This commit is contained in:
Benoit Marty
2025-06-04 16:14:29 +02:00
committed by GitHub
parent 7a3db35ccb
commit 525870f30d
10 changed files with 232 additions and 56 deletions

View File

@@ -0,0 +1,37 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.features.messages.impl.timeline
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.timeline.ReceiptType
import kotlinx.coroutines.launch
import timber.log.Timber
import javax.inject.Inject
interface MarkAsFullyRead {
operator fun invoke(roomId: RoomId)
}
@ContributesBinding(SessionScope::class)
class DefaultMarkAsFullyRead @Inject constructor(
private val matrixClient: MatrixClient,
) : MarkAsFullyRead {
override fun invoke(roomId: RoomId) {
matrixClient.sessionCoroutineScope.launch {
matrixClient.getRoom(roomId)?.use { room ->
room.markAsRead(receiptType = ReceiptType.FULLY_READ)
.onFailure {
Timber.e("Failed to mark room $roomId as fully read", it)
}
}
}
}
}

View File

@@ -8,6 +8,7 @@
package io.element.android.features.messages.impl.timeline
import androidx.compose.runtime.Composable
import androidx.compose.runtime.DisposableEffect
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.collectAsState
@@ -76,6 +77,7 @@ class TimelinePresenter @AssistedInject constructor(
private val resolveVerifiedUserSendFailurePresenter: Presenter<ResolveVerifiedUserSendFailureState>,
private val typingNotificationPresenter: Presenter<TypingNotificationState>,
private val roomCallStatePresenter: Presenter<RoomCallState>,
private val markAsFullyRead: MarkAsFullyRead,
) : Presenter<TimelineState> {
@AssistedFactory
interface Factory {
@@ -177,6 +179,12 @@ class TimelinePresenter @AssistedInject constructor(
}
}
DisposableEffect(Unit) {
onDispose {
markAsFullyRead(room.roomId)
}
}
LaunchedEffect(Unit) {
timelineItemsFactory.timelineItems
.onEach { newTimelineItems ->

View File

@@ -139,10 +139,10 @@ class MessagesPresenterTest {
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
markAsReadResult = { lambdaError() }
),
typingNoticeResult = { Result.success(Unit) },
)
assertThat(room.baseRoom.markAsReadCalls).isEmpty()
val presenter = createMessagesPresenter(joinedRoom = room)
presenter.testWithLifecycleOwner {
runCurrent()
@@ -848,12 +848,12 @@ class MessagesPresenterTest {
fun `present - permission to redact other`() = runTest {
val joinedRoom = FakeJoinedRoom(
baseRoom = FakeBaseRoom(
canRedactOtherResult = { Result.success(true) },
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(false) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
canRedactOtherResult = { Result.success(true) },
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(false) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
typingNoticeResult = { Result.success(Unit) },
)
val presenter = createMessagesPresenter(joinedRoom = joinedRoom)
@@ -897,12 +897,12 @@ class MessagesPresenterTest {
val timeline = FakeTimeline()
val room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
liveTimeline = timeline,
typingNoticeResult = { Result.success(Unit) },
)
@@ -937,12 +937,12 @@ class MessagesPresenterTest {
val analyticsService = FakeAnalyticsService()
val room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
liveTimeline = timeline,
typingNoticeResult = { Result.success(Unit) },
)
@@ -1096,12 +1096,12 @@ class MessagesPresenterTest {
}
val room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
),
liveTimeline = timeline,
typingNoticeResult = { Result.success(Unit) },
)
@@ -1134,16 +1134,16 @@ class MessagesPresenterTest {
fun `present - when room is encrypted and a DM, the DM user's identity state is fetched onResume`() = runTest {
val room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(
sessionId = A_SESSION_ID,
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
initialRoomInfo = aRoomInfo(isDirect = true, isEncrypted = true)
).apply {
givenRoomMembersState(RoomMembersState.Ready(persistentListOf(aRoomMember(userId = A_SESSION_ID), aRoomMember(userId = A_USER_ID_2))))
},
sessionId = A_SESSION_ID,
canUserSendMessageResult = { _, _ -> Result.success(true) },
canRedactOwnResult = { Result.success(true) },
canRedactOtherResult = { Result.success(true) },
canUserJoinCallResult = { Result.success(true) },
canUserPinUnpinResult = { Result.success(true) },
initialRoomInfo = aRoomInfo(isDirect = true, isEncrypted = true)
).apply {
givenRoomMembersState(RoomMembersState.Ready(persistentListOf(aRoomMember(userId = A_SESSION_ID), aRoomMember(userId = A_USER_ID_2))))
},
typingNoticeResult = { Result.success(Unit) },
)
val encryptionService = FakeEncryptionService(getUserIdentityResult = { Result.success(IdentityState.Verified) })

View File

@@ -0,0 +1,55 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
@file:OptIn(ExperimentalCoroutinesApi::class)
package io.element.android.features.messages.impl.timeline
import io.element.android.libraries.matrix.api.timeline.ReceiptType
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.FakeMatrixClient
import io.element.android.libraries.matrix.test.room.FakeBaseRoom
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest
import org.junit.Test
class DefaultMarkAsFullyReadTest {
@Test
fun `When room is not found, then no exception is thrown`() = runTest {
val markAsFullyRead = DefaultMarkAsFullyRead(
FakeMatrixClient(
sessionCoroutineScope = backgroundScope,
).apply {
givenGetRoomResult(A_ROOM_ID, null)
}
)
markAsFullyRead.invoke(A_ROOM_ID)
runCurrent()
}
@Test
fun `When room is found, the expected method is invoked`() = runTest {
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
val baseRoom = FakeBaseRoom(
markAsReadResult = markAsReadResult
)
val markAsFullyRead = DefaultMarkAsFullyRead(
FakeMatrixClient(
sessionCoroutineScope = backgroundScope,
).apply {
givenGetRoomResult(A_ROOM_ID, baseRoom)
}
)
markAsFullyRead.invoke(A_ROOM_ID)
runCurrent()
markAsReadResult.assertions().isCalledOnce().with(value(ReceiptType.FULLY_READ))
baseRoom.assertDestroyed()
}
}

View File

@@ -0,0 +1,19 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.features.messages.impl.timeline
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.tests.testutils.lambda.lambdaError
class FakeMarkAsFullyRead(
private val invokeResult: (RoomId) -> Unit = { lambdaError() }
) : MarkAsFullyRead {
override fun invoke(roomId: RoomId) {
invokeResult(roomId)
}
}

View File

@@ -29,6 +29,7 @@ import io.element.android.features.poll.test.actions.FakeEndPollAction
import io.element.android.features.poll.test.actions.FakeSendPollResponseAction
import io.element.android.features.roomcall.api.aStandByCallState
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.UniqueId
import io.element.android.libraries.matrix.api.room.RoomMembersState
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
@@ -40,6 +41,7 @@ import io.element.android.libraries.matrix.api.timeline.item.event.Receipt
import io.element.android.libraries.matrix.api.timeline.item.virtual.VirtualTimelineItem
import io.element.android.libraries.matrix.test.AN_EVENT_ID
import io.element.android.libraries.matrix.test.AN_EVENT_ID_2
import io.element.android.libraries.matrix.test.A_ROOM_ID
import io.element.android.libraries.matrix.test.A_UNIQUE_ID
import io.element.android.libraries.matrix.test.A_UNIQUE_ID_2
import io.element.android.libraries.matrix.test.A_USER_ID
@@ -58,6 +60,7 @@ import io.element.android.tests.testutils.lambda.any
import io.element.android.tests.testutils.lambda.assert
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
import io.element.android.tests.testutils.test
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -76,7 +79,9 @@ import java.util.Date
import kotlin.time.Duration
import kotlin.time.Duration.Companion.seconds
@OptIn(ExperimentalCoroutinesApi::class) class TimelinePresenterTest {
@Suppress("LargeClass")
@OptIn(ExperimentalCoroutinesApi::class)
class TimelinePresenterTest {
@get:Rule
val warmUpRule = WarmUpRule()
@@ -119,9 +124,26 @@ import kotlin.time.Duration.Companion.seconds
}
}
@OptIn(ExperimentalCoroutinesApi::class)
@Test
fun `present - on scroll finished mark a room as read if the first visible index is 0`() = runTest(StandardTestDispatcher()) {
fun `present - on scroll finished mark a room as read if the first visible index is 0 - read private`() {
`present - on scroll finished mark a room as read if the first visible index is 0`(
isSendPublicReadReceiptsEnabled = false,
expectedReceiptType = ReceiptType.READ_PRIVATE,
)
}
@Test
fun `present - on scroll finished mark a room as read if the first visible index is 0 - read`() {
`present - on scroll finished mark a room as read if the first visible index is 0`(
isSendPublicReadReceiptsEnabled = true,
expectedReceiptType = ReceiptType.READ,
)
}
private fun `present - on scroll finished mark a room as read if the first visible index is 0`(
isSendPublicReadReceiptsEnabled: Boolean,
expectedReceiptType: ReceiptType,
) = runTest(StandardTestDispatcher()) {
val timeline = FakeTimeline(
timelineItems = flowOf(
listOf(
@@ -129,11 +151,15 @@ import kotlin.time.Duration.Companion.seconds
)
)
)
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
val room = FakeJoinedRoom(
liveTimeline = timeline,
baseRoom = FakeBaseRoom(canUserSendMessageResult = { _, _ -> Result.success(true) })
baseRoom = FakeBaseRoom(
canUserSendMessageResult = { _, _ -> Result.success(true) },
markAsReadResult = markAsReadResult,
)
)
val sessionPreferencesStore = InMemorySessionPreferencesStore(isSendPublicReadReceiptsEnabled = false)
val sessionPreferencesStore = InMemorySessionPreferencesStore(isSendPublicReadReceiptsEnabled = isSendPublicReadReceiptsEnabled)
val presenter = createTimelinePresenter(
timeline = timeline,
room = room,
@@ -145,11 +171,32 @@ import kotlin.time.Duration.Companion.seconds
val initialState = awaitFirstItem()
initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
runCurrent()
assertThat(room.baseRoom.markAsReadCalls).isNotEmpty()
assert(markAsReadResult)
.isCalledOnce()
.with(value(expectedReceiptType))
cancelAndIgnoreRemainingEvents()
}
}
@Test
fun `present - once presenter is disposed, room is marked as fully read`() = runTest {
val invokeResult = lambdaRecorder<RoomId, Unit> { }
val presenter = createTimelinePresenter(
room = FakeJoinedRoom(
baseRoom = FakeBaseRoom(
canUserSendMessageResult = { _, _ -> Result.success(true) },
)
),
markAsFullyRead = FakeMarkAsFullyRead(
invokeResult = invokeResult,
)
)
presenter.test {
awaitFirstItem()
}
invokeResult.assertions().isCalledOnce().with(value(A_ROOM_ID))
}
@Test
fun `present - on scroll finished send read receipt if an event is before the index`() = runTest {
val sendReadReceiptsLambda = lambdaRecorder { _: EventId, _: ReceiptType ->
@@ -674,6 +721,7 @@ import kotlin.time.Duration.Companion.seconds
sendPollResponseAction: SendPollResponseAction = FakeSendPollResponseAction(),
sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(),
timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
markAsFullyRead: MarkAsFullyRead = FakeMarkAsFullyRead(),
): TimelinePresenter {
return TimelinePresenter(
timelineItemsFactoryCreator = aTimelineItemsFactoryCreator(),
@@ -690,6 +738,7 @@ import kotlin.time.Duration.Companion.seconds
resolveVerifiedUserSendFailurePresenter = { aResolveVerifiedUserSendFailureState() },
typingNotificationPresenter = { aTypingNotificationState() },
roomCallStatePresenter = { aStandByCallState() },
markAsFullyRead = markAsFullyRead,
)
}
}

View File

@@ -504,9 +504,18 @@ class RoomListPresenterTest {
@Test
fun `present - check that the room is marked as read with correct RR and as unread`() = runTest {
val room = FakeBaseRoom()
val room2 = FakeBaseRoom(roomId = A_ROOM_ID_2)
val room3 = FakeBaseRoom(roomId = A_ROOM_ID_3)
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
val markAsReadResult3 = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
val room = FakeBaseRoom(
markAsReadResult = markAsReadResult,
)
val room2 = FakeBaseRoom(
roomId = A_ROOM_ID_2,
)
val room3 = FakeBaseRoom(
roomId = A_ROOM_ID_3,
markAsReadResult = markAsReadResult3,
)
val allRooms = setOf(room, room2, room3)
val sessionPreferencesStore = InMemorySessionPreferencesStore()
val matrixClient = FakeMatrixClient().apply {
@@ -530,21 +539,19 @@ class RoomListPresenterTest {
}.test {
val initialState = awaitItem()
allRooms.forEach {
assertThat(it.markAsReadCalls).isEmpty()
assertThat(it.setUnreadFlagCalls).isEmpty()
}
initialState.eventSink.invoke(RoomListEvents.MarkAsRead(A_ROOM_ID))
assertThat(room.markAsReadCalls).isEqualTo(listOf(ReceiptType.READ))
markAsReadResult.assertions().isCalledOnce().with(value(ReceiptType.READ))
assertThat(room.setUnreadFlagCalls).isEqualTo(listOf(false))
clearMessagesForRoomLambda.assertions().isCalledOnce()
.with(value(A_SESSION_ID), value(A_ROOM_ID))
initialState.eventSink.invoke(RoomListEvents.MarkAsUnread(A_ROOM_ID_2))
assertThat(room2.markAsReadCalls).isEmpty()
assertThat(room2.setUnreadFlagCalls).isEqualTo(listOf(true))
// Test again with private read receipts
sessionPreferencesStore.setSendPublicReadReceipts(false)
initialState.eventSink.invoke(RoomListEvents.MarkAsRead(A_ROOM_ID_3))
assertThat(room3.markAsReadCalls).isEqualTo(listOf(ReceiptType.READ_PRIVATE))
markAsReadResult3.assertions().isCalledOnce().with(value(ReceiptType.READ_PRIVATE))
assertThat(room3.setUnreadFlagCalls).isEqualTo(listOf(false))
clearMessagesForRoomLambda.assertions().isCalledExactly(2)
.withSequence(

View File

@@ -54,6 +54,7 @@ class FakeBaseRoom(
private val canUserJoinCallResult: (UserId) -> Result<Boolean> = { lambdaError() },
private val canUserPinUnpinResult: (UserId) -> Result<Boolean> = { lambdaError() },
private val setIsFavoriteResult: (Boolean) -> Result<Unit> = { lambdaError() },
private val markAsReadResult: (ReceiptType) -> Result<Unit> = { Result.success(Unit) },
private val powerLevelsResult: () -> Result<RoomPowerLevels> = { lambdaError() },
private val leaveRoomLambda: () -> Result<Unit> = { lambdaError() },
private val updateMembersResult: () -> Unit = { lambdaError() },
@@ -183,11 +184,8 @@ class FakeBaseRoom(
return setIsFavoriteResult(isFavorite)
}
val markAsReadCalls = mutableListOf<ReceiptType>()
override suspend fun markAsRead(receiptType: ReceiptType): Result<Unit> {
markAsReadCalls.add(receiptType)
return Result.success(Unit)
return markAsReadResult(receiptType)
}
var setUnreadFlagCalls = mutableListOf<Boolean>()

View File

@@ -8,7 +8,6 @@
package io.element.android.libraries.push.impl.notifications
import android.content.Intent
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.api.MatrixClient
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.core.RoomId
@@ -224,7 +223,12 @@ class NotificationBroadcastReceiverHandlerTest {
getLambda = getLambda
)
val clearMessagesForRoomLambda = lambdaRecorder<SessionId, RoomId, Unit> { _, _ -> }
val joinedRoom = FakeJoinedRoom()
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
val joinedRoom = FakeJoinedRoom(
baseRoom = FakeBaseRoom(
markAsReadResult = markAsReadResult,
),
)
val fakeNotificationCleaner = FakeNotificationCleaner(
clearMessagesForRoomLambda = clearMessagesForRoomLambda,
)
@@ -243,7 +247,7 @@ class NotificationBroadcastReceiverHandlerTest {
clearMessagesForRoomLambda.assertions()
.isCalledOnce()
.with(value(A_SESSION_ID), value(A_ROOM_ID))
assertThat(joinedRoom.baseRoom.markAsReadCalls).isEqualTo(listOf(expectedReceiptType))
markAsReadResult.assertions().isCalledOnce().with(value(expectedReceiptType))
}
@Test

View File

@@ -48,7 +48,6 @@ class DefaultEncoder @Inject constructor(
override fun release() {
encoder?.release()
?: Timber.w("Can't release encoder that is not initialized")
encoder = null
}
}