Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-17 13:36:30 +03:00
Code Issues Packages Projects Releases Wiki Activity

feat: Don't send a notification when a group member left (#6575)

Browse Source 
When there is a broken group (which might happen with multi-transport),
people want to leave it.

The problem is that every "Group left" message notifies all other
members and pops up the chat, so that other members also want to leave
the group.

This PR makes it so that "Group left" messages don't create a
notification, don't cause a number-in-a-cirle badge counter on the chat,
and don't sort up the chat in the chatlist.

If a group is deleted, then the group won't pop up when someone leaves
it; this worked fine already before this PR, and there also is a test
for it.
This commit is contained in:
Hocuri
2025-02-26 19:00:46 +01:00
committed by GitHub
parent 8ffdd55f79
commit a4e478a071
6 changed files with 137 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
deltachat-rpc-client/src/deltachat_rpc_client/account.py
View File

@@ -301,6 +301,13 @@ class Account:
if event.kind == EventType.INCOMING_MSG:
return event
def wait_for_msgs_changed_event(self):
"""Wait for messages changed event and return it."""
while True:
event = self.wait_for_event()
if event.kind == EventType.MSGS_CHANGED:
return event
def wait_for_incoming_msg(self):
"""Wait for incoming message and return it.

4
deltachat-rpc-client/tests/test_securejoin.py
View File

@@ -653,12 +653,14 @@ def test_withdraw_securejoin_qr(acfactory):
bob_chat = bob.secure_join(qr_code)
bob.wait_for_securejoin_joiner_success()
alice.clear_all_events()
snapshot = bob.get_message_by_id(bob.wait_for_incoming_msg_event().msg_id).get_snapshot()
assert snapshot.text == "Member Me ({}) added by {}.".format(bob.get_config("addr"), alice.get_config("addr"))
assert snapshot.chat.get_basic_snapshot().is_protected
bob_chat.leave()
snapshot = alice.get_message_by_id(alice.wait_for_incoming_msg_event().msg_id).get_snapshot()
snapshot = alice.get_message_by_id(alice.wait_for_msgs_changed_event().msg_id).get_snapshot()
assert snapshot.text == "Group left by {}.".format(bob.get_config("addr"))
logging.info("Alice withdraws QR code.")

8
src/chat.rs
View File

@@ -1048,6 +1048,14 @@ impl ChatId {
Ok(count)
}
pub(crate) async fn created_timestamp(self, context: &Context) -> Result<i64> {
Ok(context
.sql
.query_get_value("SELECT created_timestamp FROM chats WHERE id=?", (self,))
.await?
.unwrap_or(0))
}
/// Returns timestamp of the latest message in the chat.
pub(crate) async fn get_timestamp(self, context: &Context) -> Result<Option<i64>> {
let timestamp = context

31
src/chat/chat_tests.rs
View File

@@ -695,16 +695,45 @@ async fn test_leave_group() -> Result<()> {
assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 2);
// Clear events so that we can later check
// that the 'Group left' message didn't trigger IncomingMsg:
alice.evtracker.clear_events();
// Shift the time so that we can later check the 'Group left' message's timestamp:
SystemTime::shift(Duration::from_secs(60));
// Bob leaves the group.
let bob_chat_id = bob_msg.chat_id;
bob_chat_id.accept(&bob).await?;
remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?;
let leave_msg = bob.pop_sent_msg().await;
alice.recv_msg(&leave_msg).await;
let rcvd_leave_msg = alice.recv_msg(&leave_msg).await;
assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 1);
assert_eq!(rcvd_leave_msg.state, MessageState::InSeen);
alice.emit_event(EventType::Test);
alice
.evtracker
.get_matching(|ev| match ev {
EventType::Test => true,
EventType::IncomingMsg { .. } => panic!("'Group left' message should be silent"),
EventType::MsgsNoticed(..) => {
panic!("'Group left' message shouldn't clear notifications")
}
_ => false,
})
.await;
// The 'Group left' message timestamp should be the same as the previous message in the chat
// so that the chat is not popped up in the chatlist:
assert_eq!(
sent_msg.load_from_db().await.timestamp_sort,
rcvd_leave_msg.timestamp_sort
);
Ok(())
}

9
src/peerstate.rs
View File

@@ -676,14 +676,7 @@ impl Peerstate {
let lastmsg = Message::load_from_db(context, *msg_id).await?;
lastmsg.timestamp_sort
} else {
context
.sql
.query_get_value(
"SELECT created_timestamp FROM chats WHERE id=?;",
(chat_id,),
)
.await?
.unwrap_or(0)
chat_id.created_timestamp(context).await?
};
if let PeerstateChange::Aeap(new_addr) = &change {

123
src/receive_imf.rs
View File

@@ -730,7 +730,7 @@ async fn add_parts(
let mut chat_id_blocked = Blocked::Not;
let mut better_msg = None;
let mut group_changes_msgs = (Vec::new(), None);
let mut group_changes = GroupChangesInfo::default();
if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled {
better_msg = Some(stock_str::msg_location_enabled_by(context, from_id).await);
}
@@ -922,7 +922,7 @@ async fn add_parts(
}
}
group_changes_msgs = apply_group_changes(
group_changes = apply_group_changes(
context,
mime_parser,
group_chat_id,
@@ -1040,7 +1040,11 @@ async fn add_parts(
}
}
state = if seen || fetching_existing_messages || is_mdn || chat_id_blocked == Blocked::Yes
state = if seen
|| fetching_existing_messages
|| is_mdn
|| chat_id_blocked == Blocked::Yes
|| group_changes.silent
// No check for `hidden` because only reactions are such and they should be `InFresh`.
{
MessageState::InSeen
@@ -1189,7 +1193,7 @@ async fn add_parts(
}
if let Some(chat_id) = chat_id {
group_changes_msgs = apply_group_changes(
group_changes = apply_group_changes(
context,
mime_parser,
chat_id,
@@ -1392,18 +1396,14 @@ async fn add_parts(
}
}
// Ensure replies to messages are sorted after the parent message.
//
// This is useful in a case where sender clocks are not
// synchronized and parent message has a Date: header with a
// timestamp higher than reply timestamp.
//
// This does not help if parent message arrives later than the
// reply.
let parent_timestamp = mime_parser.get_parent_timestamp(context).await?;
let sort_timestamp = parent_timestamp.map_or(sort_timestamp, |parent_timestamp| {
std::cmp::max(sort_timestamp, parent_timestamp)
});
let sort_timestamp = tweak_sort_timestamp(
context,
mime_parser,
group_changes.silent,
chat_id,
sort_timestamp,
)
.await?;
// if the mime-headers should be saved, find out its size
// (the mime-header ends with an empty line)
@@ -1447,24 +1447,23 @@ async fn add_parts(
let mut created_db_entries = Vec::with_capacity(mime_parser.parts.len());
if let Some(msg) = group_changes_msgs.1 {
if let Some(m) = group_changes.better_msg {
match &better_msg {
None => better_msg = Some(msg),
None => better_msg = Some(m),
Some(_) => {
if !msg.is_empty() {
group_changes_msgs.0.push(msg)
if !m.is_empty() {
group_changes.extra_msgs.push((m, is_system_message))
}
}
}
}
for group_changes_msg in group_changes_msgs.0 {
// Currently all additional group changes messages are "Member added".
for (group_changes_msg, cmd) in group_changes.extra_msgs {
chat::add_info_msg_with_cmd(
context,
chat_id,
&group_changes_msg,
SystemMessage::MemberAddedToGroup,
cmd,
sort_timestamp,
None,
None,
@@ -1801,6 +1800,40 @@ RETURNING id
})
}
async fn tweak_sort_timestamp(
context: &Context,
mime_parser: &mut MimeMessage,
silent: bool,
chat_id: ChatId,
sort_timestamp: i64,
) -> Result<i64> {
// Ensure replies to messages are sorted after the parent message.
//
// This is useful in a case where sender clocks are not
// synchronized and parent message has a Date: header with a
// timestamp higher than reply timestamp.
//
// This does not help if parent message arrives later than the
// reply.
let parent_timestamp = mime_parser.get_parent_timestamp(context).await?;
let mut sort_timestamp = parent_timestamp.map_or(sort_timestamp, |parent_timestamp| {
std::cmp::max(sort_timestamp, parent_timestamp)
});
// If the message should be silent,
// set the timestamp to be no more than the same as last message
// so that the chat is not sorted to the top of the chatlist.
if silent {
let last_msg_timestamp = if let Some(t) = chat_id.get_timestamp(context).await? {
t
} else {
chat_id.created_timestamp(context).await?
};
sort_timestamp = std::cmp::min(sort_timestamp, last_msg_timestamp);
}
Ok(sort_timestamp)
}
/// Saves attached locations to the database.
///
/// Emits an event if at least one new location was added.
@@ -2249,11 +2282,23 @@ async fn update_chats_contacts_timestamps(
Ok(modified)
}
/// The return type of [apply_group_changes].
/// Contains information on which system messages
/// should be shown in the chat.
#[derive(Default)]
struct GroupChangesInfo {
/// Optional: A better message that should replace the original system message.
/// If this is an empty string, the original system message should be trashed.
better_msg: Option<String>,
/// If true, the user should not be notified about the group change.
silent: bool,
/// A list of additional group changes messages that should be shown in the chat.
extra_msgs: Vec<(String, SystemMessage)>,
}
/// Apply group member list, name, avatar and protection status changes from the MIME message.
///
/// Returns `Vec` of group changes messages and, optionally, a better message to replace the
/// original system message. If the better message is empty, the original system message
/// should be trashed.
/// Returns [GroupChangesInfo].
///
/// * `to_ids` - contents of the `To` and `Cc` headers.
/// * `past_ids` - contents of the `Chat-Group-Past-Members` header.
@@ -2265,19 +2310,20 @@ async fn apply_group_changes(
to_ids: &[ContactId],
past_ids: &[ContactId],
verified_encryption: &VerifiedEncryption,
) -> Result<(Vec<String>, Option<String>)> {
) -> Result<GroupChangesInfo> {
if chat_id.is_special() {
// Do not apply group changes to the trash chat.
return Ok((Vec::new(), None));
return Ok(GroupChangesInfo::default());
}
let mut chat = Chat::load_from_db(context, chat_id).await?;
if chat.typ != Chattype::Group {
return Ok((Vec::new(), None));
return Ok(GroupChangesInfo::default());
}
let mut send_event_chat_modified = false;
let (mut removed_id, mut added_id) = (None, None);
let mut better_msg = None;
let mut silent = false;
// True if a Delta Chat client has explicitly added our current primary address.
let self_added =
@@ -2315,6 +2361,7 @@ async fn apply_group_changes(
removed_id = Contact::lookup_id_by_addr(context, removed_addr, Origin::Unknown).await?;
if let Some(id) = removed_id {
better_msg = if id == from_id {
silent = true;
Some(stock_str::msg_group_left_local(context, from_id).await)
} else {
Some(stock_str::msg_del_member_local(context, removed_addr, from_id).await)
@@ -2541,7 +2588,11 @@ async fn apply_group_changes(
context.emit_event(EventType::ChatModified(chat_id));
chatlist_events::emit_chatlist_item_changed(context, chat_id);
}
Ok((group_changes_msgs, better_msg))
Ok(GroupChangesInfo {
better_msg,
silent,
extra_msgs: group_changes_msgs,
})
}
/// Returns a list of strings that should be shown as info messages, informing about group membership changes.
@@ -2550,7 +2601,7 @@ async fn group_changes_msgs(
added_ids: &HashSet<ContactId>,
removed_ids: &HashSet<ContactId>,
chat_id: ChatId,
) -> Result<Vec<String>> {
) -> Result<Vec<(String, SystemMessage)>> {
let mut group_changes_msgs = Vec::new();
if !added_ids.is_empty() {
warn!(
@@ -2567,17 +2618,19 @@ async fn group_changes_msgs(
group_changes_msgs.reserve(added_ids.len() + removed_ids.len());
for contact_id in added_ids {
let contact = Contact::get_by_id(context, *contact_id).await?;
group_changes_msgs.push(
group_changes_msgs.push((
stock_str::msg_add_member_local(context, contact.get_addr(), ContactId::UNDEFINED)
.await,
);
SystemMessage::MemberAddedToGroup,
));
}
for contact_id in removed_ids {
let contact = Contact::get_by_id(context, *contact_id).await?;
group_changes_msgs.push(
group_changes_msgs.push((
stock_str::msg_del_member_local(context, contact.get_addr(), ContactId::UNDEFINED)
.await,
);
SystemMessage::MemberRemovedFromGroup,
));
}
Ok(group_changes_msgs)