fix(timeline): when loading initial receipts for main|unthreaded, load both kinds

This is a fix, because some other code elsewhere will use both kinds of
receipts whenever they're received over sync. The code that's modified
in this patch is called for the initial load of receipts, that happens
whenever we see a new event. Since the two code paths were not doing the
same thing, this would affect the displayed receipts, depending on
whether we received them during sync, or after loading the timeline for
the first time.

Author: bnjbvr

crates/matrix-sdk-ui/src/timeline/traits.rs

@@ -232,34 +232,60 @@ impl RoomDataProvider for Room {
         event_id: &'a EventId,
         receipt_thread: ReceiptThread,
     ) -> IndexMap<OwnedUserId, Receipt> {
-        let mut result = match self
-            .load_event_receipts(ReceiptType::Read, receipt_thread.clone(), event_id)
-            .await
-        {
-            Ok(receipts) => receipts.into_iter().collect(),
-            Err(e) => {
-                error!(?event_id, ?receipt_thread, "Failed to get read receipts for event: {e}");
-                IndexMap::new()
-            }
-        };
+        if matches!(receipt_thread, ReceiptThread::Unthreaded | ReceiptThread::Main) {
+            // If the requested receipt thread is unthreaded or main, we maintain maximal
+            // compatibility with clients using either unthreaded or main-thread read
+            // receipts by allowing both here.
 
-        if receipt_thread == ReceiptThread::Unthreaded {
-            // Include the main thread receipts as well, to be maximally compatible with
-            // clients using either the unthreaded or main thread receipt type.
-            let main_thread_receipts = match self
+            // First, load the main receipts.
+            let mut result = match self
                 .load_event_receipts(ReceiptType::Read, ReceiptThread::Main, event_id)
                 .await
+            {
+                Ok(receipts) => receipts.into_iter().collect(),
+                Err(e) => {
+                    error!(?event_id, "Failed to get main thread read receipts for event: {e}");
+                    IndexMap::new()
+                }
+            };
+
+            // Then, load the unthreaded receipts.
+            let unthreaded_receipts = match self
+                .load_event_receipts(ReceiptType::Read, ReceiptThread::Unthreaded, event_id)
+                .await
             {
                 Ok(receipts) => receipts,
                 Err(e) => {
                     error!(?event_id, "Failed to get main thread read receipts for event: {e}");
                     Vec::new()
                 }
             };
-            result.extend(main_thread_receipts);
+
+            // Only insert unthreaded receipts that are more recent. Ideally we'd compare
+            // the receipted event position, but we don't have access to that
+            // here.
+            for (user_id, unthreaded_receipt) in unthreaded_receipts {
+                if let Some(main_receipt) = result.get(&user_id) {
+                    if unthreaded_receipt.ts > main_receipt.ts {
+                        result.insert(user_id, unthreaded_receipt);
+                    }
+                } else {
+                    result.insert(user_id, unthreaded_receipt);
+                }
+            }
+
+            return result;
         }
 
-        result
+        // In all other cases, return what's requested, and only that (threaded
+        // receipts).
+        match self.load_event_receipts(ReceiptType::Read, receipt_thread.clone(), event_id).await {
+            Ok(receipts) => receipts.into_iter().collect(),
+            Err(e) => {
+                error!(?event_id, ?receipt_thread, "Failed to get read receipts for event: {e}");
+                IndexMap::new()
+            }
+        }
     }
 
     async fn push_context(&self) -> Option<PushContext> {