From 4e9ca9127e760527823a588f5bb4edac0d85c34d Mon Sep 17 00:00:00 2001
From: Todd Dembrey <todd.dembrey@torchbox.com>
Date: Mon, 30 Jul 2018 09:53:24 +0100
Subject: [PATCH] Refactor to control recipients better
---
opentech/apply/activity/messaging.py | 83 ++++++++++---------
.../apply/activity/tests/test_messaging.py | 18 ++--
2 files changed, 57 insertions(+), 44 deletions(-)
diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py
index b17d599d5..b63021282 100644
--- a/opentech/apply/activity/messaging.py
+++ b/opentech/apply/activity/messaging.py
@@ -50,19 +50,27 @@ class AdapterBase:
def render_message(self, message, **kwargs):
return message.format(**kwargs)
+ def extra_kwargs(self, message_type, **kwargs):
+ return {}
+
+ def recipients(self, message_type, **kwargs):
+ raise NotImplementedError()
+
def process(self, message_type, **kwargs):
message = self.message(message_type, **kwargs)
- kwargs.update(self.extra_kwargs(message_type, **kwargs))
-
if not message:
return
- if settings.SEND_MESSAGES or self.always_send:
- self.send_message(message, **kwargs)
+ recipients = self.recipients(message_type, **kwargs)
+ kwargs.update(self.extra_kwargs(message_type, **kwargs))
+
+ for recipient in recipients:
+ if settings.SEND_MESSAGES or self.always_send:
+ self.send_message(message, recipient=recipient, **kwargs)
- if not settings.SEND_MESSAGES:
- message = self.adapter_type + ': ' + message
- messages.add_message(kwargs['request'], messages.INFO, message)
+ if not settings.SEND_MESSAGES:
+ message = self.adapter_type + ': ' + message
+ messages.add_message(kwargs['request'], messages.INFO, message)
def send_message(self, message, **kwargs):
raise NotImplementedError()
@@ -81,6 +89,9 @@ class ActivityAdapter(AdapterBase):
MESSAGES.NEW_REVIEW: '{user} submitted a review'
}
+ def recipients(self, message_type, **kwargs):
+ return [None]
+
def reviewers_updated(self, added, removed, **kwargs):
message = ['Reviewers updated.']
if added:
@@ -122,23 +133,15 @@ class SlackAdapter(AdapterBase):
self.destination = settings.SLACK_DESTINATION_URL
self.target_room = settings.SLACK_DESTINATION_ROOM
- def message(self, message_type, **kwargs):
- message = super().message(message_type, **kwargs)
-
- user = kwargs['user']
- submission = kwargs['submission']
-
- slack_target = self.slack_id(submission.lead)
-
- message = ' '.join([slack_target, message]).strip()
- return message
-
def extra_kwargs(self, message_type, **kwargs):
submission = kwargs['submission']
request = kwargs['request']
link = link_to(submission, request)
return {'link': link}
+ def recipients(self, message_type, message, **kwargs):
+ return [self.slack_id(submission.lead)]
+
def notify_reviewers(self, submission, **kwargs):
reviewers_to_notify = []
for reviewer in submission.reviewers.all():
@@ -162,10 +165,12 @@ class SlackAdapter(AdapterBase):
return f'<{user.slack}>'
return ''
- def send_message(self, message, **kwargs):
+ def send_message(self, message, recipient, **kwargs):
if not self.destination or not self.target_room:
return
+ message = ' '.join([recipient, message]).strip()
+
data = {
"room": self.target_room,
"message": message,
@@ -183,22 +188,29 @@ class EmailAdapter(AdapterBase):
MESSAGES.INVITED_TO_PROPOSAL: 'messages/email/invited_to_proposal.html',
MESSAGES.READY_FOR_REVIEW: 'messages/email/ready_to_review.html',
}
- recipients = {
- MESSAGES.READY_FOR_REVIEW: 'get_reviewers',
- }
+
+ def extra_kwargs(self, message_type, submission, **kwargs):
+ if message_type == MESSAGES.READY_FOR_REVIEW:
+ subject = 'Application ready to review: {submission.title}'.format(submission=submission)
+ else:
+ subject = submission.page.specific.subject or 'Your application to Open Technology Fund: {submission.title}'.format(submission=submission)
+ return {
+ 'subject': subject,
+ }
def notify_comment(self, **kwargs):
comment = kwargs['comment']
if not comment.private:
return self.render_message('messages/email/comment.html', **kwargs)
- def get_recipients(self, submission):
- try:
- return getattr(self, self.recipients[''])
+ def recipients(self, message_type, submission, **kwargs):
+ if message_type == MESSAGES.READY_FOR_REVIEW:
+ return self.reviewers(submission)
+ return [submission.user.email]
- def get_reviewers(self, )
+ def reviewers(self, submission):
return [
- [reviewer.email]
+ reviewer.email
for reviewer in submission.reviewers.all()
if submission.phase.permissions.can_review(reviewer)
]
@@ -206,16 +218,13 @@ class EmailAdapter(AdapterBase):
def render_message(self, template, **kwargs):
return render_to_string(template, kwargs)
- def send_message(self, message, submission, **kwargs):
- subject = submission.page.specific.subject or 'Your application to Open Technology Fund: {submission.title}'.format(submission=submission)
- recipients = self.get_recipients(submission) or [(submission.user.email,)]
- for recipient in recipients:
- send_mail(
- subject,
- message,
- submission.page.specific.from_address,
- recipient,
- )
+ def send_message(self, message, submission, subject, recipient, **kwargs):
+ send_mail(
+ subject,
+ message,
+ submission.page.specific.from_address,
+ [recipient],
+ )
class MessengerBackend:
diff --git a/opentech/apply/activity/tests/test_messaging.py b/opentech/apply/activity/tests/test_messaging.py
index a27299f51..41ba22622 100644
--- a/opentech/apply/activity/tests/test_messaging.py
+++ b/opentech/apply/activity/tests/test_messaging.py
@@ -33,6 +33,9 @@ class TestAdapter(AdapterBase):
def send_message(self, message, **kwargs):
pass
+ def recipients(self, message_type, **kwargs):
+ return [message_type]
+
@override_settings(SEND_MESSAGES=True)
class TestBaseAdapter(TestCase):
@@ -46,7 +49,7 @@ class TestBaseAdapter(TestCase):
message_type = MESSAGES.UPDATE_LEAD
self.adapter.process(message_type)
- self.adapter.send_message.assert_called_once_with(message_type.value)
+ self.adapter.send_message.assert_called_once_with(message_type.value, recipient=message_type)
def test_doesnt_send_a_message_if_not_configured(self):
self.adapter.process('this_is_not_a_message_type')
@@ -61,14 +64,14 @@ class TestBaseAdapter(TestCase):
self.adapter.process(method_name)
- self.adapter.send_message.assert_called_once_with(return_message)
+ self.adapter.send_message.assert_called_once_with(return_message, recipient=method_name)
def test_that_kwargs_passed_to_send_message(self):
message_type = MESSAGES.UPDATE_LEAD
kwargs = {'test': 'that', 'these': 'exist'}
self.adapter.process(message_type, **kwargs)
- self.adapter.send_message.assert_called_once_with(message_type.value, **kwargs)
+ self.adapter.send_message.assert_called_once_with(message_type.value, recipient=message_type, **kwargs)
def test_that_message_is_formatted(self):
message_type = MESSAGES.UPDATE_LEAD
@@ -77,7 +80,7 @@ class TestBaseAdapter(TestCase):
self.adapter.process(message_type, message_to_format=message)
- self.adapter.send_message.assert_called_once_with(message, message_to_format=message)
+ self.adapter.send_message.assert_called_once_with(message, message_to_format=message, recipient=message_type)
@override_settings(SEND_MESSAGES=False)
def test_django_messages_used(self):
@@ -168,7 +171,7 @@ class TestSlackAdapter(TestCase):
@responses.activate
def test_cant_send_with_no_room(self):
adapter = SlackAdapter()
- adapter.send_message('my message')
+ adapter.send_message('my message', '')
self.assertEqual(len(responses.calls), 0)
@override_settings(
@@ -178,7 +181,7 @@ class TestSlackAdapter(TestCase):
@responses.activate
def test_cant_send_with_no_url(self):
adapter = SlackAdapter()
- adapter.send_message('my message')
+ adapter.send_message('my message', '')
self.assertEqual(len(responses.calls), 0)
@override_settings(
@@ -190,7 +193,7 @@ class TestSlackAdapter(TestCase):
responses.add(responses.POST, self.target_url, status=200)
adapter = SlackAdapter()
message = 'my message'
- adapter.send_message(message)
+ adapter.send_message(message, '')
self.assertEqual(len(responses.calls), 1)
self.assertDictEqual(
json.loads(responses.calls[0].request.body),
@@ -218,3 +221,4 @@ class TestEmailAdapter(TestCase):
adapter.process(MESSAGES.READY_FOR_REVIEW, submission=submission)
self.assertEqual(len(mail.outbox), 4)
+ self.assertTrue(mail.outbox[0].subject, 'ready to review')
--
GitLab