From 84456e510b304d092d92b52774d15ad07829926a Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sun, 5 Jan 2025 23:08:42 +0100 Subject: [PATCH 1/5] Add tilde if origin bool { self >= Origin::IncomingReplyTo } + + fn displayname_prefix(self) -> &'static str { + if self == Origin::ManuallyCreated { + "" + } else { + "~" + } + } } #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -593,6 +601,7 @@ impl Contact { .get_config(Config::Selfstatus) .await? .unwrap_or_default(); + contact.origin = Origin::ManuallyCreated; } else if contact_id == ContactId::DEVICE { contact.name = stock_str::device_messages(context).await; contact.addr = ContactId::DEVICE_ADDR.to_string(); @@ -866,33 +875,35 @@ impl Contact { } else { row_name }; - + let new_addr = if update_addr { + addr.to_string() + } else { + row_addr + }; + let new_origin = if origin > row_origin { + origin + } else { + row_origin + }; + let new_authname = if update_authname { + name.to_string() + } else { + row_authname + }; transaction .execute( "UPDATE contacts SET name=?, addr=?, origin=?, authname=? WHERE id=?;", ( - new_name, - if update_addr { - addr.to_string() - } else { - row_addr - }, - if origin > row_origin { - origin - } else { - row_origin - }, - if update_authname { - name.to_string() - } else { - row_authname - }, + new_name.clone(), + new_addr, + new_origin, + new_authname.clone(), row_id ), )?; if update_name || update_authname { - // Update the contact name also if it is used as a group name. + // The contact name is also used as the name of the 1:1 chat, which therefore also needs to be updated. // This is one of the few duplicated data, however, getting the chat list is easier this way. let chat_id: Option = transaction.query_row( "SELECT id FROM chats WHERE type=? AND id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?)", @@ -904,26 +915,13 @@ impl Contact { ).optional()?; if let Some(chat_id) = chat_id { - let contact_id = ContactId::new(row_id); - let (addr, name, authname) = - transaction.query_row( - "SELECT addr, name, authname - FROM contacts - WHERE id=?", - (contact_id,), - |row| { - let addr: String = row.get(0)?; - let name: String = row.get(1)?; - let authname: String = row.get(2)?; - Ok((addr, name, authname)) - })?; - - let chat_name = if !name.is_empty() { - name - } else if !authname.is_empty() { - authname + let prefix = origin.displayname_prefix(); + let chat_name = if !new_name.is_empty() { + format!("{prefix}{}", new_name) + } else if !new_authname.is_empty() { + format!("{prefix}{}", new_authname) } else { - addr + addr.to_string() }; let count = transaction.execute( @@ -933,7 +931,7 @@ impl Contact { if count > 0 { // Chat name updated context.emit_event(EventType::ChatModified(chat_id)); - chatlist_events::emit_chatlist_items_changed_for_contact(context, contact_id); + chatlist_events::emit_chatlist_items_changed_for_contact(context, ContactId::new(row_id)); } } } @@ -1365,14 +1363,15 @@ impl Contact { /// /// This name is typically used in lists. /// To get the name editable in a formular, use `Contact::get_name`. - pub fn get_display_name(&self) -> &str { + pub fn get_display_name(&self) -> String { + let prefix = self.origin.displayname_prefix(); if !self.name.is_empty() { - return &self.name; + return format!("{prefix}{}", self.name); } if !self.authname.is_empty() { - return &self.authname; + return format!("{prefix}{}", self.authname); } - &self.addr + self.addr.clone() } /// Get a summary of authorized name and address. @@ -1401,10 +1400,11 @@ impl Contact { /// The summary is typically used when asking the user something about the contact. /// The attached email address makes the question unique, eg. "Chat with Alan Miller (am@uniquedomain.com)?" pub fn get_name_n_addr(&self) -> String { + let prefix = self.origin.displayname_prefix(); if !self.name.is_empty() { - format!("{} ({})", self.name, self.addr) + format!("{prefix}{} ({})", self.name, self.addr) } else if !self.authname.is_empty() { - format!("{} ({})", self.authname, self.addr) + format!("{prefix}{} ({})", self.authname, self.addr) } else { (&self.addr).into() } @@ -2053,7 +2053,7 @@ mod tests { let contact = Contact::get_by_id(&context.ctx, id).await.unwrap(); assert_eq!(contact.get_name(), ""); assert_eq!(contact.get_authname(), "bob"); - assert_eq!(contact.get_display_name(), "bob"); + assert_eq!(contact.get_display_name(), "~bob"); // Search by name. let contacts = Contact::get_all(&context.ctx, 0, Some("bob")).await?; @@ -2136,7 +2136,7 @@ mod tests { assert_eq!(contact.get_id(), contact_id); assert_eq!(contact.get_name(), "Name one"); assert_eq!(contact.get_authname(), "bla foo"); - assert_eq!(contact.get_display_name(), "Name one"); + assert_eq!(contact.get_display_name(), "~Name one"); assert_eq!(contact.get_addr(), "one@eins.org"); assert_eq!(contact.get_name_n_addr(), "Name one (one@eins.org)"); @@ -2276,14 +2276,14 @@ mod tests { ) .await?; let chat_id = t.get_last_msg().await.get_chat_id(); - assert_eq!(Chat::load_from_db(&t, chat_id).await?.name, "Flobbyfoo"); + assert_eq!(Chat::load_from_db(&t, chat_id).await?.name, "~Flobbyfoo"); let chatlist = Chatlist::try_load(&t, 0, Some("flobbyfoo"), None).await?; assert_eq!(chatlist.len(), 1); let contact = Contact::get_by_id(&t, *contact_id).await?; assert_eq!(contact.get_authname(), "Flobbyfoo"); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "Flobbyfoo"); - assert_eq!(contact.get_name_n_addr(), "Flobbyfoo (f@example.org)"); + assert_eq!(contact.get_display_name(), "~Flobbyfoo"); + assert_eq!(contact.get_name_n_addr(), "~Flobbyfoo (f@example.org)"); let contacts = Contact::get_all(&t, 0, Some("f@example.org")).await?; assert_eq!(contacts.len(), 1); let contacts = Contact::get_all(&t, 0, Some("flobbyfoo")).await?; @@ -2312,8 +2312,8 @@ mod tests { let contact = Contact::get_by_id(&t, *contact_id).await?; assert_eq!(contact.get_authname(), "Foo Flobby"); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "Foo Flobby"); - assert_eq!(contact.get_name_n_addr(), "Foo Flobby (f@example.org)"); + assert_eq!(contact.get_display_name(), "~Foo Flobby"); + assert_eq!(contact.get_name_n_addr(), "~Foo Flobby (f@example.org)"); let contacts = Contact::get_all(&t, 0, Some("f@example.org")).await?; assert_eq!(contacts.len(), 1); let contacts = Contact::get_all(&t, 0, Some("flobbyfoo")).await?; @@ -2439,7 +2439,7 @@ mod tests { let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_authname(), "bob1"); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "bob1"); + assert_eq!(contact.get_display_name(), "~bob1"); // incoming mail `From: bob2 ` - this should update authname let (contact_id, sth_modified) = Contact::add_or_lookup( @@ -2455,7 +2455,7 @@ mod tests { let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_authname(), "bob2"); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "bob2"); + assert_eq!(contact.get_display_name(), "~bob2"); // manually edit name to "bob3" - authname should be still be "bob2" as given in `From:` above let contact_id = Contact::create(&t, "bob3", "bob@example.org") @@ -2510,7 +2510,8 @@ mod tests { let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_authname(), "claire1"); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "claire1"); + println!("dbg/TODO {:?}", contact.origin); + assert_eq!(contact.get_display_name(), "~claire1"); // TODO this NEEDS to be "~claire1" // incoming mail `From: claire2 ` - this should update authname let (contact_id_same, sth_modified) = Contact::add_or_lookup( @@ -2547,7 +2548,7 @@ mod tests { .await?; assert_eq!(sth_modified, Modifier::Created); let contact = Contact::get_by_id(&t, contact_id).await?; - assert_eq!(contact.get_display_name(), "Bob"); + assert_eq!(contact.get_display_name(), "~Bob"); // Incoming message from someone else with "Not Bob" in the "To:" field. let (contact_id_same, sth_modified) = Contact::add_or_lookup( @@ -2560,7 +2561,7 @@ mod tests { assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::get_by_id(&t, contact_id).await?; - assert_eq!(contact.get_display_name(), "Not Bob"); + assert_eq!(contact.get_display_name(), "~Not Bob"); // Incoming message from Bob, changing the name back. let (contact_id_same, sth_modified) = Contact::add_or_lookup( @@ -2573,7 +2574,7 @@ mod tests { assert_eq!(contact_id, contact_id_same); assert_eq!(sth_modified, Modifier::Modified); // This was None until the bugfix let contact = Contact::get_by_id(&t, contact_id).await?; - assert_eq!(contact.get_display_name(), "Bob"); + assert_eq!(contact.get_display_name(), "~Bob"); Ok(()) } diff --git a/src/events/chatlist_events.rs b/src/events/chatlist_events.rs index 4b612f3eed..1649244152 100644 --- a/src/events/chatlist_events.rs +++ b/src/events/chatlist_events.rs @@ -241,7 +241,7 @@ mod test_chatlist_events { .await; bob.recv_msg(&sent_msg).await; let alice_on_bob = bob.add_or_lookup_contact(&alice).await; - assert!(alice_on_bob.get_display_name() == "Alice"); + assert_eq!(alice_on_bob.get_display_name(), "~Alice"); wait_for_chatlist_all_items(&bob).await; diff --git a/src/message.rs b/src/message.rs index 07e597955b..09d77f3fb2 100644 --- a/src/message.rs +++ b/src/message.rs @@ -230,7 +230,7 @@ impl MsgId { let name = from_contact.get_name_n_addr(); if let Some(override_sender_name) = msg.get_override_sender_name() { let addr = from_contact.get_addr(); - ret += &format!(" by ~{override_sender_name} ({addr})"); + ret += &format!(" by {override_sender_name} ({addr})"); } else { ret += &format!(" by {name}"); } @@ -895,7 +895,7 @@ impl Message { pub fn get_override_sender_name(&self) -> Option { self.param .get(Param::OverrideSenderDisplayname) - .map(|name| name.to_string()) + .map(|name| format!("~{name}")) } // Exposing this function over the ffi instead of get_override_sender_name() would mean that at least Android Java code has @@ -2437,10 +2437,10 @@ mod tests { msg.set_override_sender_name(Some("over ride".to_string())); assert_eq!( msg.get_override_sender_name(), - Some("over ride".to_string()) + Some("~over ride".to_string()) ); - assert_eq!(msg.get_sender_name(&contact), "over ride".to_string()); - assert_ne!(contact.get_display_name(), "over ride".to_string()); + assert_eq!(msg.get_sender_name(&contact), "~over ride".to_string()); + assert_ne!(contact.get_display_name(), "~over ride".to_string()); chat::send_msg(&alice, chat.id, &mut msg).await.unwrap(); let sent_msg = alice.pop_sent_msg().await; @@ -2457,10 +2457,10 @@ mod tests { assert_eq!(msg.text, "bla blubb"); assert_eq!( msg.get_override_sender_name(), - Some("over ride".to_string()) + Some("~over ride".to_string()) ); - assert_eq!(msg.get_sender_name(&contact), "over ride".to_string()); - assert_ne!(contact.get_display_name(), "over ride".to_string()); + assert_eq!(msg.get_sender_name(&contact), "~over ride".to_string()); + assert_ne!(contact.get_display_name(), "~over ride".to_string()); // explicitly check that the message does not create a mailing list // (mailing lists may also use `Sender:`-header) @@ -2471,7 +2471,7 @@ mod tests { let msg = alice2.recv_msg(&sent_msg).await; assert_eq!( msg.get_override_sender_name(), - Some("over ride".to_string()) + Some("~over ride".to_string()) ); } diff --git a/src/peerstate.rs b/src/peerstate.rs index cf1e30e9a6..946c8c49de 100644 --- a/src/peerstate.rs +++ b/src/peerstate.rs @@ -664,7 +664,7 @@ impl Peerstate { let old_contact = Contact::get_by_id(context, contact_id).await?; stock_str::aeap_addr_changed( context, - old_contact.get_display_name(), + &old_contact.get_display_name(), &self.addr, new_addr, ) diff --git a/src/qr.rs b/src/qr.rs index 4d2ab4ea24..c8b84cc803 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -1080,7 +1080,7 @@ mod tests { assert_eq!(contact.get_addr(), "stress@test.local"); assert_eq!(contact.get_name(), "First Last"); assert_eq!(contact.get_authname(), ""); - assert_eq!(contact.get_display_name(), "First Last"); + assert_eq!(contact.get_display_name(), "~First Last"); assert!(draft.is_none()); } else { bail!("Wrong QR code type"); diff --git a/src/reaction.rs b/src/reaction.rs index f0f18b2598..2150140353 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -731,7 +731,7 @@ Here's my footer -- bob@example.net" assert_eq!(summary.state, MessageState::InFresh); // state refers to message, not to reaction assert!(summary.prefix.is_none()); assert!(summary.thumbnail_path.is_none()); - assert_summary(&alice, "BOB reacted 👍 to \"Party?\"").await; + assert_summary(&alice, "~BOB reacted 👍 to \"Party?\"").await; // Alice reacts to own message as well SystemTime::shift(Duration::from_secs(10)); @@ -742,7 +742,7 @@ Here's my footer -- bob@example.net" expect_no_unwanted_events(&bob).await; assert_summary(&alice, "You reacted 🍿 to \"Party?\"").await; - assert_summary(&bob, "ALICE reacted 🍿 to \"Party?\"").await; + assert_summary(&bob, "~ALICE reacted 🍿 to \"Party?\"").await; // Alice sends a newer message, this overwrites reaction summaries SystemTime::shift(Duration::from_secs(10)); @@ -759,7 +759,7 @@ Here's my footer -- bob@example.net" bob.recv_msg_opt(&alice_send_reaction).await; assert_summary(&alice, "You reacted 🤘 to \"Party?\"").await; - assert_summary(&bob, "ALICE reacted 🤘 to \"Party?\"").await; + assert_summary(&bob, "~ALICE reacted 🤘 to \"Party?\"").await; // Retracted reactions remove all summary reactions SystemTime::shift(Duration::from_secs(10)); diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index 398471719e..9683177478 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -165,7 +165,7 @@ async fn test_adhoc_group_show_accepted_contact_accepted() { chat_id.accept(&t).await.unwrap(); let chat = chat::Chat::load_from_db(&t, chat_id).await.unwrap(); assert_eq!(chat.typ, Chattype::Single); - assert_eq!(chat.name, "Bob"); + assert_eq!(chat.name, "~Bob"); assert_eq!(chat::get_chat_contacts(&t, chat_id).await.unwrap().len(), 1); assert_eq!(chat::get_chat_msgs(&t, chat_id).await.unwrap().len(), 1); @@ -584,7 +584,7 @@ async fn test_escaped_recipients() { .unwrap(); let contact = Contact::get_by_id(&t, carl_contact_id).await.unwrap(); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "h2"); + assert_eq!(contact.get_display_name(), "~h2"); let chats = Chatlist::try_load(&t, 0, None, None).await.unwrap(); let msg = Message::load_from_db(&t, chats.get_msg_id(0).unwrap().unwrap()) @@ -631,7 +631,7 @@ async fn test_cc_to_contact() { .unwrap(); let contact = Contact::get_by_id(&t, carl_contact_id).await.unwrap(); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "Carl"); + assert_eq!(contact.get_display_name(), "~Carl"); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -1011,8 +1011,8 @@ async fn test_github_mailing_list() -> Result<()> { let contact2 = Contact::get_by_id(&t.ctx, msg2.from_id).await?; assert_eq!(contact2.get_addr(), "notifications@github.com"); - assert_eq!(msg1.get_override_sender_name().unwrap(), "Max Mustermann"); - assert_eq!(msg2.get_override_sender_name().unwrap(), "Github"); + assert_eq!(msg1.get_override_sender_name().unwrap(), "~Max Mustermann"); + assert_eq!(msg2.get_override_sender_name().unwrap(), "~Github"); Ok(()) } @@ -2077,7 +2077,7 @@ async fn check_alias_reply(from_dc: bool, chat_request: bool, group_request: boo } assert_eq!( answer.get_override_sender_name().unwrap(), - "bob@example.net" + "~bob@example.net" ); // Bob is not part of the group, so override-sender-name should be set // Check that Claire also gets the message in the same chat. @@ -2089,7 +2089,7 @@ async fn check_alias_reply(from_dc: bool, chat_request: bool, group_request: boo assert_eq!(answer.chat_id, request.chat_id); assert_eq!( answer.get_override_sender_name().unwrap(), - "bob@example.net" + "~bob@example.net" ); } @@ -2313,12 +2313,12 @@ Second signature"; receive_imf(&alice, first_message, false).await?; let contact = Contact::get_by_id(&alice, bob_contact_id).await?; assert_eq!(contact.get_status(), "First signature"); - assert_eq!(contact.get_display_name(), "Bob1"); + assert_eq!(contact.get_display_name(), "~Bob1"); receive_imf(&alice, second_message, false).await?; let contact = Contact::get_by_id(&alice, bob_contact_id).await?; assert_eq!(contact.get_status(), "Second signature"); - assert_eq!(contact.get_display_name(), "Bob2"); + assert_eq!(contact.get_display_name(), "~Bob2"); // Duplicate message, should be ignored receive_imf(&alice, first_message, false).await?; @@ -2326,7 +2326,7 @@ Second signature"; // No change because last message is duplicate of the first. let contact = Contact::get_by_id(&alice, bob_contact_id).await?; assert_eq!(contact.get_status(), "Second signature"); - assert_eq!(contact.get_display_name(), "Bob2"); + assert_eq!(contact.get_display_name(), "~Bob2"); Ok(()) } @@ -5230,7 +5230,7 @@ async fn test_list_from() -> Result<()> { let raw = include_bytes!("../../test-data/message/list-from.eml"); let received = receive_imf(t, raw, false).await?.unwrap(); let msg = Message::load_from_db(t, *received.msg_ids.last().unwrap()).await?; - assert_eq!(msg.get_override_sender_name().unwrap(), "ÖAMTC"); + assert_eq!(msg.get_override_sender_name().unwrap(), "~ÖAMTC"); let sender_contact = Contact::get_by_id(t, msg.from_id).await?; assert_eq!( sender_contact.get_display_name(), diff --git a/src/stock_str.rs b/src/stock_str.rs index 9d2ab1a65f..a72126f4f5 100644 --- a/src/stock_str.rs +++ b/src/stock_str.rs @@ -822,7 +822,7 @@ pub(crate) async fn secure_join_started( translated(context, StockMessage::SecureJoinStarted) .await .replace1(&contact.get_name_n_addr()) - .replace2(contact.get_display_name()) + .replace2(&contact.get_display_name()) } else { format!("secure_join_started: unknown contact {inviter_contact_id}") } diff --git a/src/test_utils.rs b/src/test_utils.rs index e680d2025a..73d8cd851e 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -770,7 +770,7 @@ impl TestContext { } else { assert_eq!( actual, expected, - "To update the expected value, run `UPDATE_GOLDEN_TESTS=1 cargo test`" + "To update the expected value, run `UPDATE_GOLDEN_TESTS=1 cargo nextest run`" ); } } diff --git a/src/tests/aeap.rs b/src/tests/aeap.rs index a1863690ab..6587e69d92 100644 --- a/src/tests/aeap.rs +++ b/src/tests/aeap.rs @@ -288,7 +288,8 @@ async fn check_that_transition_worked( let info_msg = get_last_info_msg(bob, *group).await.unwrap(); let expected_text = - stock_str::aeap_addr_changed(bob, name, old_alice_addr, new_alice_addr).await; + stock_str::aeap_addr_changed(bob, &format!("{name}"), old_alice_addr, new_alice_addr) + .await; assert_eq!(info_msg.text, expected_text); assert_eq!(info_msg.from_id, ContactId::INFO); diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs index c8531c5844..5b572d7e5d 100644 --- a/src/tests/verified_chats.rs +++ b/src/tests/verified_chats.rs @@ -947,8 +947,8 @@ async fn test_no_unencrypted_name_if_encrypted() -> Result<()> { let msg = alice.recv_msg(msg).await; let contact = Contact::get_by_id(&alice, msg.from_id).await?; - - assert_eq!(Contact::get_display_name(&contact), "Bob Smith"); + println!("dbg/TODO origin: {:?}", contact.origin); + assert_eq!(Contact::get_display_name(&contact), "~Bob Smith"); } Ok(()) } diff --git a/test-data/golden/test_old_message_1 b/test-data/golden/test_old_message_1 index 61e54d2989..88f0db077a 100644 --- a/test-data/golden/test_old_message_1 +++ b/test-data/golden/test_old_message_1 @@ -1,6 +1,6 @@ -Single#Chat#10: Bob [bob@example.net] +Single#Chat#10: ~Bob [bob@example.net] -------------------------------------------------------------------------------- Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] -Msg#11: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#11: info (Contact#Contact#Info): ~Bob sent a message from another device. [NOTICED][INFO 🛡️❌] Msg#12: (Contact#Contact#10): Message from Thunderbird [SEEN] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_old_message_2 b/test-data/golden/test_old_message_2 index d7e7712abe..da43ae5c9b 100644 --- a/test-data/golden/test_old_message_2 +++ b/test-data/golden/test_old_message_2 @@ -1,7 +1,7 @@ -Single#Chat#10: Bob [bob@example.net] +Single#Chat#10: ~Bob [bob@example.net] -------------------------------------------------------------------------------- Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] -Msg#11: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#11: info (Contact#Contact#Info): ~Bob sent a message from another device. [NOTICED][INFO 🛡️❌] Msg#12: (Contact#Contact#10): Somewhat old message [FRESH] Msg#13: (Contact#Contact#10): Even older message, that must NOT be shown before the info message [SEEN] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_old_message_3 b/test-data/golden/test_old_message_3 index 82630528aa..441c5b34d0 100644 --- a/test-data/golden/test_old_message_3 +++ b/test-data/golden/test_old_message_3 @@ -1,8 +1,8 @@ -Single#Chat#10: Bob [bob@example.net] 🛡️ +Single#Chat#10: ~Bob [bob@example.net] 🛡️ -------------------------------------------------------------------------------- Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] Msg#11🔒: (Contact#Contact#10): Heyho from my verified device! [FRESH] -Msg#12: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#12: info (Contact#Contact#Info): ~Bob sent a message from another device. [NOTICED][INFO 🛡️❌] Msg#13: (Contact#Contact#10): Old, unverified message [SEEN] Msg#14: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_old_message_5 b/test-data/golden/test_old_message_5 index 75d3c0af0d..6fbe1e16e0 100644 --- a/test-data/golden/test_old_message_5 +++ b/test-data/golden/test_old_message_5 @@ -1,4 +1,4 @@ -Single#Chat#10: Bob [bob@example.net] +Single#Chat#10: ~Bob [bob@example.net] -------------------------------------------------------------------------------- Msg#10: Me (Contact#Contact#Self): Happy birthday, Bob! √ Msg#11: (Contact#Contact#10): Happy birthday to me, Alice! [FRESH] diff --git a/test-data/golden/test_verified_oneonone_chat_enable_disable b/test-data/golden/test_verified_oneonone_chat_enable_disable index 4da4a67d01..ce6b966665 100644 --- a/test-data/golden/test_verified_oneonone_chat_enable_disable +++ b/test-data/golden/test_verified_oneonone_chat_enable_disable @@ -1,11 +1,11 @@ -Single#Chat#10: Bob [bob@example.net] 🛡️ +Single#Chat#10: ~Bob [bob@example.net] 🛡️ -------------------------------------------------------------------------------- Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] -Msg#11: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#11: info (Contact#Contact#Info): ~Bob sent a message from another device. [NOTICED][INFO 🛡️❌] Msg#12: (Contact#Contact#10): Message from Thunderbird [FRESH] Msg#13: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] Msg#14🔒: (Contact#Contact#10): Hello from DC [FRESH] -Msg#15: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#15: info (Contact#Contact#Info): ~Bob sent a message from another device. [NOTICED][INFO 🛡️❌] Msg#16: (Contact#Contact#10): Message from Thunderbird [FRESH] Msg#17: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] Msg#18🔒: (Contact#Contact#10): Hello from DC [FRESH] From 9b7e74092633a8d7a70c4144f3d9bf86109f84c9 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 6 Jan 2025 14:50:40 +0100 Subject: [PATCH 2/5] Fix almost all test failures. One of the failures is real, though. --- src/chat.rs | 4 ++-- src/chatlist.rs | 2 +- src/contact.rs | 11 ++++++----- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 83b8be9c11..99769fbcf0 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4677,7 +4677,7 @@ mod tests { { "id": 10, "type": 100, - "name": "~bob", + "name": "bob", "archived": false, "param": "", "gossiped_timestamp": 0, @@ -4975,7 +4975,7 @@ mod tests { // This is the name that will be sent outside. assert_eq!(alice_bob_contact.get_authname(), "Bob"); - assert_eq!(alice_bob_contact.get_display_name(), "~robert"); + assert_eq!(alice_bob_contact.get_display_name(), "robert"); } // Create and promote a group. diff --git a/src/chatlist.rs b/src/chatlist.rs index af541999c9..b29337e55d 100644 --- a/src/chatlist.rs +++ b/src/chatlist.rs @@ -727,7 +727,7 @@ mod tests { let test_id = Contact::create(&t, "Bob Nickname", "bob@example.org").await?; assert_eq!(contact_id, test_id); let chat = Chat::load_from_db(&t, chat_id).await?; - assert_eq!(chat.get_name(), "~Bob Nickname"); + assert_eq!(chat.get_name(), "Bob Nickname"); let chats = Chatlist::try_load(&t, 0, Some("bob@example.org"), None).await?; assert_eq!(chats.len(), 0); // email-addresses are searchable in contacts, not in chats let chats = Chatlist::try_load(&t, 0, Some("Bob Nickname"), None).await?; diff --git a/src/contact.rs b/src/contact.rs index e655754fbc..4152569e6b 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -2110,6 +2110,7 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_add_or_lookup() { // add some contacts, this also tests add_address_book() + // TODO it's unsure whether we should put a ~ in front of these. let t = TestContext::new().await; let book = concat!( " Name one \n one@eins.org \n", @@ -2138,7 +2139,7 @@ mod tests { assert_eq!(contact.get_authname(), "bla foo"); assert_eq!(contact.get_display_name(), "~Name one"); assert_eq!(contact.get_addr(), "one@eins.org"); - assert_eq!(contact.get_name_n_addr(), "Name one (one@eins.org)"); + assert_eq!(contact.get_name_n_addr(), "~Name one (one@eins.org)"); // modify first added contact let (contact_id_test, sth_modified) = Contact::add_or_lookup( @@ -2185,7 +2186,7 @@ mod tests { assert_eq!(contact_id, contact_id_test); assert_eq!(sth_modified, Modifier::Modified); let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); - assert_eq!(contact.get_name_n_addr(), "m. serious (three@drei.sam)"); + assert_eq!(contact.get_name_n_addr(), "~m. serious (three@drei.sam)"); assert!(!contact.is_blocked()); // manually edit name of third contact (does not changed authorized name) @@ -2217,9 +2218,9 @@ mod tests { assert_eq!(sth_modified, Modifier::None); let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_name(), "Wonderland, Alice"); - assert_eq!(contact.get_display_name(), "Wonderland, Alice"); + assert_eq!(contact.get_display_name(), "~Wonderland, Alice"); assert_eq!(contact.get_addr(), "alice@w.de"); - assert_eq!(contact.get_name_n_addr(), "Wonderland, Alice (alice@w.de)"); + assert_eq!(contact.get_name_n_addr(), "~Wonderland, Alice (alice@w.de)"); // check SELF let contact = Contact::get_by_id(&t, ContactId::SELF).await.unwrap(); @@ -2304,7 +2305,7 @@ mod tests { ) .await?; let chat_id = t.get_last_msg().await.get_chat_id(); - assert_eq!(Chat::load_from_db(&t, chat_id).await?.name, "Foo Flobby"); + assert_eq!(Chat::load_from_db(&t, chat_id).await?.name, "~Foo Flobby"); let chatlist = Chatlist::try_load(&t, 0, Some("Flobbyfoo"), None).await?; assert_eq!(chatlist.len(), 0); let chatlist = Chatlist::try_load(&t, 0, Some("Foo Flobby"), None).await?; From 1f8224146561c815ad971b40675056fdaf50389e Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 6 Jan 2025 15:32:04 +0100 Subject: [PATCH 3/5] Fix all tests --- src/chatlist.rs | 2 +- src/contact.rs | 41 +++++++++++++------------------------ src/qr.rs | 2 +- src/tests/verified_chats.rs | 1 - 4 files changed, 16 insertions(+), 30 deletions(-) diff --git a/src/chatlist.rs b/src/chatlist.rs index b29337e55d..b079d5cec8 100644 --- a/src/chatlist.rs +++ b/src/chatlist.rs @@ -682,7 +682,7 @@ mod tests { let test_id = Contact::create(&t, "", "bob@example.org").await?; assert_eq!(contact_id, test_id); let chat = Chat::load_from_db(&t, chat_id).await?; - assert_eq!(chat.get_name(), "Bob Authname"); + assert_eq!(chat.get_name(), "~Bob Authname"); let chats = Chatlist::try_load(&t, 0, Some("bob authname"), None).await?; assert_eq!(chats.len(), 1); let chats = Chatlist::try_load(&t, 0, Some("bob nickname"), None).await?; diff --git a/src/contact.rs b/src/contact.rs index 4152569e6b..1db44265b4 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -510,14 +510,6 @@ impl Origin { pub fn is_known(self) -> bool { self >= Origin::IncomingReplyTo } - - fn displayname_prefix(self) -> &'static str { - if self == Origin::ManuallyCreated { - "" - } else { - "~" - } - } } #[derive(Debug, PartialEq, Eq, Clone, Copy)] @@ -886,7 +878,7 @@ impl Contact { row_origin }; let new_authname = if update_authname { - name.to_string() + name } else { row_authname }; @@ -915,11 +907,10 @@ impl Contact { ).optional()?; if let Some(chat_id) = chat_id { - let prefix = origin.displayname_prefix(); let chat_name = if !new_name.is_empty() { - format!("{prefix}{}", new_name) + new_name } else if !new_authname.is_empty() { - format!("{prefix}{}", new_authname) + format!("~{}", new_authname) } else { addr.to_string() }; @@ -1364,12 +1355,11 @@ impl Contact { /// This name is typically used in lists. /// To get the name editable in a formular, use `Contact::get_name`. pub fn get_display_name(&self) -> String { - let prefix = self.origin.displayname_prefix(); if !self.name.is_empty() { - return format!("{prefix}{}", self.name); + return self.name.clone(); } if !self.authname.is_empty() { - return format!("{prefix}{}", self.authname); + return format!("~{}", self.authname); } self.addr.clone() } @@ -1400,11 +1390,10 @@ impl Contact { /// The summary is typically used when asking the user something about the contact. /// The attached email address makes the question unique, eg. "Chat with Alan Miller (am@uniquedomain.com)?" pub fn get_name_n_addr(&self) -> String { - let prefix = self.origin.displayname_prefix(); if !self.name.is_empty() { - format!("{prefix}{} ({})", self.name, self.addr) + format!("{} ({})", self.name, self.addr) } else if !self.authname.is_empty() { - format!("{prefix}{} ({})", self.authname, self.addr) + format!("~{} ({})", self.authname, self.addr) } else { (&self.addr).into() } @@ -2110,7 +2099,6 @@ mod tests { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_add_or_lookup() { // add some contacts, this also tests add_address_book() - // TODO it's unsure whether we should put a ~ in front of these. let t = TestContext::new().await; let book = concat!( " Name one \n one@eins.org \n", @@ -2137,9 +2125,9 @@ mod tests { assert_eq!(contact.get_id(), contact_id); assert_eq!(contact.get_name(), "Name one"); assert_eq!(contact.get_authname(), "bla foo"); - assert_eq!(contact.get_display_name(), "~Name one"); + assert_eq!(contact.get_display_name(), "Name one"); assert_eq!(contact.get_addr(), "one@eins.org"); - assert_eq!(contact.get_name_n_addr(), "~Name one (one@eins.org)"); + assert_eq!(contact.get_name_n_addr(), "Name one (one@eins.org)"); // modify first added contact let (contact_id_test, sth_modified) = Contact::add_or_lookup( @@ -2218,9 +2206,9 @@ mod tests { assert_eq!(sth_modified, Modifier::None); let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_name(), "Wonderland, Alice"); - assert_eq!(contact.get_display_name(), "~Wonderland, Alice"); + assert_eq!(contact.get_display_name(), "Wonderland, Alice"); assert_eq!(contact.get_addr(), "alice@w.de"); - assert_eq!(contact.get_name_n_addr(), "~Wonderland, Alice (alice@w.de)"); + assert_eq!(contact.get_name_n_addr(), "Wonderland, Alice (alice@w.de)"); // check SELF let contact = Contact::get_by_id(&t, ContactId::SELF).await.unwrap(); @@ -2511,8 +2499,7 @@ mod tests { let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_authname(), "claire1"); assert_eq!(contact.get_name(), ""); - println!("dbg/TODO {:?}", contact.origin); - assert_eq!(contact.get_display_name(), "~claire1"); // TODO this NEEDS to be "~claire1" + assert_eq!(contact.get_display_name(), "~claire1"); // incoming mail `From: claire2 ` - this should update authname let (contact_id_same, sth_modified) = Contact::add_or_lookup( @@ -2528,7 +2515,7 @@ mod tests { let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_authname(), "claire2"); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "claire2"); + assert_eq!(contact.get_display_name(), "~claire2"); } /// Regression test. @@ -2612,7 +2599,7 @@ mod tests { let contact = Contact::get_by_id(&t, contact_id).await.unwrap(); assert_eq!(contact.get_authname(), "dave2"); assert_eq!(contact.get_name(), ""); - assert_eq!(contact.get_display_name(), "dave2"); + assert_eq!(contact.get_display_name(), "~dave2"); } #[test] diff --git a/src/qr.rs b/src/qr.rs index c8b84cc803..4d2ab4ea24 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -1080,7 +1080,7 @@ mod tests { assert_eq!(contact.get_addr(), "stress@test.local"); assert_eq!(contact.get_name(), "First Last"); assert_eq!(contact.get_authname(), ""); - assert_eq!(contact.get_display_name(), "~First Last"); + assert_eq!(contact.get_display_name(), "First Last"); assert!(draft.is_none()); } else { bail!("Wrong QR code type"); diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs index 5b572d7e5d..7af994ed53 100644 --- a/src/tests/verified_chats.rs +++ b/src/tests/verified_chats.rs @@ -947,7 +947,6 @@ async fn test_no_unencrypted_name_if_encrypted() -> Result<()> { let msg = alice.recv_msg(msg).await; let contact = Contact::get_by_id(&alice, msg.from_id).await?; - println!("dbg/TODO origin: {:?}", contact.origin); assert_eq!(Contact::get_display_name(&contact), "~Bob Smith"); } Ok(()) From 057501cacd4641185fa766db0ca038f25a4adc2a Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 6 Jan 2025 15:44:39 +0100 Subject: [PATCH 4/5] Revert the refactoring for now, it maybe things too difficult to review --- src/contact.rs | 64 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/src/contact.rs b/src/contact.rs index 1db44265b4..13328ac9b4 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -867,35 +867,33 @@ impl Contact { } else { row_name }; - let new_addr = if update_addr { - addr.to_string() - } else { - row_addr - }; - let new_origin = if origin > row_origin { - origin - } else { - row_origin - }; - let new_authname = if update_authname { - name - } else { - row_authname - }; + transaction .execute( "UPDATE contacts SET name=?, addr=?, origin=?, authname=? WHERE id=?;", ( - new_name.clone(), - new_addr, - new_origin, - new_authname.clone(), + new_name, + if update_addr { + addr.to_string() + } else { + row_addr + }, + if origin > row_origin { + origin + } else { + row_origin + }, + if update_authname { + name.to_string() + } else { + row_authname + }, row_id ), )?; if update_name || update_authname { - // The contact name is also used as the name of the 1:1 chat, which therefore also needs to be updated. + // Update the contact name also if it is used as a group name. // This is one of the few duplicated data, however, getting the chat list is easier this way. let chat_id: Option = transaction.query_row( "SELECT id FROM chats WHERE type=? AND id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?)", @@ -907,12 +905,26 @@ impl Contact { ).optional()?; if let Some(chat_id) = chat_id { - let chat_name = if !new_name.is_empty() { - new_name - } else if !new_authname.is_empty() { - format!("~{}", new_authname) + let contact_id = ContactId::new(row_id); + let (addr, name, authname) = + transaction.query_row( + "SELECT addr, name, authname + FROM contacts + WHERE id=?", + (contact_id,), + |row| { + let addr: String = row.get(0)?; + let name: String = row.get(1)?; + let authname: String = row.get(2)?; + Ok((addr, name, authname)) + })?; + + let chat_name = if !name.is_empty() { + name + } else if !authname.is_empty() { + format!("~{}", authname) } else { - addr.to_string() + addr }; let count = transaction.execute( @@ -922,7 +934,7 @@ impl Contact { if count > 0 { // Chat name updated context.emit_event(EventType::ChatModified(chat_id)); - chatlist_events::emit_chatlist_items_changed_for_contact(context, ContactId::new(row_id)); + chatlist_events::emit_chatlist_items_changed_for_contact(context, contact_id); } } } From b39abc84d3b842f298de77f361413f012ed60a1d Mon Sep 17 00:00:00 2001 From: Hocuri Date: Mon, 6 Jan 2025 15:45:11 +0100 Subject: [PATCH 5/5] No need to set the origin anymore --- src/contact.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/contact.rs b/src/contact.rs index 13328ac9b4..7f21305fdb 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -593,7 +593,6 @@ impl Contact { .get_config(Config::Selfstatus) .await? .unwrap_or_default(); - contact.origin = Origin::ManuallyCreated; } else if contact_id == ContactId::DEVICE { contact.name = stock_str::device_messages(context).await; contact.addr = ContactId::DEVICE_ADDR.to_string();