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

This commit is contained in:
Benjamin Bouvier
2025-07-21 18:06:31 +02:00
parent d1163b75bf
commit e2148e46bc
8 changed files with 177 additions and 99 deletions

View File

@@ -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.

View File

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

View File

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

View File

@@ -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)?;
} else {
let fut = timeline
timeline
.room()
.send_attachment(filename, &mime_type, data, config)
.send_queue()
.send_attachment(filename, mime_type, data, sdk_config)
.await
.map_err(|_| Error::FailedSendingAttachment)?;
} else {
timeline
.room()
.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(())

View File

@@ -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()`]

View File

@@ -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 = {

View File

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

View File

@@ -184,13 +184,29 @@ impl Thumbnail {
/// Configuration for sending an attachment.
#[derive(Debug, Default)]
pub struct AttachmentConfig {
pub(crate) txn_id: Option<OwnedTransactionId>,
pub(crate) info: Option<AttachmentInfo>,
pub(crate) thumbnail: Option<Thumbnail>,
pub(crate) caption: Option<String>,
pub(crate) formatted_caption: Option<FormattedBody>,
pub(crate) mentions: Option<Mentions>,
pub(crate) reply: Option<Reply>,
/// A fixed transaction id to be used for sending this attachment.
///
/// Otherwise, a random one will be generated.
pub txn_id: Option<OwnedTransactionId>,
/// Type-specific metadata about the attachment.
pub info: Option<AttachmentInfo>,
/// An optional thumbnail to send with the attachment.
pub thumbnail: Option<Thumbnail>,
/// An optional caption for the attachment.
pub caption: Option<String>,
/// An optional formatted caption for the attachment.
pub formatted_caption: Option<FormattedBody>,
/// Intentional mentions to be included in the media event.
pub mentions: Option<Mentions>,
/// Reply parameters for the attachment (replied-to event and thread-related
/// metadata).
pub reply: Option<Reply>,
}
impl AttachmentConfig {