fix: stop notifying about messages in contact request chats

This commit is contained in:
link2xt
2025-10-29 17:32:50 +00:00
committed by l
parent 8b4c718b6b
commit 885a5efa39
8 changed files with 110 additions and 45 deletions

View File

@@ -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!"
if msg.get_snapshot().text == "Hello!":
break

View File

@@ -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()

View File

@@ -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()

View File

@@ -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

View File

@@ -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()

View File

@@ -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")

View File

@@ -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,7 +1392,8 @@ 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(
if let Some((new_chat_id, new_chat_id_blocked, new_created)) =
lookup_or_create_adhoc_group(
context,
mime_parser,
to_ids,
@@ -1392,6 +1405,7 @@ async fn do_chat_assignment(
{
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,7 +1524,8 @@ async fn do_chat_assignment(
}
}
ChatAssignment::AdHocGroup => {
if let Some((new_chat_id, new_chat_id_blocked)) = lookup_or_create_adhoc_group(
if let Some((new_chat_id, new_chat_id_blocked, new_chat_created)) =
lookup_or_create_adhoc_group(
context,
mime_parser,
to_ids,
@@ -1519,6 +1537,7 @@ async fn do_chat_assignment(
{
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)

View File

@@ -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(