fix(timeline): don't send a main-thread receipt on a thread aggregation

crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs

@@ -718,6 +718,12 @@ impl Aggregations {
         self.inverted_map.insert(to, target);
         true
     }
+
+    /// Returns the id of the event this aggregation relates to, if it's a known
+    /// aggregation.
+    pub fn is_aggregation_of(&self, item: &TimelineEventItemId) -> Option<&TimelineEventItemId> {
+        self.inverted_map.get(item)
+    }
 }
 
 /// Look at all the edits of a given event, and apply the most recent one, if

crates/matrix-sdk-ui/src/timeline/controller/mod.rs

@@ -1588,22 +1588,42 @@ impl TimelineController {
                 // For event-focused timelines, filtering is handled in the event cache layer.
                 false
             }
-            _ => true,
+            TimelineFocusKind::PinnedEvents => true,
         };
 
-        // In some timelines, threaded events are added to the `AllRemoteEvents`
-        // collection since they need to be taken into account to calculate read
-        // receipts, but we don't want to actually take them into account for returning
-        // the latest event id since they're not visibly in the timeline
         state
             .items
             .all_remote_events()
             .iter()
             .rev()
-            .filter_map(|item| {
-                if !filter_out_thread_events || item.thread_root_id.is_none() {
-                    Some(item.event_id.clone())
+            .filter_map(|event_meta| {
+                if !filter_out_thread_events {
+                    // For an unthreaded timeline, the last event is always the latest event.
+                    Some(event_meta.event_id.clone())
+                } else if event_meta.thread_root_id.is_none() {
+                    // For the main-thread timeline, only non-threaded events are valid candidates
+                    // for the latest event.
+                    //
+                    // But! An event could be an aggregation that relate to an in-thread
+                    // event. In this case, it's not a valid latest event.
+                    if let Some(TimelineEventItemId::EventId(target_event_id)) =
+                        state.meta.aggregations.is_aggregation_of(&TimelineEventItemId::EventId(
+                            event_meta.event_id.clone(),
+                        ))
+                        && let Some(target_meta) =
+                            state.items.all_remote_events().get_by_event_id(target_event_id)
+                        && target_meta.thread_root_id.is_some()
+                    {
+                        // This event is an aggregation of an in-thread event, so skip it.
+                        None
+                    } else {
+                        // Not in a thread, and not the aggregation of an in-thread event, so it's
+                        // a valid candidate for the latest event.
+                        Some(event_meta.event_id.clone())
+                    }
                 } else {
+                    // An in-thread event, when we're filtering out threaded events, is never a
+                    // valid candidate for the latest event.
                     None
                 }
             })

crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs

@@ -19,7 +19,7 @@ use assert_matches2::assert_let;
 use eyeball_im::VectorDiff;
 use futures_util::StreamExt;
 use matrix_sdk::{
-    assert_let_timeout,
+    ThreadingSupport, assert_let_timeout,
     room::Receipts,
     test_utils::mocks::{MatrixMockServer, RoomMessagesResponseTemplate},
 };
@@ -34,7 +34,10 @@ use ruma::{
     events::{
         AnySyncMessageLikeEvent, AnySyncTimelineEvent, RoomAccountDataEventType,
         receipt::{ReceiptThread, ReceiptType as EventReceiptType},
-        room::message::{MessageType, RoomMessageEventContent, SyncRoomMessageEvent},
+        room::message::{
+            MessageType, RoomMessageEventContent, RoomMessageEventContentWithoutRelation,
+            SyncRoomMessageEvent,
+        },
     },
     owned_event_id, room_id,
     room_version_rules::RoomVersionRules,
@@ -1175,6 +1178,83 @@ async fn test_mark_as_read() {
     assert!(has_sent);
 }
 
+#[async_test]
+async fn test_mark_as_read_after_threaded_edit() {
+    let server = MatrixMockServer::new().await;
+
+    // Considering a client that's thread-aware,
+    let client = server
+        .client_builder()
+        .on_builder(|builder| {
+            builder.with_threading_support(ThreadingSupport::Enabled { with_subscriptions: false })
+        })
+        .build()
+        .await;
+
+    let room_id = room_id!("!a98sd12bjh:example.org");
+    let room = server.sync_joined_room(&client, room_id).await;
+
+    server.mock_room_state_encryption().plain().mount().await;
+
+    // On a main-thread timeline,
+    let timeline = room
+        .timeline_builder()
+        .with_focus(TimelineFocus::Live { hide_threaded_events: true })
+        .build()
+        .await
+        .unwrap();
+
+    let (initial_events, mut stream) = timeline.subscribe().await;
+    assert!(initial_events.is_empty());
+
+    let f = EventFactory::new().sender(*BOB);
+    let thread_root = event_id!("$thread_root");
+    let threaded_reply = event_id!("$threaded_reply");
+
+    // When the latest event is an edit of an in-thread event,
+    server
+        .sync_room(
+            &client,
+            JoinedRoomBuilder::new(room_id)
+                .add_timeline_event(
+                    f.text_msg("I like big Rust and I cannot lie").event_id(thread_root),
+                )
+                .add_timeline_event(
+                    f.text_msg("threaded reply")
+                        .in_thread(thread_root, thread_root)
+                        .event_id(threaded_reply),
+                )
+                .add_timeline_event(
+                    f.text_msg("* edit of the threaded reply")
+                        .edit(
+                            threaded_reply,
+                            RoomMessageEventContentWithoutRelation::text_plain(
+                                "edit of the threaded reply",
+                            ),
+                        )
+                        .event_id(event_id!("$edit_of_threaded_reply")),
+                ),
+        )
+        .await;
+
+    // Let the timeline react to the new events.
+    assert_let_timeout!(Some(updates) = stream.next());
+    // New thread root, thread summary update + date insertion.
+    assert_eq!(updates.len(), 3);
+
+    server
+        .mock_send_receipt(CreateReceiptType::Read)
+        .match_event_id(thread_root)
+        .ok()
+        .mock_once()
+        .mount()
+        .await;
+
+    // I can mark the room as read.
+    let has_sent = timeline.mark_as_read(CreateReceiptType::Read).await.unwrap();
+    assert!(has_sent);
+}
+
 #[async_test]
 async fn test_mark_as_read_with_unread_flag() {
     let server = MatrixMockServer::new().await;