From ad51a7cd85f473beefd51ba5a4ed5c784fc32239 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Tue, 7 Nov 2023 06:30:46 -0300 Subject: [PATCH] feat: apply_group_changes: Add system messages about implicitly added members --- src/chat.rs | 8 +-- src/receive_imf.rs | 66 ++++++++++++------- src/stock_str.rs | 10 ++- src/test_utils.rs | 7 +- .../chat_test_simultaneous_member_remove | 7 ++ 5 files changed, 64 insertions(+), 34 deletions(-) create mode 100644 test-data/golden/chat_test_simultaneous_member_remove diff --git a/src/chat.rs b/src/chat.rs index c08b351af..c6495cbf7 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4473,13 +4473,11 @@ mod tests { remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?; // Bob receives a msg about Alice adding Claire to the group. - let bob_received_add_msg = bob.recv_msg(&alice_sent_add_msg).await; + bob.recv_msg(&alice_sent_add_msg).await; // Test that add message is rewritten. - assert_eq!( - bob_received_add_msg.get_text(), - "Member claire@example.net added by alice@example.org." - ); + bob.golden_test_chat(bob_chat_id, "chat_test_simultaneous_member_remove") + .await; // Bob receives a msg about Alice removing him from the group. let bob_received_remove_msg = bob.recv_msg(&alice_sent_remove_msg).await; diff --git a/src/receive_imf.rs b/src/receive_imf.rs index fd33eb4b2..86d54914a 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -457,6 +457,7 @@ async fn add_parts( let mut chat_id_blocked = Blocked::Not; let mut better_msg = None; + let mut group_changes_msgs = Vec::new(); if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled { better_msg = Some(stock_str::msg_location_enabled_by(context, from_id).await); } @@ -649,7 +650,7 @@ async fn add_parts( } } - better_msg = better_msg.or(apply_group_changes( + group_changes_msgs = apply_group_changes( context, mime_parser, sent_timestamp, @@ -658,7 +659,7 @@ async fn add_parts( to_ids, is_partial_download.is_some(), ) - .await?); + .await?; } if chat_id.is_none() { @@ -892,7 +893,7 @@ async fn add_parts( } if let Some(chat_id) = chat_id { - better_msg = better_msg.or(apply_group_changes( + group_changes_msgs = apply_group_changes( context, mime_parser, sent_timestamp, @@ -901,7 +902,7 @@ async fn add_parts( to_ids, is_partial_download.is_some(), ) - .await?); + .await?; } if chat_id.is_none() && self_sent { @@ -1115,7 +1116,9 @@ async fn add_parts( let mut created_db_entries = Vec::with_capacity(mime_parser.parts.len()); - for part in &mut mime_parser.parts { + group_changes_msgs = group_changes_msgs.into_iter().rev().collect(); + let mut parts = mime_parser.parts.iter_mut().peekable(); + while let Some(part) = parts.peek() { if part.is_reaction { let reaction_str = simplify::remove_footers(part.msg.as_str()); set_msg_reaction( @@ -1148,7 +1151,10 @@ async fn add_parts( } let mut txt_raw = "".to_string(); - let (msg, typ): (&str, Viewtype) = if let Some(better_msg) = &better_msg { + let group_changes_msg = group_changes_msgs.pop(); + let (msg, typ): (&str, Viewtype) = if let Some(msg) = &group_changes_msg { + (msg, Viewtype::Text) + } else if let Some(better_msg) = &better_msg { (better_msg, Viewtype::Text) } else { (&part.msg, part.typ) @@ -1276,6 +1282,10 @@ RETURNING id debug_assert!(!row_id.is_special()); created_db_entries.push(row_id); + + if group_changes_msg.is_none() || (group_changes_msgs.is_empty() && better_msg.is_none()) { + parts.next(); + } } // check all parts whether they contain a new logging webxdc @@ -1704,15 +1714,15 @@ async fn apply_group_changes( from_id: ContactId, to_ids: &[ContactId], is_partial_download: bool, -) -> Result> { +) -> Result> { let mut chat = Chat::load_from_db(context, chat_id).await?; if chat.typ != Chattype::Group { - return Ok(None); + return Ok(Vec::new()); } let mut send_event_chat_modified = false; let (mut removed_id, mut added_id) = (None, None); - let mut better_msg = None; + let mut group_changes_msgs = Vec::new(); // True if a Delta Chat client has explicitly added our current primary address. let self_added = @@ -1777,11 +1787,11 @@ async fn apply_group_changes( if let Some(removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) { removed_id = Contact::lookup_id_by_addr(context, removed_addr, Origin::Unknown).await?; - better_msg = if removed_id == Some(from_id) { - Some(stock_str::msg_group_left_local(context, from_id).await) + group_changes_msgs.push(if removed_id == Some(from_id) { + stock_str::msg_group_left_local(context, from_id).await } else { - Some(stock_str::msg_del_member_local(context, removed_addr, from_id).await) - }; + stock_str::msg_del_member_local(context, removed_addr, from_id).await + }); if removed_id.is_some() { if !allow_member_list_changes { @@ -1794,7 +1804,8 @@ async fn apply_group_changes( warn!(context, "Removed {removed_addr:?} has no contact id.") } } else if let Some(added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { - better_msg = Some(stock_str::msg_add_member_local(context, added_addr, from_id).await); + group_changes_msgs + .push(stock_str::msg_add_member_local(context, added_addr, from_id).await); if allow_member_list_changes { if !recreate_member_list { @@ -1835,21 +1846,20 @@ async fn apply_group_changes( send_event_chat_modified = true; } - better_msg = Some(stock_str::msg_grp_name(context, old_name, grpname, from_id).await); + group_changes_msgs + .push(stock_str::msg_grp_name(context, old_name, grpname, from_id).await); } } else if let Some(value) = mime_parser.get_header(HeaderDef::ChatContent) { if value == "group-avatar-changed" { if let Some(avatar_action) = &mime_parser.group_avatar { // this is just an explicit message containing the group-avatar, // apart from that, the group-avatar is send along with various other messages - better_msg = match avatar_action { - AvatarAction::Delete => { - Some(stock_str::msg_grp_img_deleted(context, from_id).await) - } + group_changes_msgs.push(match avatar_action { + AvatarAction::Delete => stock_str::msg_grp_img_deleted(context, from_id).await, AvatarAction::Change(_) => { - Some(stock_str::msg_grp_img_changed(context, from_id).await) + stock_str::msg_grp_img_changed(context, from_id).await } - }; + }); } } } @@ -1883,6 +1893,18 @@ async fn apply_group_changes( if !diff.is_empty() { warn!(context, "Implicit addition of {diff:?} to chat {chat_id}."); } + group_changes_msgs.reserve(diff.len()); + for contact_id in diff { + let contact = Contact::get_by_id(context, contact_id).await?; + group_changes_msgs.push( + stock_str::msg_add_member_local( + context, + contact.get_addr(), + ContactId::UNDEFINED, + ) + .await, + ); + } } if let Some(removed_id) = removed_id { new_members.remove(&removed_id); @@ -1949,7 +1971,7 @@ async fn apply_group_changes( if send_event_chat_modified { context.emit_event(EventType::ChatModified(chat_id)); } - Ok(better_msg) + Ok(group_changes_msgs) } static LIST_ID_REGEX: Lazy = Lazy::new(|| Regex::new(r"^(.+)<(.+)>$").unwrap()); diff --git a/src/stock_str.rs b/src/stock_str.rs index 55693da14..1a70e213b 100644 --- a/src/stock_str.rs +++ b/src/stock_str.rs @@ -416,6 +416,9 @@ pub enum StockMessage { #[strum(props(fallback = "Others will only see this group after you sent a first message."))] NewGroupSendFirstMessage = 172, + + #[strum(props(fallback = "Member %1$s added."))] + MsgAddMember = 173, } impl StockMessage { @@ -603,6 +606,7 @@ pub(crate) async fn msg_grp_img_changed(context: &Context, by_contact: ContactId } /// Stock string: `I added member %1$s.`. +/// This one is for sending in group chats. /// /// The `added_member_addr` parameter should be an email address and is looked up in the /// contacts to combine with the authorized display name. @@ -637,7 +641,11 @@ pub(crate) async fn msg_add_member_local( .unwrap_or_else(|_| addr.to_string()), _ => addr.to_string(), }; - if by_contact == ContactId::SELF { + if by_contact == ContactId::UNDEFINED { + translated(context, StockMessage::MsgAddMember) + .await + .replace1(whom) + } else if by_contact == ContactId::SELF { translated(context, StockMessage::MsgYouAddMember) .await .replace1(whom) diff --git a/src/test_utils.rs b/src/test_utils.rs index a3011a142..5980ca41c 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -514,12 +514,7 @@ impl TestContext { .await .expect("receive_imf() seems not to have added a new message to the db"); - assert_eq!( - received.msg_ids.len(), - 1, - "recv_msg() can currently only receive messages with exactly one part" - ); - let msg = Message::load_from_db(self, received.msg_ids[0]) + let msg = Message::load_from_db(self, *received.msg_ids.last().unwrap()) .await .unwrap(); diff --git a/test-data/golden/chat_test_simultaneous_member_remove b/test-data/golden/chat_test_simultaneous_member_remove new file mode 100644 index 000000000..81dbe5057 --- /dev/null +++ b/test-data/golden/chat_test_simultaneous_member_remove @@ -0,0 +1,7 @@ +Group#Chat#10: Group chat [4 member(s)] +-------------------------------------------------------------------------------- +Msg#10: (Contact#Contact#10): Hi! I created a group. [FRESH] +Msg#11: Me (Contact#Contact#Self): You left the group. [INFO] o +Msg#12: (Contact#Contact#10): Member claire@example.net added by alice@example.org. [FRESH][INFO] +Msg#13: (Contact#Contact#10): Member Me (bob@example.net) added. [FRESH][INFO] +--------------------------------------------------------------------------------