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.
This commit is contained in:
B. Petersen
2020-02-18 01:18:10 +01:00
committed by holger krekel
parent fa3d98a492
commit 8fdb048b6a

View File

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