feat: apply_group_changes: Add system messages about implicitly added members

This commit is contained in:
iequidoo
2023-11-07 06:30:46 -03:00
committed by iequidoo
parent 28952789a4
commit ad51a7cd85
5 changed files with 64 additions and 34 deletions

View File

@@ -4473,13 +4473,11 @@ mod tests {
remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?; remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?;
// Bob receives a msg about Alice adding Claire to the group. // 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. // Test that add message is rewritten.
assert_eq!( bob.golden_test_chat(bob_chat_id, "chat_test_simultaneous_member_remove")
bob_received_add_msg.get_text(), .await;
"Member claire@example.net added by alice@example.org."
);
// Bob receives a msg about Alice removing him from the group. // Bob receives a msg about Alice removing him from the group.
let bob_received_remove_msg = bob.recv_msg(&alice_sent_remove_msg).await; let bob_received_remove_msg = bob.recv_msg(&alice_sent_remove_msg).await;

View File

@@ -457,6 +457,7 @@ async fn add_parts(
let mut chat_id_blocked = Blocked::Not; let mut chat_id_blocked = Blocked::Not;
let mut better_msg = None; let mut better_msg = None;
let mut group_changes_msgs = Vec::new();
if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled { if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled {
better_msg = Some(stock_str::msg_location_enabled_by(context, from_id).await); 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, context,
mime_parser, mime_parser,
sent_timestamp, sent_timestamp,
@@ -658,7 +659,7 @@ async fn add_parts(
to_ids, to_ids,
is_partial_download.is_some(), is_partial_download.is_some(),
) )
.await?); .await?;
} }
if chat_id.is_none() { if chat_id.is_none() {
@@ -892,7 +893,7 @@ async fn add_parts(
} }
if let Some(chat_id) = chat_id { if let Some(chat_id) = chat_id {
better_msg = better_msg.or(apply_group_changes( group_changes_msgs = apply_group_changes(
context, context,
mime_parser, mime_parser,
sent_timestamp, sent_timestamp,
@@ -901,7 +902,7 @@ async fn add_parts(
to_ids, to_ids,
is_partial_download.is_some(), is_partial_download.is_some(),
) )
.await?); .await?;
} }
if chat_id.is_none() && self_sent { 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()); 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 { if part.is_reaction {
let reaction_str = simplify::remove_footers(part.msg.as_str()); let reaction_str = simplify::remove_footers(part.msg.as_str());
set_msg_reaction( set_msg_reaction(
@@ -1148,7 +1151,10 @@ async fn add_parts(
} }
let mut txt_raw = "".to_string(); 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) (better_msg, Viewtype::Text)
} else { } else {
(&part.msg, part.typ) (&part.msg, part.typ)
@@ -1276,6 +1282,10 @@ RETURNING id
debug_assert!(!row_id.is_special()); debug_assert!(!row_id.is_special());
created_db_entries.push(row_id); 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 // check all parts whether they contain a new logging webxdc
@@ -1704,15 +1714,15 @@ async fn apply_group_changes(
from_id: ContactId, from_id: ContactId,
to_ids: &[ContactId], to_ids: &[ContactId],
is_partial_download: bool, is_partial_download: bool,
) -> Result<Option<String>> { ) -> Result<Vec<String>> {
let mut chat = Chat::load_from_db(context, chat_id).await?; let mut chat = Chat::load_from_db(context, chat_id).await?;
if chat.typ != Chattype::Group { if chat.typ != Chattype::Group {
return Ok(None); return Ok(Vec::new());
} }
let mut send_event_chat_modified = false; let mut send_event_chat_modified = false;
let (mut removed_id, mut added_id) = (None, None); 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. // True if a Delta Chat client has explicitly added our current primary address.
let self_added = let self_added =
@@ -1777,11 +1787,11 @@ async fn apply_group_changes(
if let Some(removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) { if let Some(removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) {
removed_id = Contact::lookup_id_by_addr(context, removed_addr, Origin::Unknown).await?; removed_id = Contact::lookup_id_by_addr(context, removed_addr, Origin::Unknown).await?;
better_msg = if removed_id == Some(from_id) { group_changes_msgs.push(if removed_id == Some(from_id) {
Some(stock_str::msg_group_left_local(context, from_id).await) stock_str::msg_group_left_local(context, from_id).await
} else { } 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 removed_id.is_some() {
if !allow_member_list_changes { if !allow_member_list_changes {
@@ -1794,7 +1804,8 @@ async fn apply_group_changes(
warn!(context, "Removed {removed_addr:?} has no contact id.") warn!(context, "Removed {removed_addr:?} has no contact id.")
} }
} else if let Some(added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { } 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 allow_member_list_changes {
if !recreate_member_list { if !recreate_member_list {
@@ -1835,21 +1846,20 @@ async fn apply_group_changes(
send_event_chat_modified = true; 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) { } else if let Some(value) = mime_parser.get_header(HeaderDef::ChatContent) {
if value == "group-avatar-changed" { if value == "group-avatar-changed" {
if let Some(avatar_action) = &mime_parser.group_avatar { if let Some(avatar_action) = &mime_parser.group_avatar {
// this is just an explicit message containing the 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 // apart from that, the group-avatar is send along with various other messages
better_msg = match avatar_action { group_changes_msgs.push(match avatar_action {
AvatarAction::Delete => { AvatarAction::Delete => stock_str::msg_grp_img_deleted(context, from_id).await,
Some(stock_str::msg_grp_img_deleted(context, from_id).await)
}
AvatarAction::Change(_) => { 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() { if !diff.is_empty() {
warn!(context, "Implicit addition of {diff:?} to chat {chat_id}."); 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 { if let Some(removed_id) = removed_id {
new_members.remove(&removed_id); new_members.remove(&removed_id);
@@ -1949,7 +1971,7 @@ async fn apply_group_changes(
if send_event_chat_modified { if send_event_chat_modified {
context.emit_event(EventType::ChatModified(chat_id)); context.emit_event(EventType::ChatModified(chat_id));
} }
Ok(better_msg) Ok(group_changes_msgs)
} }
static LIST_ID_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"^(.+)<(.+)>$").unwrap()); static LIST_ID_REGEX: Lazy<Regex> = Lazy::new(|| Regex::new(r"^(.+)<(.+)>$").unwrap());

View File

@@ -416,6 +416,9 @@ pub enum StockMessage {
#[strum(props(fallback = "Others will only see this group after you sent a first message."))] #[strum(props(fallback = "Others will only see this group after you sent a first message."))]
NewGroupSendFirstMessage = 172, NewGroupSendFirstMessage = 172,
#[strum(props(fallback = "Member %1$s added."))]
MsgAddMember = 173,
} }
impl StockMessage { 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.`. /// 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 /// The `added_member_addr` parameter should be an email address and is looked up in the
/// contacts to combine with the authorized display name. /// 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()), .unwrap_or_else(|_| addr.to_string()),
_ => 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) translated(context, StockMessage::MsgYouAddMember)
.await .await
.replace1(whom) .replace1(whom)

View File

@@ -514,12 +514,7 @@ impl TestContext {
.await .await
.expect("receive_imf() seems not to have added a new message to the db"); .expect("receive_imf() seems not to have added a new message to the db");
assert_eq!( let msg = Message::load_from_db(self, *received.msg_ids.last().unwrap())
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])
.await .await
.unwrap(); .unwrap();

View File

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