test: Add test_leave_broadcast, fix bugs I found along the way

deltachat-rpc-client/tests/test_securejoin.py

@@ -124,12 +124,12 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online):
         alice2.start_io()
         bob2.start_io()
 
-    logging.info("Alice creates a broadcast")
+    logging.info("===================== Alice creates a broadcast =====================")
     alice_chat = alice.create_broadcast("Broadcast channel for everyone!")
     snapshot = alice_chat.get_basic_snapshot()
     assert not snapshot.is_unpromoted  # Broadcast channels are never unpromoted
 
-    logging.info("Bob joins the broadcast")
+    logging.info("===================== Bob joins the broadcast =====================")
 
     qr_code = alice_chat.get_qr_code()
     bob.secure_join(qr_code)
@@ -223,7 +223,7 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online):
     check_account(bob2, bob2.create_contact(alice), inviter_side=False)
 
     # The QR code token is synced, so alice2 must be able to handle join requests.
-    logging.info("Fiona joins the group via alice2")
+    logging.info("===================== Fiona joins the group via alice2 =====================")
     alice.stop_io()
     fiona.secure_join(qr_code)
     alice2.wait_for_securejoin_inviter_success()

deltachat-rpc-client/tests/test_something.py

@@ -10,7 +10,7 @@ from unittest.mock import MagicMock
 
 import pytest
 
-from deltachat_rpc_client import Contact, EventType, Message, events
+from deltachat_rpc_client import Chat, Contact, EventType, Message, events
 from deltachat_rpc_client.const import DownloadState, MessageState
 from deltachat_rpc_client.pytestplugin import E2EE_INFO_MSGS
 from deltachat_rpc_client.rpc import JsonRpcError
@@ -874,3 +874,46 @@ def test_delete_deltachat_folder(acfactory, direct_imap):
     assert msg.text == "hello"
 
     assert "DeltaChat" in ac1_direct_imap.list_folders()
+
+
+@pytest.mark.parametrize("all_devices_online", [True, False])
+def test_leave_broadcast(acfactory, all_devices_online):
+    alice, bob = acfactory.get_online_accounts(2)
+
+    bob2 = bob.clone()
+
+    if all_devices_online:
+        bob2.start_io()
+
+    logging.info("===================== Alice creates a broadcast =====================")
+    alice_chat = alice.create_broadcast("Broadcast channel for everyone!")
+
+    logging.info("===================== Bob joins the broadcast =====================")
+    qr_code = alice_chat.get_qr_code()
+    bob.secure_join(qr_code)
+    alice.wait_for_securejoin_inviter_success()
+    bob.wait_for_securejoin_joiner_success()
+
+    alice_bob_contact = alice.create_contact(bob)
+    alice_contacts = alice_chat.get_contacts()
+    assert len(alice_contacts) == 1  # 1 recipient
+    assert alice_contacts[0].id == alice_bob_contact.id
+
+    snapshot = bob.wait_for_incoming_msg().get_snapshot()
+    assert snapshot.text == f"Member Me added by {alice.get_config('addr')}."
+    bob_chat = Chat(bob, snapshot.chat_id)
+
+    logging.info("===================== Bob leaves the broadcast =====================")
+    assert bob_chat.get_full_snapshot().self_in_group
+    assert len(bob_chat.get_contacts()) == 2  # Alice and Bob
+
+    bob_chat.leave()
+    assert not bob_chat.get_full_snapshot().self_in_group
+    assert len(bob_chat.get_contacts()) == 1  # Only Alice
+
+    logging.info("===================== Test Alice's device =====================")
+    while len(alice_chat.get_contacts()) != 0:  # After Bob left, there will be 0 recipients
+        alice.wait_for_event(EventType.CHAT_MODIFIED)
+
+    # TODO check that the devices show the correct msgs
+    # TODO check Bob's second device

src/chat.rs

@@ -31,7 +31,6 @@ use crate::debug_logging::maybe_set_logging_xdc;
 use crate::download::DownloadState;
 use crate::ephemeral::{Timer as EphemeralTimer, start_chat_ephemeral_timers};
 use crate::events::EventType;
-use crate::key::self_fingerprint;
 use crate::location;
 use crate::log::{LogExt, error, info, warn};
 use crate::logged_debug_assert;
@@ -2859,8 +2858,9 @@ pub async fn is_contact_in_chat(
 ) -> Result<bool> {
     // this function works for group and for normal chats, however, it is more useful
     // for group chats.
-    // ContactId::SELF may be used to check, if the user itself is in a group
-    // chat (ContactId::SELF is not added to normal chats)
+    // ContactId::SELF may be used to check, if the user itself is in a
+    // group or incoming broadcast chat
+    // (ContactId::SELF is not added to 1:1 chats or outgoing broadcast channels)
 
     let exists = context
         .sql
@@ -4274,9 +4274,16 @@ pub async fn remove_contact_from_chat(
         !contact_id.is_special() || contact_id == ContactId::SELF,
         "Cannot remove special contact"
     );
-
     let chat = Chat::load_from_db(context, chat_id).await?;
-    if chat.typ == Chattype::Group || chat.typ == Chattype::OutBroadcast {
+
+    if chat.typ == Chattype::InBroadcast {
+        ensure!(
+            contact_id == ContactId::SELF,
+            "Cannot remove other member from incoming broadcast channel"
+        );
+    }
+
+    if chat.typ == Chattype::Group || chat.is_any_broadcast() {
         if !chat.is_self_in_chat(context).await? {
             let err_msg = format!(
                 "Cannot remove contact {contact_id} from chat {chat_id}: self not in group."
@@ -4327,13 +4334,6 @@ 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?;
-        let fingerprint = self_fingerprint(context).await?;
-        send_member_removal_msg(context, chat_id, contact_id, &self_addr, Some(fingerprint))
-            .await?;
     } else {
         bail!("Cannot remove members from non-group chats.");
     }

src/receive_imf.rs

@@ -3508,11 +3508,13 @@ async fn apply_out_broadcast_changes(
     .await?;
 
     if let Some(removed_fpr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemovedFpr) {
+        send_event_chat_modified = true;
         let removed_id = lookup_key_contact_by_fingerprint(context, removed_fpr).await?;
         if removed_id == Some(from_id) {
             // The sender of the message left the broadcast channel
             // Silently remove them without notifying the user
             chat::remove_from_chat_contacts_table_without_trace(context, chat.id, from_id).await?;
+            info!(context, "Broadcast leave message (TRASH)");
             better_msg = Some("".to_string());
         } else if from_id == ContactId::SELF {
             if let Some(removed_id) = removed_id {