diff --git a/errbot/backends/base.py b/errbot/backends/base.py index 041a6f38a..083b40754 100644 --- a/errbot/backends/base.py +++ b/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 diff --git a/errbot/backends/hipchat.py b/errbot/backends/hipchat.py index b91ac83c1..0b97b3edf 100644 --- a/errbot/backends/hipchat.py +++ b/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 diff --git a/errbot/backends/irc.py b/errbot/backends/irc.py index ca09e4950..7ee526e4a 100644 --- a/errbot/backends/irc.py +++ b/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: diff --git a/errbot/backends/null.py b/errbot/backends/null.py index ac4e70362..fd3e426ae 100644 --- a/errbot/backends/null.py +++ b/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): diff --git a/errbot/backends/slack.py b/errbot/backends/slack.py index a1f21641d..db92ce3fb 100644 --- a/errbot/backends/slack.py +++ b/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 = "" 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 diff --git a/errbot/backends/telegram_messenger.py b/errbot/backends/telegram_messenger.py index 2fca8bdb6..7a8e83903 100644 --- a/errbot/backends/telegram_messenger.py +++ b/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: diff --git a/errbot/backends/test.py b/errbot/backends/test.py index 23d2395b5..2d9be17a6 100644 --- a/errbot/backends/test.py +++ b/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 diff --git a/errbot/backends/text.py b/errbot/backends/text.py index cd8285bf8..174504570 100644 --- a/errbot/backends/text.py +++ b/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: diff --git a/errbot/backends/xmpp.py b/errbot/backends/xmpp.py index 569b56fc9..5dea669df 100644 --- a/errbot/backends/xmpp.py +++ b/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 diff --git a/errbot/bootstrap.py b/errbot/bootstrap.py index 5b9634c46..da6db884f 100644 --- a/errbot/bootstrap.py +++ b/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'): diff --git a/errbot/botplugin.py b/errbot/botplugin.py index 52a3802fe..8efbb7b14 100644 --- a/errbot/botplugin.py +++ b/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: """ diff --git a/errbot/config-template.py b/errbot/config-template.py index ffdf8fc4c..0d60714c9 100644 --- a/errbot/config-template.py +++ b/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 diff --git a/errbot/core.py b/errbot/core.py index bbcbba078..073e376aa 100644 --- a/errbot/core.py +++ b/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 diff --git a/tests/base_backend_test.py b/tests/base_backend_test.py index d93c01859..d61be7eed 100644 --- a/tests/base_backend_test.py +++ b/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():