Repository: scrapinghub/spidermon

Issue:
Allow pass extra optional arguments to Slack API
When sending a [Slack message](https://github.com/scrapinghub/spidermon/blob/master/spidermon/contrib/actions/slack/__init__.py#L111) we don't have an easy way to provide extra arguments to [postMessage](https://api.slack.com/methods/chat.postMessage) method (`mrkdwn` or `thread_ts` for example). It would be good to be able to provide extra arguments.

My use case is that I have an already created thread on  a monitoring Slack channel and I want that Spidermon notifications goes to a specific thread instead of going as a new message on the main channel.


Discussion:

Patch:
diff --git a/spidermon/contrib/actions/jobs/tags.py b/spidermon/contrib/actions/jobs/tags.py
index f44e516..fad1614 100644
--- a/spidermon/contrib/actions/jobs/tags.py
+++ b/spidermon/contrib/actions/jobs/tags.py
@@ -23,8 +23,9 @@ class JobTagsAction(Action):
             if not self.data.job:
                 raise NotConfigured("Job not available!")
             job_metadata = self.data.job.metadata
-            self.process_tags(job_metadata)
-            job_metadata.save()
+            job_tags = job_metadata.get("tags")
+            self.process_tags(job_tags)
+            job_metadata.set("tags", job_tags)
 
     def process_tags(self, job_metadata):
         raise NotImplementedError
@@ -33,16 +34,13 @@ class JobTagsAction(Action):
 class AddJobTags(JobTagsAction):
     tag_settings = "SPIDERMON_JOB_TAGS_TO_ADD"
 
-    def process_tags(self, job_metadata):
-        for tag in self.tags:
-            if tag not in job_metadata["tags"]:
-                job_metadata["tags"].append(tag)
+    def process_tags(self, job_tags):
+        tags_to_add = [tag for tag in self.tags if tag not in job_tags]
+        job_tags += tags_to_add
 
 
 class RemoveJobTags(JobTagsAction):
     tag_settings = "SPIDERMON_JOB_TAGS_TO_REMOVE"
 
-    def process_tags(self, job_metadata):
-        for tag in self.tags:
-            if tag in job_metadata["tags"]:
-                job_metadata["tags"].remove(tag)
+    def process_tags(self, job_tags):
+        job_tags[:] = [tag for tag in job_tags if tag not in self.tags]
diff --git a/spidermon/contrib/actions/slack/__init__.py b/spidermon/contrib/actions/slack/__init__.py
index 04bdc62..23779ee 100644
--- a/spidermon/contrib/actions/slack/__init__.py
+++ b/spidermon/contrib/actions/slack/__init__.py
@@ -39,7 +39,14 @@ class SlackMessageManager:
         return self._users
 
     def send_message(
-        self, to, text, parse=None, link_names=1, attachments=None, use_mention=False
+        self,
+        to,
+        text,
+        parse=None,
+        link_names=1,
+        attachments=None,
+        use_mention=False,
+        **kwargs,
     ):
         if self.fake:
             logger.info(text)
@@ -56,6 +63,7 @@ class SlackMessageManager:
                     link_names=link_names,
                     attachments=attachments,
                     use_mention=use_mention,
+                    **kwargs,
                 )
                 for recipient in to
             ]
@@ -66,6 +74,7 @@ class SlackMessageManager:
                 parse=parse,
                 link_names=link_names,
                 attachments=attachments,
+                **kwargs,
             )
         else:
             if use_mention:
@@ -79,6 +88,7 @@ class SlackMessageManager:
                 parse=parse,
                 link_names=link_names,
                 attachments=attachments,
+                **kwargs,
             )
 
     def _get_user_id(self, username):
@@ -95,7 +105,7 @@ class SlackMessageManager:
         )
 
     def _send_user_message(
-        self, username, text, parse="full", link_names=1, attachments=None
+        self, username, text, parse="full", link_names=1, attachments=None, **kwargs
     ):
         user_id = self._get_user_id(username)
         if user_id:
@@ -105,10 +115,11 @@ class SlackMessageManager:
                 parse=parse,
                 link_names=link_names,
                 attachments=attachments,
+                **kwargs,
             )
 
     def _send_channel_message(
-        self, channel, text, parse="full", link_names=1, attachments=None
+        self, channel, text, parse="full", link_names=1, attachments=None, **kwargs
     ):
         self._client.chat_postMessage(
             channel=channel,
@@ -118,6 +129,7 @@ class SlackMessageManager:
             attachments=self._parse_attachments(attachments),
             username=self.sender_name,
             icon_url=self._get_icon_url(),
+            **kwargs,
         )
 
     def _get_icon_url(self):
@@ -184,6 +196,7 @@ class SendSlackMessage(ActionWithTemplates):
         attachments_template=None,
         include_attachments=None,
         fake=None,
+        **kwargs,
     ):
         super().__init__()
 
@@ -207,6 +220,8 @@ class SendSlackMessage(ActionWithTemplates):
         self.attachments = attachments or self.attachments
         self.attachments_template = attachments_template or self.attachments_template
 
+        self.kwargs = kwargs or {}
+
         if not self.fake and not self.recipients:
             raise NotConfigured(
                 "You must provide a value for SPIDERMON_SLACK_RECIPIENTS setting."
@@ -238,6 +253,7 @@ class SendSlackMessage(ActionWithTemplates):
             to=self.recipients,
             text=self.get_message(),
             attachments=self.get_attachments(),
+            **self.kwargs,
         )
 
     def get_message(self):


Test patch:
diff --git a/tests/contrib/actions/jobs/test_tags.py b/tests/contrib/actions/jobs/test_tags.py
new file mode 100644
index 0000000..f95c69c
--- /dev/null
+++ b/tests/contrib/actions/jobs/test_tags.py
@@ -0,0 +1,61 @@
+from unittest.mock import MagicMock, patch
+
+import pytest
+from scrapy.utils.test import get_crawler
+
+from spidermon.contrib.actions.jobs.tags import AddJobTags, JobTagsAction, RemoveJobTags
+from spidermon.exceptions import NotConfigured
+
+
+@pytest.fixture
+def test_settings():
+    return {
+        "SPIDERMON_JOB_TAGS_TO_ADD": ["add_foo", "add_bar"],
+        "SPIDERMON_JOB_TAGS_TO_REMOVE": ["remove_foo", "remove_bar"],
+    }
+
+
+class SettableDict(dict):
+    def set(self, key, value):
+        self[key] = value
+
+
+def test_run_action(test_settings):
+    crawler = get_crawler(settings_dict=test_settings)
+    job_tags_action = JobTagsAction.from_crawler(crawler)
+
+    job_tags_action.tags = MagicMock(return_value=["foo", "bar"])
+    job_tags_action.data = MagicMock()
+    job_tags_action.data.job = None
+    with pytest.raises(NotConfigured):
+        job_tags_action.run_action()
+
+    job_tags_action.data.job = MagicMock()
+    with pytest.raises(NotImplementedError):
+        job_tags_action.run_action()
+
+
+def test_add_job_tags(test_settings):
+    crawler = get_crawler(settings_dict=test_settings)
+    add_job_tags = AddJobTags.from_crawler(crawler)
+
+    assert add_job_tags.tags == ["add_foo", "add_bar"]
+
+    add_job_tags.data = MagicMock()
+    add_job_tags.data.job.metadata = SettableDict({"tags": []})
+    add_job_tags.run_action()
+    assert add_job_tags.data.job.metadata["tags"] == ["add_foo", "add_bar"]
+
+
+def test_remove_job_tags(test_settings):
+    crawler = get_crawler(settings_dict=test_settings)
+    remove_job_tags = RemoveJobTags.from_crawler(crawler)
+
+    assert remove_job_tags.tags == ["remove_foo", "remove_bar"]
+
+    remove_job_tags.data = MagicMock()
+    remove_job_tags.data.job.metadata = SettableDict(
+        {"tags": ["remove_foo", "remove_bar"]}
+    )
+    remove_job_tags.run_action()
+    assert remove_job_tags.data.job.metadata["tags"] == []
diff --git a/tests/contrib/actions/slack/test_slack_action.py b/tests/contrib/actions/slack/test_slack_action.py
index 5cdd058..5427a22 100644
--- a/tests/contrib/actions/slack/test_slack_action.py
+++ b/tests/contrib/actions/slack/test_slack_action.py
@@ -1,7 +1,8 @@
 import sys
 import pytest
 
-from spidermon.contrib.actions.slack import SlackMessageManager
+from unittest.mock import MagicMock, patch
+from spidermon.contrib.actions.slack import SendSlackMessage, SlackMessageManager
 
 
 @pytest.fixture
@@ -58,3 +59,56 @@ def test_do_not_log_text_and_attach_when_fake_is_not_set(logger_info):
     )
 
     assert logger_info.call_count == 0
+
+
+@patch("spidermon.contrib.actions.slack.WebClient")
+def test_pass_arbitrary_args_to_manager_send_message_channel(_slack_mock):
+    manager = SlackMessageManager(
+        sender_token="anything",
+        sender_name="@someone",
+    )
+
+    manager.send_message(
+        to=["channel"], text="a message", use_mention=True, arbitrary_arg=True
+    )
+
+    _, kwargs = manager._client.chat_postMessage.call_args_list[0]
+    assert "arbitrary_arg" in kwargs
+
+
+@patch("spidermon.contrib.actions.slack.WebClient")
+def test_pass_arbitrary_args_to_manager_send_message_user(_slack_mock):
+    manager = SlackMessageManager(
+        sender_token="anything",
+        sender_name="@someone",
+    )
+
+    manager._users = {"user": {"id": 10}}
+
+    manager.send_message(
+        to=["@user"], text="a message", use_mention=True, arbitrary_arg=True
+    )
+
+    _, kwargs = manager._client.chat_postMessage.call_args_list[0]
+    assert "arbitrary_arg" in kwargs
+
+
+def test_message_sender_pass_kwargs():
+    sender = SendSlackMessage(
+        sender_token="anything",
+        sender_name="@someone",
+        recipients=["user"],
+        a_new_arg="hello",
+    )
+
+    sender.manager._client = MagicMock()
+    sender.get_message = MagicMock()
+    sender.get_attachments = MagicMock()
+
+    sender.get_message.return_value = "a message"
+    sender.get_attachments.return_value = None
+
+    sender.run_action()
+
+    _, kwargs = sender.manager._client.chat_postMessage.call_args_list[0]
+    assert "a_new_arg" in kwargs