From 7beda7990ff98fcd09bf88e722a2d94ce6fe6158 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Thu, 19 Mar 2026 14:10:12 +0100 Subject: [PATCH] fix(timeline): don't send a main-thread receipt on a thread aggregation --- .../src/timeline/controller/aggregations.rs | 6 ++ .../src/timeline/controller/mod.rs | 36 ++++++-- .../integration/timeline/read_receipts.rs | 84 ++++++++++++++++++- 3 files changed, 116 insertions(+), 10 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs b/crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs index 4a398ca9d..5418f2c36 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/aggregations.rs +++ b/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 diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 3abc3d816..0d35df047 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/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 } }) diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs b/crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs index 0f0fd45f4..8f3f5a68e 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/read_receipts.rs +++ b/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;