From f409d4b3b6efabd339035e3e2abe7535f22830a1 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 21 Jul 2025 13:55:21 +0200 Subject: [PATCH 1/6] feat(timeline): `Timeline::send()` automatically includes the thread relationship for thread foci --- crates/matrix-sdk-ui/CHANGELOG.md | 6 + crates/matrix-sdk-ui/src/timeline/mod.rs | 71 ++++++++-- .../tests/integration/timeline/thread.rs | 133 ++++++++++++++++-- labs/multiverse/src/widgets/room_view/mod.rs | 82 ++--------- 4 files changed, 197 insertions(+), 95 deletions(-) diff --git a/crates/matrix-sdk-ui/CHANGELOG.md b/crates/matrix-sdk-ui/CHANGELOG.md index 0281af412ed..8952bc21a43 100644 --- a/crates/matrix-sdk-ui/CHANGELOG.md +++ b/crates/matrix-sdk-ui/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes to this project will be documented in this file. ## [Unreleased] - ReleaseDate +### Features + +- `Timeline::send()` will now automatically fill the thread relationship, if the timeline has a + thread focus, and the sent event doesn't have a prefilled `relates_to` field (i.e. a relationship). + ([5427](https://github.com/matrix-org/matrix-rust-sdk/pull/5427)) + ### Refactor - [**breaking**] The MSRV has been bumped to Rust 1.88. diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index cc68a3e1261..d24c579018a 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -33,7 +33,11 @@ use matrix_sdk::{ deserialized_responses::TimelineEvent, event_cache::{EventCacheDropHandles, RoomEventCache}, executor::JoinHandle, - room::{Receipts, Room, edit::EditedContent, reply::Reply}, + room::{ + Receipts, Room, + edit::EditedContent, + reply::{EnforceThread, Reply}, + }, send_queue::{RoomSendQueueError, SendHandle}, }; use mime::Mime; @@ -46,7 +50,7 @@ use ruma::{ poll::unstable_start::{NewUnstablePollStartEventContent, UnstablePollStartEventContent}, receipt::{Receipt, ReceiptThread}, room::{ - message::RoomMessageEventContentWithoutRelation, + message::{ReplyWithinThread, RoomMessageEventContentWithoutRelation}, pinned_events::RoomPinnedEventsEventContent, }, }, @@ -270,18 +274,65 @@ impl Timeline { /// If sending the message fails, the local echo item will change its /// `send_state` to [`EventSendState::SendingFailed`]. /// + /// This will do the right thing in the presence of threads: + /// - if this timeline is not focused on a thread, then it will send the + /// event as is. + /// - if this is a threaded timeline, and the event to send is a room + /// message without a relationship, it will automatically mark it as a + /// thread reply with the correct reply fallback, and send it. + /// /// # Arguments /// /// * `content` - The content of the message event. - /// - /// [`MessageLikeUnsigned`]: ruma::events::MessageLikeUnsigned - /// [`SyncMessageLikeEvent`]: ruma::events::SyncMessageLikeEvent #[instrument(skip(self, content), fields(room_id = ?self.room().room_id()))] - pub async fn send( - &self, - content: AnyMessageLikeEventContent, - ) -> Result { - self.room().send_queue().send(content).await + pub async fn send(&self, content: AnyMessageLikeEventContent) -> Result { + // If this is a room event we're sending in a threaded timeline, we add the + // thread relation ourselves. + if let AnyMessageLikeEventContent::RoomMessage(ref room_msg_content) = content + && room_msg_content.relates_to.is_none() + && let Some(thread_root) = self.controller.thread_root() + { + // The latest event id is used for the reply-to fallback, for clients which + // don't handle threads. It should be correctly set to the latest + // event in the thread, which the timeline instance might or might + // not know about; in this case, we do a best effort of filling it, and resort + // to using the thread root if we don't know about any event. + // + // Note: we could trigger a back-pagination if the timeline is empty, and wait + // for the results, if the timeline is too often empty. + let latest_event_id = self + .controller + .items() + .await + .iter() + .rev() + .find_map(|item| { + if let TimelineItemKind::Event(event) = item.kind() { + event.event_id().map(ToOwned::to_owned) + } else { + None + } + }) + .unwrap_or(thread_root); + + let content = self + .room() + .make_reply_event( + // Note: this `.into()` gets rid of the relation, but we've checked previously + // that the `relates_to` field wasn't set. + room_msg_content.clone().into(), + Reply { + event_id: latest_event_id, + enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No), + }, + ) + .await?; + + Ok(self.room().send_queue().send(content.into()).await?) + } else { + // Otherwise, we send the message as is. + Ok(self.room().send_queue().send(content).await?) + } } /// Send a reply to the given event. diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs index e026c7f7b22..e0a9dc7847d 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs @@ -19,7 +19,6 @@ use eyeball_im::VectorDiff; use futures_util::StreamExt as _; use matrix_sdk::{ assert_let_timeout, - room::reply::{EnforceThread, Reply}, test_utils::mocks::{MatrixMockServer, RoomRelationsResponseTemplate}, }; use matrix_sdk_test::{ @@ -32,8 +31,9 @@ use ruma::{ api::client::receipt::create_receipt::v3::ReceiptType as SendReceiptType, event_id, events::{ + AnySyncTimelineEvent, receipt::{ReceiptThread, ReceiptType}, - room::message::{ReplyWithinThread, RoomMessageEventContentWithoutRelation}, + room::message::{Relation, ReplacementMetadata, RoomMessageEventContent}, }, owned_event_id, room_id, user_id, }; @@ -732,17 +732,12 @@ async fn test_thread_timeline_gets_local_echoes() { // update. let sent_event_id = event_id!("$sent_msg"); server.mock_room_state_encryption().plain().mount().await; - server.mock_room_send().ok(sent_event_id).mock_once().mount().await; - timeline - .send_reply( - RoomMessageEventContentWithoutRelation::text_plain("hello to you too!"), - Reply { - event_id: threaded_event_id.to_owned(), - enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No), - }, - ) - .await - .unwrap(); + + let (event_receiver, mock_builder) = + server.mock_room_send().ok_with_capture(sent_event_id, *ALICE); + mock_builder.mock_once().mount().await; + + timeline.send(RoomMessageEventContent::text_plain("hello to you too!").into()).await.unwrap(); // I get the local echo for the in-thread event. assert_let_timeout!(Some(timeline_updates) = stream.next()); @@ -753,6 +748,11 @@ async fn test_thread_timeline_gets_local_echoes() { assert!(event_item.is_local_echo()); assert!(event_item.event_id().is_none()); + // The thread information is properly filled. + let msglike = event_item.content().as_msglike().unwrap(); + assert_eq!(msglike.thread_root.as_ref(), Some(&thread_root_event_id)); + assert!(msglike.in_reply_to.is_none()); + // Then the local echo morphs into a sent local echo. assert_let_timeout!(Some(timeline_updates) = stream.next()); assert_eq!(timeline_updates.len(), 1); @@ -760,12 +760,29 @@ async fn test_thread_timeline_gets_local_echoes() { assert_let!(VectorDiff::Set { index: 2, value } = &timeline_updates[0]); let event_item = value.as_event().unwrap(); assert_eq!(event_item.event_id(), Some(sent_event_id)); - assert_eq!(event_item.event_id(), Some(sent_event_id)); assert!(event_item.content().reactions().unwrap().is_empty()); // Then nothing else. assert_pending!(stream); + // The raw event includes the correctly reply-to fallback. + { + let raw_event = event_receiver.await.unwrap(); + let event = raw_event.deserialize().unwrap(); + assert_let!( + AnySyncTimelineEvent::MessageLike( + ruma::events::AnySyncMessageLikeEvent::RoomMessage(event) + ) = event + ); + let event = event.as_original().unwrap(); + assert_let!(Some(Relation::Thread(thread)) = event.content.relates_to.clone()); + assert_eq!(thread.event_id, thread_root_event_id); + assert!(thread.is_falling_back); + // The reply-to fallback is set to the latest in-thread event, not the thread + // root. + assert_eq!(thread.in_reply_to.unwrap().event_id, threaded_event_id); + } + // If I send a reaction for the in-thread event, the timeline gets updated, even // though the reaction doesn't mention the thread directly. server.mock_room_send().ok(event_id!("$reaction_id")).mock_once().mount().await; @@ -791,6 +808,94 @@ async fn test_thread_timeline_gets_local_echoes() { assert_pending!(stream); } +#[async_test] +async fn test_thread_timeline_can_send_edit() { + // If I send an edit to a threaded timeline, it just works (aka the system to + // set the threaded relationship doesn't kick in, since there's already a + // relationship). + + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let room_id = room_id!("!a:b.c"); + let thread_root_event_id = owned_event_id!("$root"); + let threaded_event_id = event_id!("$threaded_event"); + + let room = server.sync_joined_room(&client, room_id).await; + + let timeline = room + .timeline_builder() + .with_focus(TimelineFocus::Thread { root_event_id: thread_root_event_id.clone() }) + .build() + .await + .unwrap(); + + let (initial_items, mut stream) = timeline.subscribe().await; + + // At first, the timeline is empty. + assert!(initial_items.is_empty()); + assert_pending!(stream); + + // Start the timeline with an in-thread event. + let f = EventFactory::new(); + server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id).add_timeline_event( + f.text_msg("hello world") + .sender(*ALICE) + .event_id(threaded_event_id) + .in_thread(&thread_root_event_id, threaded_event_id) + .server_ts(MilliSecondsSinceUnixEpoch::now()), + ), + ) + .await; + + // Sanity check: I receive the event and the date divider. + assert_let_timeout!(Some(timeline_updates) = stream.next()); + assert_eq!(timeline_updates.len(), 2); + + // If I send an edit to an in-thread event, the timeline receives an update. + // Note: it's a bit far fetched to send an edit without using + // `Timeline::edit()`, but since it's possible… + let sent_event_id = event_id!("$sent_msg"); + server.mock_room_state_encryption().plain().mount().await; + + // No mock_once here: this endpoint may or may not be called, depending on + // timing of the end of the test. + server.mock_room_send().ok(sent_event_id).mount().await; + + timeline + .send( + RoomMessageEventContent::text_plain("bonjour monde") + .make_replacement( + ReplacementMetadata::new(threaded_event_id.to_owned(), None), + None, + ) + .into(), + ) + .await + .unwrap(); + + // I get the local echo for the in-thread event. + assert_let_timeout!(Some(timeline_updates) = stream.next()); + assert_eq!(timeline_updates.len(), 1); + + assert_let!(VectorDiff::Set { index: 1, value } = &timeline_updates[0]); + let event_item = value.as_event().unwrap(); + + // The thread information is (still) present. + let msglike = event_item.content().as_msglike().unwrap(); + assert_eq!(msglike.thread_root.as_ref(), Some(&thread_root_event_id)); + assert!(msglike.in_reply_to.is_none()); + + // Text is eagerly updated. + assert_eq!(msglike.as_message().unwrap().body(), "bonjour monde"); + + // Then we're done. + assert_pending!(stream); +} + #[async_test] async fn test_sending_read_receipt_with_no_events_doesnt_unset_read_flag() { // If a thread timeline has no events, then marking it as read doesn't unset the diff --git a/labs/multiverse/src/widgets/room_view/mod.rs b/labs/multiverse/src/widgets/room_view/mod.rs index 7f48cb1f8c9..a22b478edf7 100644 --- a/labs/multiverse/src/widgets/room_view/mod.rs +++ b/labs/multiverse/src/widgets/room_view/mod.rs @@ -8,13 +8,9 @@ use invited_room::InvitedRoomView; use matrix_sdk::{ Client, Room, RoomState, locks::Mutex, - room::reply::{EnforceThread::Threaded, Reply}, ruma::{ - OwnedEventId, OwnedRoomId, RoomId, UserId, - api::client::receipt::create_receipt::v3::ReceiptType, - events::room::message::{ - ReplyWithinThread, RoomMessageEventContent, RoomMessageEventContentWithoutRelation, - }, + OwnedRoomId, RoomId, UserId, api::client::receipt::create_receipt::v3::ReceiptType, + events::room::message::RoomMessageEventContent, }, }; use matrix_sdk_ui::{ @@ -56,8 +52,6 @@ enum TimelineKind { Thread { room: OwnedRoomId, - /// The root event ID of the thread. - root: OwnedEventId, /// The threaded-focused timeline for this thread. timeline: Arc>>, /// Items in the thread timeline (to avoid recomputing them every single @@ -150,7 +144,7 @@ impl RoomView { let i = items.clone(); let t = thread_timeline.clone(); - let root = root_event_id.clone(); + let root = root_event_id; let r = room.clone(); let task = spawn(async move { let timeline = TimelineBuilder::new(&r) @@ -178,7 +172,6 @@ impl RoomView { self.kind = TimelineKind::Thread { room: room.room_id().to_owned(), - root: root_event_id, timeline: thread_timeline, items, task, @@ -492,70 +485,17 @@ impl RoomView { } async fn send_message(&mut self, message: String) { - match &self.kind { - TimelineKind::Room { .. } => { - if let Some(sdk_timeline) = self.get_selected_timeline() { - match sdk_timeline - .send(RoomMessageEventContent::text_plain(message).into()) - .await - { - Ok(_) => { - self.input.clear(); - } - Err(err) => { - self.status_handle - .set_message(format!("error when sending event: {err}")); - } - } - } else { - self.status_handle.set_message("missing timeline for room".to_owned()); + if let Some(sdk_timeline) = self.get_selected_timeline() { + match sdk_timeline.send(RoomMessageEventContent::text_plain(message).into()).await { + Ok(_) => { + self.input.clear(); } - } - - TimelineKind::Thread { root, .. } => { - let root = root.clone(); - if let Some(sdk_timeline) = self.get_selected_timeline() { - // Pretend a reply to the previous item that can be - // replied to. - let prev_item_event_id = { - let items = sdk_timeline.items().await; - items - .iter() - .rev() - .find_map(|item| { - let event_item = item.as_event()?; - if event_item.can_be_replied_to() { - event_item.event_id().map(ToOwned::to_owned) - } else { - None - } - }) - .unwrap_or(root) - }; - - // TODO: ogod this is awful - match sdk_timeline - .send_reply( - RoomMessageEventContentWithoutRelation::text_plain(message), - Reply { - event_id: prev_item_event_id, - enforce_thread: Threaded(ReplyWithinThread::No), - }, - ) - .await - { - Ok(_) => { - self.input.clear(); - } - Err(err) => { - self.status_handle - .set_message(format!("error when sending event: {err}")); - } - } - } else { - self.status_handle.set_message("missing timeline for room".to_owned()); + Err(err) => { + self.status_handle.set_message(format!("error when sending event: {err}")); } } + } else { + self.status_handle.set_message("missing timeline for room".to_owned()); } } From 763fa8efd944b4026fc7b61e5a6445950abd14e2 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 21 Jul 2025 13:55:40 +0200 Subject: [PATCH 2/6] feat!(timeline): `Timeline::send_reply()` automatically includes the thread relationship too --- bindings/matrix-sdk-ffi/CHANGELOG.md | 5 ++ bindings/matrix-sdk-ffi/src/timeline/mod.rs | 11 ++-- crates/matrix-sdk-ui/CHANGELOG.md | 5 ++ crates/matrix-sdk-ui/src/timeline/mod.rs | 28 +++++--- .../tests/integration/timeline/replies.rs | 64 ++++++++----------- 5 files changed, 64 insertions(+), 49 deletions(-) diff --git a/bindings/matrix-sdk-ffi/CHANGELOG.md b/bindings/matrix-sdk-ffi/CHANGELOG.md index 3635b2af6cf..1ebbf998195 100644 --- a/bindings/matrix-sdk-ffi/CHANGELOG.md +++ b/bindings/matrix-sdk-ffi/CHANGELOG.md @@ -8,6 +8,11 @@ All notable changes to this project will be documented in this file. ### Features: +- [**breaking**] [`Timeline::send_reply()`] now automatically fills in the thread relationship, + based on the timeline focus. As a result, it only takes an `OwnedEventId` parameter, instead of + the `Reply` type. The proper way to start a thread is now thus to create a threaded-focused + timeline, and then use `Timeline::send()`. + ([5427](https://github.com/matrix-org/matrix-rust-sdk/pull/5427)) - Add `HomeserverLoginDetails::supports_sso_login` for legacy SSO support information. This is primarily for Element X to give a dedicated error message in case it connects a homeserver with only this method available. diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index 641560f8999..06e2cd192e2 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -529,9 +529,10 @@ impl Timeline { pub async fn send_reply( &self, msg: Arc, - reply_params: ReplyParameters, + event_id: String, ) -> Result<(), ClientError> { - self.inner.send_reply((*msg).clone(), reply_params.try_into()?).await?; + let event_id = EventId::parse(&event_id).map_err(|_| RoomError::InvalidRepliedToEventId)?; + self.inner.send_reply((*msg).clone(), event_id).await?; Ok(()) } @@ -585,7 +586,7 @@ impl Timeline { description: Option, zoom_level: Option, asset_type: Option, - reply_params: Option, + replied_to_event_id: Option, ) -> Result<(), ClientError> { let mut location_event_message_content = LocationMessageEventContent::new(body, geo_uri.clone()); @@ -604,8 +605,8 @@ impl Timeline { MessageType::Location(location_event_message_content), ); - if let Some(reply_params) = reply_params { - self.send_reply(Arc::new(room_message_event_content), reply_params).await + if let Some(replied_to_event_id) = replied_to_event_id { + self.send_reply(Arc::new(room_message_event_content), replied_to_event_id).await } else { self.send(Arc::new(room_message_event_content)).await?; Ok(()) diff --git a/crates/matrix-sdk-ui/CHANGELOG.md b/crates/matrix-sdk-ui/CHANGELOG.md index 8952bc21a43..f40bb5120e9 100644 --- a/crates/matrix-sdk-ui/CHANGELOG.md +++ b/crates/matrix-sdk-ui/CHANGELOG.md @@ -8,6 +8,11 @@ All notable changes to this project will be documented in this file. ### Features +- [**breaking**] [`Timeline::send_reply()`] now automatically fills in the thread relationship, + based on the timeline focus. As a result, it only takes an `OwnedEventId` parameter, instead of + the `Reply` type. The proper way to start a thread is now thus to create a threaded-focused + timeline, and then use `Timeline::send()`. + ([5427](https://github.com/matrix-org/matrix-rust-sdk/pull/5427)) - `Timeline::send()` will now automatically fill the thread relationship, if the timeline has a thread focus, and the sent event doesn't have a prefilled `relates_to` field (i.e. a relationship). ([5427](https://github.com/matrix-org/matrix-rust-sdk/pull/5427)) diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index d24c579018a..0e545c4e63b 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -339,26 +339,38 @@ impl Timeline { /// /// Currently it only supports events with an event ID and JSON being /// available (which can be removed by local redactions). This is subject to - /// change. Please check [`EventTimelineItem::can_be_replied_to`] to decide - /// whether to render a reply button. + /// change. Use [`EventTimelineItem::can_be_replied_to`] to decide whether + /// to render a reply button. /// /// The sender will be added to the mentions of the reply if /// and only if the event has not been written by the sender. /// - /// # Arguments + /// This will do the right thing in the presence of threads: + /// - if this timeline is not focused on a thread, then it will forward the + /// thread relationship of the replied-to event, if present. + /// - if this is a threaded timeline, it will mark the reply as an in-thread + /// reply. /// - /// * `content` - The content of the reply + /// # Arguments /// - /// * `event_id` - The ID of the event to reply to + /// * `content` - The content of the reply. /// - /// * `enforce_thread` - Whether to enforce a thread relation on the reply + /// * `event_id` - The ID of the event to reply to. #[instrument(skip(self, content))] pub async fn send_reply( &self, content: RoomMessageEventContentWithoutRelation, - reply: Reply, + replied_to: OwnedEventId, ) -> Result<(), Error> { - let content = self.room().make_reply_event(content, reply).await?; + let enforce_thread = if self.controller.thread_root().is_some() { + EnforceThread::Threaded(ReplyWithinThread::Yes) + } else { + EnforceThread::MaybeThreaded + }; + let content = self + .room() + .make_reply_event(content, Reply { event_id: replied_to, enforce_thread }) + .await?; self.send(content.into()).await?; Ok(()) } diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs b/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs index f4dbc079ef2..62604824adf 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs @@ -4,17 +4,14 @@ use assert_matches::assert_matches; use assert_matches2::assert_let; use eyeball_im::VectorDiff; use futures_util::StreamExt; -use matrix_sdk::{ - room::reply::{EnforceThread, Reply}, - test_utils::mocks::MatrixMockServer, -}; +use matrix_sdk::test_utils::mocks::MatrixMockServer; use matrix_sdk_base::timeout::timeout; use matrix_sdk_test::{ ALICE, BOB, CAROL, JoinedRoomBuilder, async_test, event_factory::EventFactory, }; use matrix_sdk_ui::timeline::{ Error as TimelineError, EventSendState, MsgLikeContent, MsgLikeKind, RoomExt, TimelineDetails, - TimelineEventItemId, TimelineItemContent, + TimelineEventItemId, TimelineFocus, TimelineItemContent, }; use ruma::{ MilliSecondsSinceUnixEpoch, UInt, event_id, @@ -27,7 +24,7 @@ use ruma::{ encrypted::{ EncryptedEventScheme, MegolmV1AesSha2ContentInit, RoomEncryptedEventContent, }, - message::{Relation, ReplyWithinThread, RoomMessageEventContentWithoutRelation}, + message::{Relation, RoomMessageEventContent, RoomMessageEventContentWithoutRelation}, }, sticker::{StickerEventContent, StickerMediaSource}, }, @@ -706,10 +703,7 @@ async fn test_send_reply() { timeline .send_reply( RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"), - Reply { - event_id: event_id_from_bob.into(), - enforce_thread: EnforceThread::MaybeThreaded, - }, + event_id_from_bob.into(), ) .await .unwrap(); @@ -808,10 +802,7 @@ async fn test_send_reply_to_self() { timeline .send_reply( RoomMessageEventContentWithoutRelation::text_plain("Replying to self"), - Reply { - event_id: event_id_from_self.into(), - enforce_thread: EnforceThread::MaybeThreaded, - }, + event_id_from_self.into(), ) .await .unwrap(); @@ -876,7 +867,7 @@ async fn test_send_reply_to_threaded() { timeline .send_reply( RoomMessageEventContentWithoutRelation::text_plain("Hello, Bob!"), - Reply { event_id: event_id_1.into(), enforce_thread: EnforceThread::MaybeThreaded }, + event_id_1.into(), ) .await .unwrap(); @@ -978,10 +969,7 @@ async fn test_send_reply_with_event_id() { timeline .send_reply( RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"), - Reply { - event_id: event_id_from_bob.into(), - enforce_thread: EnforceThread::MaybeThreaded, - }, + event_id_from_bob.into(), ) .await .unwrap(); @@ -1063,14 +1051,16 @@ async fn test_send_reply_enforce_thread() { .mount() .await; - timeline - .send_reply( - RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"), - Reply { - event_id: event_id_from_bob.into(), - enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No), - }, - ) + // Starting a thread. + let thread_timeline = room + .timeline_builder() + .with_focus(TimelineFocus::Thread { root_event_id: event_id_from_bob.to_owned() }) + .build() + .await + .unwrap(); + + thread_timeline + .send(RoomMessageEventContent::text_plain("Replying to Bob").into()) .await .unwrap(); @@ -1162,13 +1152,18 @@ async fn test_send_reply_enforce_thread_is_reply() { .mount() .await; - timeline + // Starting a thread, and making an explicit reply inside the thread. + let thread_timeline = room + .timeline_builder() + .with_focus(TimelineFocus::Thread { root_event_id: event_id_from_bob.to_owned() }) + .build() + .await + .unwrap(); + + thread_timeline .send_reply( RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"), - Reply { - event_id: event_id_from_bob.into(), - enforce_thread: EnforceThread::Threaded(ReplyWithinThread::Yes), - }, + event_id_from_bob.into(), ) .await .unwrap(); @@ -1260,10 +1255,7 @@ async fn test_send_reply_with_event_id_that_is_redacted() { timeline .send_reply( RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"), - Reply { - event_id: redacted_event_id_from_bob.into(), - enforce_thread: EnforceThread::MaybeThreaded, - }, + redacted_event_id_from_bob.into(), ) .await .unwrap(); From eca6d083103218f6e466dedc45b1de849f2db30d Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 21 Jul 2025 18:06:31 +0200 Subject: [PATCH 3/6] feat!(timeline): infer the reply type automatically when sending an attachment --- bindings/matrix-sdk-ffi/src/timeline/mod.rs | 30 +++-- crates/matrix-sdk-ui/CHANGELOG.md | 8 ++ .../src/timeline/controller/mod.rs | 5 + crates/matrix-sdk-ui/src/timeline/futures.rs | 32 +++-- crates/matrix-sdk-ui/src/timeline/mod.rs | 125 +++++++++++------- .../tests/integration/timeline/media.rs | 39 +++--- .../tests/integration/timeline/thread.rs | 5 +- crates/matrix-sdk/src/attachment.rs | 30 ++++- 8 files changed, 176 insertions(+), 98 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index 06e2cd192e2..50a68d2d3d2 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -20,8 +20,7 @@ use eyeball_im::VectorDiff; use futures_util::pin_mut; use matrix_sdk::{ attachment::{ - AttachmentConfig, AttachmentInfo, BaseAudioInfo, BaseFileInfo, BaseImageInfo, - BaseVideoInfo, Thumbnail, + AttachmentInfo, BaseAudioInfo, BaseFileInfo, BaseImageInfo, BaseVideoInfo, Thumbnail, }, deserialized_responses::{ShieldState as SdkShieldState, ShieldStateCode}, event_cache::RoomPaginationStatus, @@ -35,7 +34,7 @@ use matrix_sdk_common::{ stream::StreamExt, }; use matrix_sdk_ui::timeline::{ - self, AttachmentSource, EventItemOrigin, Profile, TimelineDetails, + self, AttachmentConfig, AttachmentSource, EventItemOrigin, Profile, TimelineDetails, TimelineUniqueId as SdkTimelineUniqueId, }; use mime::Mime; @@ -111,19 +110,26 @@ impl Timeline { let mime_str = mime_type.as_ref().ok_or(RoomError::InvalidAttachmentMimeType)?; let mime_type = mime_str.parse::().map_err(|_| RoomError::InvalidAttachmentMimeType)?; + let replied_to_event_id = params + .in_reply_to + .map(EventId::parse) + .transpose() + .map_err(|_| RoomError::InvalidRepliedToEventId)?; let formatted_caption = formatted_body_from( params.caption.as_deref(), params.formatted_caption.map(Into::into), ); - let attachment_config = AttachmentConfig::new() - .thumbnail(thumbnail) - .info(attachment_info) - .caption(params.caption) - .formatted_caption(formatted_caption) - .mentions(params.mentions.map(Into::into)) - .reply(params.reply_params.map(|p| p.try_into()).transpose()?); + let attachment_config = AttachmentConfig { + info: Some(attachment_info), + thumbnail, + caption: params.caption, + formatted_caption, + mentions: params.mentions.map(Into::into), + replied_to: replied_to_event_id, + ..Default::default() + }; let handle = SendAttachmentJoinHandle::new(get_runtime_handle().spawn(async move { let mut request = @@ -205,8 +211,8 @@ pub struct UploadParameters { formatted_caption: Option, /// Optional intentional mentions to be sent with the media. mentions: Option, - /// Optional parameters for sending the media as (threaded) reply. - reply_params: Option, + /// Optional Event ID to reply to. + in_reply_to: Option, /// Should the media be sent with the send queue, or synchronously? /// /// Watching progress only works with the synchronous method, at the moment. diff --git a/crates/matrix-sdk-ui/CHANGELOG.md b/crates/matrix-sdk-ui/CHANGELOG.md index f40bb5120e9..141548b0adf 100644 --- a/crates/matrix-sdk-ui/CHANGELOG.md +++ b/crates/matrix-sdk-ui/CHANGELOG.md @@ -8,6 +8,14 @@ All notable changes to this project will be documented in this file. ### Features +- [**breaking**] [`Timeline::send_attachment()`] now automatically fills in the thread + relationship, based on the timeline focus. As a result, there's a new + `matrix_sdk_ui::timeline::AttachmentConfig` type in town, that has a simplified optional parameter + `replied_to` of type `OwnedEventId` instead of the `Reply` type and that must be used in place of + `matrix_sdk::attachment::AttachmentConfig`. The proper way to start a thread with a media + attachment is now thus to create a threaded-focused timeline, and then use + `Timeline::send_attachment()`. + ([5427](https://github.com/matrix-org/matrix-rust-sdk/pull/5427)) - [**breaking**] [`Timeline::send_reply()`] now automatically fills in the thread relationship, based on the timeline focus. As a result, it only takes an `OwnedEventId` parameter, instead of the `Reply` type. The proper way to start a thread is now thus to create a threaded-focused diff --git a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs index 98fcede14e3..9a642dc56ce 100644 --- a/crates/matrix-sdk-ui/src/timeline/controller/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/controller/mod.rs @@ -591,6 +591,11 @@ impl TimelineController { matches!(&*self.focus, TimelineFocusKind::Live { .. }) } + /// Is this timeline focused on a thread? + pub(super) fn is_threaded(&self) -> bool { + matches!(&*self.focus, TimelineFocusKind::Thread { .. }) + } + pub(super) fn thread_root(&self) -> Option { as_variant!(&*self.focus, TimelineFocusKind::Thread { root_event_id } => root_event_id.clone()) } diff --git a/crates/matrix-sdk-ui/src/timeline/futures.rs b/crates/matrix-sdk-ui/src/timeline/futures.rs index 164de00b50e..05cffc73673 100644 --- a/crates/matrix-sdk-ui/src/timeline/futures.rs +++ b/crates/matrix-sdk-ui/src/timeline/futures.rs @@ -1,12 +1,13 @@ use std::future::IntoFuture; use eyeball::SharedObservable; -use matrix_sdk::{TransmissionProgress, attachment::AttachmentConfig}; +use matrix_sdk::TransmissionProgress; use matrix_sdk_base::boxed_into_future; use mime::Mime; use tracing::{Instrument as _, Span}; use super::{AttachmentSource, Error, Timeline}; +use crate::timeline::AttachmentConfig; pub struct SendAttachment<'a> { timeline: &'a Timeline, @@ -72,17 +73,32 @@ impl<'a> IntoFuture for SendAttachment<'a> { let fut = async move { let (data, filename) = source.try_into_bytes_and_filename()?; + let reply = timeline.infer_reply(config.replied_to).await; + let sdk_config = matrix_sdk::attachment::AttachmentConfig { + txn_id: config.txn_id, + info: config.info, + thumbnail: config.thumbnail, + caption: config.caption, + formatted_caption: config.formatted_caption, + mentions: config.mentions, + reply, + }; + if use_send_queue { - let send_queue = timeline.room().send_queue(); - let fut = send_queue.send_attachment(filename, mime_type, data, config); - fut.await.map_err(|_| Error::FailedSendingAttachment)?; + timeline + .room() + .send_queue() + .send_attachment(filename, mime_type, data, sdk_config) + .await + .map_err(|_| Error::FailedSendingAttachment)?; } else { - let fut = timeline + timeline .room() - .send_attachment(filename, &mime_type, data, config) + .send_attachment(filename, &mime_type, data, sdk_config) .with_send_progress_observable(send_progress) - .store_in_cache(); - fut.await.map_err(|_| Error::FailedSendingAttachment)?; + .store_in_cache() + .await + .map_err(|_| Error::FailedSendingAttachment)?; } Ok(()) diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 0e545c4e63b..6f9f5f3441f 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -25,11 +25,9 @@ use eyeball_im::VectorDiff; use futures::SendGallery; use futures_core::Stream; use imbl::Vector; -#[cfg(feature = "unstable-msc4274")] -use matrix_sdk::attachment::{AttachmentInfo, Thumbnail}; use matrix_sdk::{ Result, - attachment::AttachmentConfig, + attachment::{AttachmentInfo, Thumbnail}, deserialized_responses::TimelineEvent, event_cache::{EventCacheDropHandles, RoomEventCache}, executor::JoinHandle, @@ -43,24 +41,19 @@ use matrix_sdk::{ use mime::Mime; use pinned_events_loader::PinnedEventsRoom; use ruma::{ - EventId, OwnedEventId, UserId, + EventId, OwnedEventId, OwnedTransactionId, UserId, api::client::receipt::create_receipt::v3::ReceiptType, events::{ - AnyMessageLikeEventContent, AnySyncTimelineEvent, + AnyMessageLikeEventContent, AnySyncTimelineEvent, Mentions, poll::unstable_start::{NewUnstablePollStartEventContent, UnstablePollStartEventContent}, receipt::{Receipt, ReceiptThread}, room::{ - message::{ReplyWithinThread, RoomMessageEventContentWithoutRelation}, + message::{FormattedBody, ReplyWithinThread, RoomMessageEventContentWithoutRelation}, pinned_events::RoomPinnedEventsEventContent, }, }, room_version_rules::RoomVersionRules, }; -#[cfg(feature = "unstable-msc4274")] -use ruma::{ - OwnedTransactionId, - events::{Mentions, room::message::FormattedBody}, -}; use subscriber::TimelineWithDropHandle; use thiserror::Error; use tracing::{instrument, trace, warn}; @@ -176,6 +169,22 @@ pub enum DateDividerMode { Monthly, } +/// Configuration for sending an attachment. +/// +/// Like [`matrix_sdk::attachment::AttachmentConfig`], but instead of the +/// `reply` field, there's only a `replied_to` event id; it's the timeline +/// deciding to fill the rest of the reply parameters. +#[derive(Debug, Default)] +pub struct AttachmentConfig { + pub txn_id: Option, + pub info: Option, + pub thumbnail: Option, + pub caption: Option, + pub formatted_caption: Option, + pub mentions: Option, + pub replied_to: Option, +} + impl Timeline { /// Returns the room for this timeline. pub fn room(&self) -> &Room { @@ -290,44 +299,21 @@ impl Timeline { // thread relation ourselves. if let AnyMessageLikeEventContent::RoomMessage(ref room_msg_content) = content && room_msg_content.relates_to.is_none() - && let Some(thread_root) = self.controller.thread_root() + && self.controller.is_threaded() { - // The latest event id is used for the reply-to fallback, for clients which - // don't handle threads. It should be correctly set to the latest - // event in the thread, which the timeline instance might or might - // not know about; in this case, we do a best effort of filling it, and resort - // to using the thread root if we don't know about any event. - // - // Note: we could trigger a back-pagination if the timeline is empty, and wait - // for the results, if the timeline is too often empty. - let latest_event_id = self - .controller - .items() + let reply = self + .infer_reply(None) .await - .iter() - .rev() - .find_map(|item| { - if let TimelineItemKind::Event(event) = item.kind() { - event.event_id().map(ToOwned::to_owned) - } else { - None - } - }) - .unwrap_or(thread_root); - + .expect("a reply will always be set for threaded timelines"); let content = self .room() .make_reply_event( // Note: this `.into()` gets rid of the relation, but we've checked previously // that the `relates_to` field wasn't set. room_msg_content.clone().into(), - Reply { - event_id: latest_event_id, - enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No), - }, + reply, ) .await?; - Ok(self.room().send_queue().send(content.into()).await?) } else { // Otherwise, we send the message as is. @@ -362,19 +348,62 @@ impl Timeline { content: RoomMessageEventContentWithoutRelation, replied_to: OwnedEventId, ) -> Result<(), Error> { - let enforce_thread = if self.controller.thread_root().is_some() { - EnforceThread::Threaded(ReplyWithinThread::Yes) - } else { - EnforceThread::MaybeThreaded - }; - let content = self - .room() - .make_reply_event(content, Reply { event_id: replied_to, enforce_thread }) - .await?; + let reply = self + .infer_reply(Some(replied_to)) + .await + .expect("the reply will always be set because we provided a replied-to event id"); + let content = self.room().make_reply_event(content, reply).await?; self.send(content.into()).await?; Ok(()) } + /// Given a message or media to send, and an optional `replied_to` event, + /// automatically fills the [`Reply`] information based on the current + /// timeline focus. + pub(crate) async fn infer_reply(&self, replied_to: Option) -> Option { + // If there's a replied-to event id, the reply is pretty straightforward, and we + // should only infer the `EnforceThread` based on the current focus. + if let Some(replied_to) = replied_to { + let enforce_thread = if self.controller.is_threaded() { + EnforceThread::Threaded(ReplyWithinThread::Yes) + } else { + EnforceThread::MaybeThreaded + }; + return Some(Reply { event_id: replied_to, enforce_thread }); + } + + let thread_root = self.controller.thread_root()?; + + // The latest event id is used for the reply-to fallback, for clients which + // don't handle threads. It should be correctly set to the latest + // event in the thread, which the timeline instance might or might + // not know about; in this case, we do a best effort of filling it, and resort + // to using the thread root if we don't know about any event. + // + // Note: we could trigger a back-pagination if the timeline is empty, and wait + // for the results, if the timeline is too often empty. + + let latest_event_id = self + .controller + .items() + .await + .iter() + .rev() + .find_map(|item| { + if let TimelineItemKind::Event(event) = item.kind() { + event.event_id().map(ToOwned::to_owned) + } else { + None + } + }) + .unwrap_or(thread_root); + + Some(Reply { + event_id: latest_event_id, + enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No), + }) + } + /// Edit an event given its [`TimelineEventItemId`] and some new content. /// /// Only supports events for which [`EventTimelineItem::is_editable()`] diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/media.rs b/crates/matrix-sdk-ui/tests/integration/timeline/media.rs index c5c43529b3e..4790ef8b31d 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/media.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/media.rs @@ -20,14 +20,11 @@ use eyeball_im::VectorDiff; use futures_util::StreamExt; #[cfg(feature = "unstable-msc4274")] use matrix_sdk::attachment::{AttachmentInfo, BaseFileInfo}; -use matrix_sdk::{ - assert_let_timeout, - attachment::AttachmentConfig, - room::reply::{EnforceThread, Reply}, - test_utils::mocks::MatrixMockServer, -}; +use matrix_sdk::{assert_let_timeout, test_utils::mocks::MatrixMockServer}; use matrix_sdk_test::{ALICE, JoinedRoomBuilder, async_test, event_factory::EventFactory}; -use matrix_sdk_ui::timeline::{AttachmentSource, EventSendState, RoomExt}; +use matrix_sdk_ui::timeline::{ + AttachmentConfig, AttachmentSource, EventSendState, RoomExt, TimelineFocus, +}; #[cfg(feature = "unstable-msc4274")] use matrix_sdk_ui::timeline::{GalleryConfig, GalleryItemInfo}; #[cfg(feature = "unstable-msc4274")] @@ -36,10 +33,7 @@ use ruma::events::room::message::GalleryItemType; use ruma::owned_mxc_uri; use ruma::{ event_id, - events::room::{ - MediaSource, - message::{MessageType, ReplyWithinThread}, - }, + events::room::{MediaSource, message::MessageType}, room_id, }; use serde_json::json; @@ -115,12 +109,19 @@ async fn test_send_attachment_from_file() { mock.mock_room_send().ok(event_id!("$media")).mock_once().mount().await; - // Queue sending of an attachment. - let config = AttachmentConfig::new().caption(Some("caption".to_owned())).reply(Some(Reply { - event_id: event_id.to_owned(), - enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No), - })); - timeline.send_attachment(&file_path, mime::TEXT_PLAIN, config).use_send_queue().await.unwrap(); + // Queue sending of an attachment in the thread. + let thread_timeline = room + .timeline_builder() + .with_focus(TimelineFocus::Thread { root_event_id: event_id.to_owned() }) + .build() + .await + .unwrap(); + let config = AttachmentConfig { caption: Some("caption".to_owned()), ..Default::default() }; + thread_timeline + .send_attachment(&file_path, mime::TEXT_PLAIN, config) + .use_send_queue() + .await + .unwrap(); { assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next()); @@ -222,7 +223,7 @@ async fn test_send_attachment_from_bytes() { mock.mock_room_send().ok(event_id!("$media")).mock_once().mount().await; // Queue sending of an attachment. - let config = AttachmentConfig::new().caption(Some("caption".to_owned())); + let config = AttachmentConfig { caption: Some("caption".to_owned()), ..Default::default() }; timeline.send_attachment(source, mime::TEXT_PLAIN, config).use_send_queue().await.unwrap(); { @@ -422,7 +423,7 @@ async fn test_react_to_local_media() { let (_tmp_dir, file_path) = create_temporary_file("test.bin"); // Queue sending of an attachment (no captions). - let config = AttachmentConfig::new(); + let config = AttachmentConfig::default(); timeline.send_attachment(&file_path, mime::TEXT_PLAIN, config).use_send_queue().await.unwrap(); let item_id = { diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs index e0a9dc7847d..6c00e9150c8 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs @@ -868,10 +868,7 @@ async fn test_thread_timeline_can_send_edit() { timeline .send( RoomMessageEventContent::text_plain("bonjour monde") - .make_replacement( - ReplacementMetadata::new(threaded_event_id.to_owned(), None), - None, - ) + .make_replacement(ReplacementMetadata::new(threaded_event_id.to_owned(), None)) .into(), ) .await diff --git a/crates/matrix-sdk/src/attachment.rs b/crates/matrix-sdk/src/attachment.rs index c8c405c642f..20712b5d500 100644 --- a/crates/matrix-sdk/src/attachment.rs +++ b/crates/matrix-sdk/src/attachment.rs @@ -184,13 +184,29 @@ impl Thumbnail { /// Configuration for sending an attachment. #[derive(Debug, Default)] pub struct AttachmentConfig { - pub(crate) txn_id: Option, - pub(crate) info: Option, - pub(crate) thumbnail: Option, - pub(crate) caption: Option, - pub(crate) formatted_caption: Option, - pub(crate) mentions: Option, - pub(crate) reply: Option, + /// A fixed transaction id to be used for sending this attachment. + /// + /// Otherwise, a random one will be generated. + pub txn_id: Option, + + /// Type-specific metadata about the attachment. + pub info: Option, + + /// An optional thumbnail to send with the attachment. + pub thumbnail: Option, + + /// An optional caption for the attachment. + pub caption: Option, + + /// An optional formatted caption for the attachment. + pub formatted_caption: Option, + + /// Intentional mentions to be included in the media event. + pub mentions: Option, + + /// Reply parameters for the attachment (replied-to event and thread-related + /// metadata). + pub reply: Option, } impl AttachmentConfig { From 0ea8629be45c5a443358721ee341093d0cfddea9 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 21 Jul 2025 18:09:44 +0200 Subject: [PATCH 4/6] refactor(timeline): homogeneize naming of replied_to vs in_reply_to --- bindings/matrix-sdk-ffi/src/timeline/mod.rs | 4 ++-- crates/matrix-sdk-ui/src/timeline/futures.rs | 2 +- crates/matrix-sdk-ui/src/timeline/mod.rs | 16 ++++++++-------- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index 50a68d2d3d2..1e2e6c32483 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -110,7 +110,7 @@ impl Timeline { let mime_str = mime_type.as_ref().ok_or(RoomError::InvalidAttachmentMimeType)?; let mime_type = mime_str.parse::().map_err(|_| RoomError::InvalidAttachmentMimeType)?; - let replied_to_event_id = params + let in_reply_to_event_id = params .in_reply_to .map(EventId::parse) .transpose() @@ -127,7 +127,7 @@ impl Timeline { caption: params.caption, formatted_caption, mentions: params.mentions.map(Into::into), - replied_to: replied_to_event_id, + in_reply_to: in_reply_to_event_id, ..Default::default() }; diff --git a/crates/matrix-sdk-ui/src/timeline/futures.rs b/crates/matrix-sdk-ui/src/timeline/futures.rs index 05cffc73673..1cb47aac9b2 100644 --- a/crates/matrix-sdk-ui/src/timeline/futures.rs +++ b/crates/matrix-sdk-ui/src/timeline/futures.rs @@ -73,7 +73,7 @@ impl<'a> IntoFuture for SendAttachment<'a> { let fut = async move { let (data, filename) = source.try_into_bytes_and_filename()?; - let reply = timeline.infer_reply(config.replied_to).await; + let reply = timeline.infer_reply(config.in_reply_to).await; let sdk_config = matrix_sdk::attachment::AttachmentConfig { txn_id: config.txn_id, info: config.info, diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 6f9f5f3441f..839f4811a5b 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -172,7 +172,7 @@ pub enum DateDividerMode { /// Configuration for sending an attachment. /// /// Like [`matrix_sdk::attachment::AttachmentConfig`], but instead of the -/// `reply` field, there's only a `replied_to` event id; it's the timeline +/// `reply` field, there's only a `in_reply_to` event id; it's the timeline /// deciding to fill the rest of the reply parameters. #[derive(Debug, Default)] pub struct AttachmentConfig { @@ -182,7 +182,7 @@ pub struct AttachmentConfig { pub caption: Option, pub formatted_caption: Option, pub mentions: Option, - pub replied_to: Option, + pub in_reply_to: Option, } impl Timeline { @@ -346,10 +346,10 @@ impl Timeline { pub async fn send_reply( &self, content: RoomMessageEventContentWithoutRelation, - replied_to: OwnedEventId, + in_reply_to: OwnedEventId, ) -> Result<(), Error> { let reply = self - .infer_reply(Some(replied_to)) + .infer_reply(Some(in_reply_to)) .await .expect("the reply will always be set because we provided a replied-to event id"); let content = self.room().make_reply_event(content, reply).await?; @@ -357,19 +357,19 @@ impl Timeline { Ok(()) } - /// Given a message or media to send, and an optional `replied_to` event, + /// Given a message or media to send, and an optional `in_reply_to` event, /// automatically fills the [`Reply`] information based on the current /// timeline focus. - pub(crate) async fn infer_reply(&self, replied_to: Option) -> Option { + pub(crate) async fn infer_reply(&self, in_reply_to: Option) -> Option { // If there's a replied-to event id, the reply is pretty straightforward, and we // should only infer the `EnforceThread` based on the current focus. - if let Some(replied_to) = replied_to { + if let Some(in_reply_to) = in_reply_to { let enforce_thread = if self.controller.is_threaded() { EnforceThread::Threaded(ReplyWithinThread::Yes) } else { EnforceThread::MaybeThreaded }; - return Some(Reply { event_id: replied_to, enforce_thread }); + return Some(Reply { event_id: in_reply_to, enforce_thread }); } let thread_root = self.controller.thread_root()?; From 0a75d73e8d50b39518a6a2c03c6bd5dde33e82f4 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 21 Jul 2025 18:20:41 +0200 Subject: [PATCH 5/6] feat!(timeline): infer the reply type automatically when sending a media gallery --- bindings/matrix-sdk-ffi/CHANGELOG.md | 5 ++ bindings/matrix-sdk-ffi/src/timeline/mod.rs | 55 +++++--------------- crates/matrix-sdk-ui/CHANGELOG.md | 7 +++ crates/matrix-sdk-ui/src/timeline/futures.rs | 27 ++++++++-- crates/matrix-sdk-ui/src/timeline/mod.rs | 32 ++---------- 5 files changed, 54 insertions(+), 72 deletions(-) diff --git a/bindings/matrix-sdk-ffi/CHANGELOG.md b/bindings/matrix-sdk-ffi/CHANGELOG.md index 1ebbf998195..d68fdd61f5f 100644 --- a/bindings/matrix-sdk-ffi/CHANGELOG.md +++ b/bindings/matrix-sdk-ffi/CHANGELOG.md @@ -8,6 +8,11 @@ All notable changes to this project will be documented in this file. ### Features: +- [**breaking**] [`GalleryUploadParameters::reply`] and [`UploadParameters::reply`] have been both + replaced with a new optional `in_reply_to` field, that's a string which will be parsed into an + `OwnedEventId` when sending the event. The thread relationship will be automatically filled in, + based on the timeline focus. + ([5427](https://github.com/matrix-org/matrix-rust-sdk/pull/5427)) - [**breaking**] [`Timeline::send_reply()`] now automatically fills in the thread relationship, based on the timeline focus. As a result, it only takes an `OwnedEventId` parameter, instead of the `Reply` type. The proper way to start a thread is now thus to create a threaded-focused diff --git a/bindings/matrix-sdk-ffi/src/timeline/mod.rs b/bindings/matrix-sdk-ffi/src/timeline/mod.rs index 1e2e6c32483..503d61392ac 100644 --- a/bindings/matrix-sdk-ffi/src/timeline/mod.rs +++ b/bindings/matrix-sdk-ffi/src/timeline/mod.rs @@ -24,10 +24,7 @@ use matrix_sdk::{ }, deserialized_responses::{ShieldState as SdkShieldState, ShieldStateCode}, event_cache::RoomPaginationStatus, - room::{ - edit::EditedContent as SdkEditedContent, - reply::{EnforceThread, Reply}, - }, + room::edit::EditedContent as SdkEditedContent, }; use matrix_sdk_common::{ executor::{AbortHandle, JoinHandle}, @@ -51,8 +48,7 @@ use ruma::{ }, }, room::message::{ - LocationMessageEventContent, MessageType, ReplyWithinThread, - RoomMessageEventContentWithoutRelation, + LocationMessageEventContent, MessageType, RoomMessageEventContentWithoutRelation, }, AnyMessageLikeEventContent, }, @@ -245,37 +241,6 @@ impl From for AttachmentSource { } } -#[derive(uniffi::Record)] -pub struct ReplyParameters { - /// The ID of the event to reply to. - event_id: String, - /// Whether to enforce a thread relation. - enforce_thread: bool, - /// If enforcing a threaded relation, whether the message is a reply on a - /// thread. - reply_within_thread: bool, -} - -impl TryInto for ReplyParameters { - type Error = RoomError; - - fn try_into(self) -> Result { - let event_id = - EventId::parse(&self.event_id).map_err(|_| RoomError::InvalidRepliedToEventId)?; - let enforce_thread = if self.enforce_thread { - EnforceThread::Threaded(if self.reply_within_thread { - ReplyWithinThread::Yes - } else { - ReplyWithinThread::No - }) - } else { - EnforceThread::MaybeThreaded - }; - - Ok(Reply { event_id, enforce_thread }) - } -} - #[matrix_sdk_ffi_macros::export] impl Timeline { pub async fn add_listener(&self, listener: Box) -> Arc { @@ -1401,6 +1366,7 @@ mod galleries { use matrix_sdk_common::executor::{AbortHandle, JoinHandle}; use matrix_sdk_ui::timeline::GalleryConfig; use mime::Mime; + use ruma::EventId; use tokio::sync::Mutex; use tracing::error; @@ -1408,7 +1374,7 @@ mod galleries { error::RoomError, ruma::{AudioInfo, FileInfo, FormattedBody, ImageInfo, Mentions, VideoInfo}, runtime::get_runtime_handle, - timeline::{build_thumbnail_info, ReplyParameters, Timeline}, + timeline::{build_thumbnail_info, Timeline}, }; #[derive(uniffi::Record)] @@ -1419,8 +1385,8 @@ mod galleries { formatted_caption: Option, /// Optional intentional mentions to be sent with the gallery. mentions: Option, - /// Optional parameters for sending the media as (threaded) reply. - reply_params: Option, + /// Optional Event ID to reply to. + in_reply_to: Option, } #[derive(uniffi::Enum)] @@ -1605,11 +1571,18 @@ mod galleries { params.formatted_caption.map(Into::into), ); + let in_reply_to = params + .in_reply_to + .as_ref() + .map(|event_id| EventId::parse(event_id)) + .transpose() + .map_err(|_| RoomError::InvalidRepliedToEventId)?; + let mut gallery_config = GalleryConfig::new() .caption(params.caption) .formatted_caption(formatted_caption) .mentions(params.mentions.map(Into::into)) - .reply(params.reply_params.map(|p| p.try_into()).transpose()?); + .in_reply_to(in_reply_to); for item_info in item_infos { gallery_config = gallery_config.add_item(item_info.try_into()?); diff --git a/crates/matrix-sdk-ui/CHANGELOG.md b/crates/matrix-sdk-ui/CHANGELOG.md index 141548b0adf..5d450fc68ff 100644 --- a/crates/matrix-sdk-ui/CHANGELOG.md +++ b/crates/matrix-sdk-ui/CHANGELOG.md @@ -8,6 +8,13 @@ All notable changes to this project will be documented in this file. ### Features +- [**breaking**] [`Timeline::send_gallery()`] now automatically fills in the thread relationship, + based on the timeline focus. As a result, the `GalleryConfig::reply()` builder method has been + replaced with `GalleryConfig::in_reply_to`, and only takes an optional event id (the event that is + effectively replied to) instead of the `Reply` type. The proper way to start a thread with a + gallery event is now thus to create a threaded-focused timeline, and then use + `Timeline::send_gallery()`. + ([5427](https://github.com/matrix-org/matrix-rust-sdk/pull/5427)) - [**breaking**] [`Timeline::send_attachment()`] now automatically fills in the thread relationship, based on the timeline focus. As a result, there's a new `matrix_sdk_ui::timeline::AttachmentConfig` type in town, that has a simplified optional parameter diff --git a/crates/matrix-sdk-ui/src/timeline/futures.rs b/crates/matrix-sdk-ui/src/timeline/futures.rs index 1cb47aac9b2..68661c975c0 100644 --- a/crates/matrix-sdk-ui/src/timeline/futures.rs +++ b/crates/matrix-sdk-ui/src/timeline/futures.rs @@ -141,9 +141,30 @@ mod galleries { let Self { timeline, gallery, tracing_span } = self; let fut = async move { - let send_queue = timeline.room().send_queue(); - let fut = send_queue.send_gallery(gallery.try_into()?); - fut.await.map_err(|_| Error::FailedSendingAttachment)?; + let reply = timeline.infer_reply(gallery.in_reply_to).await; + + let mut config = matrix_sdk::attachment::GalleryConfig::new(); + + if let Some(txn_id) = gallery.txn_id { + config = config.txn_id(txn_id); + } + + for item in gallery.items { + config = config.add_item(item.try_into()?); + } + + config = config + .caption(gallery.caption) + .formatted_caption(gallery.formatted_caption) + .mentions(gallery.mentions) + .reply(reply); + + timeline + .room() + .send_queue() + .send_gallery(config) + .await + .map_err(|_| Error::FailedSendingAttachment)?; Ok(()) }; diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 839f4811a5b..295edcff930 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -956,7 +956,7 @@ pub struct GalleryConfig { pub(crate) caption: Option, pub(crate) formatted_caption: Option, pub(crate) mentions: Option, - pub(crate) reply: Option, + pub(crate) in_reply_to: Option, } #[cfg(feature = "unstable-msc4274")] @@ -1024,9 +1024,9 @@ impl GalleryConfig { /// /// # Arguments /// - /// * `reply` - The reply information of the message. - pub fn reply(mut self, reply: Option) -> Self { - self.reply = reply; + /// * `event_id` - The event ID to reply to. + pub fn in_reply_to(mut self, event_id: Option) -> Self { + self.in_reply_to = event_id; self } @@ -1041,30 +1041,6 @@ impl GalleryConfig { } } -#[cfg(feature = "unstable-msc4274")] -impl TryFrom for matrix_sdk::attachment::GalleryConfig { - type Error = Error; - - fn try_from(value: GalleryConfig) -> Result { - let mut config = matrix_sdk::attachment::GalleryConfig::new(); - - if let Some(txn_id) = value.txn_id { - config = config.txn_id(txn_id); - } - - for item in value.items { - config = config.add_item(item.try_into()?); - } - - config = config.caption(value.caption); - config = config.formatted_caption(value.formatted_caption); - config = config.mentions(value.mentions); - config = config.reply(value.reply); - - Ok(config) - } -} - #[cfg(feature = "unstable-msc4274")] #[derive(Debug)] /// Metadata for a gallery item From f75dca25f599c526b8a6406dd2bb3b47e5d373cd Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Wed, 23 Jul 2025 11:47:40 +0200 Subject: [PATCH 6/6] feat(timeline): add support for sending sticker/polls in thread automatically too --- crates/matrix-sdk-ui/src/timeline/mod.rs | 69 ++++--- .../tests/integration/timeline/thread.rs | 174 +++++++++++++++++- 2 files changed, 218 insertions(+), 25 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 295edcff930..13ab5bbab21 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -47,8 +47,12 @@ use ruma::{ AnyMessageLikeEventContent, AnySyncTimelineEvent, Mentions, poll::unstable_start::{NewUnstablePollStartEventContent, UnstablePollStartEventContent}, receipt::{Receipt, ReceiptThread}, + relation::Thread, room::{ - message::{FormattedBody, ReplyWithinThread, RoomMessageEventContentWithoutRelation}, + message::{ + FormattedBody, Relation, RelationWithoutReplacement, ReplyWithinThread, + RoomMessageEventContentWithoutRelation, + }, pinned_events::RoomPinnedEventsEventContent, }, }, @@ -294,31 +298,50 @@ impl Timeline { /// /// * `content` - The content of the message event. #[instrument(skip(self, content), fields(room_id = ?self.room().room_id()))] - pub async fn send(&self, content: AnyMessageLikeEventContent) -> Result { + pub async fn send(&self, mut content: AnyMessageLikeEventContent) -> Result { // If this is a room event we're sending in a threaded timeline, we add the // thread relation ourselves. - if let AnyMessageLikeEventContent::RoomMessage(ref room_msg_content) = content - && room_msg_content.relates_to.is_none() - && self.controller.is_threaded() + if content.relation().is_none() + && let Some(reply) = self.infer_reply(None).await { - let reply = self - .infer_reply(None) - .await - .expect("a reply will always be set for threaded timelines"); - let content = self - .room() - .make_reply_event( - // Note: this `.into()` gets rid of the relation, but we've checked previously - // that the `relates_to` field wasn't set. - room_msg_content.clone().into(), - reply, - ) - .await?; - Ok(self.room().send_queue().send(content.into()).await?) - } else { - // Otherwise, we send the message as is. - Ok(self.room().send_queue().send(content).await?) + match &mut content { + AnyMessageLikeEventContent::RoomMessage(room_msg_content) => { + content = self + .room() + .make_reply_event( + // Note: this `.into()` gets rid of the relation, but we've checked + // previously that the `relates_to` field wasn't + // set. + room_msg_content.clone().into(), + reply, + ) + .await? + .into(); + } + + AnyMessageLikeEventContent::UnstablePollStart( + UnstablePollStartEventContent::New(poll), + ) => { + if let Some(thread_root) = self.controller.thread_root() { + poll.relates_to = Some(RelationWithoutReplacement::Thread(Thread::plain( + thread_root, + reply.event_id, + ))); + } + } + + AnyMessageLikeEventContent::Sticker(sticker) => { + if let Some(thread_root) = self.controller.thread_root() { + sticker.relates_to = + Some(Relation::Thread(Thread::plain(thread_root, reply.event_id))); + } + } + + _ => {} + } } + + Ok(self.room().send_queue().send(content).await?) } /// Send a reply to the given event. @@ -341,7 +364,7 @@ impl Timeline { /// /// * `content` - The content of the reply. /// - /// * `event_id` - The ID of the event to reply to. + /// * `in_reply_to` - The ID of the event to reply to. #[instrument(skip(self, content))] pub async fn send_reply( &self, diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs index 6c00e9150c8..4dfa216c4ac 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs @@ -32,10 +32,18 @@ use ruma::{ event_id, events::{ AnySyncTimelineEvent, + poll::unstable_start::{ + NewUnstablePollStartEventContent, UnstablePollAnswer, UnstablePollStartContentBlock, + UnstablePollStartEventContent, + }, receipt::{ReceiptThread, ReceiptType}, - room::message::{Relation, ReplacementMetadata, RoomMessageEventContent}, + room::{ + ImageInfo, + message::{Relation, ReplacementMetadata, RoomMessageEventContent}, + }, + sticker::{StickerEventContent, StickerMediaSource}, }, - owned_event_id, room_id, user_id, + owned_event_id, owned_mxc_uri, room_id, user_id, }; use stream_assert::assert_pending; use tokio::task::yield_now; @@ -893,6 +901,168 @@ async fn test_thread_timeline_can_send_edit() { assert_pending!(stream); } +#[async_test] +async fn test_send_sticker_thread() { + // If I send a sticker to a threaded timeline, it just works (aka the system to + // set the threaded relationship does kick in). + + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let room_id = room_id!("!a:b.c"); + let thread_root_event_id = owned_event_id!("$root"); + let threaded_event_id = event_id!("$threaded_event"); + + let room = server.sync_joined_room(&client, room_id).await; + + let timeline = room + .timeline_builder() + .with_focus(TimelineFocus::Thread { root_event_id: thread_root_event_id.clone() }) + .build() + .await + .unwrap(); + + let (initial_items, mut stream) = timeline.subscribe().await; + + // At first, the timeline is empty. + assert!(initial_items.is_empty()); + assert_pending!(stream); + + // Start the timeline with an in-thread event. + let f = EventFactory::new(); + server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id).add_timeline_event( + f.text_msg("hello world") + .sender(*ALICE) + .event_id(threaded_event_id) + .in_thread(&thread_root_event_id, threaded_event_id) + .server_ts(MilliSecondsSinceUnixEpoch::now()), + ), + ) + .await; + + // Sanity check: I receive the event and the date divider. + assert_let_timeout!(Some(timeline_updates) = stream.next()); + assert_eq!(timeline_updates.len(), 2); + + server.mock_room_state_encryption().plain().mount().await; + + let sent_event_id = event_id!("$sent_msg"); + server.mock_room_send().ok(sent_event_id).mount().await; + + let media_src = owned_mxc_uri!("mxc://example.com/1"); + timeline + .send( + StickerEventContent::new("sticker!".to_owned(), ImageInfo::new(), media_src.clone()) + .into(), + ) + .await + .unwrap(); + + // I get the local echo for the in-thread event. + assert_let_timeout!(Some(timeline_updates) = stream.next()); + assert_eq!(timeline_updates.len(), 1); + + assert_let!(VectorDiff::PushBack { value } = &timeline_updates[0]); + let event_item = value.as_event().unwrap(); + + // The content matches what we expect. + let sticker_item = event_item.content().as_sticker().unwrap(); + let content = sticker_item.content(); + assert_eq!(content.body, "sticker!"); + assert_let!(StickerMediaSource::Plain(plain) = content.source.clone()); + assert_eq!(plain, media_src); + + // Then we're done. + assert_pending!(stream); +} + +#[async_test] +async fn test_send_poll_thread() { + // If I send a poll to a threaded timeline, it just works (aka the system to + // set the threaded relationship does kick in). + + let server = MatrixMockServer::new().await; + let client = server.client_builder().build().await; + + let room_id = room_id!("!a:b.c"); + let thread_root_event_id = owned_event_id!("$root"); + let threaded_event_id = event_id!("$threaded_event"); + + let room = server.sync_joined_room(&client, room_id).await; + + let timeline = room + .timeline_builder() + .with_focus(TimelineFocus::Thread { root_event_id: thread_root_event_id.clone() }) + .build() + .await + .unwrap(); + + let (initial_items, mut stream) = timeline.subscribe().await; + + // At first, the timeline is empty. + assert!(initial_items.is_empty()); + assert_pending!(stream); + + // Start the timeline with an in-thread event. + let f = EventFactory::new(); + server + .sync_room( + &client, + JoinedRoomBuilder::new(room_id).add_timeline_event( + f.text_msg("hello world") + .sender(*ALICE) + .event_id(threaded_event_id) + .in_thread(&thread_root_event_id, threaded_event_id) + .server_ts(MilliSecondsSinceUnixEpoch::now()), + ), + ) + .await; + + // Sanity check: I receive the event and the date divider. + assert_let_timeout!(Some(timeline_updates) = stream.next()); + assert_eq!(timeline_updates.len(), 2); + + server.mock_room_state_encryption().plain().mount().await; + + let sent_event_id = event_id!("$sent_msg"); + server.mock_room_send().ok(sent_event_id).mount().await; + + timeline + .send( + UnstablePollStartEventContent::New(NewUnstablePollStartEventContent::plain_text( + "let's vote", + UnstablePollStartContentBlock::new( + "what day is it today?", + vec![ + UnstablePollAnswer::new("0", "monday"), + UnstablePollAnswer::new("1", "friday"), + ] + .try_into() + .unwrap(), + ), + )) + .into(), + ) + .await + .unwrap(); + + // I get the local echo for the in-thread event. + assert_let_timeout!(Some(timeline_updates) = stream.next()); + assert_eq!(timeline_updates.len(), 1); + + assert_let!(VectorDiff::PushBack { value } = &timeline_updates[0]); + let event_item = value.as_event().unwrap(); + + // The content is a poll. + assert!(event_item.content().is_poll()); + + // Then we're done. + assert_pending!(stream); +} + #[async_test] async fn test_sending_read_receipt_with_no_events_doesnt_unset_read_flag() { // If a thread timeline has no events, then marking it as read doesn't unset the