diff --git a/deltachat-rpc-client/tests/test_calls.py b/deltachat-rpc-client/tests/test_calls.py index e83d736f04..cd10c4beed 100644 --- a/deltachat-rpc-client/tests/test_calls.py +++ b/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 diff --git a/deltachat-rpc-client/tests/test_chatlist_events.py b/deltachat-rpc-client/tests/test_chatlist_events.py index 7c9b63466a..05bec0795f 100644 --- a/deltachat-rpc-client/tests/test_chatlist_events.py +++ b/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() diff --git a/deltachat-rpc-client/tests/test_iroh_webxdc.py b/deltachat-rpc-client/tests/test_iroh_webxdc.py index c2433d2f76..978f3298a4 100644 --- a/deltachat-rpc-client/tests/test_iroh_webxdc.py +++ b/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() diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index f219f2bcd7..85f61ee182 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/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 diff --git a/deltachat-rpc-client/tests/test_vcard.py b/deltachat-rpc-client/tests/test_vcard.py index c0fdb2c94c..f01a64431d 100644 --- a/deltachat-rpc-client/tests/test_vcard.py +++ b/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("charlie@example.org", "Charlie") alice_contact_charlie_snapshot = alice_contact_charlie.get_snapshot() diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index 65b3ef46a1..c62473230d 100644 --- a/python/tests/test_1_online.py +++ b/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") diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 0dbec21e7f..e49bf466f4 100644 --- a/src/receive_imf.rs +++ b/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], past_ids: &[Option], @@ -1291,11 +1300,12 @@ async fn do_chat_assignment( mime_parser: &mut MimeMessage, is_partial_download: Option, parent_message: Option, -) -> 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> { +) -> Result> { // 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 { /// /// `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> { +) -> Result> { 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) diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index fb250533dd..6d4ee760fa 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -2709,17 +2709,19 @@ async fn test_gmx_forwarded_msg() -> Result<()> { Ok(()) } -/// Tests that user is notified about new incoming contact requests. +/// Tests that user is notified about new incoming contact requests, +/// but not about additional messages arriving in the contact request chat. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_incoming_contact_request() -> Result<()> { - let t = TestContext::new_alice().await; + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; - receive_imf(&t, MSGRMSG, false).await?; - let msg = t.get_last_msg().await; - let chat = chat::Chat::load_from_db(&t, msg.chat_id).await?; + let msg = tcm.send_recv(alice, bob, "Hello!").await; + let chat = chat::Chat::load_from_db(bob, msg.chat_id).await?; assert!(chat.is_contact_request()); - let event = t + let event = bob .evtracker .get_matching(|evt| matches!(evt, EventType::IncomingMsg { .. })) .await; @@ -2727,10 +2729,39 @@ async fn test_incoming_contact_request() -> Result<()> { EventType::IncomingMsg { chat_id, msg_id } => { assert_eq!(msg.chat_id, chat_id); assert_eq!(msg.id, msg_id); - Ok(()) } _ => unreachable!(), } + + // Bob ignores contact request. + // The second and third message does not result in notification. + for text in ["Hello!!??", "Hello!!!!????"] { + let msg = tcm.send_recv(alice, bob, text).await; + + // There are only `MsgsChanged` events for each message, + // but no `IncomingMsg` before or after. + let event = bob + .evtracker + .get_matching(|evt| { + matches!( + evt, + EventType::MsgsChanged { .. } | EventType::IncomingMsg { .. } + ) + }) + .await; + match event { + EventType::MsgsChanged { chat_id, msg_id } => { + assert_eq!(msg.chat_id, chat_id); + assert_eq!(msg.id, msg_id); + + let msg = Message::load_from_db(bob, msg_id).await?; + assert_eq!(msg.text, text); + } + _ => unreachable!(), + } + } + + Ok(()) } async fn get_parent_message(