diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index b78e251977e..d2f6ba85baa 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -281,14 +281,13 @@ impl Timeline { /// Send a reply to the given event. /// - /// Currently only supports events events with an event ID and JSON being + /// 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. /// - /// If the `content.mentions` is `Some(_)`, the sender of `reply_item` will - /// be added to the mentions of the reply. If `content.mentions` is `None`, - /// it will be kept as-is. + /// The sender of `reply_item` will be added to the mentions of the reply if + /// and only if `reply_item` has not been written by the sender. /// /// # Arguments /// @@ -312,8 +311,24 @@ impl Timeline { return Err(UnsupportedReplyItem::MISSING_EVENT_ID); }; - let add_mentions = - if content.mentions.is_some() { AddMentions::Yes } else { AddMentions::No }; + let client = self.room().client(); + + // [The specification](https://spec.matrix.org/v1.10/client-server-api/#user-and-room-mentions) says: + // + // > Users should not add their own Matrix ID to the `m.mentions` property as + // > outgoing messages cannot self-notify. + // + // OK, so let's start by assuming we always want to mention the sender of the + // replied event (`reply_item`)… + let mut mention_the_sender_of_the_replied_event = AddMentions::Yes; + + // … but if `reply_item` has been written by the current user, let's toggle + // to `AddMentions::No`. + if let Some(current_user_id) = client.user_id() { + if current_user_id == &reply_item.sender { + mention_the_sender_of_the_replied_event = AddMentions::No; + } + } let content = match reply_item.content() { TimelineItemContent::Message(msg) => { @@ -325,7 +340,11 @@ impl Timeline { content: msg.to_content(), unsigned: Default::default(), }; - content.make_reply_to(&event, forward_thread, add_mentions) + content.make_reply_to( + &event, + forward_thread, + mention_the_sender_of_the_replied_event, + ) } _ => { let Some(raw_event) = reply_item.latest_json() else { @@ -337,7 +356,7 @@ impl Timeline { event_id.to_owned(), self.room().room_id(), forward_thread, - add_mentions, + mention_the_sender_of_the_replied_event, ) } }; diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs b/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs index 79ca15abd82..95524e9f3cb 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/replies.rs @@ -20,7 +20,7 @@ use ruma::{ AddMentions, ForwardThread, OriginalRoomMessageEvent, Relation, ReplyWithinThread, RoomMessageEventContent, RoomMessageEventContentWithoutRelation, }, - MessageLikeUnsigned, + Mentions, MessageLikeUnsigned, }, owned_event_id, owned_room_id, owned_user_id, room_id, uint, MilliSecondsSinceUnixEpoch, }; @@ -28,7 +28,7 @@ use serde_json::json; use stream_assert::assert_next_matches; use wiremock::{ matchers::{header, method, path_regex}, - Mock, ResponseTemplate, + Mock, Request, ResponseTemplate, }; use crate::{mock_encryption_state, mock_sync}; @@ -273,12 +273,12 @@ async fn test_send_reply() { let (_, mut timeline_stream) = timeline.subscribe_filter_map(|item| item.as_event().cloned()).await; - let event_id_1 = event_id!("$event1"); + let event_id_from_bob = event_id!("$event_from_bob"); sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( event_builder.make_sync_message_event_with_id( &BOB, - event_id_1, - RoomMessageEventContent::text_plain("Hello, World!"), + event_id_from_bob, + RoomMessageEventContent::text_plain("Hello from Bob"), ), )); @@ -286,7 +286,7 @@ async fn test_send_reply() { let _response = client.sync_once(sync_settings.clone()).await.unwrap(); server.reset().await; - let hello_world_item = + let event_from_bob = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); // Clear the timeline to make sure the old item does not need to be @@ -295,19 +295,35 @@ async fn test_send_reply() { assert_next_matches!(timeline_stream, VectorDiff::Clear); mock_encryption_state(&server, false).await; + + // Now, let's reply to a message sent by `BOB`. Mock::given(method("PUT")) .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) - .respond_with( - ResponseTemplate::new(200).set_body_json(json!({ "event_id": "$reply_event" })), - ) + .respond_with(move |req: &Request| { + use ruma::events::room::message::RoomMessageEventContent; + + let reply_event = req + .body_json::() + .expect("Failed to deserialize the event"); + + assert_matches!(reply_event.relates_to, Some(Relation::Reply { in_reply_to: InReplyTo { event_id, .. } }) => { + assert_eq!(event_id, event_id_from_bob); + }); + assert_matches!(reply_event.mentions, Some(Mentions { user_ids, room: false, .. }) => { + assert_eq!(user_ids.len(), 1); + assert!(user_ids.contains(*BOB)); + }); + + ResponseTemplate::new(200).set_body_json(json!({ "event_id": "$reply_event" })) + }) .expect(1) .mount(&server) .await; timeline .send_reply( - RoomMessageEventContentWithoutRelation::text_plain("Hello, Bob!"), - &hello_world_item, + RoomMessageEventContentWithoutRelation::text_plain("Replying to Bob"), + &event_from_bob, ForwardThread::Yes, ) .await @@ -317,9 +333,9 @@ async fn test_send_reply() { assert_matches!(reply_item.send_state(), Some(EventSendState::NotSentYet)); let reply_message = reply_item.content().as_message().unwrap(); - assert_eq!(reply_message.body(), "Hello, Bob!"); + assert_eq!(reply_message.body(), "Replying to Bob"); let in_reply_to = reply_message.in_reply_to().unwrap(); - assert_eq!(in_reply_to.event_id, event_id_1); + assert_eq!(in_reply_to.event_id, event_id_from_bob); // Right now, we don't pass along the replied-to event to the event handler, // so it's not available if the timeline got cleared. Not critical, but // there's notable room for improvement here. @@ -333,14 +349,106 @@ async fn test_send_reply() { assert_matches!(reply_item_remote_echo.send_state(), Some(EventSendState::Sent { .. })); let reply_message = reply_item_remote_echo.content().as_message().unwrap(); - assert_eq!(reply_message.body(), "Hello, Bob!"); + assert_eq!(reply_message.body(), "Replying to Bob"); let in_reply_to = reply_message.in_reply_to().unwrap(); - assert_eq!(in_reply_to.event_id, event_id_1); + assert_eq!(in_reply_to.event_id, event_id_from_bob); // Same as above. // // let replied_to_event = - // assert_matches!(&in_reply_to.event, TimelineDetails::Ready(ev) => ev); - // assert_eq!(replied_to_event.sender(), *BOB); + // assert_matches!(&in_reply_to.event, TimelineDetails::Ready(ev) => + // ev); assert_eq!(replied_to_event.sender(), *BOB); + + server.verify().await; +} + +#[async_test] +async fn test_send_reply_to_self() { + let room_id = room_id!("!a98sd12bjh:example.org"); + let (client, server) = logged_in_client_with_server().await; + let event_builder = EventBuilder::new(); + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let mut sync_builder = SyncResponseBuilder::new(); + sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + let room = client.get_room(room_id).unwrap(); + let timeline = room.timeline().await.unwrap(); + let (_, mut timeline_stream) = + timeline.subscribe_filter_map(|item| item.as_event().cloned()).await; + + let event_id_from_self = event_id!("$event_from_self"); + sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( + event_builder.make_sync_message_event_with_id( + client.user_id().expect("Client must have a user ID"), + event_id_from_self, + RoomMessageEventContent::text_plain("Hello from self"), + ), + )); + + mock_sync(&server, sync_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + let event_from_self = + assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + + // Clear the timeline to make sure the old item does not need to be + // available in it for the reply to work. + timeline.clear().await; + assert_next_matches!(timeline_stream, VectorDiff::Clear); + + mock_encryption_state(&server, false).await; + + // Now, let's reply to a message sent by the current user. + Mock::given(method("PUT")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*")) + .respond_with(move |req: &Request| { + use ruma::events::room::message::RoomMessageEventContent; + + let reply_event = req + .body_json::() + .expect("Failed to deserialize the event"); + + assert_matches!(reply_event.relates_to, Some(Relation::Reply { in_reply_to: InReplyTo { event_id, .. } }) => { + assert_eq!(event_id, event_id_from_self); + }); + assert!(reply_event.mentions.is_none()); + + ResponseTemplate::new(200).set_body_json(json!({ "event_id": "$reply_event" })) + }) + .expect(1) + .mount(&server) + .await; + + timeline + .send_reply( + RoomMessageEventContentWithoutRelation::text_plain("Replying to self"), + &event_from_self, + ForwardThread::Yes, + ) + .await + .unwrap(); + + let reply_item = assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => value); + + assert_matches!(reply_item.send_state(), Some(EventSendState::NotSentYet)); + let reply_message = reply_item.content().as_message().unwrap(); + assert_eq!(reply_message.body(), "Replying to self"); + let in_reply_to = reply_message.in_reply_to().unwrap(); + assert_eq!(in_reply_to.event_id, event_id_from_self); + + let diff = timeout(timeline_stream.next(), Duration::from_secs(1)).await.unwrap().unwrap(); + assert_let!(VectorDiff::Set { index: 0, value: reply_item_remote_echo } = diff); + + assert_matches!(reply_item_remote_echo.send_state(), Some(EventSendState::Sent { .. })); + let reply_message = reply_item_remote_echo.content().as_message().unwrap(); + assert_eq!(reply_message.body(), "Replying to self"); + let in_reply_to = reply_message.in_reply_to().unwrap(); + assert_eq!(in_reply_to.event_id, event_id_from_self); server.verify().await; }