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:
parent
712a9226e6
commit
96d90a75d9
8 changed files with 128 additions and 0 deletions
1
changelog.d/671.feature
Normal file
1
changelog.d/671.feature
Normal file
|
|
@ -0,0 +1 @@
|
||||||
|
Send read receipts for rooms.
|
||||||
|
|
@ -21,4 +21,5 @@ import io.element.android.libraries.matrix.api.core.EventId
|
||||||
sealed interface TimelineEvents {
|
sealed interface TimelineEvents {
|
||||||
object LoadMore : TimelineEvents
|
object LoadMore : TimelineEvents
|
||||||
data class SetHighlightedEvent(val eventId: EventId?) : TimelineEvents
|
data class SetHighlightedEvent(val eventId: EventId?) : TimelineEvents
|
||||||
|
data class OnScrollFinished(val firstIndex: Int) : TimelineEvents
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -20,14 +20,18 @@ import androidx.compose.runtime.Composable
|
||||||
import androidx.compose.runtime.LaunchedEffect
|
import androidx.compose.runtime.LaunchedEffect
|
||||||
import androidx.compose.runtime.MutableState
|
import androidx.compose.runtime.MutableState
|
||||||
import androidx.compose.runtime.collectAsState
|
import androidx.compose.runtime.collectAsState
|
||||||
|
import androidx.compose.runtime.getValue
|
||||||
import androidx.compose.runtime.mutableStateOf
|
import androidx.compose.runtime.mutableStateOf
|
||||||
import androidx.compose.runtime.rememberCoroutineScope
|
import androidx.compose.runtime.rememberCoroutineScope
|
||||||
import androidx.compose.runtime.saveable.rememberSaveable
|
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.factories.TimelineItemsFactory
|
||||||
|
import io.element.android.features.messages.impl.timeline.model.TimelineItem
|
||||||
import io.element.android.libraries.architecture.Presenter
|
import io.element.android.libraries.architecture.Presenter
|
||||||
import io.element.android.libraries.matrix.api.core.EventId
|
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.room.MatrixRoom
|
||||||
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
|
import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
|
||||||
|
import kotlinx.collections.immutable.ImmutableList
|
||||||
import kotlinx.coroutines.CoroutineScope
|
import kotlinx.coroutines.CoroutineScope
|
||||||
import kotlinx.coroutines.flow.launchIn
|
import kotlinx.coroutines.flow.launchIn
|
||||||
import kotlinx.coroutines.flow.onEach
|
import kotlinx.coroutines.flow.onEach
|
||||||
|
|
@ -52,6 +56,9 @@ class TimelinePresenter @Inject constructor(
|
||||||
mutableStateOf(null)
|
mutableStateOf(null)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var lastReadMarkerIndex by rememberSaveable { mutableStateOf(Int.MAX_VALUE) }
|
||||||
|
var lastReadMarkerId by rememberSaveable { mutableStateOf<EventId?>(null) }
|
||||||
|
|
||||||
val timelineItems = timelineItemsFactory
|
val timelineItems = timelineItemsFactory
|
||||||
.flow()
|
.flow()
|
||||||
.collectAsState()
|
.collectAsState()
|
||||||
|
|
@ -64,6 +71,15 @@ class TimelinePresenter @Inject constructor(
|
||||||
when (event) {
|
when (event) {
|
||||||
TimelineEvents.LoadMore -> localCoroutineScope.loadMore(paginationState.value)
|
TimelineEvents.LoadMore -> localCoroutineScope.loadMore(paginationState.value)
|
||||||
is TimelineEvents.SetHighlightedEvent -> highlightedEventId.value = event.eventId
|
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 {
|
private fun CoroutineScope.loadMore(paginationState: MatrixTimeline.PaginationState) = launch {
|
||||||
if (paginationState.canBackPaginate && !paginationState.isBackPaginating) {
|
if (paginationState.canBackPaginate && !paginationState.isBackPaginating) {
|
||||||
timeline.paginateBackwards(backPaginationEventLimit, backPaginationPageSize)
|
timeline.paginateBackwards(backPaginationEventLimit, backPaginationPageSize)
|
||||||
|
|
@ -94,4 +119,8 @@ class TimelinePresenter @Inject constructor(
|
||||||
Timber.v("Can't back paginate as paginationState = $paginationState")
|
Timber.v("Can't back paginate as paginationState = $paginationState")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun CoroutineScope.sendReadReceipt(eventId: EventId) = launch {
|
||||||
|
timeline.sendReadReceipt(eventId)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -85,6 +85,14 @@ fun TimelineView(
|
||||||
// TODO implement this logic once we have support to 'jump to event X' in sliding sync
|
// 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) {
|
Box(modifier = modifier) {
|
||||||
LazyColumn(
|
LazyColumn(
|
||||||
modifier = Modifier.fillMaxSize(),
|
modifier = Modifier.fillMaxSize(),
|
||||||
|
|
|
||||||
|
|
@ -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.fixtures.aTimelineItemsFactory
|
||||||
import io.element.android.features.messages.impl.timeline.TimelineEvents
|
import io.element.android.features.messages.impl.timeline.TimelineEvents
|
||||||
import io.element.android.features.messages.impl.timeline.TimelinePresenter
|
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.AN_EVENT_ID
|
||||||
import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
|
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 kotlinx.coroutines.test.runTest
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
|
|
||||||
|
|
@ -87,4 +91,73 @@ class TimelinePresenterTest {
|
||||||
assertThat(withoutHighlightedState.highlightedEventId).isNull()
|
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)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -32,4 +32,6 @@ interface MatrixTimeline {
|
||||||
|
|
||||||
suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result<Unit>
|
suspend fun paginateBackwards(requestSize: Int, untilNumberOfItems: Int): Result<Unit>
|
||||||
suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit>
|
suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit>
|
||||||
|
|
||||||
|
suspend fun sendReadReceipt(eventId: EventId): Result<Unit>
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -107,4 +107,10 @@ class RustMatrixTimeline(
|
||||||
Timber.v("Success back paginating for room ${matrixRoom.roomId}")
|
Timber.v("Success back paginating for room ${matrixRoom.roomId}")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override suspend fun sendReadReceipt(eventId: EventId) = withContext(coroutineDispatchers.io) {
|
||||||
|
runCatching {
|
||||||
|
innerRoom.sendReadReceipt(eventId = eventId.value)
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -32,6 +32,9 @@ class FakeMatrixTimeline(
|
||||||
private val paginationState: MutableStateFlow<MatrixTimeline.PaginationState> = MutableStateFlow(initialPaginationState)
|
private val paginationState: MutableStateFlow<MatrixTimeline.PaginationState> = MutableStateFlow(initialPaginationState)
|
||||||
private val timelineItems: MutableStateFlow<List<MatrixTimelineItem>> = MutableStateFlow(initialTimelineItems)
|
private val timelineItems: MutableStateFlow<List<MatrixTimelineItem>> = MutableStateFlow(initialTimelineItems)
|
||||||
|
|
||||||
|
var sendReadReceiptCount = 0
|
||||||
|
private set
|
||||||
|
|
||||||
fun updatePaginationState(update: (MatrixTimeline.PaginationState.() -> MatrixTimeline.PaginationState)) {
|
fun updatePaginationState(update: (MatrixTimeline.PaginationState.() -> MatrixTimeline.PaginationState)) {
|
||||||
paginationState.value = update(paginationState.value)
|
paginationState.value = update(paginationState.value)
|
||||||
}
|
}
|
||||||
|
|
@ -66,4 +69,9 @@ class FakeMatrixTimeline(
|
||||||
override suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit> {
|
override suspend fun fetchDetailsForEvent(eventId: EventId): Result<Unit> {
|
||||||
return Result.success(Unit)
|
return Result.success(Unit)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override suspend fun sendReadReceipt(eventId: EventId): Result<Unit> {
|
||||||
|
sendReadReceiptCount++
|
||||||
|
return Result.success(Unit)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue