diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py index 1fda69d7f9132a5df95fba288304c1a178208a90..62f76957567deeec3e496a059e82864a0287e64c 100644 --- a/opentech/apply/activity/messaging.py +++ b/opentech/apply/activity/messaging.py @@ -1,26 +1,40 @@ from enum import Enum + +import requests + +from django.conf import settings from django.contrib import messages from .models import Activity +def link_to(target, request): + return request.scheme + '://' + request.get_host() + target.get_absolute_url() + + class MESSAGES(Enum): UPDATE_LEAD = 'update_lead' NEW_SUBMISSION = 'new_submission' TRANSITION = 'transition' - NEW_DETERMINATION = 'new_determination' DETERMINATION_OUTCOME = 'determination_outcome' INVITED_TO_PROPOSAL = 'invited_to_proposal' REVIEWERS_UPDATED = 'reviewers_updated' NEW_REVIEW = 'new_review' COMMENT = 'comment' + PROPOSAL_SUBMITTED = 'proposal_submitted' class AdapterBase: messages = {} + always_send = False def message(self, message_type, **kwargs): - message = self.messages[message_type] + try: + message = self.messages[message_type] + except KeyError: + # We don't know how to handle that message type + return + try: # see if its a method on the adapter method = getattr(self, message) @@ -30,33 +44,30 @@ class AdapterBase: return method(**kwargs) def process(self, message_type, **kwargs): - try: - message = self.message(message_type, **kwargs) - except KeyError: - return - self.send_message(message, **kwargs) + message = self.message(message_type, **kwargs) - def send_message(self, message, **kwargs): - raise NotImplementedError() + if not message: + return + if settings.SEND_MESSAGES or self.always_send: + self.send_message(message, **kwargs) -class MessageAdapter(AdapterBase): - messages = { - enum: enum.value - for enum in MESSAGES.__members__.values() - } + if not settings.SEND_MESSAGES: + message = self.adapter_type + ': ' + message + messages.add_message(kwargs['request'], messages.INFO, message) def send_message(self, message, **kwargs): - messages.add_message(kwargs['request'], messages.INFO, message) + raise NotImplementedError() class ActivityAdapter(AdapterBase): + adapter_type = "Activity Feed" + always_send = True messages = { MESSAGES.TRANSITION: 'Progressed from {old_phase.display_name} to {submission.phase}', MESSAGES.NEW_SUBMISSION: 'Submitted {submission.title} for {submission.page.title}', MESSAGES.UPDATE_LEAD: 'Lead changed from {old.lead} to {submission.lead}', - MESSAGES.NEW_DETERMINATION: 'Created a determination for {submission.title}', - MESSAGES.DETERMINATION_OUTCOME: 'Sent a {submission.determination.get_outcome_display} determination for {submission.title}:\r\n{determination.clean_message}', + MESSAGES.DETERMINATION_OUTCOME: 'Sent a {submission.determination.get_outcome_display} determination for {submission.title}:\r\n{submission.determination.clean_message}', MESSAGES.INVITED_TO_PROPOSAL: '{submission.title} has been invited to submit a proposal.', MESSAGES.REVIEWERS_UPDATED: 'reviewers_updated', MESSAGES.NEW_REVIEW: 'Created a review for {submission.title}' @@ -82,6 +93,55 @@ class ActivityAdapter(AdapterBase): ) +class SlackAdapter(AdapterBase): + adapter_type = "Slack" + always_send = True + messages = { + MESSAGES.NEW_SUBMISSION: 'A new submission has been submitted for {{submission.page.title}}: <{link}|{submission.title}>', + MESSAGES.UPDATE_LEAD: 'The lead of <{link}|{submission.title}> has been updated from {old.lead} to {submission.lead} by {user}', + MESSAGES.COMMENT: 'A new comment has been posted on <{link}|{submission.title}>', + MESSAGES.REVIEWERS_UPDATED: '{user} has updated the reviewers on <{link}|{submission.title}>', + MESSAGES.TRANSITION: '{user} has updated the status of <{link}|{submission.title}>: {old_phase.display_name} → {submission.phase}', + MESSAGES.DETERMINATION_OUTCOME: 'A determination for <{link}|{submission.title}> was sent by email: {submission.determination.get_outcome_display}', + MESSAGES.PROPOSAL_SUBMITTED: 'A proposal has been submitted for review: <{link}|{submission.title}>', + MESSAGES.INVITED_TO_PROPOSAL: '<{link}|{submission.title}> by {submission.user} has been invited to submit a proposal', + MESSAGES.NEW_REVIEW: '{user} has submitted a review for <{link}|{submission.title}>. Outcome: {review.outcome} Score: {review.score}' + } + + def __init__(self): + super().__init__() + self.destination = settings.SLACK_DESTINATION_URL + self.target_room = settings.SLACK_DESTINATION_ROOM + + def message(self, message_type, **kwargs): + submission = kwargs['submission'] + request = kwargs['request'] + link = link_to(submission, request) + + message = super().message(message_type, link=link, **kwargs) + + if submission.lead.slack: + slack_target = self.slack_id(submission.lead) + else: + slack_target = '' + + message = ' '.join([slack_target, message]).strip() + return message + + def slack_id(self, user): + return f'<{user.slack}>' + + def send_message(self, message, **kwargs): + if not self.destination or not self.target_room: + return + + data = { + "room": self.target_room, + "message": message, + } + requests.post(self.destination, json=data) + + class MessengerBackend: def __init__(self, *adpaters): self.adapters = adpaters @@ -96,7 +156,7 @@ class MessengerBackend: adapters = [ ActivityAdapter(), - MessageAdapter(), + SlackAdapter(), ] diff --git a/opentech/apply/activity/tests/test_messaging.py b/opentech/apply/activity/tests/test_messaging.py index a1441c82d798d30d713f2f3371d9fc94c4222fef..c0fa21b54e069485630e4d27fc1c86cd92351cad 100644 --- a/opentech/apply/activity/tests/test_messaging.py +++ b/opentech/apply/activity/tests/test_messaging.py @@ -1,6 +1,9 @@ +import json from unittest.mock import Mock, patch -from django.test import TestCase +import responses + +from django.test import TestCase, override_settings from django.contrib.messages import get_messages from opentech.apply.utils.testing import make_request @@ -11,14 +14,15 @@ from ..models import Activity from ..messaging import ( AdapterBase, ActivityAdapter, - MessageAdapter, MessengerBackend, MESSAGES, + SlackAdapter, ) class TestAdapter(AdapterBase): """A test class which will pass the message type to send_message""" + adapter_type = 'Test Adapter' messages = { enum: enum.value for enum in MESSAGES.__members__.values() @@ -28,6 +32,7 @@ class TestAdapter(AdapterBase): pass +@override_settings(SEND_MESSAGES=True) class TestBaseAdapter(TestCase): def setUp(self): patched_class = patch.object(TestAdapter, 'send_message') @@ -72,6 +77,17 @@ class TestBaseAdapter(TestCase): self.adapter.send_message.assert_called_once_with(message, message=message) + @override_settings(SEND_MESSAGES=False) + def test_django_messages_used(self): + request = make_request() + + self.adapter.process(MESSAGES.UPDATE_LEAD, request=request) + + messages = list(get_messages(request)) + self.assertEqual(len(messages), 1) + self.assertTrue(MESSAGES.UPDATE_LEAD.value in messages[0].message) + self.assertTrue(self.adapter.adapter_type in messages[0].message) + class TestMessageBackend(TestCase): def setUp(self): @@ -98,19 +114,7 @@ class TestMessageBackend(TestCase): self.assertEqual(adapter.process.call_count, len(adapters)) -class TestDjangoMessagesAdapter(TestCase): - def test_message_added(self): - adapter = MessageAdapter() - request = make_request() - - message = 'test message' - adapter.send_message(message, request=request) - - messages = list(get_messages(request)) - self.assertEqual(len(messages), 1) - self.assertEqual(messages[0].message, message) - - +@override_settings(SEND_MESSAGES=True) class TestActivityAdapter(TestCase): def setUp(self): self.adapter = ActivityAdapter() @@ -149,3 +153,47 @@ class TestActivityAdapter(TestCase): self.assertTrue('Removed' in message) self.assertTrue('1' in message) self.assertTrue('2' in message) + + +class TestSlackAdapter(TestCase): + target_url = 'https://my-slack-backend.com/incoming/my-very-secret-key' + target_room = '<ROOM ID>' + + @override_settings( + SLACK_DESTINATION_URL=target_url, + SLACK_DESTINATION_ROOM=None, + ) + @responses.activate + def test_cant_send_with_no_room(self): + adapter = SlackAdapter() + adapter.send_message('my message') + self.assertEqual(len(responses.calls), 0) + + @override_settings( + SLACK_DESTINATION_URL=None, + SLACK_DESTINATION_ROOM=target_room, + ) + @responses.activate + def test_cant_send_with_no_url(self): + adapter = SlackAdapter() + adapter.send_message('my message') + self.assertEqual(len(responses.calls), 0) + + @override_settings( + SLACK_DESTINATION_URL=target_url, + SLACK_DESTINATION_ROOM=target_room, + ) + @responses.activate + def test_correct_payload(self): + responses.add(responses.POST, self.target_url, status=200) + adapter = SlackAdapter() + message = 'my message' + adapter.send_message(message) + self.assertEqual(len(responses.calls), 1) + self.assertDictEqual( + json.loads(responses.calls[0].request.body), + { + 'room': self.target_room, + 'message': message, + } + ) diff --git a/opentech/apply/activity/views.py b/opentech/apply/activity/views.py index fb9f59bc1cc4059d1bcb23055096e8713bd955c3..68a8cefad26c990891810ebfd78bb426ffdfc71d 100644 --- a/opentech/apply/activity/views.py +++ b/opentech/apply/activity/views.py @@ -38,13 +38,15 @@ class CommentFormView(DelegatedViewMixin, CreateView): form.instance.user = self.request.user form.instance.submission = self.kwargs['submission'] form.instance.type = COMMENT + response = super().form_valid(form) messenger( MESSAGES.COMMENT, + request=self.request, user=self.request.user, - submission=self.submission, - comment=form.instance, + submission=self.object.submission, + comment=self.object, ) - return super().form_valid(form) + return response def get_success_url(self): return self.object.submission.get_absolute_url() + '#communications' diff --git a/opentech/apply/determinations/forms.py b/opentech/apply/determinations/forms.py index 9ad3f2652d48bb74d482974f057be2b3c6e2bb35..900485459eaa106c56c3c8a0662af876cbb5474b 100644 --- a/opentech/apply/determinations/forms.py +++ b/opentech/apply/determinations/forms.py @@ -1,7 +1,7 @@ from django import forms from django.core.exceptions import NON_FIELD_ERRORS -from opentech.apply.funds.workflow import DETERMINATION_RESPONSE_TRANSITIONS +from opentech.apply.funds.workflow import DETERMINATION_OUTCOMES from .models import Determination, DETERMINATION_CHOICES, NEEDS_MORE_INFO, REJECTED, ACCEPTED, \ DETERMINATION_TRANSITION_SUFFIX @@ -75,7 +75,7 @@ class BaseDeterminationForm(forms.ModelForm): return super().save(commit) def get_determination_from_action_name(self, action_name): - if action_name in DETERMINATION_RESPONSE_TRANSITIONS: + if action_name in DETERMINATION_OUTCOMES: if 'more_info' in action_name: return NEEDS_MORE_INFO elif 'accepted' in action_name or 'invited_to_proposal' in action_name: diff --git a/opentech/apply/determinations/views.py b/opentech/apply/determinations/views.py index 9b03cd36b9abd2fbf60c86913e4e3d1d76d6ccdd..3d41e04038eea8cc40d5ab8db488adc95a30f660 100644 --- a/opentech/apply/determinations/views.py +++ b/opentech/apply/determinations/views.py @@ -74,8 +74,6 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): return self.submission.get_absolute_url() def form_valid(self, form): - is_new = not form.instance.id - response = super().form_valid(form) if not self.object.is_draft: @@ -88,14 +86,6 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): action_name = self.get_action_name_from_determination(int(form.cleaned_data.get('outcome'))) self.submission.perform_transition(action_name, self.request.user, request=self.request) - elif is_new: - messenger( - MESSAGES.NEW_DETERMINATION, - request=self.request, - user=self.object.author, - submission=self.object.submission, - ) - return self.progress_stage(self.submission) or response def progress_stage(self, instance): diff --git a/opentech/apply/funds/models.py b/opentech/apply/funds/models.py index 8c93ec46e2204bf1ae788b33eeac0fd5922a15f7..4e474bb3563a006e24fb554c22098d63f42ea19c 100644 --- a/opentech/apply/funds/models.py +++ b/opentech/apply/funds/models.py @@ -46,13 +46,13 @@ from .blocks import ApplicationCustomFormFieldsBlock, REQUIRED_BLOCK_NAMES from .edit_handlers import FilteredFieldPanel, ReadOnlyPanel, ReadOnlyInlinePanel from .workflow import ( active_statuses, + DETERMINATION_PHASES, + DETERMINATION_RESPONSE_PHASES, get_review_statuses, INITIAL_STATE, review_statuses, UserPermissions, WORKFLOWS, - DETERMINATION_PHASES, - DETERMINATION_RESPONSE_PHASES, ) LIMIT_TO_STAFF = {'groups__name': STAFF_GROUP_NAME} diff --git a/opentech/apply/funds/views.py b/opentech/apply/funds/views.py index 0f0f962f0db72ca0b110b69ff4a810d82543152f..4a2c4ed1fae5973d84dd04999ed8ee0c373bdac1 100644 --- a/opentech/apply/funds/views.py +++ b/opentech/apply/funds/views.py @@ -19,7 +19,7 @@ from opentech.apply.activity.views import ( DelegatedViewMixin, ) from opentech.apply.activity.messaging import messenger, MESSAGES -from opentech.apply.funds.workflow import DETERMINATION_RESPONSE_TRANSITIONS +from opentech.apply.funds.workflow import DETERMINATION_OUTCOMES from opentech.apply.review.views import ReviewContextMixin from opentech.apply.users.decorators import staff_required from opentech.apply.utils.views import DelegateableView, ViewDispatcher @@ -38,7 +38,7 @@ class SubmissionListView(AllActivityContextMixin, SingleTableMixin, FilterView): filterset_class = SubmissionFilter def get_queryset(self): - return self.filterset_class._meta.model.objects.current() + return self.filterset_class._meta.model.objects.active().current() def get_context_data(self, **kwargs): active_filters = self.filterset.data @@ -77,13 +77,21 @@ class ProgressSubmissionView(DelegatedViewMixin, UpdateView): def form_valid(self, form): action = form.cleaned_data.get('action') # Defer to the determination form for any of the determination transitions - if action in DETERMINATION_RESPONSE_TRANSITIONS: + if action in DETERMINATION_OUTCOMES and self.object.can_have_determination: return HttpResponseRedirect(reverse_lazy( 'apply:submissions:determinations:form', args=(form.instance.id,)) + "?action=" + action) self.object.perform_transition(action, self.request.user, request=self.request) + if self.object.phase.name == 'proposal_discussion' and action == 'proposal_discussion': + messenger( + MESSAGES.PROPOSAL_SUBMITTED, + request=self.request, + user=self.request.user, + submission=self.object.instance, + ) + return super().form_valid(form) diff --git a/opentech/apply/funds/workflow.py b/opentech/apply/funds/workflow.py index 3d1afa63f60c6fbd89f7500fd9a2de78d46f6013..2f7dca237e69fa403973684ea27a4d5c93519226 100644 --- a/opentech/apply/funds/workflow.py +++ b/opentech/apply/funds/workflow.py @@ -427,4 +427,4 @@ def get_determination_transitions(): return transitions -DETERMINATION_RESPONSE_TRANSITIONS = get_determination_transitions() +DETERMINATION_OUTCOMES = get_determination_transitions() diff --git a/opentech/apply/review/forms.py b/opentech/apply/review/forms.py index d15bb4ab211f0baf9bca2529d53d5f9d14de406f..d16b99a989fe44c4049fb5e55c720b4dea085f48 100644 --- a/opentech/apply/review/forms.py +++ b/opentech/apply/review/forms.py @@ -76,7 +76,7 @@ class ReviewModelForm(StreamBaseForm, forms.ModelForm, metaclass=MixedMetaClass) def save(self, commit=True): self.instance.score = self.calculate_score(self.cleaned_data) - self.instance.recommendation = self.cleaned_data[get_recommendation_field(self.instance.form_fields)] + self.instance.recommendation = int(self.cleaned_data[get_recommendation_field(self.instance.form_fields)]) self.instance.is_draft = self.draft_button_name in self.data self.instance.form_data = self.cleaned_data['form_data'] diff --git a/opentech/apply/review/models.py b/opentech/apply/review/models.py index 8b46555950d041799b4d515e9e0d5fbf0ddca66d..5d093e7b87e00f8a4c5c69a8db60fd0f68388e56 100644 --- a/opentech/apply/review/models.py +++ b/opentech/apply/review/models.py @@ -87,6 +87,10 @@ class Review(BaseStreamForm, models.Model): class Meta: unique_together = ('author', 'submission') + @property + def outcome(self): + return self.get_recommendation_display() + def get_absolute_url(self): return reverse('apply:reviews:review', args=(self.id,)) diff --git a/opentech/apply/review/views.py b/opentech/apply/review/views.py index 1fbe123477922e0dc56dcc144a12bbfa73c94722..588c896b34c65cdf2b4c4a0785a48ee1c85d29d3 100644 --- a/opentech/apply/review/views.py +++ b/opentech/apply/review/views.py @@ -90,6 +90,7 @@ class ReviewCreateOrUpdateView(BaseStreamForm, CreateOrUpdateView): request=self.request, user=self.object.author, submission=self.submission, + review=self.object, ) return response diff --git a/opentech/apply/users/templates/users/login.html b/opentech/apply/users/templates/users/login.html index 179e21bb3074716facfdc157ab21fcbffaea7b31..5f542f6ea06646eac09272371c5be708b8e11788 100644 --- a/opentech/apply/users/templates/users/login.html +++ b/opentech/apply/users/templates/users/login.html @@ -5,14 +5,6 @@ {% block content %} <div class="wrapper wrapper--small"> - {% if messages %} - <ul class="messages"> - {% for message in messages %} - <li{% if message.tags %} class="{{ message.tags }}" {% endif %}>{{ message }}</li> - {% endfor %} - </ul> - {% endif %} - <form class="form form--with-p-tags" method="post"> {% csrf_token %} {{ form.as_p }} diff --git a/opentech/settings/base.py b/opentech/settings/base.py index 7dae6e7776aa76cde3fe3e50ad867e9beb8c50a8..c4da25f92ef4553be9dbb4358242a49b94d39b46 100644 --- a/opentech/settings/base.py +++ b/opentech/settings/base.py @@ -5,6 +5,8 @@ Django settings for opentech project. # Build paths inside the project like this: os.path.join(BASE_DIR, ...) import os +env = os.environ.copy() + PROJECT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) BASE_DIR = os.path.dirname(PROJECT_DIR) @@ -348,3 +350,9 @@ BLEACH_STRIP_COMMENTS = True HIJACK_LOGIN_REDIRECT_URL = '/dashboard/' HIJACK_LOGOUT_REDIRECT_URL = '/account/' HIJACK_DECORATOR = 'opentech.apply.users.decorators.superuser_decorator' + + +# Messaging Settings +SEND_MESSAGES = env.get('SEND_MESSAGES', 'false').lower() == 'true' +SLACK_DESTINATION_URL = env.get('SLACK_DESTINATION_URL', None) +SLACK_DESTINATION_ROOM = env.get('SLACK_DESTINATION_ROOM', None) diff --git a/opentech/templates/base-apply.html b/opentech/templates/base-apply.html index 5133d16d2aa53a727c37e70a60a7d3366a8d08b6..1fb279f94c5f709acf4794ed3da81cafa4a306b8 100644 --- a/opentech/templates/base-apply.html +++ b/opentech/templates/base-apply.html @@ -45,19 +45,7 @@ {% wagtailuserbar %} - {% if messages %} - <ul class="messages"> - {% for message in messages %} - <li class="messages__text{% if message.tags %} messages__text--{{ message.tags }} js-message"{% endif %}> - {{ message }} - <a href="#" class="messages__close js-close-message"> - <svg class="icon icon--close"><use xlink:href="#cross"></use></svg> - </a> - </li> - {% endfor %} - </ul> - {% endif %} - + {% include 'includes/messages.html' %} <header class="header"> <div class="header__inner wrapper wrapper--large"> diff --git a/opentech/templates/base.html b/opentech/templates/base.html index 076ab638694ac2423551dff88488b55e9299c469..33d99863760cd73b4c3f675dfabccb98e15e8a15 100644 --- a/opentech/templates/base.html +++ b/opentech/templates/base.html @@ -70,6 +70,8 @@ {% wagtailuserbar %} + {% include 'includes/messages.html' %} + {% block header %} {% image page.header_image fill-2560x410 as header_image %} <header class="header header--standard {% if page.header_image %}header--has-bg-image{% endif %} {% block header_modifier %}{% endblock %}" {% if page.header_image %}style="background-image:url('{{ header_image.url }}')"{% endif %}> diff --git a/opentech/templates/includes/messages.html b/opentech/templates/includes/messages.html new file mode 100644 index 0000000000000000000000000000000000000000..94e7d5fb6033ea923bbcc03c4ed091ed00f98699 --- /dev/null +++ b/opentech/templates/includes/messages.html @@ -0,0 +1,12 @@ +{% if messages %} + <ul class="messages"> + {% for message in messages %} + <li class="messages__text{% if message.tags %} messages__text--{{ message.tags }} js-message"{% endif %}> + {{ message }} + <a href="#" class="messages__close js-close-message"> + <svg class="icon icon--close"><use xlink:href="#cross"></use></svg> + </a> + </li> + {% endfor %} + </ul> +{% endif %} diff --git a/requirements.txt b/requirements.txt index c70c8301fb265f72ceedb8649edfec45da1ac284..beb571c642a3e18eba3c76d4ec83563eaed9a745 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,6 +16,7 @@ django-hijack==2.1.9 factory_boy==2.9.2 # wagtail_factories - waiting on merge and release form master branch git+git://github.com/mvantellingen/wagtail-factories.git#egg=wagtail_factories +responses == 0.9.0 flake8