From b04dce377ca98d67d806c0c8fc40f1b5849dd429 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Tue, 7 Apr 2026 22:01:24 -0300 Subject: [PATCH] fix: Scale up contacts messaged in groups to IncomingTo This makes such contacts appear in the contact list. `IncomingTo` is used because `ChatId::accept_ex()` does so for groups, so sending a group message is effectively accepting the group again in regards to contact searchability. This fixes up b549e7633d5735af72c7a7c9b35f984abdff938e which made it impossible to find contacts from groups even if we've written there. --- src/contact/contact_tests.rs | 45 ++++++++++++++++++++++++++++ src/mimefactory.rs | 19 +++++++----- src/receive_imf/receive_imf_tests.rs | 19 +++++++----- 3 files changed, 67 insertions(+), 16 deletions(-) diff --git a/src/contact/contact_tests.rs b/src/contact/contact_tests.rs index 300345438..b821301ce 100644 --- a/src/contact/contact_tests.rs +++ b/src/contact/contact_tests.rs @@ -4,6 +4,7 @@ use super::*; use crate::chat::{Chat, get_chat_contacts, send_text_msg}; use crate::chatlist::Chatlist; use crate::receive_imf::receive_imf; +use crate::securejoin::get_securejoin_qr; use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote, sync}; #[test] @@ -154,6 +155,50 @@ async fn test_get_contacts() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_search_contacts_from_group() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let fiona = &tcm.fiona().await; + + let alice_chat_id = chat::create_group(alice, "").await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; + let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; + tcm.exec_securejoin_qr(fiona, alice, &qr).await; + + // Workaround for "member added" for fiona not sent to bob. + let gossip_period = alice.get_config_int(Config::GossipPeriod).await?; + SystemTime::shift(Duration::from_secs(gossip_period.try_into()?)); + send_text_msg(alice, alice_chat_id, "hello".to_string()).await?; + let sent_msg = alice.pop_sent_msg().await; + bob.recv_msg(&sent_msg).await; + fiona.recv_msg(&sent_msg).await; + + let contacts = Contact::get_all(bob, 0, None).await?; + let bob_alice_id = bob.add_or_lookup_contact_id(alice).await; + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0], bob_alice_id); + + let contacts = Contact::get_all(fiona, 0, None).await?; + let fiona_alice_id = fiona.add_or_lookup_contact_id(alice).await; + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0], fiona_alice_id); + + // Sending to the group adds new members to the contact list. + send_text_msg(bob, bob_chat_id, "hello".to_string()).await?; + fiona.recv_msg(&bob.pop_sent_msg().await).await; + let contacts = Contact::get_all(bob, 0, None).await?; + let bob_fiona_id = bob.add_or_lookup_contact_id(fiona).await; + assert_eq!(contacts.len(), 2); + assert_eq!(contacts[0], bob_alice_id); + assert_eq!(contacts[1], bob_fiona_id); + let contacts = Contact::get_all(fiona, 0, None).await?; + assert_eq!(contacts.len(), 1); + assert_eq!(contacts[0], fiona_alice_id); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_is_self_addr() -> Result<()> { let t = TestContext::new().await; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0e0eefd6e..91d682be8 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -463,18 +463,21 @@ impl MimeFactory { .into_iter() .filter(|id| *id != ContactId::SELF) .collect(); - if recipient_ids.len() == 1 - && !matches!( - msg.param.get_cmd(), - SystemMessage::MemberRemovedFromGroup | SystemMessage::SecurejoinMessage - ) - && !matches!(chat.typ, Chattype::OutBroadcast | Chattype::InBroadcast) + if !matches!( + msg.param.get_cmd(), + SystemMessage::MemberRemovedFromGroup | SystemMessage::SecurejoinMessage + ) && !matches!(chat.typ, Chattype::OutBroadcast | Chattype::InBroadcast) { + let origin = match recipient_ids.len() { + 1 => Origin::OutgoingTo, + // Use the same origin as ChatId::accept_ex() does for groups. + _ => Origin::IncomingTo, + }; info!( context, - "Scale up origin of {} recipients to OutgoingTo.", chat.id + "Scale up origin of {} recipients to {origin:?}.", chat.id ); - ContactId::scaleup_origin(context, &recipient_ids, Origin::OutgoingTo).await?; + ContactId::scaleup_origin(context, &recipient_ids, origin).await?; } if !msg.is_system_message() diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 809ead1d6..47337dda8 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -3875,21 +3875,17 @@ async fn test_group_contacts_goto_bottom() -> Result<()> { assert_eq!(Contact::get_all(bob, 0, None).await?.len(), 0); bob_chat_id.accept(bob).await?; let contacts = Contact::get_all(bob, 0, None).await?; - assert_eq!(contacts.len(), 2); let bob_fiona_id = bob.add_or_lookup_contact_id(fiona).await; + // Fiona hasn't been online, so she goes after Alice. + assert_eq!(contacts.len(), 2); assert_eq!(contacts[1], bob_fiona_id); - ChatId::create_for_contact(bob, bob_fiona_id).await?; + let bob_fiona_chat_id = ChatId::create_for_contact(bob, bob_fiona_id).await?; let contacts = Contact::get_all(bob, 0, None).await?; assert_eq!(contacts.len(), 2); assert_eq!(contacts[0], bob_fiona_id); - send_text_msg( - bob, - bob_chat_id, - "Hi Alice, stay down in my contact list".to_string(), - ) - .await?; + send_text_msg(bob, bob_chat_id, "Hi all".to_string()).await?; bob.pop_sent_msg().await; let contacts = Contact::get_all(bob, 0, None).await?; assert_eq!(contacts[0], bob_fiona_id); @@ -3905,6 +3901,13 @@ async fn test_group_contacts_goto_bottom() -> Result<()> { bob.pop_sent_msg().await; let contacts = Contact::get_all(bob, 0, None).await?; let bob_alice_id = bob.add_or_lookup_contact_id(alice).await; + // As the group only contains Alice, the sent message promotes her in the contact list. + assert_eq!(contacts[0], bob_alice_id); + + send_text_msg(bob, bob_fiona_chat_id, "Hi Fiona".to_string()).await?; + bob.pop_sent_msg().await; + let contacts = Contact::get_all(bob, 0, None).await?; + // Alice is still the 0th contact because Fiona hasn't been online. assert_eq!(contacts[0], bob_alice_id); Ok(()) }