From 8fdb048b6a77d91bb0c96bc583aa8bb2b659f3ea Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Tue, 18 Feb 2020 01:18:10 +0100 Subject: [PATCH] alter the memberlist more carefully up to now, Chat-Group-Member-Added and -Removed commands result in a complete recreation of the memberlist by collecting all addresses from the From: and To: headers. this easily results in missed and accidentally removed members, esp. when several people at the same time scan a qr code to join a group. this commit changes the behavior of adding members by not removing members on the Chat-Group-Member-Added command. instead the existing memberlist is conjuncted with the memberlist seen in the message. only adding the member from the Chat-Group-Member-Added seems not to be sufficient - imaging a group of Alice and Bob: - Alice adds Claire - Bob adds Dave _before_ seeing that Alice added Claire - Dave would never get the information that Claire is in the group wrt Chat-Group-Member-Removed: this command does no longer recreate the memberlist but just remove _exactly_ the member mentioned in the header. there are situations, where a just removed member will be readded by out-of-order-messages, however, compared to missed members, this is seems to be acceptable - also as this is more visible and easier to fix (just remove the member again). might be that, in practise, this is not a big issue. while adding members is typically done in masses on bootstraping a group, this is typically not true for removing members. --- src/dc_receive_imf.rs | 29 +++++++++++------------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 5375d7b81..f2f333632 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -985,7 +985,7 @@ fn create_or_lookup_group( // XXX insert code in a different PR :) // execute group commands - if X_MrAddToGrp.is_some() || X_MrRemoveFromGrp.is_some() { + if X_MrAddToGrp.is_some() { recreate_member_list = true; } else if X_MrGrpNameChanged { if let Some(ref grpname) = grpname { @@ -1021,39 +1021,32 @@ fn create_or_lookup_group( } // add members to group/check members - // for recreation: we should add a timestamp if recreate_member_list { - // TODO: the member list should only be recreated if the corresponding message is newer - // than the one that is responsible for the current member list, see - // https://github.com/deltachat/deltachat-core/issues/127 - - let skip = X_MrRemoveFromGrp.as_ref(); - sql::execute( - context, - &context.sql, - "DELETE FROM chats_contacts WHERE chat_id=?;", - params![chat_id], - ) - .ok(); - if skip.is_none() || !addr_cmp(&self_addr, skip.unwrap()) { + if !chat::is_contact_in_chat(context, chat_id, DC_CONTACT_ID_SELF) { chat::add_to_chat_contacts_table(context, chat_id, DC_CONTACT_ID_SELF); } if from_id > DC_CHAT_ID_LAST_SPECIAL && !Contact::addr_equals_contact(context, &self_addr, from_id as u32) - && (skip.is_none() - || !Contact::addr_equals_contact(context, skip.unwrap(), from_id as u32)) + && !chat::is_contact_in_chat(context, chat_id, from_id) { chat::add_to_chat_contacts_table(context, chat_id, from_id as u32); } for &to_id in to_ids.iter() { info!(context, "adding to={:?} to chat id={}", to_id, chat_id); if !Contact::addr_equals_contact(context, &self_addr, to_id) - && (skip.is_none() || !Contact::addr_equals_contact(context, skip.unwrap(), to_id)) + && !chat::is_contact_in_chat(context, chat_id, to_id) { chat::add_to_chat_contacts_table(context, chat_id, to_id); } } send_EVENT_CHAT_MODIFIED = true; + } else if let Some(removed_addr) = X_MrRemoveFromGrp { + let contact_id = Contact::lookup_id_by_addr(context, removed_addr); + if contact_id != 0 { + info!(context, "remove {:?} from chat id={}", contact_id, chat_id); + chat::remove_from_chat_contacts_table(context, chat_id, contact_id); + } + send_EVENT_CHAT_MODIFIED = true; } if send_EVENT_CHAT_MODIFIED {