This adds threading support officially. (#1057)

* This adds threading support officially.

It is only implemented on Slack at the moment.

- adds a parent field to Message signaling to backends that it is a threaded
response.
- retrofit in_reply_to as a signal to thread by putting this message as
parent without changing the frm/to behavior.
- adds an optional threaded boolean for the build_reply & co that needs
to be implemented by the backends (similar category as private).

* Test miss
  :wq

* Filter out notifications of threads status (#1060)

* Added DIVERT_TO_THREAD.

Similar to DIVERT_TO_PRIVATE to force some commands into a thread so it
is less noisy on chat.

errbot/backends/base.py

@@ -227,28 +227,31 @@ class Message(object):
     A chat message.
 
     This class represents chat messages that are sent or received by
-    the bot. It is modeled after XMPP messages so not all methods
-    make sense in the context of other back-ends.
+    the bot.
     """
 
     def __init__(self,
                  body: str='',
                  frm: Identifier=None,
                  to: Identifier=None,
+                 parent: 'Message'=None,
                  delayed: bool=False,
                  extras: Mapping=None,
                  flow=None):
         """
         :param body:
-            The plaintext body of the message.
+            The markdown body of the message.
         :param extras:
             Extra data attached by a backend
         :param flow:
             The flow in which this message has been triggered.
+        :param parent:
+            The parent message of this message in a thread. (Not supported by all backends)
         """
         self._body = body
         self._from = frm
         self._to = to
+        self._parent = parent
         self._delayed = delayed
         self._extras = extras or dict()
         self._flow = flow
@@ -260,7 +263,7 @@ def __init__(self,
             self.ctx = {}
 
     def clone(self):
-        return Message(self._body, self._from, self._to, self._delayed, self.extras)
+        return Message(self._body, self._from, self._to, self._parent, self._delayed, self.extras)
 
     @property
     def to(self) -> Identifier:
@@ -325,6 +328,14 @@ def delayed(self) -> bool:
     def delayed(self, delayed: bool):
         self._delayed = delayed
 
+    @property
+    def parent(self):
+        return self._parent
+
+    @parent.setter
+    def parent(self, parent: 'Message'):
+        self._parent = parent
+
     @property
     def extras(self) -> Mapping:
         return self._extras
@@ -350,6 +361,10 @@ def is_direct(self) -> bool:
     def is_group(self) -> bool:
         return isinstance(self.to, Room)
 
+    @property
+    def is_threaded(self) -> bool:
+        return self._parent is not None
+
 
 class Card(Message):
     """
@@ -361,6 +376,7 @@ def __init__(self,
                  body: str='',
                  frm: Identifier=None,
                  to: Identifier=None,
+                 parent: Message=None,
                  summary: str=None,
                  title: str='',
                  link: str=None,
@@ -373,6 +389,7 @@ def __init__(self,
         :param body: main text of the card in markdown.
         :param frm: the card is sent from this identifier.
         :param to: the card is sent to this identifier (Room, RoomOccupant, Person...).
+        :param parent: the parent message this card replies to. (threads the message if the backend supports it).
         :param summary: (optional) One liner summary of the card, possibly collapsed to it.
         :param title: (optional) Title possibly linking.
         :param link: (optional) url the title link is pointing to.
@@ -381,7 +398,7 @@ def __init__(self,
         :param color: (optional) background color or color indicator.
         :param fields: (optional) a tuple of (key, value) pairs.
         """
-        super().__init__(body=body, frm=frm, to=to)
+        super().__init__(body=body, frm=frm, to=to, parent=parent)
         self._summary = summary
         self._title = title
         self._link = link
@@ -640,7 +657,7 @@ def change_presence(self, status: str=ONLINE, message: str='') -> None:
         """Signal a presence change for the bot. Should be overridden by backends with a super().send_message() call."""
 
     @abstractmethod
-    def build_reply(self, msg: Message, text: str=None, private: bool=False):
+    def build_reply(self, msg: Message, text: str=None, private: bool=False, threaded: bool=False):
         """ Should be implemented by the backend """
 
     @abstractmethod

errbot/backends/hipchat.py

@@ -462,8 +462,8 @@ def query_room(self, room):
 
         return HipChatRoom(name, self)
 
-    def build_reply(self, msg, text=None, private=False):
-        response = super().build_reply(msg=msg, text=text, private=private)
+    def build_reply(self, msg, text=None, private=False, threaded=False):
+        response = super().build_reply(msg=msg, text=text, private=private, threaded=threaded)
         if msg.is_group and msg.frm == response.to:
             # HipChat violates the XMPP spec :( This results in a valid XMPP JID
             # but HipChat mangles them into stuff like

errbot/backends/irc.py

@@ -692,7 +692,7 @@ def change_presence(self, status: str = ONLINE, message: str = '') -> None:
     def send_stream_request(self, identifier, fsource, name=None, size=None, stream_type=None):
         return self.conn.send_stream_request(identifier, fsource, name, size, stream_type)
 
-    def build_reply(self, msg, text=None, private=False):
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         response = self.build_message(text)
         if msg.is_group:
             if private:

errbot/backends/null.py

@@ -59,7 +59,7 @@ def shutdown(self):
     def change_presence(self, status: str = ONLINE, message: str = '') -> None:
         pass
 
-    def build_reply(self, msg, text=None, private=False):
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         pass
 
     def prefix_groupchat_reply(self, message, identifier):

errbot/backends/slack.py

@@ -451,7 +451,7 @@ def _message_event_handler(self, event):
 
         subtype = event.get('subtype', None)
 
-        if subtype in ("message_deleted", "channel_topic"):
+        if subtype in ("message_deleted", "channel_topic", "message_replied"):
             log.debug("Message of type %s, ignoring this event", subtype)
             return
 
@@ -619,6 +619,16 @@ def _prepare_message(self, msg):  # or card
 
     def send_message(self, msg):
         super().send_message(msg)
+
+        if msg.parent is not None:
+            # we are asked to reply to a specify thread.
+            try:
+                msg.extras['thread_ts'] = self._ts_for_message(msg.parent)
+            except KeyError:
+                # Gives to the user a more interesting explanation if we cannot find a ts from the parent.
+                log.exception('The provided parent message is not a Slack message '
+                              'or does not contain a Slack timestamp.')
+
         to_humanreadable = "<unknown>"
         try:
             if msg.is_group:
@@ -858,11 +868,14 @@ def build_identifier(self, txtrep):
     def is_from_self(self, msg: Message) -> bool:
         return self.bot_identifier.userid == msg.frm.userid
 
-    def build_reply(self, msg, text=None, private=False):
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         response = self.build_message(text)
 
-        # If we reply to a threaded message, keep it in the thread.
-        if 'thread_ts' in msg.extras['slack_event']:
+        if threaded:
+            response.parent = msg
+
+        elif 'thread_ts' in msg.extras['slack_event']:
+            # If we reply to a threaded message, keep it in the thread.
             response.extras['thread_ts'] = msg.extras['slack_event']['thread_ts']
 
         response.frm = self.bot_identifier

errbot/backends/telegram_messenger.py

@@ -307,7 +307,7 @@ def build_identifier(self, txtrep):
         else:
             return TelegramRoom(id=id_)
 
-    def build_reply(self, msg, text=None, private=False):
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         response = self.build_message(text)
         response.frm = self.bot_identifier
         if private:

errbot/backends/test.py

@@ -280,7 +280,7 @@ def connect(self):
     def build_identifier(self, text_representation):
         return TestPerson(text_representation)
 
-    def build_reply(self, msg, text=None, private=False):
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         msg = self.build_message(text)
         msg.frm = self.bot_identifier
         msg.to = msg.frm

errbot/backends/text.py

@@ -407,7 +407,7 @@ def build_identifier(self, text_representation):
             raise ValueError('An identifier for the Text backend needs to start with # for a room or @ for a person.')
         return TextPerson(text_representation[1:])
 
-    def build_reply(self, msg, text=None, private=False):
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         response = self.build_message(text)
         response.frm = self.bot_identifier
         if private:

errbot/backends/xmpp.py

@@ -543,10 +543,7 @@ def build_identifier(self, txtrep):
         log.debug('This is a person ! %s', txtrep)
         return self._build_person(txtrep)
 
-    def build_reply(self, msg, text=None, private=False):
-        """Build a message for responding to another message.
-        Message is NOT sent"""
-        log.debug("build reply ...")
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         response = self.build_message(text)
         response.frm = self.bot_identifier
 

errbot/bootstrap.py

@@ -41,6 +41,8 @@ def bot_config_defaults(config):
         config.BOT_ALT_PREFIX_CASEINSENSITIVE = False
     if not hasattr(config, 'DIVERT_TO_PRIVATE'):
         config.DIVERT_TO_PRIVATE = ()
+    if not hasattr(config, 'DIVERT_TO_THREAD'):
+        config.DIVERT_TO_THREAD = ()
     if not hasattr(config, 'MESSAGE_SIZE_LIMIT'):
         config.MESSAGE_SIZE_LIMIT = 10000  # Corresponds with what HipChat accepts
     if not hasattr(config, 'GROUPCHAT_NICK_PREFIXED'):

errbot/botplugin.py

@@ -519,6 +519,7 @@ def send(self,
 
             :param groupchat_nick_reply: if True the message will mention the user in the chatroom.
             :param in_reply_to: the original message this message is a reply to (optional).
+                                In some backends it will start a thread.
             :param text: markdown formatted text to send to the user.
             :param identifier: An Identifier representing the user or room to message.
                                Identifiers may be created with :func:`build_identifier`.
@@ -560,7 +561,7 @@ def send_card(self,
             if in_reply_to is None:
                 raise ValueError('Either to or in_reply_to needs to be set.')
             to = in_reply_to.frm
-        self._bot.send_card(Card(body, frm, to, summary, title, link, image, thumbnail, color, fields))
+        self._bot.send_card(Card(body, frm, to, in_reply_to, summary, title, link, image, thumbnail, color, fields))
 
     def change_presence(self, status: str = ONLINE, message: str = '') -> None:
         """

errbot/config-template.py

@@ -294,6 +294,11 @@
 # DIVERT_TO_PRIVATE = ('help', 'about', 'status')
 DIVERT_TO_PRIVATE = ()
 
+# A list of commands which should be responded to in a thread if the backend supports it.
+# For example:
+# DIVERT_TO_THREAD = ('help', 'about', 'status')
+DIVERT_TO_THREAD = ()
+
 # Chat relay
 # Can be used to relay one to one message from specific users to the bot
 # to MUCs. This can be useful with XMPP notifiers like for example the

errbot/core.py

@@ -134,6 +134,7 @@ def send(self, identifier, text, in_reply_to=None, groupchat_nick_reply=False):
         msg = self.build_message(text)
         msg.to = identifier
         msg.frm = in_reply_to.to if in_reply_to else self.bot_identifier
+        msg.parent = in_reply_to
 
         nick_reply = self.bot_config.GROUPCHAT_NICK_PREFIXED
         if isinstance(identifier, Room) and in_reply_to and (nick_reply or groupchat_nick_reply):
@@ -186,14 +187,15 @@ def send_card(self, card):
         """
         self.send_templated(card.to, 'card', {'card': card})
 
-    def send_simple_reply(self, msg, text, private=False):
+    def send_simple_reply(self, msg, text, private=False, threaded=False):
         """Send a simple response to a given incoming message
 
         :param private: if True will force a response in private.
+        :param threaded: if True and if the backend supports it, sends the response in a threaded message.
         :param text: the markdown text of the message.
         :param msg: the message you are replying to.
         """
-        reply = self.build_reply(msg, text, private)
+        reply = self.build_reply(msg, text, private=private, threaded=threaded)
         if isinstance(reply.to, Room) and self.bot_config.GROUPCHAT_NICK_PREFIXED:
             self.prefix_groupchat_reply(reply, msg.frm)
         self.split_and_send_message(reply)
@@ -433,6 +435,7 @@ def _execute_and_send(self, cmd, args, match, msg, template_name=None):
 
         """
         private = cmd in self.bot_config.DIVERT_TO_PRIVATE
+        threaded = cmd in self.bot_config.DIVERT_TO_THREAD
         commands = self.re_commands if match else self.commands
         try:
             with self._gbl:
@@ -451,11 +454,11 @@ def _execute_and_send(self, cmd, args, match, msg, template_name=None):
                 replies = method(msg, match) if match else method(msg, args)
                 for reply in replies:
                     if reply:
-                        self.send_simple_reply(msg, self.process_template(template_name, reply), private)
+                        self.send_simple_reply(msg, self.process_template(template_name, reply), private, threaded)
             else:
                 reply = method(msg, match) if match else method(msg, args)
                 if reply:
-                    self.send_simple_reply(msg, self.process_template(template_name, reply), private)
+                    self.send_simple_reply(msg, self.process_template(template_name, reply), private, threaded)
 
             # The command is a success, check if this has not made a flow progressed
             self.flow_executor.trigger(cmd, msg.frm, msg.ctx)
@@ -464,14 +467,14 @@ def _execute_and_send(self, cmd, args, match, msg, template_name=None):
             reason = command_error.reason
             if command_error.template:
                 reason = self.process_template(command_error.template, reason)
-            self.send_simple_reply(msg, reason, private)
+            self.send_simple_reply(msg, reason, private, threaded)
 
         except Exception as e:
             tb = traceback.format_exc()
             log.exception('An error happened while processing '
                           'a message ("%s"): %s"' %
                           (msg.body, tb))
-            self.send_simple_reply(msg, self.MSG_ERROR_OCCURRED + ':\n %s' % e, private)
+            self.send_simple_reply(msg, self.MSG_ERROR_OCCURRED + ':\n %s' % e, private, threaded)
 
     def unknown_command(self, _, cmd, args):
         """ Override the default unknown command behavior

tests/base_backend_test.py

@@ -116,10 +116,12 @@ def __init__(self, extra_config=None):
     def build_identifier(self, text_representation):
         return TestPerson(text_representation)
 
-    def build_reply(self, msg, text=None, private=False):
+    def build_reply(self, msg, text=None, private=False, threaded=False):
         reply = self.build_message(text)
         reply.frm = self.bot_identifier
         reply.to = msg.frm
+        if threaded:
+            reply.parent = msg
         return reply
 
     def send_message(self, msg):
@@ -249,6 +251,16 @@ def test_buildreply(dummy_backend):
     assert str(resp.to) == 'user'
     assert str(resp.frm) == 'err'
     assert str(resp.body) == 'Response'
+    assert resp.parent is None
+
+
+def test_buildreply_with_parent(dummy_backend):
+    m = dummy_backend.build_message('Content')
+    m.frm = dummy_backend.build_identifier('user')
+    m.to = dummy_backend.build_identifier('somewhere')
+    resp = dummy_backend.build_reply(m, 'Response', threaded=True)
+
+    assert resp.parent is not None
 
 
 def test_bot_admins_unique_string():