fix: stop notifying about messages in contact request chats

Author: link2xt

deltachat-rpc-client/tests/test_calls.py

@@ -103,7 +103,7 @@ def test_no_contact_request_call(acfactory) -> None:
         # There should be no incoming call notification.
         assert event.kind != EventType.INCOMING_CALL
 
-        if event.kind == EventType.INCOMING_MSG:
+        if event.kind == EventType.MSGS_CHANGED:
             msg = bob.get_message_by_id(event.msg_id)
-            assert msg.get_snapshot().text == "Hello!"
-            break
+            if msg.get_snapshot().text == "Hello!":
+                break

deltachat-rpc-client/tests/test_chatlist_events.py

@@ -169,6 +169,8 @@ def test_imap_sync_seen_msgs(acfactory: ACFactory) -> None:
     """
     alice, alice_second_device, bob, alice_chat_bob = get_multi_account_test_setup(acfactory)
 
+    bob.create_chat(alice)
+
     alice_chat_bob.send_text("hello")
 
     msg = bob.wait_for_incoming_msg()

deltachat-rpc-client/tests/test_iroh_webxdc.py

@@ -214,7 +214,9 @@ def test_advertisement_after_chatting(acfactory, path_to_webxdc):
     ac1_ac2_chat = ac1.create_chat(ac2)
     ac1_webxdc_msg = ac1_ac2_chat.send_message(text="WebXDC", file=path_to_webxdc)
     ac2_webxdc_msg = ac2.wait_for_incoming_msg()
-    assert ac2_webxdc_msg.get_snapshot().text == "WebXDC"
+    ac2_webxdc_msg_snapshot = ac2_webxdc_msg.get_snapshot()
+    assert ac2_webxdc_msg_snapshot.text == "WebXDC"
+    ac2_webxdc_msg_snapshot.chat.accept()
 
     ac1_ac2_chat.send_text("Hello!")
     ac2_hello_msg = ac2.wait_for_incoming_msg()

deltachat-rpc-client/tests/test_something.py

@@ -347,6 +347,7 @@ def test_receive_imf_failure(acfactory) -> None:
     assert snapshot.download_state == DownloadState.AVAILABLE
     assert snapshot.error is not None
     assert snapshot.show_padlock
+    snapshot.chat.accept()
 
     # The failed message doesn't break the IMAP loop.
     bob.set_config("fail_on_receiving_full_msg", "0")
@@ -888,10 +889,12 @@ def test_rename_group(acfactory):
     bob_msg = bob.wait_for_incoming_msg()
     bob_chat = bob_msg.get_snapshot().chat
     assert bob_chat.get_basic_snapshot().name == "Test group"
+    bob.wait_for_event(EventType.CHATLIST_ITEM_CHANGED)
 
     for name in ["Baz", "Foo bar", "Xyzzy"]:
         alice_group.set_name(name)
-        bob.wait_for_incoming_msg_event()
+        bob.wait_for_event(EventType.CHATLIST_ITEM_CHANGED)
+        bob.wait_for_event(EventType.CHATLIST_ITEM_CHANGED)
         assert bob_chat.get_basic_snapshot().name == name
 
 

deltachat-rpc-client/tests/test_vcard.py

@@ -1,6 +1,7 @@
 def test_vcard(acfactory) -> None:
     alice, bob, fiona = acfactory.get_online_accounts(3)
 
+    bob.create_chat(alice)
     alice_contact_bob = alice.create_contact(bob, "Bob")
     alice_contact_charlie = alice.create_contact("[email protected]", "Charlie")
     alice_contact_charlie_snapshot = alice_contact_charlie.get_snapshot()

python/tests/test_1_online.py

@@ -460,7 +460,7 @@ def test_forward_own_message(acfactory, lp):
 
 def test_resend_message(acfactory, lp):
     ac1, ac2 = acfactory.get_online_accounts(2)
-    chat1 = ac1.create_chat(ac2)
+    chat1 = acfactory.get_accepted_chat(ac1, ac2)
 
     lp.sec("ac1: send message to ac2")
     chat1.send_text("message")

src/receive_imf.rs

@@ -715,7 +715,9 @@ pub(crate) async fn receive_imf_inner(
         mark_recipients_as_verified(context, from_id, &mime_parser).await?;
     }
 
+    let is_old_contact_request;
     let received_msg = if let Some(received_msg) = received_msg {
+        is_old_contact_request = false;
         received_msg
     } else {
         let is_dc_message = if mime_parser.has_chat_version() {
@@ -754,9 +756,9 @@ pub(crate) async fn receive_imf_inner(
             to_ids.first().copied().flatten().unwrap_or(ContactId::SELF)
         };
 
-        let (chat_id, chat_id_blocked) = do_chat_assignment(
+        let (chat_id, chat_id_blocked, is_created) = do_chat_assignment(
             context,
-            chat_assignment,
+            &chat_assignment,
             from_id,
             &to_ids,
             &past_ids,
@@ -767,6 +769,7 @@ pub(crate) async fn receive_imf_inner(
             parent_message,
         )
         .await?;
+        is_old_contact_request = chat_id_blocked == Blocked::Request && !is_created;
 
         // Add parts
         add_parts(
@@ -984,8 +987,9 @@ pub(crate) async fn receive_imf_inner(
         let fresh = received_msg.state == MessageState::InFresh
             && mime_parser.is_system_message != SystemMessage::CallAccepted
             && mime_parser.is_system_message != SystemMessage::CallEnded;
+        let important = mime_parser.incoming && fresh && !is_old_contact_request;
         for msg_id in &received_msg.msg_ids {
-            chat_id.emit_msg_event(context, *msg_id, mime_parser.incoming && fresh);
+            chat_id.emit_msg_event(context, *msg_id, important);
         }
     }
     context.new_msgs_notify.notify_one();
@@ -1279,10 +1283,15 @@ async fn decide_chat_assignment(
 /// Assigns the message to a chat.
 ///
 /// Creates a new chat if necessary.
+///
+/// Returns the chat ID,
+/// whether it is blocked
+/// and if the chat was created by this function
+/// (as opposed to being looked up among existing chats).
 #[expect(clippy::too_many_arguments)]
 async fn do_chat_assignment(
     context: &Context,
-    chat_assignment: ChatAssignment,
+    chat_assignment: &ChatAssignment,
     from_id: ContactId,
     to_ids: &[Option<ContactId>],
     past_ids: &[Option<ContactId>],
@@ -1291,11 +1300,12 @@ async fn do_chat_assignment(
     mime_parser: &mut MimeMessage,
     is_partial_download: Option<u32>,
     parent_message: Option<Message>,
-) -> Result<(ChatId, Blocked)> {
+) -> Result<(ChatId, Blocked, bool)> {
     let is_bot = context.get_config_bool(Config::Bot).await?;
 
     let mut chat_id = None;
     let mut chat_id_blocked = Blocked::Not;
+    let mut chat_created = false;
 
     if mime_parser.incoming {
         let test_normal_chat = ChatIdBlocked::lookup_by_contact(context, from_id).await?;
@@ -1350,12 +1360,13 @@ async fn do_chat_assignment(
                     {
                         chat_id = Some(new_chat_id);
                         chat_id_blocked = new_chat_id_blocked;
+                        chat_created = true;
                     }
                 }
             }
             ChatAssignment::MailingListOrBroadcast => {
                 if let Some(mailinglist_header) = mime_parser.get_mailinglist_header() {
-                    if let Some((new_chat_id, new_chat_id_blocked)) =
+                    if let Some((new_chat_id, new_chat_id_blocked, new_chat_created)) =
                         create_or_lookup_mailinglist_or_broadcast(
                             context,
                             allow_creation,
@@ -1367,6 +1378,7 @@ async fn do_chat_assignment(
                     {
                         chat_id = Some(new_chat_id);
                         chat_id_blocked = new_chat_id_blocked;
+                        chat_created = new_chat_created;
 
                         apply_mailinglist_changes(context, mime_parser, new_chat_id).await?;
                     }
@@ -1380,18 +1392,20 @@ async fn do_chat_assignment(
                 chat_id_blocked = *new_chat_id_blocked;
             }
             ChatAssignment::AdHocGroup => {
-                if let Some((new_chat_id, new_chat_id_blocked)) = lookup_or_create_adhoc_group(
-                    context,
-                    mime_parser,
-                    to_ids,
-                    allow_creation || test_normal_chat.is_some(),
-                    create_blocked,
-                    is_partial_download.is_some(),
-                )
-                .await?
+                if let Some((new_chat_id, new_chat_id_blocked, new_created)) =
+                    lookup_or_create_adhoc_group(
+                        context,
+                        mime_parser,
+                        to_ids,
+                        allow_creation || test_normal_chat.is_some(),
+                        create_blocked,
+                        is_partial_download.is_some(),
+                    )
+                    .await?
                 {
                     chat_id = Some(new_chat_id);
                     chat_id_blocked = new_chat_id_blocked;
+                    chat_created = new_created;
                 }
             }
             ChatAssignment::OneOneChat => {}
@@ -1427,6 +1441,7 @@ async fn do_chat_assignment(
                     .context("Failed to get (new) chat for contact")?;
                 chat_id = Some(chat.id);
                 chat_id_blocked = chat.blocked;
+                chat_created = true;
             }
 
             if let Some(chat_id) = chat_id {
@@ -1479,6 +1494,7 @@ async fn do_chat_assignment(
                     {
                         chat_id = Some(new_chat_id);
                         chat_id_blocked = new_chat_id_blocked;
+                        chat_created = true;
                     }
                 }
             }
@@ -1499,6 +1515,7 @@ async fn do_chat_assignment(
                         {
                             id
                         } else {
+                            chat_created = true;
                             let name =
                                 compute_mailinglist_name(mailinglist_header, &listid, mime_parser);
                             chat::create_broadcast_ex(context, Nosync, listid, name).await?
@@ -1507,18 +1524,20 @@ async fn do_chat_assignment(
                 }
             }
             ChatAssignment::AdHocGroup => {
-                if let Some((new_chat_id, new_chat_id_blocked)) = lookup_or_create_adhoc_group(
-                    context,
-                    mime_parser,
-                    to_ids,
-                    allow_creation,
-                    Blocked::Not,
-                    is_partial_download.is_some(),
-                )
-                .await?
+                if let Some((new_chat_id, new_chat_id_blocked, new_chat_created)) =
+                    lookup_or_create_adhoc_group(
+                        context,
+                        mime_parser,
+                        to_ids,
+                        allow_creation,
+                        Blocked::Not,
+                        is_partial_download.is_some(),
+                    )
+                    .await?
                 {
                     chat_id = Some(new_chat_id);
                     chat_id_blocked = new_chat_id_blocked;
+                    chat_created = new_chat_created;
                 }
             }
             ChatAssignment::OneOneChat => {}
@@ -1538,6 +1557,7 @@ async fn do_chat_assignment(
                     let chat = ChatIdBlocked::get_for_contact(context, to_id, Blocked::Not).await?;
                     chat_id = Some(chat.id);
                     chat_id_blocked = chat.blocked;
+                    chat_created = true;
                 }
             }
             if chat_id.is_none() && mime_parser.has_chat_version() {
@@ -1575,7 +1595,7 @@ async fn do_chat_assignment(
         info!(context, "No chat id for message (TRASH).");
         DC_CHAT_ID_TRASH
     });
-    Ok((chat_id, chat_id_blocked))
+    Ok((chat_id, chat_id_blocked, chat_created))
 }
 
 /// Creates a `ReceivedMsg` from given parts which might consist of
@@ -2402,7 +2422,7 @@ async fn lookup_or_create_adhoc_group(
     allow_creation: bool,
     create_blocked: Blocked,
     is_partial_download: bool,
-) -> Result<Option<(ChatId, Blocked)>> {
+) -> Result<Option<(ChatId, Blocked, bool)>> {
     // Partial download may be an encrypted message with protected Subject header. We do not want to
     // create a group with "..." or "Encrypted message" as a subject. The same is for undecipherable
     // messages. Instead, assign the message to 1:1 chat with the sender.
@@ -2492,12 +2512,12 @@ async fn lookup_or_create_adhoc_group(
             context,
             "Assigning message to ad-hoc group {chat_id} with matching name and members."
         );
-        return Ok(Some((chat_id, blocked)));
+        return Ok(Some((chat_id, blocked, false)));
     }
     if !allow_creation {
         return Ok(None);
     }
-    create_adhoc_group(
+    Ok(create_adhoc_group(
         context,
         mime_parser,
         create_blocked,
@@ -2506,7 +2526,8 @@ async fn lookup_or_create_adhoc_group(
         &grpname,
     )
     .await
-    .context("Could not create ad hoc group")
+    .context("Could not create ad hoc group")?
+    .map(|(chat_id, blocked)| (chat_id, blocked, true)))
 }
 
 /// If this method returns true, the message shall be assigned to the 1:1 chat with the sender.
@@ -3146,17 +3167,22 @@ fn mailinglist_header_listid(list_id_header: &str) -> Result<String> {
 ///
 /// `mime_parser` is the corresponding message
 /// and is used to figure out the mailing list name from different header fields.
+///
+/// Returns the chat ID,
+/// whether it is blocked
+/// and if the chat was created by this function
+/// (as opposed to being looked up among existing chats).
 async fn create_or_lookup_mailinglist_or_broadcast(
     context: &Context,
     allow_creation: bool,
     list_id_header: &str,
     from_id: ContactId,
     mime_parser: &MimeMessage,
-) -> Result<Option<(ChatId, Blocked)>> {
+) -> Result<Option<(ChatId, Blocked, bool)>> {
     let listid = mailinglist_header_listid(list_id_header)?;
 
     if let Some((chat_id, blocked)) = chat::get_chat_id_by_grpid(context, &listid).await? {
-        return Ok(Some((chat_id, blocked)));
+        return Ok(Some((chat_id, blocked, false)));
     }
 
     let chattype = if mime_parser.was_encrypted() {
@@ -3220,7 +3246,7 @@ async fn create_or_lookup_mailinglist_or_broadcast(
             )
             .await?;
         }
-        Ok(Some((chat_id, blocked)))
+        Ok(Some((chat_id, blocked, true)))
     } else {
         info!(context, "Creating list forbidden by caller.");
         Ok(None)