Send read receipts (#713)

* Send read receipts

* Add changelog

* Add tests.

* Optimise how sending read receipts work in the timeline.

* Move the check for send read receipts to the presenter

Also improve the way we find the first visible `Event` if we have `Virtual` ones in the timeline.
This commit is contained in:
Jorge Martin Espinosa 2023-06-29 12:08:19 +02:00 committed by GitHub
parent f4e17cf12d
commit 469b54f204
8 changed files with 128 additions and 0 deletions

1
changelog.d/671.feature Normal file
View file

@ -0,0 +1 @@
Send read receipts for rooms.

View file

@ -21,4 +21,5 @@ import io.element.android.libraries.matrix.api.core.EventId
sealed interface TimelineEvents {
object LoadMore : TimelineEvents
data class SetHighlightedEvent(val eventId: EventId?) : TimelineEvents
data class OnScrollFinished(val firstIndex: Int) : TimelineEvents
}

View file

@ -20,14 +20,18 @@ import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.MutableState
import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import io.element.android.features.messages.impl.timeline.factories.TimelineItemsFactory
import io.element.android.features.messages.impl.timeline.model.TimelineItem
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
import kotlinx.collections.immutable.ImmutableList
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
@ -52,6 +56,9 @@ class TimelinePresenter @Inject constructor(
mutableStateOf(null)
}
var lastReadMarkerIndex by rememberSaveable { mutableStateOf(Int.MAX_VALUE) }
var lastReadMarkerId by rememberSaveable { mutableStateOf<EventId?>(null) }
val timelineItems = timelineItemsFactory
.flow()
.collectAsState()
@ -64,6 +71,15 @@ class TimelinePresenter @Inject constructor(
when (event) {
TimelineEvents.LoadMore -> localCoroutineScope.loadMore(paginationState.value)
is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId
is TimelineEvents.OnScrollFinished -> {
// Get last valid EventId seen by the user, as the first index might refer to a Virtual item
val eventId = getLastEventIdBeforeOrAt(event.firstIndex, timelineItems.value) ?: return
if (event.firstIndex <= lastReadMarkerIndex && eventId != lastReadMarkerId) {
lastReadMarkerIndex = event.firstIndex
lastReadMarkerId = eventId
localCoroutineScope.sendReadReceipt(eventId)
}
}
}
}
@ -87,6 +103,15 @@ class TimelinePresenter @Inject constructor(
)
}
private fun getLastEventIdBeforeOrAt(index: Int, items: ImmutableList<TimelineItem>): EventId? {
for (item in items.subList(index, items.count())) {
if (item is TimelineItem.Event) {
return item.eventId
}
}
return null
}
private fun CoroutineScope.loadMore(paginationState: MatrixTimeline.PaginationState) = launch {
if (paginationState.canBackPaginate && !paginationState.isBackPaginating) {
timeline.paginateBackwards(backPaginationEventLimit, backPaginationPageSize)
@ -94,4 +119,8 @@ class TimelinePresenter @Inject constructor(
Timber.v("Can't back paginate as paginationState = $paginationState")
}
}
private fun CoroutineScope.sendReadReceipt(eventId: EventId) = launch {
timeline.sendReadReceipt(eventId)
}
}

View file

@ -85,6 +85,14 @@ fun TimelineView(
// TODO implement this logic once we have support to 'jump to event X' in sliding sync
}
// Send an event to the presenter when the scrolling is finished, with the first visible index at the bottom.
val firstVisibleIndex by remember { derivedStateOf { lazyListState.firstVisibleItemIndex } }
val isScrollFinished by remember { derivedStateOf { !lazyListState.isScrollInProgress } }
LaunchedEffect(firstVisibleIndex, isScrollFinished) {
if (!isScrollFinished) return@LaunchedEffect
state.eventSink(TimelineEvents.OnScrollFinished(firstVisibleIndex))
}
Box(modifier = modifier) {
LazyColumn(
modifier = Modifier.fillMaxSize(),

View file

@ -23,8 +23,12 @@ import com.google.common.truth.Truth.assertThat
import io.element.android.features.messages.fixtures.aTimelineItemsFactory
import io.element.android.features.messages.impl.timeline.TimelineEvents
import io.element.android.features.messages.impl.timeline.TimelinePresenter
import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem
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.room.FakeMatrixRoom
import io.element.android.libraries.matrix.test.room.anEventTimelineItem
import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline
import kotlinx.coroutines.test.runTest
import org.junit.Test
@ -87,4 +91,73 @@ class TimelinePresenterTest {
assertThat(withoutHighlightedState.highlightedEventId).isNull()
}
}
@Test
fun `present - on scroll finished send read receipt if an event is before the index`() = runTest {
val timeline = FakeMatrixTimeline()
val timelineItemsFactory = aTimelineItemsFactory().apply {
replaceWith(listOf(MatrixTimelineItem.Event(anEventTimelineItem())))
}
val room = FakeMatrixRoom(matrixTimeline = timeline)
val presenter = TimelinePresenter(
timelineItemsFactory = timelineItemsFactory,
room = room,
)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
val initialState = awaitItem()
initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(0))
assertThat(timeline.sendReadReceiptCount).isEqualTo(1)
}
}
@Test
fun `present - on scroll finished will not send read receipt no event is before the index`() = runTest {
val timeline = FakeMatrixTimeline()
val timelineItemsFactory = aTimelineItemsFactory().apply {
replaceWith(listOf(MatrixTimelineItem.Event(anEventTimelineItem())))
}
val room = FakeMatrixRoom(matrixTimeline = timeline)
val presenter = TimelinePresenter(
timelineItemsFactory = timelineItemsFactory,
room = room,
)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
val initialState = awaitItem()
initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(1))
assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
}
}
@Test
fun `present - on scroll finished will not send read receipt only virtual events exist before the index`() = runTest {
val timeline = FakeMatrixTimeline()
val timelineItemsFactory = aTimelineItemsFactory().apply {
replaceWith(listOf(MatrixTimelineItem.Virtual(VirtualTimelineItem.ReadMarker)))
}
val room = FakeMatrixRoom(matrixTimeline = timeline)
val presenter = TimelinePresenter(
timelineItemsFactory = timelineItemsFactory,
room = room,
)
moleculeFlow(RecompositionClock.Immediate) {
presenter.present()
}.test {
assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
val initialState = awaitItem()
initialState.eventSink.invoke(TimelineEvents.OnScrollFinished(1))
assertThat(timeline.sendReadReceiptCount).isEqualTo(0)
}
}
}

View file

@ -32,4 +32,6 @@ interface MatrixTimeline {
suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result<Unit>
suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit>
suspend fun sendReadReceipt(eventId: EventId): Result<Unit>
}

View file

@ -107,4 +107,10 @@ class RustMatrixTimeline(
Timber.v("Success back paginating for room ${matrixRoom.roomId}")
}
}
override suspend fun sendReadReceipt(eventId: EventId) = withContext(coroutineDispatchers.io) {
runCatching {
innerRoom.sendReadReceipt(eventId = eventId.value)
}
}
}

View file

@ -32,6 +32,9 @@ class FakeMatrixTimeline(
private val paginationState: MutableStateFlow<MatrixTimeline.PaginationState> = MutableStateFlow(initialPaginationState)
private val timelineItems: MutableStateFlow<List<MatrixTimelineItem>> = MutableStateFlow(initialTimelineItems)
var sendReadReceiptCount = 0
private set
fun updatePaginationState(update: (MatrixTimeline.PaginationState.() -> MatrixTimeline.PaginationState)) {
paginationState.value = update(paginationState.value)
}
@ -66,4 +69,9 @@ class FakeMatrixTimeline(
override suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit> {
return Result.success(Unit)
}
override suspend fun sendReadReceipt(eventId: EventId): Result<Unit> {
sendReadReceiptCount++
return Result.success(Unit)
}
}