fix: trash no-op messages about self being added to groups

src/receive_imf.rs

@@ -773,6 +773,7 @@ pub(crate) async fn receive_imf_inner(
             chat_id,
             chat_id_blocked,
             is_dc_message,
+            is_created,
         )
         .await
         .context("add_parts error")?
@@ -1809,6 +1810,7 @@ async fn add_parts(
     mut chat_id: ChatId,
     mut chat_id_blocked: Blocked,
     is_dc_message: MessengerMessage,
+    is_chat_created: bool,
 ) -> Result<ReceivedMsg> {
     let to_id = if mime_parser.incoming {
         ContactId::SELF
@@ -1872,7 +1874,16 @@ async fn add_parts(
             apply_out_broadcast_changes(context, mime_parser, &mut chat, from_id).await?
         }
         Chattype::Group => {
-            apply_group_changes(context, mime_parser, &mut chat, from_id, to_ids, past_ids).await?
+            apply_group_changes(
+                context,
+                mime_parser,
+                &mut chat,
+                from_id,
+                to_ids,
+                past_ids,
+                is_chat_created,
+            )
+            .await?
         }
         Chattype::InBroadcast => {
             apply_in_broadcast_changes(context, mime_parser, &mut chat, from_id).await?
@@ -2889,8 +2900,7 @@ async fn create_group(
     let mut chat_id = None;
     let mut chat_id_blocked = Default::default();
 
-    if chat_id.is_none()
-            && !mime_parser.is_mailinglist_message()
+    if !mime_parser.is_mailinglist_message()
             && !grpid.is_empty()
             && mime_parser.get_header(HeaderDef::ChatGroupName).is_some()
             // otherwise, a pending "quit" message may pop up
@@ -3082,6 +3092,7 @@ async fn apply_group_changes(
     from_id: ContactId,
     to_ids: &[Option<ContactId>],
     past_ids: &[Option<ContactId>],
+    is_chat_created: bool,
 ) -> Result<GroupChangesInfo> {
     let from_is_key_contact = Contact::get_by_id(context, from_id).await?.is_key_contact();
     ensure!(from_is_key_contact || chat.grpid.is_empty());
@@ -3295,19 +3306,15 @@ async fn apply_group_changes(
         .difference(&new_chat_contacts)
         .copied()
         .collect();
-
-    if let Some(added_id) = added_id
-        && !added_ids.remove(&added_id)
-        && added_id != ContactId::SELF
-    {
-        // No-op "Member added" message. An exception is self-addition messages because they at
-        // least must be shown when a chat is created on our side.
-        info!(context, "No-op 'Member added' message (TRASH)");
-        better_msg = Some(String::new());
-    }
+    let id_was_already_added = if let Some(added_id) = added_id {
+        !added_ids.remove(&added_id)
+    } else {
+        false
+    };
     if let Some(removed_id) = removed_id {
         removed_ids.remove(&removed_id);
     }
+
     let group_changes_msgs = if !chat_contacts.contains(&ContactId::SELF)
         && new_chat_contacts.contains(&ContactId::SELF)
     {
@@ -3316,6 +3323,11 @@ async fn apply_group_changes(
         group_changes_msgs(context, &added_ids, &removed_ids, chat.id).await?
     };
 
+    if id_was_already_added && group_changes_msgs.is_empty() && !is_chat_created {
+        info!(context, "No-op 'Member added' message (TRASH)");
+        better_msg = Some(String::new());
+    }
+
     if send_event_chat_modified {
         context.emit_event(EventType::ChatModified(chat.id));
         chatlist_events::emit_chatlist_item_changed(context, chat.id);

src/securejoin/securejoin_tests.rs

@@ -1584,3 +1584,51 @@ async fn test_auth_token_is_synchronized() -> Result<()> {
 
     Ok(())
 }
+
+/// Tests that Bob deduplicates messages about being added to the group.
+///
+/// If Alice (inviter) has multiple devices,
+/// Bob may receive multiple "member added" messages.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_deduplicate_member_added() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice1 = &tcm.alice().await;
+    let alice2 = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+
+    // Enable sync messages so Alice QR code tokens
+    // are synchronized.
+    alice1.set_config_bool(Config::SyncMsgs, true).await?;
+    alice2.set_config_bool(Config::SyncMsgs, true).await?;
+
+    tcm.section("Alice creates a group");
+    let alice1_chat_id = chat::create_group(alice1, "Group").await?;
+
+    tcm.section("Alice creates group QR code and synchronizes it");
+    let qr = get_securejoin_qr(alice1, Some(alice1_chat_id)).await?;
+    sync(alice1, alice2).await;
+
+    // Import Alice's key to skip key request step.
+    tcm.section("Bob imports Alice's vCard");
+    bob.add_or_lookup_contact_id(alice1).await;
+
+    tcm.section("Bob scans the QR code");
+    let bob_chat_id = join_securejoin(bob, &qr).await?;
+
+    tcm.section("Both Alice's devices add Bob to chat");
+    let sent = bob.pop_sent_msg().await;
+    alice1.recv_msg_trash(&sent).await;
+    alice2.recv_msg_trash(&sent).await;
+
+    let sent1 = alice1.pop_sent_msg().await;
+    let sent2 = alice2.pop_sent_msg().await;
+
+    let bob_rcvd = bob.recv_msg(&sent1).await;
+    assert_eq!(bob_rcvd.chat_id, bob_chat_id);
+    assert_eq!(bob_rcvd.text, "Member Me added by alice@example.org.");
+
+    // Second message is a no-op, so it is trashed.
+    bob.recv_msg_trash(&sent2).await;
+
+    Ok(())
+}

src/tests/verified_chats.rs

@@ -507,9 +507,8 @@ async fn test_verified_member_added_reordering() -> Result<()> {
 
     assert_eq!(fiona_received_message.get_text(), "Hi");
 
-    // Fiona receives late "Member added" message
-    // and the chat becomes protected.
-    fiona.recv_msg(&alice_sent_member_added).await;
+    // Fiona receives late "Member added" message.
+    fiona.recv_msg_trash(&alice_sent_member_added).await;
 
     Ok(())
 }