Skip to content

Commit 6406f30

Browse files
Hocurilink2xt
andauthored
feat: Make it possible to leave broadcast channels (#6984)
Part of #6884. The channel owner will not be notified in any way that you left, they will only see that there is one member less. For the translated stock strings, this is what we agreed on in the group: - Add a new "Leave Channel" stock string (will need to be done in UIs) - Reword the existing "Are you sure you want to leave this group?" string to "Are you sure you want to leave?" (the options are "Cancel" and "Leave Group" / "Leave Channel", so it's clear what you are leaving) (will need to be done in the deltachat-android repo, other UIs will pick it up automatically) - Reword the existing "You left the group." string to "You left". (done here, I will adapt the strings in deltachat-android, too) I adapted DC Android by pushing deltachat/deltachat-android@6df2740 to deltachat/deltachat-android#3783. --------- Co-authored-by: l <link2xt@testrun.org>
1 parent e5e0f0c commit 6406f30

File tree

7 files changed

+191
-29
lines changed

7 files changed

+191
-29
lines changed

deltachat-ffi/deltachat.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7424,7 +7424,7 @@ void dc_event_unref(dc_event_t* event);
74247424
/// Used in status messages.
74257425
#define DC_STR_REMOVE_MEMBER_BY_OTHER 131
74267426

7427-
/// "You left the group."
7427+
/// "You left."
74287428
///
74297429
/// Used in status messages.
74307430
#define DC_STR_GROUP_LEFT_BY_YOU 132

src/chat.rs

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2905,10 +2905,12 @@ async fn prepare_send_msg(
29052905
// If the chat is a contact request, let the user accept it later.
29062906
msg.param.get_cmd() == SystemMessage::SecurejoinMessage
29072907
}
2908-
// Allow to send "Member removed" messages so we can leave the group.
2908+
// Allow to send "Member removed" messages so we can leave the group/broadcast.
29092909
// Necessary checks should be made anyway before removing contact
29102910
// from the chat.
2911-
CantSendReason::NotAMember => msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup,
2911+
CantSendReason::NotAMember | CantSendReason::InBroadcast => {
2912+
msg.param.get_cmd() == SystemMessage::MemberRemovedFromGroup
2913+
}
29122914
CantSendReason::MissingKey => msg
29132915
.param
29142916
.get_bool(Param::ForcePlaintext)
@@ -4079,8 +4081,6 @@ pub async fn remove_contact_from_chat(
40794081
"Cannot remove special contact"
40804082
);
40814083

4082-
let mut msg = Message::new(Viewtype::default());
4083-
40844084
let chat = Chat::load_from_db(context, chat_id).await?;
40854085
if chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast {
40864086
if !chat.is_self_in_chat(context).await? {
@@ -4110,19 +4110,10 @@ pub async fn remove_contact_from_chat(
41104110
// in case of the database becoming inconsistent due to a bug.
41114111
if let Some(contact) = Contact::get_by_id_optional(context, contact_id).await? {
41124112
if chat.typ == Chattype::Group && chat.is_promoted() {
4113-
msg.viewtype = Viewtype::Text;
4114-
if contact_id == ContactId::SELF {
4115-
msg.text = stock_str::msg_group_left_local(context, ContactId::SELF).await;
4116-
} else {
4117-
msg.text =
4118-
stock_str::msg_del_member_local(context, contact_id, ContactId::SELF)
4119-
.await;
4120-
}
4121-
msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup);
4122-
msg.param.set(Param::Arg, contact.get_addr().to_lowercase());
4123-
msg.param
4124-
.set(Param::ContactAddedRemoved, contact.id.to_u32() as i32);
4125-
let res = send_msg(context, chat_id, &mut msg).await;
4113+
let addr = contact.get_addr();
4114+
4115+
let res = send_member_removal_msg(context, chat_id, contact_id, addr).await;
4116+
41264117
if contact_id == ContactId::SELF {
41274118
res?;
41284119
set_group_explicitly_left(context, &chat.grpid).await?;
@@ -4141,13 +4132,40 @@ pub async fn remove_contact_from_chat(
41414132
chat.sync_contacts(context).await.log_err(context).ok();
41424133
}
41434134
}
4135+
} else if chat.typ == Chattype::InBroadcast && contact_id == ContactId::SELF {
4136+
// For incoming broadcast channels, it's not possible to remove members,
4137+
// but it's possible to leave:
4138+
let self_addr = context.get_primary_self_addr().await?;
4139+
send_member_removal_msg(context, chat_id, contact_id, &self_addr).await?;
41444140
} else {
41454141
bail!("Cannot remove members from non-group chats.");
41464142
}
41474143

41484144
Ok(())
41494145
}
41504146

4147+
async fn send_member_removal_msg(
4148+
context: &Context,
4149+
chat_id: ChatId,
4150+
contact_id: ContactId,
4151+
addr: &str,
4152+
) -> Result<MsgId> {
4153+
let mut msg = Message::new(Viewtype::Text);
4154+
4155+
if contact_id == ContactId::SELF {
4156+
msg.text = stock_str::msg_group_left_local(context, ContactId::SELF).await;
4157+
} else {
4158+
msg.text = stock_str::msg_del_member_local(context, contact_id, ContactId::SELF).await;
4159+
}
4160+
4161+
msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup);
4162+
msg.param.set(Param::Arg, addr.to_lowercase());
4163+
msg.param
4164+
.set(Param::ContactAddedRemoved, contact_id.to_u32());
4165+
4166+
send_msg(context, chat_id, &mut msg).await
4167+
}
4168+
41514169
async fn set_group_explicitly_left(context: &Context, grpid: &str) -> Result<()> {
41524170
if !is_group_explicitly_left(context, grpid).await? {
41534171
context

src/chat/chat_tests.rs

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::ephemeral::Timer;
55
use crate::headerdef::HeaderDef;
66
use crate::imex::{ImexMode, has_backup, imex};
77
use crate::message::{MessengerMessage, delete_msgs};
8-
use crate::mimeparser;
8+
use crate::mimeparser::{self, MimeMessage};
99
use crate::receive_imf::receive_imf;
1010
use crate::test_utils::{
1111
AVATAR_64x64_BYTES, AVATAR_64x64_DEDUPLICATED, TestContext, TestContextManager,
@@ -374,7 +374,10 @@ async fn test_member_add_remove() -> Result<()> {
374374
// Alice leaves the chat.
375375
remove_contact_from_chat(&alice, alice_chat_id, ContactId::SELF).await?;
376376
let sent = alice.pop_sent_msg().await;
377-
assert_eq!(sent.load_from_db().await.get_text(), "You left the group.");
377+
assert_eq!(
378+
sent.load_from_db().await.get_text(),
379+
stock_str::msg_group_left_local(&alice, ContactId::SELF).await
380+
);
378381

379382
Ok(())
380383
}
@@ -2931,6 +2934,108 @@ async fn test_broadcast_channel_protected_listid() -> Result<()> {
29312934
Ok(())
29322935
}
29332936

2937+
/// Test that if Bob leaves a broadcast channel,
2938+
/// Alice (the channel owner) won't see him as a member anymore,
2939+
/// but won't be notified about this in any way.
2940+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2941+
async fn test_leave_broadcast() -> Result<()> {
2942+
let mut tcm = TestContextManager::new();
2943+
let alice = &tcm.alice().await;
2944+
let bob = &tcm.bob().await;
2945+
2946+
tcm.section("Alice creates broadcast channel with Bob.");
2947+
let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?;
2948+
let bob_contact = alice.add_or_lookup_contact(bob).await.id;
2949+
add_contact_to_chat(alice, alice_chat_id, bob_contact).await?;
2950+
2951+
tcm.section("Alice sends first message to broadcast.");
2952+
let sent_msg = alice.send_text(alice_chat_id, "Hello!").await;
2953+
let bob_msg = bob.recv_msg(&sent_msg).await;
2954+
2955+
assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 1);
2956+
2957+
// Clear events so that we can later check
2958+
// that the 'Broadcast channel left' message didn't trigger IncomingMsg:
2959+
alice.evtracker.clear_events();
2960+
2961+
// Shift the time so that we can later check the "Broadcast channel left" message's timestamp:
2962+
SystemTime::shift(Duration::from_secs(60));
2963+
2964+
tcm.section("Bob leaves the broadcast channel.");
2965+
let bob_chat_id = bob_msg.chat_id;
2966+
bob_chat_id.accept(bob).await?;
2967+
remove_contact_from_chat(bob, bob_chat_id, ContactId::SELF).await?;
2968+
2969+
let leave_msg = bob.pop_sent_msg().await;
2970+
alice.recv_msg_trash(&leave_msg).await;
2971+
2972+
assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 0);
2973+
2974+
alice.emit_event(EventType::Test);
2975+
alice
2976+
.evtracker
2977+
.get_matching(|ev| match ev {
2978+
EventType::Test => true,
2979+
EventType::IncomingMsg { .. } => {
2980+
panic!("'Broadcast channel left' message should be silent")
2981+
}
2982+
EventType::MsgsNoticed(..) => {
2983+
panic!("'Broadcast channel left' message shouldn't clear notifications")
2984+
}
2985+
EventType::MsgsChanged { .. } => {
2986+
panic!("Broadcast channels should be left silently, without any message");
2987+
}
2988+
_ => false,
2989+
})
2990+
.await;
2991+
2992+
Ok(())
2993+
}
2994+
2995+
/// Tests that if Bob leaves a broadcast channel with one device,
2996+
/// the other device shows a correct info message "You left.".
2997+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2998+
async fn test_leave_broadcast_multidevice() -> Result<()> {
2999+
let mut tcm = TestContextManager::new();
3000+
let alice = &tcm.alice().await;
3001+
let bob0 = &tcm.bob().await;
3002+
let bob1 = &tcm.bob().await;
3003+
3004+
tcm.section("Alice creates broadcast channel with Bob.");
3005+
let alice_chat_id = create_broadcast(alice, "foo".to_string()).await?;
3006+
let bob_contact = alice.add_or_lookup_contact(bob0).await.id;
3007+
add_contact_to_chat(alice, alice_chat_id, bob_contact).await?;
3008+
3009+
tcm.section("Alice sends first message to broadcast.");
3010+
let sent_msg = alice.send_text(alice_chat_id, "Hello!").await;
3011+
let bob0_hello = bob0.recv_msg(&sent_msg).await;
3012+
let bob1_hello = bob1.recv_msg(&sent_msg).await;
3013+
3014+
tcm.section("Bob leaves the broadcast channel with his first device.");
3015+
let bob_chat_id = bob0_hello.chat_id;
3016+
bob_chat_id.accept(bob0).await?;
3017+
remove_contact_from_chat(bob0, bob_chat_id, ContactId::SELF).await?;
3018+
3019+
let leave_msg = bob0.pop_sent_msg().await;
3020+
let parsed = MimeMessage::from_bytes(bob1, leave_msg.payload().as_bytes(), None).await?;
3021+
assert_eq!(
3022+
parsed.parts[0].msg,
3023+
stock_str::msg_group_left_remote(bob0).await
3024+
);
3025+
3026+
let rcvd = bob1.recv_msg(&leave_msg).await;
3027+
3028+
assert_eq!(rcvd.chat_id, bob1_hello.chat_id);
3029+
assert!(rcvd.is_info());
3030+
assert_eq!(rcvd.get_info_type(), SystemMessage::MemberRemovedFromGroup);
3031+
assert_eq!(
3032+
rcvd.text,
3033+
stock_str::msg_group_left_local(bob1, ContactId::SELF).await
3034+
);
3035+
3036+
Ok(())
3037+
}
3038+
29343039
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
29353040
async fn test_create_for_contact_with_blocked() -> Result<()> {
29363041
let t = TestContext::new().await;

src/mimefactory.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ impl MimeFactory {
789789
}
790790

791791
if let Loaded::Message { chat, .. } = &self.loaded {
792-
if chat.typ == Chattype::OutBroadcast {
792+
if chat.typ == Chattype::OutBroadcast || chat.typ == Chattype::InBroadcast {
793793
headers.push((
794794
"List-ID",
795795
mail_builder::headers::text::Text::new(format!(
@@ -1319,7 +1319,10 @@ impl MimeFactory {
13191319
}
13201320
}
13211321

1322-
if chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast {
1322+
if chat.typ == Chattype::Group
1323+
|| chat.typ == Chattype::OutBroadcast
1324+
|| chat.typ == Chattype::InBroadcast
1325+
{
13231326
headers.push((
13241327
"Chat-Group-Name",
13251328
mail_builder::headers::text::Text::new(chat.name.to_string()).into(),

src/receive_imf.rs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ use mailparse::SingleInfo;
1414
use num_traits::FromPrimitive;
1515
use regex::Regex;
1616

17-
use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ProtectionStatus};
17+
use crate::chat::{
18+
self, Chat, ChatId, ChatIdBlocked, ProtectionStatus, remove_from_chat_contacts_table,
19+
};
1820
use crate::config::Config;
1921
use crate::constants::{Blocked, Chattype, DC_CHAT_ID_TRASH, EDITED_PREFIX, ShowEmails};
2022
use crate::contact::{Contact, ContactId, Origin, mark_contact_id_as_verified};
@@ -1687,7 +1689,9 @@ async fn add_parts(
16871689
_ if chat.id.is_special() => GroupChangesInfo::default(),
16881690
Chattype::Single => GroupChangesInfo::default(),
16891691
Chattype::Mailinglist => GroupChangesInfo::default(),
1690-
Chattype::OutBroadcast => GroupChangesInfo::default(),
1692+
Chattype::OutBroadcast => {
1693+
apply_out_broadcast_changes(context, mime_parser, &mut chat, from_id).await?
1694+
}
16911695
Chattype::Group => {
16921696
apply_group_changes(
16931697
context,
@@ -1701,7 +1705,7 @@ async fn add_parts(
17011705
.await?
17021706
}
17031707
Chattype::InBroadcast => {
1704-
apply_broadcast_changes(context, mime_parser, &mut chat, from_id).await?
1708+
apply_in_broadcast_changes(context, mime_parser, &mut chat, from_id).await?
17051709
}
17061710
};
17071711

@@ -3438,7 +3442,30 @@ async fn apply_mailinglist_changes(
34383442
Ok(())
34393443
}
34403444

3441-
async fn apply_broadcast_changes(
3445+
async fn apply_out_broadcast_changes(
3446+
context: &Context,
3447+
mime_parser: &MimeMessage,
3448+
chat: &mut Chat,
3449+
from_id: ContactId,
3450+
) -> Result<GroupChangesInfo> {
3451+
ensure!(chat.typ == Chattype::OutBroadcast);
3452+
3453+
if let Some(_removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) {
3454+
// The sender of the message left the broadcast channel
3455+
remove_from_chat_contacts_table(context, chat.id, from_id).await?;
3456+
3457+
return Ok(GroupChangesInfo {
3458+
better_msg: Some("".to_string()),
3459+
added_removed_id: None,
3460+
silent: true,
3461+
extra_msgs: vec![],
3462+
});
3463+
}
3464+
3465+
Ok(GroupChangesInfo::default())
3466+
}
3467+
3468+
async fn apply_in_broadcast_changes(
34423469
context: &Context,
34433470
mime_parser: &MimeMessage,
34443471
chat: &mut Chat,
@@ -3459,6 +3486,15 @@ async fn apply_broadcast_changes(
34593486
)
34603487
.await?;
34613488

3489+
if let Some(_removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) {
3490+
// The only member added/removed message that is ever sent is "I left.",
3491+
// so, this is the only case we need to handle here
3492+
if from_id == ContactId::SELF {
3493+
better_msg
3494+
.get_or_insert(stock_str::msg_group_left_local(context, ContactId::SELF).await);
3495+
}
3496+
}
3497+
34623498
if send_event_chat_modified {
34633499
context.emit_event(EventType::ChatModified(chat.id));
34643500
chatlist_events::emit_chatlist_item_changed(context, chat.id);

src/stock_str.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ pub enum StockMessage {
285285
#[strum(props(fallback = "Member %1$s removed by %2$s."))]
286286
MsgDelMemberBy = 131,
287287

288-
#[strum(props(fallback = "You left the group."))]
288+
#[strum(props(fallback = "You left."))]
289289
MsgYouLeftGroup = 132,
290290

291291
#[strum(props(fallback = "Group left by %1$s."))]
@@ -685,7 +685,7 @@ pub(crate) async fn msg_group_left_remote(context: &Context) -> String {
685685
translated(context, StockMessage::MsgILeftGroup).await
686686
}
687687

688-
/// Stock string: `You left the group.` or `Group left by %1$s.`.
688+
/// Stock string: `You left.` or `Group left by %1$s.`.
689689
pub(crate) async fn msg_group_left_local(context: &Context, by_contact: ContactId) -> String {
690690
if by_contact == ContactId::SELF {
691691
translated(context, StockMessage::MsgYouLeftGroup).await
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Group#Chat#10: Group chat [3 member(s)]
22
--------------------------------------------------------------------------------
33
Msg#10🔒: (Contact#Contact#10): Hi! I created a group. [FRESH]
4-
Msg#11🔒: Me (Contact#Contact#Self): You left the group. [INFO] √
4+
Msg#11🔒: Me (Contact#Contact#Self): You left. [INFO] √
55
Msg#12🔒: (Contact#Contact#10): Member charlie@example.net added by alice@example.org. [FRESH][INFO]
66
Msg#13🔒: (Contact#Contact#10): What a silence! [FRESH]
77
--------------------------------------------------------------------------------

0 commit comments

Comments
 (0)