From c7df433a44fb7c1da53f7e3ccbfe25671696628f Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 2 Jul 2019 12:08:44 +0200 Subject: [PATCH] Fix / send read marker for collapsed items Also remove unnecessary check on matrix id format --- .../session/room/read/DefaultReadService.kt | 1 + .../session/room/read/SetReadMarkersTask.kt | 25 ++++++++++++------- .../room/send/LocalEchoEventFactory.kt | 9 ++++--- .../home/room/detail/RoomDetailViewModel.kt | 4 ++- .../timeline/TimelineEventController.kt | 4 ++- ...lineEventVisibilityStateChangedListener.kt | 15 +++++++++++ 6 files changed, 44 insertions(+), 14 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt index 3edfed82..7b90a8ac 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt @@ -39,6 +39,7 @@ internal class DefaultReadService @Inject constructor(private val roomId: String private val credentials: Credentials) : ReadService { override fun markAllAsRead(callback: MatrixCallback) { + //TODO shouldn't it be latest synced event? val latestEvent = getLatestEvent() val params = SetReadMarkersTask.Params(roomId, fullyReadEventId = latestEvent?.eventId, readReceiptEventId = latestEvent?.eventId) setReadMarkersTask.configureWith(params).dispatchTo(callback).executeBy(taskExecutor) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt index 2146fdf0..7ff2d009 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt @@ -18,7 +18,6 @@ package im.vector.matrix.android.internal.session.room.read import arrow.core.Try import com.zhuinden.monarchy.Monarchy -import im.vector.matrix.android.api.MatrixPatterns import im.vector.matrix.android.api.auth.data.Credentials import im.vector.matrix.android.internal.database.model.ChunkEntity import im.vector.matrix.android.internal.database.model.EventEntity @@ -29,10 +28,11 @@ import im.vector.matrix.android.internal.database.query.findLastLiveChunkFromRoo import im.vector.matrix.android.internal.database.query.latestEvent import im.vector.matrix.android.internal.database.query.where import im.vector.matrix.android.internal.network.executeRequest -import im.vector.matrix.android.internal.session.SessionScope import im.vector.matrix.android.internal.session.room.RoomAPI +import im.vector.matrix.android.internal.session.room.send.LocalEchoEventFactory import im.vector.matrix.android.internal.task.Task import im.vector.matrix.android.internal.util.tryTransactionAsync +import timber.log.Timber import javax.inject.Inject internal interface SetReadMarkersTask : Task { @@ -48,21 +48,28 @@ private const val READ_MARKER = "m.fully_read" private const val READ_RECEIPT = "m.read" internal class DefaultSetReadMarkersTask @Inject constructor(private val roomAPI: RoomAPI, - private val credentials: Credentials, - private val monarchy: Monarchy + private val credentials: Credentials, + private val monarchy: Monarchy ) : SetReadMarkersTask { override suspend fun execute(params: SetReadMarkersTask.Params): Try { val markers = HashMap() - if (params.fullyReadEventId != null && MatrixPatterns.isEventId(params.fullyReadEventId)) { - markers[READ_MARKER] = params.fullyReadEventId + if (params.fullyReadEventId != null) { + if (LocalEchoEventFactory.isLocalEchoId(params.fullyReadEventId)) { + Timber.w("Can't set read marker for local event ${params.fullyReadEventId}") + } else { + markers[READ_MARKER] = params.fullyReadEventId + } } if (params.readReceiptEventId != null - && MatrixPatterns.isEventId(params.readReceiptEventId) && !isEventRead(params.roomId, params.readReceiptEventId)) { - updateNotificationCountIfNecessary(params.roomId, params.readReceiptEventId) - markers[READ_RECEIPT] = params.readReceiptEventId + if (LocalEchoEventFactory.isLocalEchoId(params.readReceiptEventId)) { + Timber.w("Can't set read marker for local event ${params.fullyReadEventId}") + } else { + updateNotificationCountIfNecessary(params.roomId, params.readReceiptEventId) + markers[READ_RECEIPT] = params.readReceiptEventId + } } return if (markers.isEmpty()) { Try.just(Unit) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt index 6e80e7c6..eaf90f39 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/send/LocalEchoEventFactory.kt @@ -29,9 +29,7 @@ import im.vector.matrix.android.api.session.room.model.relation.ReactionInfo import im.vector.matrix.android.api.session.room.model.relation.RelationDefaultContent import im.vector.matrix.android.api.session.room.model.relation.ReplyToContent import im.vector.matrix.android.internal.database.helper.addSendingEvent -import im.vector.matrix.android.internal.database.model.ChunkEntity import im.vector.matrix.android.internal.database.model.RoomEntity -import im.vector.matrix.android.internal.database.query.findLastLiveChunkFromRoom import im.vector.matrix.android.internal.database.query.where import im.vector.matrix.android.internal.session.content.ThumbnailExtractor import im.vector.matrix.android.internal.session.room.RoomSummaryUpdater @@ -238,7 +236,7 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials } private fun dummyEventId(roomId: String): String { - return "m.${UUID.randomUUID()}" + return "$LOCAL_ID_PREFIX${UUID.randomUUID()}" } fun createReplyTextEvent(roomId: String, eventReplied: Event, replyText: String): Event? { @@ -353,4 +351,9 @@ internal class LocalEchoEventFactory @Inject constructor(private val credentials } } + companion object { + const val LOCAL_ID_PREFIX = "local." + + fun isLocalEchoId(eventId: String): Boolean = eventId.startsWith(LOCAL_ID_PREFIX) + } } diff --git a/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/RoomDetailViewModel.kt index 8d23ac0f..23302dff 100644 --- a/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/RoomDetailViewModel.kt @@ -376,7 +376,9 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro } private fun handleEventDisplayed(action: RoomDetailActions.EventDisplayed) { - displayedEventsObservable.accept(action) + if (action.event.sendState.isSent()) { //ignore pending/local events + displayedEventsObservable.accept(action) + } //We need to update this with the related m.replace also (to move read receipt) action.event.annotations?.editSummary?.sourceEvents?.forEach { room.getTimeLineEvent(it)?.let { event -> diff --git a/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/TimelineEventController.kt b/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/TimelineEventController.kt index 82e54ef9..72f73cd5 100644 --- a/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/TimelineEventController.kt +++ b/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/TimelineEventController.kt @@ -299,9 +299,11 @@ class TimelineEventController @Inject constructor(private val dateFormatter: Tim collapsedEventIds.removeAll(mergedEventIds) } val mergeId = mergedEventIds.joinToString(separator = "_") { it } - MergedHeaderItem(isCollapsed, mergeId, mergedData, avatarRenderer) { + (MergedHeaderItem(isCollapsed, mergeId, mergedData, avatarRenderer) { mergeItemCollapseStates[event.localId] = it requestModelBuild() + }).also { + it.setOnVisibilityStateChanged(MergedTimelineEventVisibilityStateChangedListener(callback, mergedEvents)) } } } diff --git a/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/helper/TimelineEventVisibilityStateChangedListener.kt b/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/helper/TimelineEventVisibilityStateChangedListener.kt index ba6cf27a..0b39bdf5 100644 --- a/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/helper/TimelineEventVisibilityStateChangedListener.kt +++ b/vector/src/main/java/im/vector/riotredesign/features/home/room/detail/timeline/helper/TimelineEventVisibilityStateChangedListener.kt @@ -31,4 +31,19 @@ class TimelineEventVisibilityStateChangedListener(private val callback: Timeline } } +} + + +class MergedTimelineEventVisibilityStateChangedListener(private val callback: TimelineEventController.Callback?, + private val events: List) + : VectorEpoxyModel.OnVisibilityStateChangedListener { + + override fun onVisibilityStateChanged(visibilityState: Int) { + if (visibilityState == VisibilityState.VISIBLE) { + events.forEach { + callback?.onEventVisible(it) + } + } + } + } \ No newline at end of file