From dd48f825b0286ba73637b612928cf519f7b263d0 Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Fri, 1 Mar 2019 11:14:21 +0000
Subject: [PATCH] GH-1061: Update the slack message handling to be more
 graceful

---
 opentech/apply/activity/messaging.py          | 36 ++++++++-----------
 .../apply/activity/tests/test_messaging.py    | 31 +++++++++++++---
 2 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py
index f4db03d66..1f7a5a8fd 100644
--- a/opentech/apply/activity/messaging.py
+++ b/opentech/apply/activity/messaging.py
@@ -428,37 +428,29 @@ class SlackAdapter(AdapterBase):
             return f'<{user.slack}>'
         return ''
 
-    def slack_channel(self, submission):
+    def slack_channels(self, submission):
+        target_rooms = [self.target_room]
         try:
-            target_rooms = submission.get_from_parent('slack_channel').split(',')
+            extra_rooms = submission.get_from_parent('slack_channel').split(',')
         except AttributeError:
-            # If not a submission object, set room to default.
-            target_rooms = self.target_room
+            # Not a submission object, no extra rooms.
+            pass
         else:
-            if not target_rooms:
-                # If no custom room, set to default.
-                target_rooms = self.target_room
-            else:
-                # Always send a copy to the default channel as well.
-                target_rooms.append(self.target_room)
+            target_rooms.extend(extra_rooms)
 
         # Make sure each channel name starts with a "#".
-        for i, target_room in enumerate(target_rooms):
-            if target_room and not target_room.startswith('#'):
-                target_rooms[i] = f"#{target_room}"
+        target_rooms = [
+            room if room.startswith('#') else '#' + room
+            for room in target_rooms
+            if room
+        ]
 
         return target_rooms
 
-    def send_message(self, message, recipient, **kwargs):
-        try:
-            submission = kwargs['submission']
-        except Exception:
-            # If no submission, set room to default.
-            target_rooms = self.target_room
-        else:
-            target_rooms = self.slack_channel(submission)
+    def send_message(self, message, recipient, submission, **kwargs):
+        target_rooms = self.slack_channels(submission)
 
-        if not self.destination or not target_rooms:
+        if not self.destination or not any(target_rooms):
             errors = list()
             if not self.destination:
                 errors.append('Destination URL')
diff --git a/opentech/apply/activity/tests/test_messaging.py b/opentech/apply/activity/tests/test_messaging.py
index c7e1d8540..48f9f519c 100644
--- a/opentech/apply/activity/tests/test_messaging.py
+++ b/opentech/apply/activity/tests/test_messaging.py
@@ -321,7 +321,8 @@ class TestSlackAdapter(AdapterMixin, TestCase):
     @responses.activate
     def test_cant_send_with_no_room(self):
         adapter = SlackAdapter()
-        adapter.send_message('my message', '')
+        submission = ApplicationSubmissionFactory()
+        adapter.send_message('my message', '', submission)
         self.assertEqual(len(responses.calls), 0)
 
     @override_settings(
@@ -331,7 +332,8 @@ class TestSlackAdapter(AdapterMixin, TestCase):
     @responses.activate
     def test_cant_send_with_no_url(self):
         adapter = SlackAdapter()
-        adapter.send_message('my message', '')
+        submission = ApplicationSubmissionFactory()
+        adapter.send_message('my message', '', submission)
         self.assertEqual(len(responses.calls), 0)
 
     @override_settings(
@@ -341,14 +343,35 @@ class TestSlackAdapter(AdapterMixin, TestCase):
     @responses.activate
     def test_correct_payload(self):
         responses.add(responses.POST, self.target_url, status=200, body='OK')
+        submission = ApplicationSubmissionFactory()
+        adapter = SlackAdapter()
+        message = 'my message'
+        adapter.send_message(message, '', submission)
+        self.assertEqual(len(responses.calls), 1)
+        self.assertDictEqual(
+            json.loads(responses.calls[0].request.body),
+            {
+                'room': [self.target_room],
+                'message': message,
+            }
+        )
+
+    @override_settings(
+        SLACK_DESTINATION_URL=target_url,
+        SLACK_DESTINATION_ROOM=target_room,
+    )
+    @responses.activate
+    def test_round_slack_channel(self):
+        responses.add(responses.POST, self.target_url, status=200, body='OK')
+        submission = ApplicationSubmissionFactory(page__slack_channel='dummy')
         adapter = SlackAdapter()
         message = 'my message'
-        adapter.send_message(message, '')
+        adapter.send_message(message, '', submission)
         self.assertEqual(len(responses.calls), 1)
         self.assertDictEqual(
             json.loads(responses.calls[0].request.body),
             {
-                'room': self.target_room,
+                'room': [self.target_room, '#dummy'],
                 'message': message,
             }
         )
-- 
GitLab