feat!(timeline): Timeline::send_reply() automatically includes the thread relationship too

Author: bnjbvr

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.

bindings/matrix-sdk-ffi/src/timeline/mod.rs

@@ -529,9 +529,10 @@ impl Timeline {
     pub async fn send_reply(
         &self,
         msg: Arc<RoomMessageEventContentWithoutRelation>,
-        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<String>,
         zoom_level: Option<u8>,
         asset_type: Option<AssetType>,
-        reply_params: Option<ReplyParameters>,
+        replied_to_event_id: Option<String>,
     ) -> 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(())

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))

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(())
     }

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();