From 1f336f89a6a60003897183bd1b5ac5be719e479e Mon Sep 17 00:00:00 2001 From: iequidoo Date: Thu, 23 Nov 2023 22:28:25 -0300 Subject: [PATCH] feat: Sync `Config::Displayname` across devices (#4893) We already synchronise status/footer when we see a self-sent message with a Chat-Version header. Would be nice to do the same for display name. But let's do it the same way as for `Config::{MdnsEnabled,ShowEmails}`. Otherwise, if we sync the display name using the "From" header, smth like `Param::StatusTimestamp` is needed then to reject outdated display names. Also this timestamp needs to be updated when `Config::Displayname` is set locally. Also this wouldn't work if system time isn't synchronised on devices. Also using multiple approaches to sync different config values would lead to more code and bugs while having almost no value -- using "From" only saves some bytes and allows to sync some things w/o the synchronisation itself to be enabled. But the latter also can be a downside -- if it's usual synchonisation, you can (potentially) disable it and share the same email account across people in some organisation allowing them to have different display names. With using "From" for synchronisation such a capability definitely requires a new config option. --- python/tests/test_4_lowlevel.py | 2 ++ src/config.rs | 37 +++++++++++++++++++-------------- src/receive_imf.rs | 10 ++++----- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/python/tests/test_4_lowlevel.py b/python/tests/test_4_lowlevel.py index 23ef3dbe3..4d9865ec6 100644 --- a/python/tests/test_4_lowlevel.py +++ b/python/tests/test_4_lowlevel.py @@ -156,6 +156,8 @@ def test_markseen_invalid_message_ids(acfactory): chat = contact1.create_chat() chat.send_text("one message") ac1._evtracker.get_matching("DC_EVENT_MSGS_CHANGED") + # Skip configuration-related warnings, but not errors. + ac1._evtracker.ensure_event_not_queued("DC_EVENT_ERROR") msg_ids = [9] lib.dc_markseen_msgs(ac1._dc_context, msg_ids, len(msg_ids)) ac1._evtracker.ensure_event_not_queued("DC_EVENT_WARNING|DC_EVENT_ERROR") diff --git a/src/config.rs b/src/config.rs index 192b4e25c..a71f3ca53 100644 --- a/src/config.rs +++ b/src/config.rs @@ -356,7 +356,10 @@ impl Config { /// multiple users are sharing an account. Another example is `Self::SyncMsgs` itself which /// mustn't be controlled by other devices. pub(crate) fn is_synced(&self) -> bool { - matches!(self, Self::MdnsEnabled | Self::ShowEmails) + matches!( + self, + Self::Displayname | Self::MdnsEnabled | Self::ShowEmails + ) } } @@ -487,8 +490,9 @@ impl Context { &self, sync: sync::Sync, key: Config, - value: Option<&str>, + mut value: Option<&str>, ) -> Result<()> { + let better_value; match key { Config::Selfavatar => { self.sql @@ -515,10 +519,11 @@ impl Context { ret? } Config::Displayname => { - let value = value.map(improve_single_line_input); - self.sql - .set_raw_config(key.as_ref(), value.as_deref()) - .await?; + if let Some(v) = value { + better_value = improve_single_line_input(v); + value = Some(&better_value); + } + self.sql.set_raw_config(key.as_ref(), value).await?; } Config::Socks5Enabled | Config::BccSelf @@ -873,16 +878,6 @@ mod tests { mdns_enabled ); - // Usual sync scenario. - alice0 - .set_config_bool(Config::MdnsEnabled, !mdns_enabled) - .await?; - sync(&alice0, &alice1).await?; - assert_eq!( - alice1.get_config_bool(Config::MdnsEnabled).await?, - !mdns_enabled - ); - // Reset to default. Test that it's not synced because defaults may differ across client // versions. alice0.set_config(Config::MdnsEnabled, None).await?; @@ -908,6 +903,16 @@ mod tests { sync(&alice0, &alice1).await?; assert!(alice1.get_config_bool(Config::MdnsEnabled).await?); + // Usual sync scenario. + alice0 + .set_config(Config::Displayname, Some("Alice Sync")) + .await?; + sync(&alice0, &alice1).await?; + assert_eq!( + alice1.get_config(Config::Displayname).await?, + Some("Alice Sync".to_string()) + ); + Ok(()) } } diff --git a/src/receive_imf.rs b/src/receive_imf.rs index f5374015c..baa1578ff 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -451,7 +451,10 @@ pub(crate) async fn receive_imf_inner( /// /// Also returns whether it is blocked or not and its origin. /// -/// * `prevent_rename`: passed through to `add_or_lookup_contacts_by_address_list()` +/// * `prevent_rename`: if true, the display_name of this contact will not be changed. Useful for +/// mailing lists: In some mailing lists, many users write from the same address but with different +/// display names. We don't want the display name to change every time the user gets a new email from +/// a mailing list. /// /// Returns `None` if From field does not contain a valid contact address. pub async fn from_field_to_contact_id( @@ -2600,11 +2603,6 @@ pub(crate) async fn get_prefetch_parent_message( /// Looks up contact IDs from the database given the list of recipients. /// /// Returns vector of IDs guaranteed to be unique. -/// -/// * param `prevent_rename`: if true, the display_name of this contact will not be changed. Useful for -/// mailing lists: In some mailing lists, many users write from the same address but with different -/// display names. We don't want the display name to change every time the user gets a new email from -/// a mailing list. async fn add_or_lookup_contacts_by_address_list( context: &Context, address_list: &[SingleInfo],