fix: apply_group_changes: Don't implicitly delete members locally, add absent ones instead (#4934)

This is another approach to provide group membership consistency for all members. Considerations:
- Classical MUA users usually don't intend to remove users from an email thread, so if they removed
  a recipient then it was probably by accident.
- DC users could miss new member additions and then better to handle this in the same way as for
  classical MUA messages. Moreover, if we remove a member implicitly, they will never know that and
  continue to think they're still here.

But it shouldn't be a big problem if somebody missed a member removal, because they will likely
recreate the member list from the next received message. The problem occurs only if that "somebody"
managed to reply earlier. Really, it's a problem for big groups with high message rate, but let it
be for now.
This commit is contained in:
iequidoo
2023-11-06 04:10:44 -03:00
committed by link2xt
parent 836f65376c
commit ce32f76265
2 changed files with 41 additions and 29 deletions

View File

@@ -3411,14 +3411,24 @@ async fn test_dont_recreate_contacts_on_add_remove() -> Result<()> {
alice.recv_msg(&bob.pop_sent_msg().await).await;
// Bob didn't receive the addition of Fiona, so Alice must remove Fiona from the members list
// back to make their group members view consistent.
assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 3);
// Bob didn't receive the addition of Fiona, but Alice mustn't remove Fiona from the members
// list back. Instead, Bob must add Fiona from the next Alice's message to make their group
// members view consistent.
assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 4);
// Just a dumb check for remove_contact_from_chat(). Let's have it in this only place.
remove_contact_from_chat(&bob, bob_chat_id, bob_blue).await?;
alice.recv_msg(&bob.pop_sent_msg().await).await;
assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 2);
assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 3);
send_text_msg(
&alice,
alice_chat_id,
"Finally add Fiona please".to_string(),
)
.await?;
bob.recv_msg(&alice.pop_sent_msg().await).await;
assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 3);
Ok(())
}
@@ -3502,8 +3512,11 @@ async fn test_dont_readd_with_normal_msg() -> Result<()> {
bob.recv_msg(&alice.pop_sent_msg().await).await;
// Alice didn't receive Bobs leave message, but bob shouldn't readded himself just because of that.
assert!(!is_contact_in_chat(&bob, bob_chat_id, ContactId::SELF).await?);
// Alice didn't receive Bob's leave message, so Bob must readd themselves otherwise other
// members would think Bob is still here while they aren't, and then retry to leave if they
// think that Alice didn't re-add them on purpose (which is possible if Alice uses a classical
// MUA).
assert!(is_contact_in_chat(&bob, bob_chat_id, ContactId::SELF).await?);
Ok(())
}