feat!(timeline): infer the reply type automatically when sending an attachment

Author: bnjbvr

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::<Mime>().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<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.

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

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

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<OwnedTransactionId>,
+    pub info: Option<AttachmentInfo>,
+    pub thumbnail: Option<Thumbnail>,
+    pub caption: Option<String>,
+    pub formatted_caption: Option<FormattedBody>,
+    pub mentions: Option<Mentions>,
+    pub replied_to: Option<OwnedEventId>,
+}
+
 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<OwnedEventId>) -> Option<Reply> {
+        // 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()`]