Fix marking a room as read re-instantiates its timeline (#5628)
* Add `Timeline.markAsRead` to avoid reinstantiating the timeline using `Room.markAsRead` * Mark as read when exiting the room screen, destroy the timeline when fully closed * Ensure `MarkAsFullyReadAndExit` event can only be processed once * Fix `DelayedVisibility` not being displayed in previews
This commit is contained in:
committed by
GitHub
parent
988815217a
commit
1f5f6896c6
@@ -26,6 +26,7 @@ import io.element.android.libraries.designsystem.theme.components.CircularProgre
|
||||
import io.element.android.libraries.designsystem.theme.components.Scaffold
|
||||
import io.element.android.libraries.designsystem.theme.components.Text
|
||||
import io.element.android.libraries.designsystem.theme.components.TopAppBar
|
||||
import io.element.android.libraries.designsystem.utils.DelayedVisibility
|
||||
import io.element.android.libraries.matrix.ui.room.LoadingRoomState
|
||||
import io.element.android.libraries.matrix.ui.room.LoadingRoomStateProvider
|
||||
import io.element.android.libraries.ui.strings.CommonStrings
|
||||
@@ -57,7 +58,9 @@ fun LoadingRoomNodeView(
|
||||
style = ElementTheme.typography.fontBodyMdRegular,
|
||||
)
|
||||
} else {
|
||||
CircularProgressIndicator()
|
||||
DelayedVisibility {
|
||||
CircularProgressIndicator()
|
||||
}
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -37,7 +37,10 @@ import kotlinx.coroutines.test.TestScope
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
class JoinedRoomLoadedFlowNodeTest {
|
||||
@get:Rule
|
||||
val instantTaskExecutorRule = InstantTaskExecutorRule()
|
||||
@@ -140,6 +143,7 @@ class JoinedRoomLoadedFlowNodeTest {
|
||||
spaceEntryPoint: SpaceEntryPoint = FakeSpaceEntryPoint(),
|
||||
forwardEntryPoint: ForwardEntryPoint = FakeForwardEntryPoint(),
|
||||
activeRoomsHolder: ActiveRoomsHolder = ActiveRoomsHolder(),
|
||||
matrixClient: FakeMatrixClient = FakeMatrixClient(),
|
||||
) = JoinedRoomLoadedFlowNode(
|
||||
buildContext = BuildContext.root(savedStateMap = null),
|
||||
plugins = plugins,
|
||||
@@ -148,9 +152,9 @@ class JoinedRoomLoadedFlowNodeTest {
|
||||
spaceEntryPoint = spaceEntryPoint,
|
||||
forwardEntryPoint = forwardEntryPoint,
|
||||
appNavigationStateService = FakeAppNavigationStateService(),
|
||||
sessionCoroutineScope = this,
|
||||
sessionCoroutineScope = backgroundScope,
|
||||
roomGraphFactory = FakeRoomGraphFactory(),
|
||||
matrixClient = FakeMatrixClient(),
|
||||
matrixClient = matrixClient,
|
||||
activeRoomsHolder = activeRoomsHolder,
|
||||
)
|
||||
|
||||
|
||||
@@ -18,6 +18,7 @@ sealed interface MessagesEvents {
|
||||
data class InviteDialogDismissed(val action: InviteDialogAction) : MessagesEvents
|
||||
data class OnUserClicked(val user: MatrixUser) : MessagesEvents
|
||||
data object Dismiss : MessagesEvents
|
||||
data object MarkAsFullyReadAndExit : MessagesEvents
|
||||
}
|
||||
|
||||
enum class InviteDialogAction {
|
||||
|
||||
@@ -23,4 +23,5 @@ interface MessagesNavigator {
|
||||
fun onPreviewAttachment(attachments: ImmutableList<Attachment>, inReplyToEventId: EventId?)
|
||||
fun onNavigateToRoom(roomId: RoomId, eventId: EventId?, serverNames: List<String>)
|
||||
fun onOpenThread(threadRootId: ThreadId, focusedEventId: EventId?)
|
||||
fun onNavigateUp()
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ package io.element.android.features.messages.impl
|
||||
|
||||
import android.app.Activity
|
||||
import android.content.Context
|
||||
import androidx.activity.compose.BackHandler
|
||||
import androidx.activity.compose.LocalActivity
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
@@ -263,6 +264,8 @@ class MessagesNode(
|
||||
context.toast(CommonStrings.screen_room_permalink_same_room_android)
|
||||
}
|
||||
|
||||
override fun onNavigateUp() = navigateUp()
|
||||
|
||||
@Composable
|
||||
override fun View(modifier: Modifier) {
|
||||
val activity = requireNotNull(LocalActivity.current)
|
||||
@@ -271,6 +274,11 @@ class MessagesNode(
|
||||
LocalTimelineItemPresenterFactories provides timelineItemPresenterFactories,
|
||||
) {
|
||||
val state = presenter.present()
|
||||
|
||||
BackHandler {
|
||||
state.eventSink(MessagesEvents.MarkAsFullyReadAndExit)
|
||||
}
|
||||
|
||||
OnLifecycleEvent { _, event ->
|
||||
when (event) {
|
||||
Lifecycle.Event.ON_PAUSE -> state.composerState.eventSink(MessageComposerEvents.SaveDraft)
|
||||
@@ -279,7 +287,7 @@ class MessagesNode(
|
||||
}
|
||||
MessagesView(
|
||||
state = state,
|
||||
onBackClick = this::navigateUp,
|
||||
onBackClick = { state.eventSink(MessagesEvents.MarkAsFullyReadAndExit) },
|
||||
onRoomDetailsClick = this::onRoomDetailsClick,
|
||||
onEventContentClick = { isLive, event ->
|
||||
if (isLive) {
|
||||
|
||||
@@ -36,6 +36,7 @@ import io.element.android.features.messages.impl.link.LinkState
|
||||
import io.element.android.features.messages.impl.messagecomposer.MessageComposerEvents
|
||||
import io.element.android.features.messages.impl.messagecomposer.MessageComposerState
|
||||
import io.element.android.features.messages.impl.pinned.banner.PinnedMessagesBannerState
|
||||
import io.element.android.features.messages.impl.timeline.MarkAsFullyRead
|
||||
import io.element.android.features.messages.impl.timeline.TimelineController
|
||||
import io.element.android.features.messages.impl.timeline.TimelineEvents
|
||||
import io.element.android.features.messages.impl.timeline.TimelineState
|
||||
@@ -65,6 +66,7 @@ import io.element.android.libraries.designsystem.components.avatar.AvatarSize
|
||||
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarDispatcher
|
||||
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage
|
||||
import io.element.android.libraries.designsystem.utils.snackbar.collectSnackbarMessageAsState
|
||||
import io.element.android.libraries.di.annotations.SessionCoroutineScope
|
||||
import io.element.android.libraries.featureflag.api.FeatureFlagService
|
||||
import io.element.android.libraries.featureflag.api.FeatureFlags
|
||||
import io.element.android.libraries.matrix.api.core.toThreadId
|
||||
@@ -93,6 +95,7 @@ import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withContext
|
||||
import timber.log.Timber
|
||||
import java.util.concurrent.atomic.AtomicBoolean
|
||||
|
||||
@AssistedInject
|
||||
class MessagesPresenter(
|
||||
@@ -122,6 +125,8 @@ class MessagesPresenter(
|
||||
private val encryptionService: EncryptionService,
|
||||
private val featureFlagService: FeatureFlagService,
|
||||
private val addRecentEmoji: AddRecentEmoji,
|
||||
private val markAsFullyRead: MarkAsFullyRead,
|
||||
@SessionCoroutineScope private val sessionCoroutineScope: CoroutineScope,
|
||||
) : Presenter<MessagesState> {
|
||||
@AssistedFactory
|
||||
interface Factory {
|
||||
@@ -138,10 +143,13 @@ class MessagesPresenter(
|
||||
timelineMode = timelineController.mainTimelineMode()
|
||||
)
|
||||
|
||||
private val markingAsReadAndExiting = AtomicBoolean(false)
|
||||
|
||||
@Composable
|
||||
override fun present(): MessagesState {
|
||||
htmlConverterProvider.Update()
|
||||
|
||||
val coroutineScope = rememberCoroutineScope()
|
||||
val roomInfo by room.roomInfoFlow.collectAsState()
|
||||
val localCoroutineScope = rememberCoroutineScope()
|
||||
val composerState = composerPresenter.present()
|
||||
@@ -239,6 +247,22 @@ class MessagesPresenter(
|
||||
is MessagesEvents.OnUserClicked -> {
|
||||
roomMemberModerationState.eventSink(RoomMemberModerationEvents.ShowActionsForUser(event.user))
|
||||
}
|
||||
is MessagesEvents.MarkAsFullyReadAndExit -> coroutineScope.launch {
|
||||
if (!markingAsReadAndExiting.getAndSet(true)) {
|
||||
val latestEventId = room.liveTimeline.getLatestEventId().getOrElse {
|
||||
Timber.w(it, "Failed to get latest event id to mark as fully read")
|
||||
navigator.onNavigateUp()
|
||||
return@launch
|
||||
}
|
||||
latestEventId?.let { eventId ->
|
||||
sessionCoroutineScope.launch {
|
||||
markAsFullyRead(room.roomId, eventId)
|
||||
}
|
||||
}
|
||||
navigator.onNavigateUp()
|
||||
markingAsReadAndExiting.set(false)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -229,6 +229,8 @@ class ThreadedMessagesNode(
|
||||
callbacks.forEach { it.onOpenThread(threadRootId, focusedEventId) }
|
||||
}
|
||||
|
||||
override fun onNavigateUp() = navigateUp()
|
||||
|
||||
private fun onSendLocationClick() {
|
||||
callbacks.forEach { it.onSendLocationClick() }
|
||||
}
|
||||
|
||||
@@ -8,29 +8,26 @@
|
||||
package io.element.android.features.messages.impl.timeline
|
||||
|
||||
import dev.zacsweers.metro.ContributesBinding
|
||||
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
|
||||
import io.element.android.libraries.di.SessionScope
|
||||
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
|
||||
import io.element.android.libraries.matrix.api.timeline.ReceiptType
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withContext
|
||||
import timber.log.Timber
|
||||
|
||||
interface MarkAsFullyRead {
|
||||
operator fun invoke(roomId: RoomId)
|
||||
suspend operator fun invoke(roomId: RoomId, eventId: EventId): Result<Unit>
|
||||
}
|
||||
|
||||
@ContributesBinding(SessionScope::class)
|
||||
class DefaultMarkAsFullyRead(
|
||||
private val matrixClient: MatrixClient,
|
||||
private val coroutineDispatchers: CoroutineDispatchers,
|
||||
) : 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)
|
||||
}
|
||||
}
|
||||
override suspend fun invoke(roomId: RoomId, eventId: EventId): Result<Unit> = withContext(coroutineDispatchers.io) {
|
||||
matrixClient.markRoomAsFullyRead(roomId, eventId).onFailure {
|
||||
Timber.e(it, "Failed to mark room $roomId as fully read for event $eventId")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,7 +8,6 @@
|
||||
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
|
||||
@@ -85,7 +84,6 @@ class TimelinePresenter(
|
||||
private val resolveVerifiedUserSendFailurePresenter: Presenter<ResolveVerifiedUserSendFailureState>,
|
||||
private val typingNotificationPresenter: Presenter<TypingNotificationState>,
|
||||
private val roomCallStatePresenter: Presenter<RoomCallState>,
|
||||
private val markAsFullyRead: MarkAsFullyRead,
|
||||
private val featureFlagService: FeatureFlagService,
|
||||
) : Presenter<TimelineState> {
|
||||
@AssistedFactory
|
||||
@@ -219,12 +217,6 @@ class TimelinePresenter(
|
||||
}
|
||||
}
|
||||
|
||||
DisposableEffect(Unit) {
|
||||
onDispose {
|
||||
markAsFullyRead(room.roomId)
|
||||
}
|
||||
}
|
||||
|
||||
LaunchedEffect(Unit) {
|
||||
timelineItemsFactory.timelineItems
|
||||
.onEach { newTimelineItems ->
|
||||
@@ -388,7 +380,7 @@ class TimelinePresenter(
|
||||
) = launch(dispatchers.computation) {
|
||||
// If we are at the bottom of timeline, we mark the room as read.
|
||||
if (firstVisibleIndex == 0) {
|
||||
room.markAsRead(receiptType = readReceiptType)
|
||||
room.liveTimeline.markAsRead(receiptType = readReceiptType)
|
||||
} else {
|
||||
// Get last valid EventId seen by the user, as the first index might refer to a Virtual item
|
||||
val eventId = getLastEventIdBeforeOrAt(firstVisibleIndex, timelineItems)
|
||||
|
||||
@@ -24,6 +24,7 @@ class FakeMessagesNavigator(
|
||||
private val onPreviewAttachmentLambda: (attachments: ImmutableList<Attachment>, inReplyToEventId: EventId?) -> Unit = { _, _ -> lambdaError() },
|
||||
private val onNavigateToRoomLambda: (roomId: RoomId, threadId: EventId?, serverNames: List<String>) -> Unit = { _, _, _ -> lambdaError() },
|
||||
private val onOpenThreadLambda: (threadRootId: ThreadId, focusedEventId: EventId?) -> Unit = { _, _ -> lambdaError() },
|
||||
private val onNavigateUpLambda: () -> Unit = { lambdaError() },
|
||||
) : MessagesNavigator {
|
||||
override fun onShowEventDebugInfoClick(eventId: EventId?, debugInfo: TimelineItemDebugInfo) {
|
||||
onShowEventDebugInfoClickLambda(eventId, debugInfo)
|
||||
@@ -52,4 +53,8 @@ class FakeMessagesNavigator(
|
||||
override fun onOpenThread(threadRootId: ThreadId, focusedEventId: EventId?) {
|
||||
onOpenThreadLambda(threadRootId, focusedEventId)
|
||||
}
|
||||
|
||||
override fun onNavigateUp() {
|
||||
onNavigateUpLambda()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,6 +23,8 @@ import io.element.android.features.messages.impl.messagecomposer.MessageComposer
|
||||
import io.element.android.features.messages.impl.messagecomposer.MessageComposerState
|
||||
import io.element.android.features.messages.impl.messagecomposer.aMessageComposerState
|
||||
import io.element.android.features.messages.impl.pinned.banner.aLoadedPinnedMessagesBannerState
|
||||
import io.element.android.features.messages.impl.timeline.FakeMarkAsFullyRead
|
||||
import io.element.android.features.messages.impl.timeline.MarkAsFullyRead
|
||||
import io.element.android.features.messages.impl.timeline.TimelineController
|
||||
import io.element.android.features.messages.impl.timeline.TimelineEvents
|
||||
import io.element.android.features.messages.impl.timeline.aTimelineState
|
||||
@@ -178,7 +180,11 @@ class MessagesPresenterTest {
|
||||
liveTimeline = timeline,
|
||||
typingNoticeResult = { Result.success(Unit) },
|
||||
)
|
||||
val presenter = createMessagesPresenter(joinedRoom = room, coroutineDispatchers = coroutineDispatchers)
|
||||
val presenter = createMessagesPresenter(
|
||||
timeline = timeline,
|
||||
joinedRoom = room,
|
||||
coroutineDispatchers = coroutineDispatchers
|
||||
)
|
||||
presenter.testWithLifecycleOwner {
|
||||
skipItems(1)
|
||||
val initialState = awaitItem()
|
||||
@@ -220,7 +226,11 @@ class MessagesPresenterTest {
|
||||
liveTimeline = timeline,
|
||||
typingNoticeResult = { Result.success(Unit) },
|
||||
)
|
||||
val presenter = createMessagesPresenter(joinedRoom = room, coroutineDispatchers = coroutineDispatchers)
|
||||
val presenter = createMessagesPresenter(
|
||||
timeline = timeline,
|
||||
joinedRoom = room,
|
||||
coroutineDispatchers = coroutineDispatchers
|
||||
)
|
||||
presenter.testWithLifecycleOwner {
|
||||
val initialState = awaitItem()
|
||||
initialState.eventSink(MessagesEvents.ToggleReaction("👍", AN_EVENT_ID.toEventOrTransactionId()))
|
||||
@@ -509,6 +519,7 @@ class MessagesPresenterTest {
|
||||
val redactEventLambda = lambdaRecorder { _: EventOrTransactionId, _: String? -> Result.success(Unit) }
|
||||
liveTimeline.redactEventLambda = redactEventLambda
|
||||
val presenter = createMessagesPresenter(
|
||||
timeline = liveTimeline,
|
||||
joinedRoom = joinedRoom,
|
||||
coroutineDispatchers = coroutineDispatchers,
|
||||
)
|
||||
@@ -920,6 +931,7 @@ class MessagesPresenterTest {
|
||||
typingNoticeResult = { Result.success(Unit) },
|
||||
)
|
||||
val presenter = createMessagesPresenter(
|
||||
timeline = timeline,
|
||||
joinedRoom = room,
|
||||
analyticsService = analyticsService,
|
||||
)
|
||||
@@ -962,7 +974,11 @@ class MessagesPresenterTest {
|
||||
liveTimeline = timeline,
|
||||
typingNoticeResult = { Result.success(Unit) },
|
||||
)
|
||||
val presenter = createMessagesPresenter(joinedRoom = room, analyticsService = analyticsService)
|
||||
val presenter = createMessagesPresenter(
|
||||
timeline = timeline,
|
||||
joinedRoom = room,
|
||||
analyticsService = analyticsService
|
||||
)
|
||||
presenter.testWithLifecycleOwner {
|
||||
val messageEvent = aMessageEvent(
|
||||
content = aTimelineItemTextContent()
|
||||
@@ -1236,8 +1252,57 @@ class MessagesPresenterTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - handle MarkAsFullyReadAndExit marks the room as fully read and navigates up`() = runTest {
|
||||
val markAsFullyReadRecorder = lambdaRecorder<RoomId, EventId, Unit> { _, _ -> }
|
||||
val markAsFullyReadUseCase = FakeMarkAsFullyRead(markAsFullyReadRecorder)
|
||||
val onNavigateUpRecorder = lambdaRecorder<Unit> {}
|
||||
val navigator = FakeMessagesNavigator(onNavigateUpLambda = onNavigateUpRecorder)
|
||||
|
||||
val presenter = createMessagesPresenter(
|
||||
timeline = FakeTimeline(getLatestEventIdResult = { Result.success(AN_EVENT_ID) }),
|
||||
markAsFullyRead = markAsFullyReadUseCase,
|
||||
navigator = navigator,
|
||||
)
|
||||
presenter.testWithLifecycleOwner {
|
||||
val initialState = awaitItem()
|
||||
initialState.eventSink(MessagesEvents.MarkAsFullyReadAndExit)
|
||||
|
||||
runCurrent()
|
||||
|
||||
markAsFullyReadRecorder.assertions().isCalledOnce()
|
||||
onNavigateUpRecorder.assertions().isCalledOnce()
|
||||
|
||||
cancelAndIgnoreRemainingEvents()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `present - handle MarkAsFullyReadAndExit still navigates up if marking as read fails`() = runTest {
|
||||
val markAsFullyReadUseCase = FakeMarkAsFullyRead { _, _ -> error("boom") }
|
||||
val onNavigateUpRecorder = lambdaRecorder<Unit> {}
|
||||
val navigator = FakeMessagesNavigator(onNavigateUpLambda = onNavigateUpRecorder)
|
||||
|
||||
val presenter = createMessagesPresenter(
|
||||
timeline = FakeTimeline(getLatestEventIdResult = { Result.success(AN_EVENT_ID) }),
|
||||
markAsFullyRead = markAsFullyReadUseCase,
|
||||
navigator = navigator,
|
||||
)
|
||||
presenter.testWithLifecycleOwner {
|
||||
val initialState = awaitItem()
|
||||
initialState.eventSink(MessagesEvents.MarkAsFullyReadAndExit)
|
||||
|
||||
runCurrent()
|
||||
|
||||
onNavigateUpRecorder.assertions().isCalledOnce()
|
||||
|
||||
cancelAndIgnoreRemainingEvents()
|
||||
}
|
||||
}
|
||||
|
||||
private fun TestScope.createMessagesPresenter(
|
||||
coroutineDispatchers: CoroutineDispatchers = testCoroutineDispatchers(),
|
||||
timeline: Timeline = FakeTimeline(),
|
||||
joinedRoom: FakeJoinedRoom = FakeJoinedRoom(
|
||||
baseRoom = FakeBaseRoom(
|
||||
canUserSendMessageResult = { _, _ -> Result.success(true) },
|
||||
@@ -1248,10 +1313,9 @@ class MessagesPresenterTest {
|
||||
).apply {
|
||||
givenRoomInfo(aRoomInfo(id = roomId, name = ""))
|
||||
},
|
||||
liveTimeline = FakeTimeline(),
|
||||
liveTimeline = timeline,
|
||||
typingNoticeResult = { Result.success(Unit) },
|
||||
),
|
||||
timeline: Timeline = joinedRoom.liveTimeline,
|
||||
navigator: FakeMessagesNavigator = FakeMessagesNavigator(),
|
||||
clipboardHelper: FakeClipboardHelper = FakeClipboardHelper(),
|
||||
analyticsService: FakeAnalyticsService = FakeAnalyticsService(),
|
||||
@@ -1270,6 +1334,7 @@ class MessagesPresenterTest {
|
||||
featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(),
|
||||
actionListEventSink: (ActionListEvents) -> Unit = {},
|
||||
addRecentEmoji: AddRecentEmoji = AddRecentEmoji(FakeMatrixClient(), testCoroutineDispatchers()),
|
||||
markAsFullyRead: MarkAsFullyRead = FakeMarkAsFullyRead(),
|
||||
): MessagesPresenter {
|
||||
return MessagesPresenter(
|
||||
navigator = navigator,
|
||||
@@ -1298,6 +1363,8 @@ class MessagesPresenterTest {
|
||||
encryptionService = encryptionService,
|
||||
featureFlagService = featureFlagService,
|
||||
addRecentEmoji = addRecentEmoji,
|
||||
markAsFullyRead = markAsFullyRead,
|
||||
sessionCoroutineScope = backgroundScope,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,12 +9,15 @@
|
||||
|
||||
package io.element.android.features.messages.impl.timeline
|
||||
|
||||
import io.element.android.libraries.matrix.api.timeline.ReceiptType
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
import io.element.android.libraries.matrix.test.AN_EVENT_ID
|
||||
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 io.element.android.tests.testutils.testCoroutineDispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.test.runCurrent
|
||||
import kotlinx.coroutines.test.runTest
|
||||
@@ -22,34 +25,30 @@ import org.junit.Test
|
||||
|
||||
class DefaultMarkAsFullyReadTest {
|
||||
@Test
|
||||
fun `When room is not found, then no exception is thrown`() = runTest {
|
||||
fun `When marking as read fails, no exception is thrown`() = runTest {
|
||||
val markAsFullyRead = DefaultMarkAsFullyRead(
|
||||
FakeMatrixClient(
|
||||
sessionCoroutineScope = backgroundScope,
|
||||
matrixClient = FakeMatrixClient(
|
||||
markRoomAsFullyReadResult = { _, _ -> Result.failure(IllegalStateException("Room not found")) },
|
||||
).apply {
|
||||
givenGetRoomResult(A_ROOM_ID, null)
|
||||
}
|
||||
},
|
||||
coroutineDispatchers = testCoroutineDispatchers(),
|
||||
)
|
||||
markAsFullyRead.invoke(A_ROOM_ID)
|
||||
assertThat(markAsFullyRead.invoke(A_ROOM_ID, AN_EVENT_ID).isFailure).isTrue()
|
||||
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
|
||||
)
|
||||
fun `When marking as read is successful, the expected method is invoked`() = runTest {
|
||||
val markAsFullyReadResult = lambdaRecorder<RoomId, EventId, Result<Unit>> { _, _ -> Result.success(Unit) }
|
||||
val markAsFullyRead = DefaultMarkAsFullyRead(
|
||||
FakeMatrixClient(
|
||||
sessionCoroutineScope = backgroundScope,
|
||||
).apply {
|
||||
givenGetRoomResult(A_ROOM_ID, baseRoom)
|
||||
}
|
||||
matrixClient = FakeMatrixClient(
|
||||
markRoomAsFullyReadResult = markAsFullyReadResult,
|
||||
),
|
||||
coroutineDispatchers = testCoroutineDispatchers(),
|
||||
)
|
||||
markAsFullyRead.invoke(A_ROOM_ID)
|
||||
assertThat(markAsFullyRead.invoke(A_ROOM_ID, AN_EVENT_ID).isSuccess).isTrue()
|
||||
runCurrent()
|
||||
markAsReadResult.assertions().isCalledOnce().with(value(ReceiptType.FULLY_READ))
|
||||
baseRoom.assertDestroyed()
|
||||
markAsFullyReadResult.assertions().isCalledOnce().with(value(A_ROOM_ID), value(AN_EVENT_ID))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,13 +7,15 @@
|
||||
|
||||
package io.element.android.features.messages.impl.timeline
|
||||
|
||||
import io.element.android.libraries.core.extensions.runCatchingExceptions
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
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() }
|
||||
private val invokeResult: (RoomId, EventId) -> Unit = { _, _ -> lambdaError() },
|
||||
) : MarkAsFullyRead {
|
||||
override fun invoke(roomId: RoomId) {
|
||||
invokeResult(roomId)
|
||||
override suspend fun invoke(roomId: RoomId, eventId: EventId): Result<Unit> {
|
||||
return runCatchingExceptions { invokeResult(roomId, eventId) }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -151,19 +151,21 @@ class TimelinePresenterTest {
|
||||
isSendPublicReadReceiptsEnabled: Boolean,
|
||||
expectedReceiptType: ReceiptType,
|
||||
) = runTest(StandardTestDispatcher()) {
|
||||
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
|
||||
val sendReadReceiptLambda = lambdaRecorder<EventId, ReceiptType, Result<Unit>> { _, _ -> Result.success(Unit) }
|
||||
val timeline = FakeTimeline(
|
||||
timelineItems = flowOf(
|
||||
listOf(
|
||||
MatrixTimelineItem.Event(A_UNIQUE_ID, anEventTimelineItem())
|
||||
)
|
||||
)
|
||||
),
|
||||
markAsReadResult = markAsReadResult,
|
||||
sendReadReceiptLambda = sendReadReceiptLambda,
|
||||
)
|
||||
val markAsReadResult = lambdaRecorder<ReceiptType, Result<Unit>> { Result.success(Unit) }
|
||||
val room = FakeJoinedRoom(
|
||||
liveTimeline = timeline,
|
||||
baseRoom = FakeBaseRoom(
|
||||
canUserSendMessageResult = { _, _ -> Result.success(true) },
|
||||
markAsReadResult = markAsReadResult,
|
||||
)
|
||||
)
|
||||
val sessionPreferencesStore = InMemorySessionPreferencesStore(isSendPublicReadReceiptsEnabled = isSendPublicReadReceiptsEnabled)
|
||||
@@ -185,25 +187,6 @@ class TimelinePresenterTest {
|
||||
}
|
||||
}
|
||||
|
||||
@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 ->
|
||||
@@ -258,10 +241,10 @@ class TimelinePresenterTest {
|
||||
)
|
||||
)
|
||||
)
|
||||
)
|
||||
).apply {
|
||||
this.sendReadReceiptLambda = sendReadReceiptsLambda
|
||||
}
|
||||
),
|
||||
markAsReadResult = { Result.success(Unit) },
|
||||
sendReadReceiptLambda = sendReadReceiptsLambda,
|
||||
)
|
||||
val sessionPreferencesStore = InMemorySessionPreferencesStore(isSendPublicReadReceiptsEnabled = false)
|
||||
val presenter = createTimelinePresenter(
|
||||
timeline = timeline,
|
||||
@@ -349,7 +332,10 @@ class TimelinePresenterTest {
|
||||
@Test
|
||||
fun `present - covers newEventState scenarios`() = runTest {
|
||||
val timelineItems = MutableStateFlow(emptyList<MatrixTimelineItem>())
|
||||
val timeline = FakeTimeline(timelineItems = timelineItems)
|
||||
val timeline = FakeTimeline(
|
||||
timelineItems = timelineItems,
|
||||
markAsReadResult = { Result.success(Unit) },
|
||||
)
|
||||
val presenter = createTimelinePresenter(timeline)
|
||||
moleculeFlow(RecompositionMode.Immediate) {
|
||||
presenter.present()
|
||||
@@ -1039,7 +1025,6 @@ class TimelinePresenterTest {
|
||||
sendPollResponseAction: SendPollResponseAction = FakeSendPollResponseAction(),
|
||||
sessionPreferencesStore: InMemorySessionPreferencesStore = InMemorySessionPreferencesStore(),
|
||||
timelineItemIndexer: TimelineItemIndexer = TimelineItemIndexer(),
|
||||
markAsFullyRead: MarkAsFullyRead = FakeMarkAsFullyRead(),
|
||||
featureFlagService: FakeFeatureFlagService = FakeFeatureFlagService(),
|
||||
): TimelinePresenter {
|
||||
return TimelinePresenter(
|
||||
@@ -1057,7 +1042,6 @@ class TimelinePresenterTest {
|
||||
resolveVerifiedUserSendFailurePresenter = { aResolveVerifiedUserSendFailureState() },
|
||||
typingNotificationPresenter = { aTypingNotificationState() },
|
||||
roomCallStatePresenter = { aStandByCallState() },
|
||||
markAsFullyRead = markAsFullyRead,
|
||||
featureFlagService = featureFlagService,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
/*
|
||||
* 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.libraries.designsystem.utils
|
||||
|
||||
import androidx.compose.animation.AnimatedVisibility
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.movableContentOf
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.platform.LocalInspectionMode
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlin.time.Duration
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
|
||||
/**
|
||||
* Displays the content of [block] after a delay of [duration].
|
||||
*/
|
||||
@Composable
|
||||
fun DelayedVisibility(
|
||||
duration: Duration = 300.milliseconds,
|
||||
block: @Composable () -> Unit,
|
||||
) {
|
||||
// Technically this shouldn't be needed because `LocalInspectionMode` won't change, but let's make the linter happy
|
||||
val movableBlock = remember { movableContentOf { block() } }
|
||||
if (LocalInspectionMode.current) {
|
||||
// Just allow the contents to be displayed in the previews/screenshot tests
|
||||
movableBlock()
|
||||
} else {
|
||||
var shouldDisplay by remember { mutableStateOf(false) }
|
||||
LaunchedEffect(Unit) {
|
||||
delay(duration)
|
||||
shouldDisplay = true
|
||||
}
|
||||
AnimatedVisibility(shouldDisplay) {
|
||||
movableBlock()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -9,6 +9,7 @@ package io.element.android.libraries.matrix.api
|
||||
|
||||
import io.element.android.libraries.core.data.tryOrNull
|
||||
import io.element.android.libraries.matrix.api.core.DeviceId
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
import io.element.android.libraries.matrix.api.core.MatrixPatterns
|
||||
import io.element.android.libraries.matrix.api.core.RoomAlias
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
@@ -34,6 +35,7 @@ import io.element.android.libraries.matrix.api.roomlist.RoomListService
|
||||
import io.element.android.libraries.matrix.api.spaces.SpaceService
|
||||
import io.element.android.libraries.matrix.api.sync.SlidingSyncVersion
|
||||
import io.element.android.libraries.matrix.api.sync.SyncService
|
||||
import io.element.android.libraries.matrix.api.timeline.Timeline
|
||||
import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults
|
||||
import io.element.android.libraries.matrix.api.user.MatrixUser
|
||||
import io.element.android.libraries.matrix.api.verification.SessionVerificationService
|
||||
@@ -183,6 +185,14 @@ interface MatrixClient {
|
||||
* Adds an emoji to the list of recent emoji reactions for this account.
|
||||
*/
|
||||
suspend fun addRecentEmoji(emoji: String): Result<Unit>
|
||||
|
||||
/**
|
||||
* Marks the room with the provided [roomId] as read, sending a fully read receipt for [eventId].
|
||||
*
|
||||
* This method should be used with caution as providing the [eventId] ourselves can result in incorrect read receipts.
|
||||
* Use [Timeline.markAsRead] instead when possible.
|
||||
*/
|
||||
suspend fun markRoomAsFullyRead(roomId: RoomId, eventId: EventId): Result<Unit>
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -16,7 +16,7 @@ sealed class QrLoginException : Exception() {
|
||||
data object OidcMetadataInvalid : QrLoginException()
|
||||
data object SlidingSyncNotAvailable : QrLoginException()
|
||||
data object OtherDeviceNotSignedIn : QrLoginException()
|
||||
data object Unknown : QrLoginException()
|
||||
data object CheckCodeAlreadySent : QrLoginException()
|
||||
data object CheckCodeCannotBeSent : QrLoginException()
|
||||
data object Unknown : QrLoginException()
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ import io.element.android.libraries.matrix.api.room.powerlevels.RoomPowerLevelsV
|
||||
import io.element.android.libraries.matrix.api.room.tombstone.PredecessorRoom
|
||||
import io.element.android.libraries.matrix.api.roomdirectory.RoomVisibility
|
||||
import io.element.android.libraries.matrix.api.timeline.ReceiptType
|
||||
import io.element.android.libraries.matrix.api.timeline.Timeline
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
@@ -180,6 +181,10 @@ interface BaseRoom : Closeable {
|
||||
|
||||
/**
|
||||
* Mark the room as read by trying to attach an unthreaded read receipt to the latest room event.
|
||||
*
|
||||
* Note this will instantiate a new timeline, which is an expensive operation.
|
||||
* Prefer using [Timeline.markAsRead] instead when possible.
|
||||
*
|
||||
* @param receiptType The type of receipt to send.
|
||||
*/
|
||||
suspend fun markAsRead(receiptType: ReceiptType): Result<Unit>
|
||||
|
||||
@@ -55,6 +55,7 @@ interface Timeline : AutoCloseable {
|
||||
val mode: Mode
|
||||
val membershipChangeEventReceived: Flow<Unit>
|
||||
suspend fun sendReadReceipt(eventId: EventId, receiptType: ReceiptType): Result<Unit>
|
||||
suspend fun markAsRead(receiptType: ReceiptType): Result<Unit>
|
||||
suspend fun paginate(direction: PaginationDirection): Result<Boolean>
|
||||
|
||||
val backwardPaginationStatus: StateFlow<PaginationStatus>
|
||||
@@ -227,4 +228,9 @@ interface Timeline : AutoCloseable {
|
||||
* pinned
|
||||
*/
|
||||
suspend fun unpinEvent(eventId: EventId): Result<Boolean>
|
||||
|
||||
/**
|
||||
* Get the latest event id of the timeline.
|
||||
*/
|
||||
suspend fun getLatestEventId(): Result<EventId?>
|
||||
}
|
||||
|
||||
@@ -17,6 +17,7 @@ import io.element.android.libraries.core.extensions.runCatchingExceptions
|
||||
import io.element.android.libraries.featureflag.api.FeatureFlagService
|
||||
import io.element.android.libraries.matrix.api.MatrixClient
|
||||
import io.element.android.libraries.matrix.api.core.DeviceId
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
import io.element.android.libraries.matrix.api.core.RoomAlias
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
|
||||
@@ -713,6 +714,13 @@ class RustMatrixClient(
|
||||
}
|
||||
}
|
||||
|
||||
override suspend fun markRoomAsFullyRead(roomId: RoomId, eventId: EventId): Result<Unit> = withContext(sessionDispatcher) {
|
||||
runCatchingExceptions {
|
||||
val room = innerClient.getRoom(roomId.value) ?: error("Could not fetch associated room")
|
||||
room.markAsFullyReadUnchecked(eventId.value)
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun getCacheSize(
|
||||
includeCryptoDb: Boolean = false,
|
||||
): Long = withContext(sessionDispatcher) {
|
||||
|
||||
@@ -473,6 +473,7 @@ class JoinedRustRoom(
|
||||
override fun destroy() {
|
||||
baseRoom.destroy()
|
||||
liveInnerTimeline.destroy()
|
||||
Timber.d("Room $roomId destroyed")
|
||||
}
|
||||
|
||||
private fun InnerTimeline.map(
|
||||
|
||||
@@ -159,6 +159,12 @@ class RustTimeline(
|
||||
}
|
||||
}
|
||||
|
||||
override suspend fun markAsRead(receiptType: ReceiptType): Result<Unit> = withContext(dispatcher) {
|
||||
runCatchingExceptions {
|
||||
inner.markAsRead(receiptType.toRustReceiptType())
|
||||
}
|
||||
}
|
||||
|
||||
private fun updatePaginationStatus(direction: Timeline.PaginationDirection, update: (Timeline.PaginationStatus) -> Timeline.PaginationStatus) {
|
||||
when (direction) {
|
||||
Timeline.PaginationDirection.BACKWARDS -> backwardPaginationStatus.getAndUpdate(update)
|
||||
@@ -586,6 +592,12 @@ class RustTimeline(
|
||||
}
|
||||
}
|
||||
|
||||
override suspend fun getLatestEventId(): Result<EventId?> = withContext(dispatcher) {
|
||||
runCatchingExceptions {
|
||||
inner.latestEventId()?.let(::EventId)
|
||||
}
|
||||
}
|
||||
|
||||
private suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit> = withContext(dispatcher) {
|
||||
runCatchingExceptions {
|
||||
inner.fetchDetailsForEvent(eventId.value)
|
||||
|
||||
@@ -9,6 +9,7 @@ package io.element.android.libraries.matrix.test
|
||||
|
||||
import io.element.android.libraries.matrix.api.MatrixClient
|
||||
import io.element.android.libraries.matrix.api.core.DeviceId
|
||||
import io.element.android.libraries.matrix.api.core.EventId
|
||||
import io.element.android.libraries.matrix.api.core.RoomAlias
|
||||
import io.element.android.libraries.matrix.api.core.RoomId
|
||||
import io.element.android.libraries.matrix.api.core.RoomIdOrAlias
|
||||
@@ -101,6 +102,7 @@ class FakeMatrixClient(
|
||||
private val getJoinedRoomIdsResult: () -> Result<Set<RoomId>> = { Result.success(emptySet()) },
|
||||
private val getRecentEmojisLambda: () -> Result<List<String>> = { Result.success(emptyList()) },
|
||||
private val addRecentEmojiLambda: (String) -> Result<Unit> = { Result.success(Unit) },
|
||||
private val markRoomAsFullyReadResult: (RoomId, EventId) -> Result<Unit> = { _, _ -> lambdaError() },
|
||||
) : MatrixClient {
|
||||
var setDisplayNameCalled: Boolean = false
|
||||
private set
|
||||
@@ -344,4 +346,8 @@ class FakeMatrixClient(
|
||||
override suspend fun getRecentEmojis(): Result<List<String>> {
|
||||
return getRecentEmojisLambda()
|
||||
}
|
||||
|
||||
override suspend fun markRoomAsFullyRead(roomId: RoomId, eventId: EventId): Result<Unit> {
|
||||
return markRoomAsFullyReadResult(roomId, eventId)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,6 +49,14 @@ class FakeTimeline(
|
||||
override val membershipChangeEventReceived: Flow<Unit> = MutableSharedFlow(),
|
||||
private val cancelSendResult: (TransactionId) -> Result<Unit> = { lambdaError() },
|
||||
override val mode: Timeline.Mode = Timeline.Mode.Live,
|
||||
private val markAsReadResult: (ReceiptType) -> Result<Unit> = { lambdaError() },
|
||||
private val getLatestEventIdResult: () -> Result<EventId?> = { lambdaError() },
|
||||
var sendReadReceiptLambda: (
|
||||
eventId: EventId,
|
||||
receiptType: ReceiptType,
|
||||
) -> Result<Unit> = { _, _ ->
|
||||
lambdaError()
|
||||
}
|
||||
) : Timeline {
|
||||
var sendMessageLambda: (
|
||||
body: String,
|
||||
@@ -397,18 +405,15 @@ class FakeTimeline(
|
||||
)
|
||||
}
|
||||
|
||||
var sendReadReceiptLambda: (
|
||||
eventId: EventId,
|
||||
receiptType: ReceiptType,
|
||||
) -> Result<Unit> = { _, _ ->
|
||||
lambdaError()
|
||||
}
|
||||
|
||||
override suspend fun sendReadReceipt(
|
||||
eventId: EventId,
|
||||
receiptType: ReceiptType,
|
||||
): Result<Unit> = sendReadReceiptLambda(eventId, receiptType)
|
||||
|
||||
override suspend fun markAsRead(receiptType: ReceiptType): Result<Unit> {
|
||||
return markAsReadResult(receiptType)
|
||||
}
|
||||
|
||||
var paginateLambda: (direction: Timeline.PaginationDirection) -> Result<Boolean> = {
|
||||
Result.success(false)
|
||||
}
|
||||
@@ -431,6 +436,10 @@ class FakeTimeline(
|
||||
return unpinEventLambda(eventId)
|
||||
}
|
||||
|
||||
override suspend fun getLatestEventId(): Result<EventId?> {
|
||||
return getLatestEventIdResult()
|
||||
}
|
||||
|
||||
var closeCounter = 0
|
||||
private set
|
||||
|
||||
|
||||
Reference in New Issue
Block a user