feat!(ffi): merge send_reply and send_thread_reply (matrix-org#4880)

This pushes down the `Reply` struct to be provided when sending a reply, and merges the `send_reply` and `send_thread_reply` FFI functions.

This is a small follow-up on https://github.com/matrix-org/matrix-rust-sdk/pull/4852/files#r2016594024.

Signed-off-by: Johannes Marbach <n0-0ne+github@mailbox.org>

Author: Johennes

bindings/matrix-sdk-ffi/CHANGELOG.md

@@ -65,6 +65,11 @@ Breaking changes:
 - `ClientBuilder::passphrase` is renamed `session_passphrase`
   ([#4870](https://github.com/matrix-org/matrix-rust-sdk/pull/4870/))
 
+- Merge `Timeline::send_thread_reply` into `Timeline::send_reply`. This
+  changes the parameters of `send_reply` which now requires passing the
+  event ID (and thread reply behaviour) inside a `ReplyParameters` struct.
+  ([#4880](https://github.com/matrix-org/matrix-rust-sdk/pull/4880/))
+
 Additions:
 
 - Add `Encryption::get_user_identity` which returns `UserIdentity`

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

@@ -23,11 +23,14 @@ use futures_util::{pin_mut, StreamExt as _};
 use matrix_sdk::{
     attachment::{
         AttachmentConfig, AttachmentInfo, BaseAudioInfo, BaseFileInfo, BaseImageInfo,
-        BaseVideoInfo, Reply, Thumbnail,
+        BaseVideoInfo, Thumbnail,
     },
     deserialized_responses::{ShieldState as SdkShieldState, ShieldStateCode},
     event_cache::RoomPaginationStatus,
-    room::{edit::EditedContent as SdkEditedContent, reply::EnforceThread},
+    room::{
+        edit::EditedContent as SdkEditedContent,
+        reply::{EnforceThread, Reply},
+    },
 };
 use matrix_sdk_ui::timeline::{
     self, EventItemOrigin, Profile, RepliedToEvent, TimelineDetails,
@@ -116,31 +119,13 @@ impl Timeline {
             params.formatted_caption.map(Into::into),
         );
 
-        let reply = if let Some(reply_params) = params.reply_params {
-            let event_id = EventId::parse(reply_params.event_id)
-                .map_err(|_| RoomError::InvalidRepliedToEventId)?;
-            let enforce_thread = if reply_params.enforce_thread {
-                EnforceThread::Threaded(if reply_params.reply_within_thread {
-                    ReplyWithinThread::Yes
-                } else {
-                    ReplyWithinThread::No
-                })
-            } else {
-                EnforceThread::MaybeThreaded
-            };
-
-            Some(Reply { event_id, enforce_thread })
-        } else {
-            None
-        };
-
         let attachment_config = AttachmentConfig::new()
             .thumbnail(thumbnail)
             .info(attachment_info)
             .caption(params.caption)
             .formatted_caption(formatted_caption)
             .mentions(params.mentions.map(Into::into))
-            .reply(reply);
+            .reply(params.reply_params.map(|p| p.try_into()).transpose()?);
 
         let handle = SendAttachmentJoinHandle::new(get_runtime_handle().spawn(async move {
             let mut request =
@@ -241,6 +226,26 @@ pub struct ReplyParameters {
     reply_within_thread: bool,
 }
 
+impl TryInto<Reply> for ReplyParameters {
+    type Error = RoomError;
+
+    fn try_into(self) -> Result<Reply, Self::Error> {
+        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<dyn TimelineListener>) -> Arc<TaskHandle> {
@@ -502,45 +507,10 @@ impl Timeline {
     pub async fn send_reply(
         &self,
         msg: Arc<RoomMessageEventContentWithoutRelation>,
-        event_id: String,
+        reply_params: ReplyParameters,
     ) -> Result<(), ClientError> {
-        let event_id = EventId::parse(event_id)?;
-        self.inner
-            .send_reply((*msg).clone(), event_id, EnforceThread::MaybeThreaded)
-            .await
-            .map_err(|err| anyhow::anyhow!(err))?;
-        Ok(())
-    }
-
-    /// Send a message on a thread.
-    ///
-    /// If the replied to event does not have a thread relation, it becomes the
-    /// root of a new thread.
-    ///
-    /// # Arguments
-    ///
-    /// * `msg` - Message content to send
-    ///
-    /// * `event_id` - ID of the event to reply to
-    ///
-    /// * `is_reply` - Whether the message is a reply on a thread
-    pub async fn send_thread_reply(
-        &self,
-        msg: Arc<RoomMessageEventContentWithoutRelation>,
-        event_id: String,
-        is_reply: bool,
-    ) -> Result<(), ClientError> {
-        let event_id = EventId::parse(event_id)?;
         self.inner
-            .send_reply(
-                (*msg).clone(),
-                event_id,
-                EnforceThread::Threaded(if is_reply {
-                    ReplyWithinThread::Yes
-                } else {
-                    ReplyWithinThread::No
-                }),
-            )
+            .send_reply((*msg).clone(), reply_params.try_into()?)
             .await
             .map_err(|err| anyhow::anyhow!(err))?;
         Ok(())

crates/matrix-sdk-ui/CHANGELOG.md

@@ -28,6 +28,9 @@ All notable changes to this project will be documented in this file.
   from an [`EventTimelineItem`] by calling `.content().reactions()`. They're also returned by
   ownership (cloned) instead of by reference.
   ([#4576](https://github.com/matrix-org/matrix-rust-sdk/pull/4576))
+- [**breaking**] The parameters `event_id` and `enforce_thread` on [`Timeline::send_reply()`]
+  have been wrapped in a `reply` struct parameter.
+  ([#4880](https://github.com/matrix-org/matrix-rust-sdk/pull/4880/))
 
 ## [0.10.0] - 2025-02-04
 

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

@@ -28,7 +28,7 @@ use matrix_sdk::{
     event_cache::{EventCacheDropHandles, RoomEventCache},
     event_handler::EventHandlerHandle,
     executor::JoinHandle,
-    room::{edit::EditedContent, reply::EnforceThread, Receipts, Room},
+    room::{edit::EditedContent, reply::Reply, Receipts, Room},
     send_queue::{RoomSendQueueError, SendHandle},
     Client, Result,
 };
@@ -278,10 +278,9 @@ impl Timeline {
     pub async fn send_reply(
         &self,
         content: RoomMessageEventContentWithoutRelation,
-        event_id: OwnedEventId,
-        enforce_thread: EnforceThread,
+        reply: Reply,
     ) -> Result<(), Error> {
-        let content = self.room().make_reply_event(content, &event_id, enforce_thread).await?;
+        let content = self.room().make_reply_event(content, reply).await?;
         self.send(content.into()).await?;
         Ok(())
     }

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

@@ -20,8 +20,8 @@ use eyeball_im::VectorDiff;
 use futures_util::{FutureExt, StreamExt};
 use matrix_sdk::{
     assert_let_timeout,
-    attachment::{AttachmentConfig, Reply},
-    room::reply::EnforceThread,
+    attachment::AttachmentConfig,
+    room::reply::{EnforceThread, Reply},
     test_utils::mocks::MatrixMockServer,
 };
 use matrix_sdk_test::{async_test, event_factory::EventFactory, JoinedRoomBuilder, ALICE};

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

@@ -4,7 +4,10 @@ use assert_matches::assert_matches;
 use assert_matches2::assert_let;
 use eyeball_im::VectorDiff;
 use futures_util::StreamExt;
-use matrix_sdk::{room::reply::EnforceThread, test_utils::mocks::MatrixMockServer};
+use matrix_sdk::{
+    room::reply::{EnforceThread, Reply},
+    test_utils::mocks::MatrixMockServer,
+};
 use matrix_sdk_base::timeout::timeout;
 use matrix_sdk_test::{
     async_test, event_factory::EventFactory, JoinedRoomBuilder, ALICE, BOB, CAROL,
@@ -692,8 +695,10 @@ async fn test_send_reply() {
     timeline
         .send_reply(
             RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"),
-            event_id_from_bob.to_owned(),
-            EnforceThread::MaybeThreaded,
+            Reply {
+                event_id: event_id_from_bob.into(),
+                enforce_thread: EnforceThread::MaybeThreaded,
+            },
         )
         .await
         .unwrap();
@@ -792,8 +797,10 @@ async fn test_send_reply_to_self() {
     timeline
         .send_reply(
             RoomMessageEventContentWithoutRelation::text_plain("Replying to self"),
-            event_id_from_self.to_owned(),
-            EnforceThread::MaybeThreaded,
+            Reply {
+                event_id: event_id_from_self.into(),
+                enforce_thread: EnforceThread::MaybeThreaded,
+            },
         )
         .await
         .unwrap();
@@ -858,8 +865,7 @@ async fn test_send_reply_to_threaded() {
     timeline
         .send_reply(
             RoomMessageEventContentWithoutRelation::text_plain("Hello, Bob!"),
-            event_id_1.to_owned(),
-            EnforceThread::MaybeThreaded,
+            Reply { event_id: event_id_1.into(), enforce_thread: EnforceThread::MaybeThreaded },
         )
         .await
         .unwrap();
@@ -961,8 +967,10 @@ async fn test_send_reply_with_event_id() {
     timeline
         .send_reply(
             RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"),
-            event_id_from_bob.to_owned(),
-            EnforceThread::MaybeThreaded,
+            Reply {
+                event_id: event_id_from_bob.into(),
+                enforce_thread: EnforceThread::MaybeThreaded,
+            },
         )
         .await
         .unwrap();
@@ -1047,8 +1055,10 @@ async fn test_send_reply_enforce_thread() {
     timeline
         .send_reply(
             RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"),
-            event_id_from_bob.to_owned(),
-            EnforceThread::Threaded(ReplyWithinThread::No),
+            Reply {
+                event_id: event_id_from_bob.into(),
+                enforce_thread: EnforceThread::Threaded(ReplyWithinThread::No),
+            },
         )
         .await
         .unwrap();
@@ -1144,8 +1154,10 @@ async fn test_send_reply_enforce_thread_is_reply() {
     timeline
         .send_reply(
             RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"),
-            event_id_from_bob.to_owned(),
-            EnforceThread::Threaded(ReplyWithinThread::Yes),
+            Reply {
+                event_id: event_id_from_bob.into(),
+                enforce_thread: EnforceThread::Threaded(ReplyWithinThread::Yes),
+            },
         )
         .await
         .unwrap();
@@ -1237,8 +1249,10 @@ async fn test_send_reply_with_event_id_that_is_redacted() {
     timeline
         .send_reply(
             RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"),
-            redacted_event_id_from_bob.to_owned(),
-            EnforceThread::MaybeThreaded,
+            Reply {
+                event_id: redacted_event_id_from_bob.into(),
+                enforce_thread: EnforceThread::MaybeThreaded,
+            },
         )
         .await
         .unwrap();

crates/matrix-sdk/CHANGELOG.md

@@ -235,6 +235,9 @@ simpler methods:
   ([#4831](https://github.com/matrix-org/matrix-rust-sdk/pull/4831))
 - [**breaking**] `Client::store` is renamed `state_store`
   ([#4851](https://github.com/matrix-org/matrix-rust-sdk/pull/4851))
+- [**breaking**] The parameters `event_id` and `enforce_thread` on [`Room::make_reply_event()`]
+  have been wrapped in a `reply` struct parameter.
+  ([#4880](https://github.com/matrix-org/matrix-rust-sdk/pull/4880/))
 
 ## [0.10.0] - 2025-02-04
 

crates/matrix-sdk/src/attachment.rs

@@ -25,10 +25,10 @@ use ruma::{
         },
         Mentions,
     },
-    OwnedEventId, OwnedTransactionId, TransactionId, UInt,
+    OwnedTransactionId, TransactionId, UInt,
 };
 
-use crate::room::reply::EnforceThread;
+use crate::room::reply::Reply;
 
 /// Base metadata about an image.
 #[derive(Debug, Clone, Default)]
@@ -181,15 +181,6 @@ impl Thumbnail {
     }
 }
 
-/// Information needed to reply to an event.
-#[derive(Debug)]
-pub struct Reply {
-    /// The event ID of the event to reply to.
-    pub event_id: OwnedEventId,
-    /// Whether to enforce a thread relation.
-    pub enforce_thread: EnforceThread,
-}
-
 /// Configuration for sending an attachment.
 #[derive(Debug, Default)]
 pub struct AttachmentConfig {

crates/matrix-sdk/src/room/mod.rs

@@ -58,6 +58,7 @@ use matrix_sdk_common::{
     timeout::timeout,
 };
 use mime::Mime;
+use reply::Reply;
 #[cfg(feature = "e2e-encryption")]
 use ruma::events::{
     room::encrypted::OriginalSyncRoomEncryptedEvent, AnySyncMessageLikeEvent, AnySyncTimelineEvent,
@@ -138,7 +139,7 @@ pub use self::{
 #[cfg(doc)]
 use crate::event_cache::EventCache;
 use crate::{
-    attachment::{AttachmentConfig, AttachmentInfo, Reply},
+    attachment::{AttachmentConfig, AttachmentInfo},
     client::WeakClient,
     config::RequestConfig,
     error::{BeaconError, WrongRoomState},
@@ -2272,9 +2273,7 @@ impl Room {
         if let Some(reply) = reply {
             // Since we just created the event, there is no relation attached to it. Thus,
             // it is safe to add the reply relation without overriding anything.
-            content = self
-                .make_reply_event(content.into(), &reply.event_id, reply.enforce_thread)
-                .await?;
+            content = self.make_reply_event(content.into(), reply).await?;
         }
         Ok(content)
     }