refine member-added and member-removed plugin hooks to signal the sender (who added/removed a contact )

add ac_chat_modified hook event
add account.get_contact_by_addr (thanks @r10s)

python/examples/group_tracking.py

@@ -22,14 +22,16 @@ class GroupTrackingPlugin:
         print("*** ac_configure_completed:", success)
 
     @account_hookimpl
-    def ac_member_added(self, chat, contact):
-        print("*** ac_member_added", contact.addr, "from", chat)
+    def ac_member_added(self, chat, contact, sender):
+        print("*** ac_member_added {} to chat {} from {}".format(
+            contact.addr, chat.id, sender.addr))
         for member in chat.get_contacts():
             print("chat member: {}".format(member.addr))
 
     @account_hookimpl
-    def ac_member_removed(self, chat, contact):
-        print("*** ac_member_removed", contact.addr, "from", chat)
+    def ac_member_removed(self, chat, contact, sender):
+        print("*** ac_member_removed {} from chat {} by {}".format(
+            contact.addr, chat.id, sender.addr))
 
 
 def main(argv=None):

python/examples/test_examples.py

@@ -43,14 +43,14 @@ def test_group_tracking_plugin(acfactory, lp):
     ac1.add_account_plugin(FFIEventLogger(ac1, "ac1"))
     ac2.add_account_plugin(FFIEventLogger(ac2, "ac2"))
 
-    lp.sec("creating bot test group with all three")
+    lp.sec("creating bot test group with bot")
     bot_contact = ac1.create_contact(botproc.addr)
     ch = ac1.create_group_chat("bot test group")
     ch.add_contact(bot_contact)
     ch.send_text("hello")
 
     botproc.fnmatch_lines("""
-        *ac_member_added {}*
+        *ac_chat_modified*
     """.format(ac1.get_config("addr")))
 
     lp.sec("adding third member {}".format(ac2.get_config("addr")))

python/src/deltachat/account.py

@@ -245,6 +245,14 @@ class Account(object):
         assert contact_id > const.DC_CHAT_ID_LAST_SPECIAL
         return bool(lib.dc_delete_contact(self._dc_context, contact_id))
 
+    def get_contact_by_addr(self, email):
+        """ get a contact for the email address or None if it's blocked or doesn't exist. """
+        _, addr = parseaddr(email)
+        addr = as_dc_charpointer(addr)
+        contact_id = lib.dc_lookup_contact_id_by_addr(self._dc_context, addr)
+        if contact_id:
+            return self.get_contact_by_id(contact_id)
+
     def get_contacts(self, query=None, with_self=False, only_verified=False):
         """ get a (filtered) list of contacts.
 
@@ -617,25 +625,46 @@ class Account(object):
                 return "ac_configure_completed", dict(success=success)
         elif name == "DC_EVENT_INCOMING_MSG":
             msg = self.get_message_by_id(ffi_event.data2)
-            return "ac_incoming_message", dict(message=msg)
+            return self._map_incoming(msg)
         elif name == "DC_EVENT_MSGS_CHANGED":
             if ffi_event.data2 != 0:
                 msg = self.get_message_by_id(ffi_event.data2)
+                if msg.is_outgoing():
+                    evname, kwargs = self._map_incoming(msg)
+                    if evname.startswith("ac_member"):
+                        return evname, kwargs
                 if msg.is_in_fresh():
-                    return "ac_incoming_message", dict(message=msg)
+                    return self._map_incoming(msg)
         elif name == "DC_EVENT_MSG_DELIVERED":
             msg = self.get_message_by_id(ffi_event.data2)
             return "ac_message_delivered", dict(message=msg)
-        elif name == "DC_EVENT_MEMBER_ADDED":
+        elif name == "DC_EVENT_CHAT_MODIFIED":
             chat = self.get_chat_by_id(ffi_event.data1)
-            contact = self.get_contact_by_id(ffi_event.data2)
-            return "ac_member_added", dict(chat=chat, contact=contact)
-        elif name == "DC_EVENT_MEMBER_REMOVED":
-            chat = self.get_chat_by_id(ffi_event.data1)
-            contact = self.get_contact_by_id(ffi_event.data2)
-            return "ac_member_removed", dict(chat=chat, contact=contact)
+            return "ac_chat_modified", dict(chat=chat)
         return None, {}
 
+    def _map_incoming(self, msg):
+        if msg.is_system_message():
+            res = parse_system_add_remove(msg.text)
+            if res:
+                contact = msg.account.get_contact_by_addr(res[1])
+                if contact:
+                    d = dict(chat=msg.chat, contact=contact, sender=msg.get_sender_contact())
+                    return "ac_member_" + res[0], d
+        return "ac_incoming_message", dict(message=msg)
+
+
+def parse_system_add_remove(text):
+    # Member Me (x@y) removed by a@b.
+    # Member x@y removed by a@b
+    text = text.lower()
+    parts = text.split()
+    if parts[0] == "member":
+        if parts[2] in ("removed", "added"):
+            return parts[2], parts[1]
+        if parts[3] in ("removed", "added"):
+            return parts[3], parts[2].strip("()")
+
 
 def _destroy_dc_context(dc_context, dc_context_unref=lib.dc_context_unref):
     # destructor for dc_context

python/src/deltachat/hookspec.py

@@ -53,11 +53,15 @@ class PerAccount:
         """ Called when an outgoing message has been delivered to SMTP. """
 
     @account_hookspec
-    def ac_member_added(self, chat, contact):
-        """ Called for each contact added to a chat. """
+    def ac_chat_modified(self, chat):
+        """ Chat was created or modified regarding membership, avatar, title. """
 
     @account_hookspec
-    def ac_member_removed(self, chat, contact):
+    def ac_member_added(self, chat, contact, sender):
+        """ Called for each contact added to an accepted chat. """
+
+    @account_hookspec
+    def ac_member_removed(self, chat, contact, sender):
         """ Called for each contact removed from a chat. """
 
 

python/src/deltachat/message.py

@@ -91,6 +91,10 @@ class Message(object):
         """mime type of the file (if it exists)"""
         return from_dc_charpointer(lib.dc_msg_get_filemime(self._dc_msg))
 
+    def is_system_message(self):
+        """ return True if this message is a system/info message. """
+        return lib.dc_msg_is_info(self._dc_msg)
+
     def is_setup_message(self):
         """ return True if this message is a setup message. """
         return lib.dc_msg_is_setupmessage(self._dc_msg)
@@ -224,6 +228,13 @@ class Message(object):
         """
         return self._msgstate == const.DC_STATE_IN_SEEN
 
+    def is_outgoing(self):
+        """Return True if Message is outgoing. """
+        return self._msgstate in (
+            const.DC_STATE_OUT_PREPARING, const.DC_STATE_OUT_PENDING,
+            const.DC_STATE_OUT_FAILED, const.DC_STATE_OUT_MDN_RCVD,
+            const.DC_STATE_OUT_DELIVERED)
+
     def is_out_preparing(self):
         """Return True if Message is outgoing, but its file is being prepared.
         """

python/tests/test_account.py

@@ -11,6 +11,17 @@ from conftest import (wait_configuration_progress,
                       wait_securejoin_inviter_progress)
 
 
+@pytest.mark.parametrize("msgtext,res", [
+    ("Member Me (tmp1@x.org) removed by tmp2@x.org.", ("removed", "tmp1@x.org")),
+    ("Member tmp1@x.org added by tmp2@x.org.", ("added", "tmp1@x.org")),
+])
+def test_parse_system_add_remove(msgtext, res):
+    from deltachat.account import parse_system_add_remove
+
+    out = parse_system_add_remove(msgtext)
+    assert out == res
+
+
 class TestOfflineAccountBasic:
     def test_wrong_db(self, tmpdir):
         p = tmpdir.join("hello.db")
@@ -170,35 +181,6 @@ class TestOfflineChat:
         else:
             pytest.fail("could not find chat")
 
-    def test_add_member_event(self, ac1):
-        chat = ac1.create_group_chat(name="title1")
-        assert chat.is_group()
-        contact1 = ac1.create_contact("some1@hello.com", name="some1")
-
-        chat.add_contact(contact1)
-        for ev in ac1.iter_events(timeout=1):
-            if ev.name == "ac_member_added":
-                assert ev.kwargs["chat"] == chat
-                if ev.kwargs["contact"] == ac1.get_self_contact():
-                    continue
-                assert ev.kwargs["contact"] == contact1
-                break
-
-    def test_remove_member_event(self, ac1):
-        chat = ac1.create_group_chat(name="title1")
-        assert chat.is_group()
-        contact1 = ac1.create_contact("some1@hello.com", name="some1")
-        chat.add_contact(contact1)
-        ac1._handle_current_events()
-        chat.remove_contact(contact1)
-        for ev in ac1.iter_events(timeout=1):
-            if ev.name == "ac_member_removed":
-                assert ev.kwargs["chat"] == chat
-                if ev.kwargs["contact"] == ac1.get_self_contact():
-                    continue
-                assert ev.kwargs["contact"] == contact1
-                break
-
     def test_group_chat_creation(self, ac1):
         contact1 = ac1.create_contact("some1@hello.com", name="some1")
         contact2 = ac1.create_contact("some2@hello.com", name="some2")
@@ -496,7 +478,7 @@ class TestOfflineChat:
         # perform plugin hooks
         ac1._handle_current_events()
 
-        assert len(in_list) == 11
+        assert len(in_list) == 10
         chat_contacts = chat.get_contacts()
         for in_cmd, in_chat, in_contact in in_list:
             assert in_cmd == "added"
@@ -504,19 +486,23 @@ class TestOfflineChat:
             assert in_contact in chat_contacts
             chat_contacts.remove(in_contact)
 
+        assert chat_contacts[0].id == 1  # self contact
+
+        in_list[:] = []
+
         lp.sec("ac1: removing two contacts and checking things are right")
         chat.remove_contact(contacts[9])
         chat.remove_contact(contacts[3])
         assert len(chat.get_contacts()) == 9
 
         ac1._handle_current_events()
-        assert len(in_list) == 13
-        assert in_list[-2][0] == "removed"
-        assert in_list[-2][1] == chat
-        assert in_list[-2][2] == contacts[9]
-        assert in_list[-1][0] == "removed"
-        assert in_list[-1][1] == chat
-        assert in_list[-1][2] == contacts[3]
+        assert len(in_list) == 2
+        assert in_list[0][0] == "removed"
+        assert in_list[0][1] == chat
+        assert in_list[0][2] == contacts[9]
+        assert in_list[1][0] == "removed"
+        assert in_list[1][1] == chat
+        assert in_list[1][2] == contacts[3]
 
 
 class TestOnlineAccount:
@@ -1297,48 +1283,77 @@ class TestOnlineAccount:
 
     def test_add_remove_member_remote_events(self, acfactory, lp):
         ac1, ac2 = acfactory.get_two_online_accounts()
+        ac1_addr = ac1.get_config("addr")
+        ac2_addr = ac2.get_config("addr")
         # activate local plugin for ac2
         in_list = queue.Queue()
 
+        class EventHolder:
+            def __init__(self, **kwargs):
+                self.__dict__.update(kwargs)
+
         class InPlugin:
             @account_hookimpl
-            def ac_member_added(self, chat, contact):
-                in_list.put(("added", chat, contact))
+            def ac_incoming_message(self, message):
+                # we immediately accept the sender because
+                # otherwise we won't see member_added contacts
+                message.accept_sender_contact()
 
             @account_hookimpl
-            def ac_member_removed(self, chat, contact):
-                in_list.put(("removed", chat, contact))
+            def ac_chat_modified(self, chat):
+                in_list.put(EventHolder(action="chat-modified", chat=chat))
+
+            @account_hookimpl
+            def ac_member_added(self, chat, contact, sender):
+                in_list.put(EventHolder(action="added", chat=chat, contact=contact, sender=sender))
+
+            @account_hookimpl
+            def ac_member_removed(self, chat, contact, sender):
+                in_list.put(EventHolder(action="removed", chat=chat, contact=contact, sender=sender))
 
         ac2.add_account_plugin(InPlugin())
 
         lp.sec("ac1: create group chat with ac2")
         chat = ac1.create_group_chat("hello")
-        contact = ac1.create_contact(email=ac2.get_config("addr"))
+        contact = ac1.create_contact(email=ac2_addr)
         chat.add_contact(contact)
 
         lp.sec("ac1: send a message to group chat to promote the group")
         chat.send_text("afterwards promoted")
-        ev1 = in_list.get()
-        ev2 = in_list.get()
-        assert ev1[2] == ac2.get_self_contact()
-        assert ev2[2].addr == ac1.get_config("addr")
+        ev = in_list.get(timeout=10)
+        assert ev.action == "chat-modified"
+        assert chat.is_promoted()
+        assert sorted(x.addr for x in chat.get_contacts()) == \
+            sorted(x.addr for x in ev.chat.get_contacts())
 
         lp.sec("ac1: add address2")
-        contact2 = ac1.create_contact(email="not@example.org")
+        # note that if the above accept_sender_contact() would not
+        # happen we would not receive a proper member_added event
+        contact2 = ac1.create_contact(email="notexistingaccountihope@testrun.org")
         chat.add_contact(contact2)
-        ev1 = in_list.get()
-        assert ev1[2].addr == contact2.addr
+        ev = in_list.get(timeout=10)
+        assert ev.action == "chat-modified"
+        ev = in_list.get(timeout=10)
+        assert ev.action == "added"
+        assert ev.sender.addr == ac1_addr
+        assert ev.contact.addr == "notexistingaccountihope@testrun.org"
 
         lp.sec("ac1: remove address2")
         chat.remove_contact(contact2)
-        ev1 = in_list.get()
-        assert ev1[0] == "removed"
-        assert ev1[2].addr == contact2.addr
+        ev = in_list.get(timeout=10)
+        assert ev.action == "chat-modified"
+        ev = in_list.get(timeout=10)
+        assert ev.action == "removed"
+        assert ev.contact.addr == contact2.addr
+        assert ev.sender.addr == ac1_addr
 
         lp.sec("ac1: remove ac2 contact from chat")
         chat.remove_contact(contact)
-        ev1 = in_list.get()
-        assert ev1[2] == ac2.get_self_contact()
+        ev = in_list.get(timeout=10)
+        assert ev.action == "chat-modified"
+        ev = in_list.get(timeout=10)
+        assert ev.action == "removed"
+        assert ev.sender.addr == ac1_addr
 
     def test_set_get_group_image(self, acfactory, data, lp):
         ac1, ac2 = acfactory.get_two_online_accounts()

src/chat.rs

@@ -2155,10 +2155,6 @@ pub fn remove_contact_from_chat(
                         msg.param.set_cmd(SystemMessage::MemberRemovedFromGroup);
                         msg.param.set(Param::Arg, contact.get_addr());
                         msg.id = send_msg(context, chat_id, &mut msg)?;
-                        context.call_cb(Event::MsgsChanged {
-                            chat_id,
-                            msg_id: msg.id,
-                        });
                     }
                 }
                 // we remove the member from the chat after constructing the