events: Handle existing mentions in make_replacement

Allows to use add_mentions before calling it.
Deprecates set_mentions.

crates/ruma-events/CHANGELOG.md

@@ -4,6 +4,8 @@ Improvements:
 
 - Implement `make_for_thread` and `make_replacement` for
   `RoomMessageEventContentWithoutRelation`
+- `RoomMessageEventContent::set_mentions` is deprecated and replaced by
+  `add_mentions` that should be called before `make_replacement`.
 
 # 0.28.0
 

crates/ruma-events/src/room/message.rs

@@ -241,9 +241,9 @@ impl RoomMessageEventContent {
     /// `original_message`.
     #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))]
     ///
-    /// If the message that is replaced contains [`Mentions`], they are copied into
-    /// `m.new_content` to keep the same mentions, but not into `content` to avoid repeated
-    /// notifications.
+    /// If this message contains [`Mentions`], they are copied into `m.new_content` to keep the same
+    /// mentions, but the ones in `content` are filtered with the ones in the
+    /// [`ReplacementMetadata`] so only new mentions will trigger a notification.
     ///
     /// # Panics
     ///
@@ -270,6 +270,7 @@ impl RoomMessageEventContent {
     /// used instead.
     ///
     /// [mentions]: https://spec.matrix.org/latest/client-server-api/#user-and-room-mentions
+    #[deprecated = "Call add_mentions before adding the relation instead."]
     pub fn set_mentions(mut self, mentions: Mentions) -> Self {
         if let Some(Relation::Replacement(replacement)) = &mut self.relates_to {
             let old_mentions = &replacement.new_content.mentions;
@@ -306,9 +307,8 @@ impl RoomMessageEventContent {
     /// mentions by extending the previous `user_ids` with the new ones, and applies a logical OR to
     /// the values of `room`.
     ///
-    /// This is recommended over [`Self::set_mentions()`] to avoid to overwrite any mentions set
-    /// automatically by another method, like [`Self::make_reply_to()`]. However, this method has no
-    /// special support for replacements.
+    /// This should be called before methods that add a relation, like [`Self::make_reply_to()`] and
+    /// [`Self::make_replacement()`], for the mentions to be correctly set.
     ///
     /// [mentions]: https://spec.matrix.org/latest/client-server-api/#user-and-room-mentions
     pub fn add_mentions(mut self, mentions: Mentions) -> Self {

crates/ruma-events/src/room/message/without_relation.rs

@@ -268,9 +268,9 @@ impl RoomMessageEventContentWithoutRelation {
     /// `original_message`.
     #[doc = include_str!(concat!(env!("CARGO_MANIFEST_DIR"), "/src/doc/rich_reply.md"))]
     ///
-    /// If the message that is replaced contains [`Mentions`], they are copied into
-    /// `m.new_content` to keep the same mentions, but not into `content` to avoid repeated
-    /// notifications.
+    /// If this message contains [`Mentions`], they are copied into `m.new_content` to keep the same
+    /// mentions, but the ones in `content` are filtered with the ones in the
+    /// [`ReplacementMetadata`] so only new mentions will trigger a notification.
     ///
     /// # Panics
     ///
@@ -285,12 +285,34 @@ impl RoomMessageEventContentWithoutRelation {
     ) -> RoomMessageEventContent {
         let metadata = metadata.into();
 
+        let mentions = self.mentions.take();
+
+        // Only set mentions that were not there before.
+        if let Some(mentions) = &mentions {
+            let new_mentions = metadata.mentions.map(|old_mentions| {
+                let mut new_mentions = Mentions::new();
+
+                new_mentions.user_ids = mentions
+                    .user_ids
+                    .iter()
+                    .filter(|u| !old_mentions.user_ids.contains(*u))
+                    .cloned()
+                    .collect();
+
+                new_mentions.room = mentions.room && !old_mentions.room;
+
+                new_mentions
+            });
+
+            self.mentions = new_mentions;
+        };
+
         // Prepare relates_to with the untouched msgtype.
         let relates_to = Relation::Replacement(Replacement {
             event_id: metadata.event_id,
             new_content: RoomMessageEventContentWithoutRelation {
                 msgtype: self.msgtype.clone(),
-                mentions: metadata.mentions,
+                mentions,
             },
         });
 

crates/ruma-events/tests/it/room_message.rs

@@ -1164,6 +1164,7 @@ fn video_msgtype_deserialization() {
 }
 
 #[test]
+#[allow(deprecated)]
 fn set_mentions() {
     let mut content = RoomMessageEventContent::text_plain("you!");
     let mentions = content.mentions.take();
@@ -1176,7 +1177,42 @@ fn set_mentions() {
 }
 
 #[test]
-fn make_replacement_set_mentions() {
+fn add_mentions_then_make_replacement() {
+    let alice = owned_user_id!("@alice:localhost");
+    let bob = owned_user_id!("@bob:localhost");
+    let original_message_json = json!({
+        "content": {
+            "body": "Hello, World!",
+            "msgtype": "m.text",
+            "m.mentions": {
+                "user_ids": [alice],
+            }
+        },
+        "event_id": "$143273582443PhrSn",
+        "origin_server_ts": 134_829_848,
+        "room_id": "!roomid:notareal.hs",
+        "sender": "@user:notareal.hs",
+        "type": "m.room.message",
+    });
+    let original_message: OriginalSyncRoomMessageEvent =
+        from_json_value(original_message_json).unwrap();
+
+    let mut content = RoomMessageEventContent::text_html(
+        "This is _an edited_ message.",
+        "This is <em>an edited</em> message.",
+    );
+    content = content.add_mentions(Mentions::with_user_ids(vec![alice.clone(), bob.clone()]));
+    content = content.make_replacement(&original_message, None);
+
+    let mentions = content.mentions.unwrap();
+    assert_eq!(mentions.user_ids, [bob.clone()].into());
+    assert_matches!(content.relates_to, Some(Relation::Replacement(replacement)));
+    let mentions = replacement.new_content.mentions.unwrap();
+    assert_eq!(mentions.user_ids, [alice, bob].into());
+}
+
+#[test]
+fn make_replacement_then_add_mentions() {
     let alice = owned_user_id!("@alice:localhost");
     let bob = owned_user_id!("@bob:localhost");
     let original_message_json = json!({
@@ -1201,19 +1237,12 @@ fn make_replacement_set_mentions() {
         "This is <em>an edited</em> message.",
     );
     content = content.make_replacement(&original_message, None);
-    let content_clone = content.clone();
+    content = content.add_mentions(Mentions::with_user_ids(vec![alice.clone(), bob.clone()]));
 
-    assert_matches!(content.mentions, None);
-    assert_matches!(content.relates_to, Some(Relation::Replacement(replacement)));
-    let mentions = replacement.new_content.mentions.unwrap();
-    assert_eq!(mentions.user_ids, [alice.clone()].into());
-
-    content = content_clone.set_mentions(Mentions::with_user_ids(vec![alice.clone(), bob.clone()]));
     let mentions = content.mentions.unwrap();
-    assert_eq!(mentions.user_ids, [bob.clone()].into());
-    assert_matches!(content.relates_to, Some(Relation::Replacement(replacement)));
-    let mentions = replacement.new_content.mentions.unwrap();
     assert_eq!(mentions.user_ids, [alice, bob].into());
+    assert_matches!(content.relates_to, Some(Relation::Replacement(replacement)));
+    assert!(replacement.new_content.mentions.is_none());
 }
 
 #[test]