Timeline: changes after pr review

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt

@@ -25,7 +25,6 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 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
@@ -62,8 +61,8 @@ class TimelinePresenter @Inject constructor(
             mutableStateOf(null)
         }
 
-        var lastReadReceiptIndex by rememberSaveable { mutableStateOf(Int.MAX_VALUE) }
+        val lastReadReceiptIndex = rememberSaveable { mutableStateOf(Int.MAX_VALUE) }
-        var lastReadReceiptId by rememberSaveable { mutableStateOf<EventId?>(null) }
+        val lastReadReceiptId = rememberSaveable { mutableStateOf<EventId?>(null) }
 
         val timelineItems by timelineItemsFactory.collectItemsAsState()
         val paginationState by timeline.paginationState.collectAsState()
@@ -73,16 +72,6 @@ class TimelinePresenter @Inject constructor(
         val prevMostRecentItemId = rememberSaveable { mutableStateOf<String?>(null) }
         val hasNewItems = remember { mutableStateOf(false) }
 
-        fun CoroutineScope.sendReadReceiptIfNeeded(firstVisibleIndex: Int) = launch(dispatchers.computation) {
-            // Get last valid EventId seen by the user, as the first index might refer to a Virtual item
-            val eventId = getLastEventIdBeforeOrAt(firstVisibleIndex, timelineItems)
-            if (eventId != null && firstVisibleIndex <= lastReadReceiptIndex && eventId != lastReadReceiptId) {
-                lastReadReceiptIndex = firstVisibleIndex
-                lastReadReceiptId = eventId
-                timeline.sendReadReceipt(eventId)
-            }
-        }
-
         fun handleEvents(event: TimelineEvents) {
             when (event) {
                 TimelineEvents.LoadMore -> localScope.paginateBackwards()
@@ -91,7 +80,12 @@ class TimelinePresenter @Inject constructor(
                     if (event.firstIndex == 0) {
                         hasNewItems.value = false
                     }
-                    appScope.sendReadReceiptIfNeeded(event.firstIndex)
+                    appScope.sendReadReceiptIfNeeded(
+                        firstVisibleIndex = event.firstIndex,
+                        timelineItems = timelineItems,
+                        lastReadReceiptIndex = lastReadReceiptIndex,
+                        lastReadReceiptId = lastReadReceiptId
+                    )
                 }
             }
         }
@@ -122,6 +116,11 @@ class TimelinePresenter @Inject constructor(
         )
     }
 
+    /**
+     * This method compute the hasNewItem state passed as a [MutableState] each time the timeline items size changes.
+     * Basically, if we got new timeline event from sync or local, either from us or another user, we update the state so we tell we have new items.
+     * The state never goes back to false from this method, but need to be reset from somewhere else.
+     */
     private suspend fun computeHasNewItems(
         timelineItems: ImmutableList<TimelineItem>,
         prevMostRecentItemId: MutableState<String?>,
@@ -130,13 +129,31 @@ class TimelinePresenter @Inject constructor(
         val newMostRecentItem = timelineItems.firstOrNull()
         val prevMostRecentItemIdValue = prevMostRecentItemId.value
         val newMostRecentItemId = newMostRecentItem?.identifier()
-        hasNewItemsState.value = prevMostRecentItemIdValue != null &&
+        val hasNewItems = prevMostRecentItemIdValue != null &&
             newMostRecentItem is TimelineItem.Event &&
             newMostRecentItem.origin != TimelineItemEventOrigin.PAGINATION &&
             newMostRecentItemId != prevMostRecentItemIdValue
+        if (hasNewItems) {
+            hasNewItemsState.value = true
+        }
         prevMostRecentItemId.value = newMostRecentItemId
     }
 
+    private fun CoroutineScope.sendReadReceiptIfNeeded(
+        firstVisibleIndex: Int,
+        timelineItems: ImmutableList<TimelineItem>,
+        lastReadReceiptIndex: MutableState<Int>,
+        lastReadReceiptId: MutableState<EventId?>,
+    ) = launch(dispatchers.computation) {
+        // Get last valid EventId seen by the user, as the first index might refer to a Virtual item
+        val eventId = getLastEventIdBeforeOrAt(firstVisibleIndex, timelineItems)
+        if (eventId != null && firstVisibleIndex <= lastReadReceiptIndex.value && eventId != lastReadReceiptId.value) {
+            lastReadReceiptIndex.value = firstVisibleIndex
+            lastReadReceiptId.value = eventId
+            timeline.sendReadReceipt(eventId)
+        }
+    }
+
     private fun getLastEventIdBeforeOrAt(index: Int, items: ImmutableList<TimelineItem>): EventId? {
         for (item in items.subList(index, items.count())) {
             if (item is TimelineItem.Event) {