diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index f0ce41afd7..25b86e95eb 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -7424,7 +7424,7 @@ void dc_event_unref(dc_event_t* event); /// Used in status messages. #define DC_STR_REMOVE_MEMBER_BY_OTHER 131 -/// "You left the group." +/// "You left." /// /// Used in status messages. #define DC_STR_GROUP_LEFT_BY_YOU 132 diff --git a/src/chat.rs b/src/chat.rs index 506ab1c32d..bee6ec0223 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2905,10 +2905,12 @@ async fn prepare_send_msg( // If the chat is a contact request, let the user accept it later. msg.param.get_cmd() == SystemMessage::SecurejoinMessage } - // Allow to send "Member removed" messages so we can leave the group. + // Allow to send "Member removed" messages so we can leave the group/broadcast. // Necessary checks should be made anyway before removing contact // from the chat. - CantSendReason::NotAMember => msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup, + CantSendReason::NotAMember | CantSendReason::InBroadcast => { + msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup + } CantSendReason::MissingKey => msg .param .get_bool(Param::ForcePlaintext) @@ -4079,8 +4081,6 @@ pub async fn remove_contact_from_chat( "Cannot remove special contact" ); - let mut msg = Message::new(Viewtype::default()); - let chat = Chat::load_from_db(context, chat_id).await?; if chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast { if !chat.is_self_in_chat(context).await? { @@ -4110,19 +4110,10 @@ pub async fn remove_contact_from_chat( // in case of the database becoming inconsistent due to a bug. if let Some(contact) = Contact::get_by_id_optional(context, contact_id).await? { if chat.typ == Chattype::Group && chat.is_promoted() { - msg.viewtype = Viewtype::Text; - if contact_id == ContactId::SELF { - msg.text = stock_str::msg_group_left_local(context, ContactId::SELF).await; - } else { - msg.text = - stock_str::msg_del_member_local(context, contact_id, ContactId::SELF) - .await; - } - msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup); - msg.param.set(Param::Arg, contact.get_addr().to_lowercase()); - msg.param - .set(Param::ContactAddedRemoved, contact.id.to_u32() as i32); - let res = send_msg(context, chat_id, &mut msg).await; + let addr = contact.get_addr(); + + let res = send_member_removal_msg(context, chat_id, contact_id, addr).await; + if contact_id == ContactId::SELF { res?; set_group_explicitly_left(context, &chat.grpid).await?; @@ -4141,6 +4132,11 @@ pub async fn remove_contact_from_chat( chat.sync_contacts(context).await.log_err(context).ok(); } } + } else if chat.typ == Chattype::InBroadcast && contact_id == ContactId::SELF { + // For incoming broadcast channels, it's not possible to remove members, + // but it's possible to leave: + let self_addr = context.get_primary_self_addr().await?; + send_member_removal_msg(context, chat_id, contact_id, &self_addr).await?; } else { bail!("Cannot remove members from non-group chats."); } @@ -4148,6 +4144,28 @@ pub async fn remove_contact_from_chat( Ok(()) } +async fn send_member_removal_msg( + context: &Context, + chat_id: ChatId, + contact_id: ContactId, + addr: &str, +) -> Result { + let mut msg = Message::new(Viewtype::Text); + + if contact_id == ContactId::SELF { + msg.text = stock_str::msg_group_left_local(context, ContactId::SELF).await; + } else { + msg.text = stock_str::msg_del_member_local(context, contact_id, ContactId::SELF).await; + } + + msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup); + msg.param.set(Param::Arg, addr.to_lowercase()); + msg.param + .set(Param::ContactAddedRemoved, contact_id.to_u32()); + + send_msg(context, chat_id, &mut msg).await +} + async fn set_group_explicitly_left(context: &Context, grpid: &str) -> Result<()> { if !is_group_explicitly_left(context, grpid).await? { context diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index cca358b395..b693934f76 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -5,7 +5,7 @@ use crate::ephemeral::Timer; use crate::headerdef::HeaderDef; use crate::imex::{ImexMode, has_backup, imex}; use crate::message::{MessengerMessage, delete_msgs}; -use crate::mimeparser; +use crate::mimeparser::{self, MimeMessage}; use crate::receive_imf::receive_imf; use crate::test_utils::{ AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext, TestContextManager, @@ -374,7 +374,10 @@ async fn test_member_add_remove() -> Result<()> { // Alice leaves the chat. remove_contact_from_chat(&alice, alice_chat_id, ContactId::SELF).await?; let sent = alice.pop_sent_msg().await; - assert_eq!(sent.load_from_db().await.get_text(), "You left the group."); + assert_eq!( + sent.load_from_db().await.get_text(), + stock_str::msg_group_left_local(&alice, ContactId::SELF).await + ); Ok(()) } @@ -2931,6 +2934,108 @@ async fn test_broadcast_channel_protected_listid() -> Result<()> { Ok(()) } +/// Test that if Bob leaves a broadcast channel, +/// Alice (the channel owner) won't see him as a member anymore, +/// but won't be notified about this in any way. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_leave_broadcast() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + tcm.section("Alice creates broadcast channel with Bob."); + let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; + let bob_contact = alice.add_or_lookup_contact(bob).await.id; + add_contact_to_chat(alice, alice_chat_id, bob_contact).await?; + + tcm.section("Alice sends first message to broadcast."); + let sent_msg = alice.send_text(alice_chat_id, "Hello!").await; + let bob_msg = bob.recv_msg(&sent_msg).await; + + assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 1); + + // Clear events so that we can later check + // that the 'Broadcast channel left' message didn't trigger IncomingMsg: + alice.evtracker.clear_events(); + + // Shift the time so that we can later check the "Broadcast channel left" message's timestamp: + SystemTime::shift(Duration::from_secs(60)); + + tcm.section("Bob leaves the broadcast channel."); + 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_trash(&leave_msg).await; + + assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 0); + + alice.emit_event(EventType::Test); + alice + .evtracker + .get_matching(|ev| match ev { + EventType::Test => true, + EventType::IncomingMsg { .. } => { + panic!("'Broadcast channel left' message should be silent") + } + EventType::MsgsNoticed(..) => { + panic!("'Broadcast channel left' message shouldn't clear notifications") + } + EventType::MsgsChanged { .. } => { + panic!("Broadcast channels should be left silently, without any message"); + } + _ => false, + }) + .await; + + Ok(()) +} + +/// Tests that if Bob leaves a broadcast channel with one device, +/// the other device shows a correct info message "You left.". +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_leave_broadcast_multidevice() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob0 = &tcm.bob().await; + let bob1 = &tcm.bob().await; + + tcm.section("Alice creates broadcast channel with Bob."); + let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?; + let bob_contact = alice.add_or_lookup_contact(bob0).await.id; + add_contact_to_chat(alice, alice_chat_id, bob_contact).await?; + + tcm.section("Alice sends first message to broadcast."); + let sent_msg = alice.send_text(alice_chat_id, "Hello!").await; + let bob0_hello = bob0.recv_msg(&sent_msg).await; + let bob1_hello = bob1.recv_msg(&sent_msg).await; + + tcm.section("Bob leaves the broadcast channel with his first device."); + let bob_chat_id = bob0_hello.chat_id; + bob_chat_id.accept(bob0).await?; + remove_contact_from_chat(bob0, bob_chat_id, ContactId::SELF).await?; + + let leave_msg = bob0.pop_sent_msg().await; + let parsed = MimeMessage::from_bytes(bob1, leave_msg.payload().as_bytes(), None).await?; + assert_eq!( + parsed.parts[0].msg, + stock_str::msg_group_left_remote(bob0).await + ); + + let rcvd = bob1.recv_msg(&leave_msg).await; + + assert_eq!(rcvd.chat_id, bob1_hello.chat_id); + assert!(rcvd.is_info()); + assert_eq!(rcvd.get_info_type(), SystemMessage::MemberRemovedFromGroup); + assert_eq!( + rcvd.text, + stock_str::msg_group_left_local(bob1, ContactId::SELF).await + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_create_for_contact_with_blocked() -> Result<()> { let t = TestContext::new().await; diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 8baa0ded9a..bd126be47e 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -789,7 +789,7 @@ impl MimeFactory { } if let Loaded::Message { chat, .. } = &self.loaded { - if chat.typ == Chattype::OutBroadcast { + if chat.typ == Chattype::OutBroadcast || chat.typ == Chattype::InBroadcast { headers.push(( "List-ID", mail_builder::headers::text::Text::new(format!( @@ -1319,7 +1319,10 @@ impl MimeFactory { } } - if chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast { + if chat.typ == Chattype::Group + || chat.typ == Chattype::OutBroadcast + || chat.typ == Chattype::InBroadcast + { headers.push(( "Chat-Group-Name", mail_builder::headers::text::Text::new(chat.name.to_string()).into(), diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 131167ad9f..82c0624837 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -14,7 +14,9 @@ use mailparse::SingleInfo; use num_traits::FromPrimitive; use regex::Regex; -use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ProtectionStatus}; +use crate::chat::{ + self, Chat, ChatId, ChatIdBlocked, ProtectionStatus, remove_from_chat_contacts_table, +}; use crate::config::Config; use crate::constants::{Blocked, Chattype, DC_CHAT_ID_TRASH, EDITED_PREFIX, ShowEmails}; use crate::contact::{Contact, ContactId, Origin, mark_contact_id_as_verified}; @@ -1687,7 +1689,9 @@ async fn add_parts( _ if chat.id.is_special() => GroupChangesInfo::default(), Chattype::Single => GroupChangesInfo::default(), Chattype::Mailinglist => GroupChangesInfo::default(), - Chattype::OutBroadcast => GroupChangesInfo::default(), + Chattype::OutBroadcast => { + apply_out_broadcast_changes(context, mime_parser, &mut chat, from_id).await? + } Chattype::Group => { apply_group_changes( context, @@ -1701,7 +1705,7 @@ async fn add_parts( .await? } Chattype::InBroadcast => { - apply_broadcast_changes(context, mime_parser, &mut chat, from_id).await? + apply_in_broadcast_changes(context, mime_parser, &mut chat, from_id).await? } }; @@ -3438,7 +3442,30 @@ async fn apply_mailinglist_changes( Ok(()) } -async fn apply_broadcast_changes( +async fn apply_out_broadcast_changes( + context: &Context, + mime_parser: &MimeMessage, + chat: &mut Chat, + from_id: ContactId, +) -> Result { + ensure!(chat.typ == Chattype::OutBroadcast); + + if let Some(_removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) { + // The sender of the message left the broadcast channel + remove_from_chat_contacts_table(context, chat.id, from_id).await?; + + return Ok(GroupChangesInfo { + better_msg: Some("".to_string()), + added_removed_id: None, + silent: true, + extra_msgs: vec![], + }); + } + + Ok(GroupChangesInfo::default()) +} + +async fn apply_in_broadcast_changes( context: &Context, mime_parser: &MimeMessage, chat: &mut Chat, @@ -3459,6 +3486,15 @@ async fn apply_broadcast_changes( ) .await?; + if let Some(_removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) { + // The only member added/removed message that is ever sent is "I left.", + // so, this is the only case we need to handle here + if from_id == ContactId::SELF { + better_msg + .get_or_insert(stock_str::msg_group_left_local(context, ContactId::SELF).await); + } + } + if send_event_chat_modified { context.emit_event(EventType::ChatModified(chat.id)); chatlist_events::emit_chatlist_item_changed(context, chat.id); diff --git a/src/stock_str.rs b/src/stock_str.rs index 26a5d17dda..86fb86b130 100644 --- a/src/stock_str.rs +++ b/src/stock_str.rs @@ -285,7 +285,7 @@ pub enum StockMessage { #[strum(props(fallback = "Member %1$s removed by %2$s."))] MsgDelMemberBy = 131, - #[strum(props(fallback = "You left the group."))] + #[strum(props(fallback = "You left."))] MsgYouLeftGroup = 132, #[strum(props(fallback = "Group left by %1$s."))] @@ -685,7 +685,7 @@ pub(crate) async fn msg_group_left_remote(context: &Context) -> String { translated(context, StockMessage::MsgILeftGroup).await } -/// Stock string: `You left the group.` or `Group left by %1$s.`. +/// Stock string: `You left.` or `Group left by %1$s.`. pub(crate) async fn msg_group_left_local(context: &Context, by_contact: ContactId) -> String { if by_contact == ContactId::SELF { translated(context, StockMessage::MsgYouLeftGroup).await diff --git a/test-data/golden/chat_test_parallel_member_remove b/test-data/golden/chat_test_parallel_member_remove index b548b3e006..449f89a510 100644 --- a/test-data/golden/chat_test_parallel_member_remove +++ b/test-data/golden/chat_test_parallel_member_remove @@ -1,7 +1,7 @@ Group#Chat#10: Group chat [3 member(s)] -------------------------------------------------------------------------------- Msg#10🔒: (Contact#Contact#10): Hi! I created a group. [FRESH] -Msg#11🔒: Me (Contact#Contact#Self): You left the group. [INFO] √ +Msg#11🔒: Me (Contact#Contact#Self): You left. [INFO] √ Msg#12🔒: (Contact#Contact#10): Member charlie@example.net added by alice@example.org. [FRESH][INFO] Msg#13🔒: (Contact#Contact#10): What a silence! [FRESH] --------------------------------------------------------------------------------