Repository: opsdroid/opsdroid

Issue:
Ability to send messages to threads (Mattermost)
# Description
The feature to post into threads was implemented for Slack in #1229 
Mattermost has the same feature but the connector seemingly does not support it yet. This is a feature request to implement message threads for Mattermost as well.


## Steps to Reproduce
Feature Enhancement


## Expected Functionality
Support a new configuration on the mattermost connector to prefer responses in Threads.


## Hints:
The field `root_id` needs to be set to the ID of the message that acts as the root of the thread here:
https://github.com/opsdroid/opsdroid/blob/main/opsdroid/connector/mattermost/__init__.py#L119

The root ID can be determined as follows:
`raw_event["data"]["post"]["root_id"]` is **not an empty string**: use it
`raw_event["data"]["post"]["id"]` otherwise, to start a new thread

However, it looks like `post` is a raw string, that needs to go through json.loads or something...

It is rather challenging with the way the firewall works for us, to prepare a PR myself so if anyone could assist, that would be a tremendous help!

Discussion:

Solution patch:
diff --git a/docs/connectors/mattermost.md b/docs/connectors/mattermost.md
index 49b297b..f91a994 100644
--- a/docs/connectors/mattermost.md
+++ b/docs/connectors/mattermost.md
@@ -22,6 +22,9 @@ connectors:
     scheme: "http" # default: https
     port: 8065 # default: 8065
     connect-timeout: 30 # default: 30
+
+    # Mattermost should always respond in a thread:
+    use-threads: True # default: False
 ```
 
 ## Usage
diff --git a/opsdroid/connector/mattermost/connector.py b/opsdroid/connector/mattermost/connector.py
index f8f40f7..bfb992e 100644
--- a/opsdroid/connector/mattermost/connector.py
+++ b/opsdroid/connector/mattermost/connector.py
@@ -14,6 +14,7 @@ CONFIG_SCHEMA = {
     Required("token"): str,
     Required("url"): str,
     Required("team-name"): str,
+    "use-threads": bool,
     "scheme": str,
     "port": int,
     "connect-timeout": int,
@@ -34,6 +35,7 @@ class ConnectorMattermost(Connector):
         self._scheme = config.get("scheme", "https")
         self._port = config.get("port", 8065)
         self._timeout = config.get("connect-timeout", 30)
+        self._use_threads = config.get("use-threads", False)
 
         self._bot_id = None
 
@@ -123,9 +125,24 @@ class ConnectorMattermost(Connector):
             myjson = await response.json()
             channel_id = myjson["id"]
 
-        async with self._mm_driver.posts.create_post(
-            json={"channel_id": channel_id, "message": message.text}
-        ) as response:
+        body = {}
+        if self._use_threads:
+            data = message.linked_event.raw_event["data"]
+            post = json.loads(data["post"])
+
+            # root_id is set if it is already a thread
+            # otherwise use the id to start a new thread from that top level message
+            root_id = ("root_id" in post and post["root_id"]) or post["id"]
+
+            body = {
+                "channel_id": channel_id,
+                "message": message.text,
+                "root_id": root_id,
+            }
+        else:
+            body = {"channel_id": channel_id, "message": message.text}
+
+        async with self._mm_driver.posts.create_post(json=body) as response:
             _LOGGER.debug(
                 _("Mattermost responds with status code '%s' to the new post"),
                 response.status,


Test patch:
diff --git a/opsdroid/connector/mattermost/tests/test_connector.py b/opsdroid/connector/mattermost/tests/test_connector.py
index ac1a5ff..d54632e 100644
--- a/opsdroid/connector/mattermost/tests/test_connector.py
+++ b/opsdroid/connector/mattermost/tests/test_connector.py
@@ -132,11 +132,93 @@ async def test_api_send_message(connector, mock_api, caplog):
     )
 
 
+@pytest.mark.add_response(*USER_ME_SUCCESS)
+@pytest.mark.add_response(*GET_CHANNEL_ID_FOR_CHANNEL_SUCCESS)
+@pytest.mark.add_response(*CREATE_POST_SUCCESS)
+async def test_api_send_message_threaded_new(connector, mock_api, caplog):
+    """Test that Mattermost correctly creates a new thread if configured appropriately."""
+    connector._use_threads = True
+
+    with caplog.at_level("DEBUG"):
+        await connector.connect()
+        await connector.send_message(
+            Message(
+                text="Unit Test Message",
+                target="unit_test_channel",
+                linked_event=Message(
+                    "Original Message",
+                    raw_event={"data": {"post": '{"id": "top-level-id"}'}},
+                ),
+            )
+        )
+
+    assert (
+        "Querying channel for team 'opsdroid' and name 'unit_test_channel'"
+        in caplog.text
+    )
+    assert (
+        "Responding with: 'Unit Test Message' in room  unit_test_channel" in caplog.text
+    )
+
+    post_call = mock_api.get_payload("/api/v4/posts", 0)
+
+    assert post_call["channel_id"] == "channel_id_for_name"
+    assert post_call["message"] == "Unit Test Message"
+
+    assert (
+        "Sending post with payload '{'channel_id': 'channel_id_for_name', 'message': 'Unit Test Message', 'root_id': 'top-level-id'}'"
+        in caplog.text
+    )
+
+
+@pytest.mark.add_response(*USER_ME_SUCCESS)
+@pytest.mark.add_response(*GET_CHANNEL_ID_FOR_CHANNEL_SUCCESS)
+@pytest.mark.add_response(*CREATE_POST_SUCCESS)
+async def test_api_send_message_threaded_existing(connector, mock_api, caplog):
+    """Test that Mattermost correctly creates a new thread if configured appropriately."""
+    connector._use_threads = True
+
+    with caplog.at_level("DEBUG"):
+        await connector.connect()
+        await connector.send_message(
+            Message(
+                text="Unit Test Message",
+                target="unit_test_channel",
+                linked_event=Message(
+                    "Original Message",
+                    raw_event={
+                        "data": {
+                            "post": '{"id": "inside-thread-id", "root_id": "top-level-id"}'
+                        }
+                    },
+                ),
+            )
+        )
+
+    assert (
+        "Querying channel for team 'opsdroid' and name 'unit_test_channel'"
+        in caplog.text
+    )
+    assert (
+        "Responding with: 'Unit Test Message' in room  unit_test_channel" in caplog.text
+    )
+
+    post_call = mock_api.get_payload("/api/v4/posts", 0)
+
+    assert post_call["channel_id"] == "channel_id_for_name"
+    assert post_call["message"] == "Unit Test Message"
+
+    assert (
+        "Sending post with payload '{'channel_id': 'channel_id_for_name', 'message': 'Unit Test Message', 'root_id': 'top-level-id'}'"
+        in caplog.text
+    )
+
+
 @pytest.mark.add_response(*USER_ME_SUCCESS)
 @pytest.mark.add_response(*WEBSOCKET_MESSAGE)
 @pytest.mark.add_response(*WEBSOCKET_HELLO)
 async def test_websocket_message(connector, mock_api, caplog):
-    """Tests that messages over the websocket are propery processed by opsdroid."""
+    """Tests that messages over the websocket are properly processed by opsdroid."""
     caplog.set_level(
         "DEBUG"
     )  # cannot use 'with caplog.at_level' because it wouldn't apply to the websocket loop