refactor(timeline): enhance the Timeline::send() and Timeline::send_reply() APIs when the timeline is threaded

bindings/matrix-sdk-ffi/CHANGELOG.md

@@ -8,6 +8,16 @@ 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
+  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

@@ -20,22 +20,18 @@ 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,
-    room::{
-        edit::EditedContent as SdkEditedContent,
-        reply::{EnforceThread, Reply},
-    },
+    room::edit::EditedContent as SdkEditedContent,
 };
 use matrix_sdk_common::{
     executor::{AbortHandle, JoinHandle},
     stream::StreamExt,
 };
 use matrix_sdk_ui::timeline::{
-    self, AttachmentSource, EventItemOrigin, Profile, TimelineDetails,
+    self, AttachmentConfig, AttachmentSource, EventItemOrigin, Profile, TimelineDetails,
     TimelineUniqueId as SdkTimelineUniqueId,
 };
 use mime::Mime;
@@ -52,8 +48,7 @@ use ruma::{
             },
         },
         room::message::{
-            LocationMessageEventContent, MessageType, ReplyWithinThread,
-            RoomMessageEventContentWithoutRelation,
+            LocationMessageEventContent, MessageType, RoomMessageEventContentWithoutRelation,
         },
         AnyMessageLikeEventContent,
     },
@@ -111,19 +106,26 @@ impl Timeline {
         let mime_str = mime_type.as_ref().ok_or(RoomError::InvalidAttachmentMimeType)?;
         let mime_type =
             mime_str.parse::<Mime>().map_err(|_| RoomError::InvalidAttachmentMimeType)?;
+        let in_reply_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),
+            in_reply_to: in_reply_to_event_id,
+            ..Default::default()
+        };
 
         let handle = SendAttachmentJoinHandle::new(get_runtime_handle().spawn(async move {
             let mut request =
@@ -205,8 +207,8 @@ pub struct UploadParameters {
     formatted_caption: Option<FormattedBody>,
     /// Optional intentional mentions to be sent with the media.
     mentions: Option<Mentions>,
-    /// Optional parameters for sending the media as (threaded) reply.
-    reply_params: Option<ReplyParameters>,
+    /// Optional Event ID to reply to.
+    in_reply_to: Option<String>,
     /// Should the media be sent with the send queue, or synchronously?
     ///
     /// Watching progress only works with the synchronous method, at the moment.
@@ -239,37 +241,6 @@ impl From<UploadSource> 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<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> {
@@ -529,9 +500,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 +557,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 +576,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(())
@@ -1394,14 +1366,15 @@ 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;
 
     use crate::{
         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)]
@@ -1412,8 +1385,8 @@ mod galleries {
         formatted_caption: Option<FormattedBody>,
         /// Optional intentional mentions to be sent with the gallery.
         mentions: Option<Mentions>,
-        /// Optional parameters for sending the media as (threaded) reply.
-        reply_params: Option<ReplyParameters>,
+        /// Optional Event ID to reply to.
+        in_reply_to: Option<String>,
     }
 
     #[derive(uniffi::Enum)]
@@ -1598,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()?);

crates/matrix-sdk-ui/CHANGELOG.md

@@ -6,6 +6,32 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased] - ReleaseDate
 
+### 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
+  `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
+  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))
+
 ### Refactor
 
 - [**breaking**] The MSRV has been bumped to Rust 1.88.

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

@@ -591,6 +591,11 @@ impl<P: RoomDataProvider, D: Decryptor> TimelineController<P, D> {
         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<OwnedEventId> {
         as_variant!(&*self.focus, TimelineFocusKind::Thread { root_event_id } => root_event_id.clone())
     }

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.in_reply_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(())
@@ -125,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(())
             };