Merge pull request #259 from vector-im/feature/fix_read_receipts_not_updated

Fix / send read marker for collapsed items

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<Unit>) {
+        //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)

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<SetReadMarkersTask.Params, Unit> {
@@ -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<Unit> {
         val markers = HashMap<String, String>()
-        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)

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)
+    }
 }

vector/src/main/java/im/vector/riotredesign/features/home/room/detail/RoomDetailViewModel.kt

@@ -380,7 +380,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 ->

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))
                 }
             }
         }

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<TimelineEvent>)
+    : VectorEpoxyModel.OnVisibilityStateChangedListener {
+
+    override fun onVisibilityStateChanged(visibilityState: Int) {
+        if (visibilityState == VisibilityState.VISIBLE) {
+            events.forEach {
+                callback?.onEventVisible(it)
+            }
+        }
+    }
+
 }