diff --git a/opentech/apply/activity/__init__.py b/opentech/apply/activity/__init__.py index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..1b5686b41eb4edfad6cdba716fe22c97f5f66734 100644 --- a/opentech/apply/activity/__init__.py +++ b/opentech/apply/activity/__init__.py @@ -0,0 +1 @@ +default_app_config = 'opentech.apply.activity.apps.ActivityConfig' diff --git a/opentech/apply/activity/admin.py b/opentech/apply/activity/admin.py index 9b9141baf43121b1d1ba3d04fde51aa917078c1d..eacc4bfbde9664469434699c67ef262ac8762e47 100644 --- a/opentech/apply/activity/admin.py +++ b/opentech/apply/activity/admin.py @@ -4,7 +4,7 @@ from .models import Event, Message class MessageInline(admin.TabularInline): model = Message - readonly_fields = ('type', 'recipient', 'content', 'status') + readonly_fields = ('type', 'recipient', 'content', 'status', 'external_id') can_delete = False def has_add_permission(self, request): diff --git a/opentech/apply/activity/apps.py b/opentech/apply/activity/apps.py index 34cfcc035c5ba7b644b8d40ef0f61bd953171b31..c224ba349ca1d29e09bb686bc0a4950f7902cb53 100644 --- a/opentech/apply/activity/apps.py +++ b/opentech/apply/activity/apps.py @@ -2,4 +2,7 @@ from django.apps import AppConfig class ActivityConfig(AppConfig): - name = 'activity' + name = 'opentech.apply.activity' + + def ready(self): + from . import signals # NOQA diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py index 815828de2c196dc3b79174c3eb5080aa711437bb..53b4fd3bfdd4dfb1375706efb952d049233dccd4 100644 --- a/opentech/apply/activity/messaging.py +++ b/opentech/apply/activity/messaging.py @@ -4,9 +4,10 @@ import requests from django.conf import settings from django.contrib import messages -from django.core.mail import send_mail from django.template.loader import render_to_string +from .tasks import send_mail + def link_to(target, request): return request.scheme + '://' + request.get_host() + target.get_absolute_url() @@ -69,13 +70,13 @@ class AdapterBase: return for recipient in self.recipients(message_type, **kwargs): + message_log = self.create_log(message, recipient, event) if settings.SEND_MESSAGES or self.always_send: - status = self.send_message(message, recipient=recipient, **kwargs) + status = self.send_message(message, recipient=recipient, message_log=message_log, **kwargs) else: status = 'Message not sent as SEND_MESSAGES==FALSE' - if status: - self.log_message(message, recipient, event, status) + message_log.update_status(status) if not settings.SEND_MESSAGES: if recipient: @@ -84,14 +85,13 @@ class AdapterBase: message = '{}: {}'.format(self.adapter_type, message) messages.add_message(kwargs['request'], messages.INFO, message) - def log_message(self, message, recipient, event, status): - from.models import Message - Message.objects.create( + def create_log(self, message, recipient, event): + from .models import Message + return Message.objects.create( type=self.adapter_type, content=message, - recipient=recipient, + recipient=recipient or '', event=event, - status=status, ) def send_message(self, message, **kwargs): @@ -253,18 +253,16 @@ class EmailAdapter(AdapterBase): def send_message(self, message, submission, subject, recipient, **kwargs): try: - emails_sent = send_mail( + send_mail( subject, message, submission.page.specific.from_address, [recipient], - fail_silently=False, + log=kwargs['message_log'] ) except Exception as e: return 'Error: ' + str(e) - return 'Emails sent: ' + str(emails_sent) - class MessengerBackend: def __init__(self, *adpaters): diff --git a/opentech/apply/activity/migrations/0008_message_external_id.py b/opentech/apply/activity/migrations/0008_message_external_id.py new file mode 100644 index 0000000000000000000000000000000000000000..6e2d8d540611072566a8f401cb26c8c6d1be7a44 --- /dev/null +++ b/opentech/apply/activity/migrations/0008_message_external_id.py @@ -0,0 +1,18 @@ +# Generated by Django 2.0.2 on 2018-08-01 09:01 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('activity', '0007_message_status'), + ] + + operations = [ + migrations.AddField( + model_name='message', + name='external_id', + field=models.CharField(blank=True, max_length=75, null=True), + ), + ] diff --git a/opentech/apply/activity/models.py b/opentech/apply/activity/models.py index 7970cf9eb5fe928833f5bf5a38eb4da0466c5227..e143e8f1cbb65be6c814518d679c5986e44aa8bf 100644 --- a/opentech/apply/activity/models.py +++ b/opentech/apply/activity/models.py @@ -1,5 +1,7 @@ from django.conf import settings from django.db import models +from django.db.models import Case, When, Value +from django.db.models.functions import Concat from .messaging import MESSAGES @@ -121,3 +123,12 @@ class Message(models.Model): recipient = models.CharField(max_length=250) event = models.ForeignKey(Event, on_delete=models.CASCADE) status = models.TextField() + external_id = models.CharField(max_length=75, null=True, blank=True) # Stores the id of the object from an external system + + def update_status(self, status): + if status: + self.status = Case( + When(status='', then=Value(status)), + default=Concat('status', Value('<br />' + status)) + ) + self.save() diff --git a/opentech/apply/activity/signals.py b/opentech/apply/activity/signals.py new file mode 100644 index 0000000000000000000000000000000000000000..11794ddc176125d54042351611b39b5bcd290ce9 --- /dev/null +++ b/opentech/apply/activity/signals.py @@ -0,0 +1,12 @@ +from anymail.signals import tracking +from django.dispatch import receiver + +from .models import Message + + +@receiver(tracking) +def handle_event(sender, event, esp_name, **kwargs): + status = 'Webhook received: {} [{}]'.format(event.event_type, event.timestamp) + if event.description: + status += ' ' + event.description + Message.objects.get(external_id=event.message_id).update_status(status) diff --git a/opentech/apply/activity/tasks.py b/opentech/apply/activity/tasks.py new file mode 100644 index 0000000000000000000000000000000000000000..73ff71b8343d368b91776ef80fc0b59cdb6a1dd0 --- /dev/null +++ b/opentech/apply/activity/tasks.py @@ -0,0 +1,49 @@ +from celery import Celery + +from django.conf import settings +from django.core.mail import EmailMessage + +app = Celery('tasks') + +app.config_from_object(settings, namespace='CELERY', force=True) + + +def send_mail(subject, message, from_address, recipients, log=None): + # Convenience method to wrap the tasks and handle the callback + send_mail_task.apply_async( + kwargs={ + 'subject': subject, + 'body': message, + 'from_email': from_address, + 'to': recipients, + }, + link=update_message_status.s(log.id), + ) + + +@app.task +def send_mail_task(**kwargs): + response = {'status': '', 'id': None} + email = EmailMessage(**kwargs) + try: + email.send() + except Exception as e: + response['status'] = 'Error: ' + str(e) + else: + try: + return { + 'status': email.anymail_status.status.pop(), + 'id': email.anymail_status.message_id, + } + except AttributeError: + response['status'] = 'sent' + + return response + + +@app.task +def update_message_status(response, message_id): + from .models import Message + message = Message.objects.get(id=message_id) + message.external_id = response['id'] + message.update_status(response['status']) diff --git a/opentech/apply/activity/templates/messages/email/determination.html b/opentech/apply/activity/templates/messages/email/determination.html index b51f4dcb3c5b7fcb1a452b8c7d3ee5eb145bf617..d224887ece86e02120b3f067de61e5dd1c2a6d76 100644 --- a/opentech/apply/activity/templates/messages/email/determination.html +++ b/opentech/apply/activity/templates/messages/email/determination.html @@ -2,7 +2,7 @@ {% load bleach_tags %} {% block content %} -{{ submission.determination.clean_message }} +{{ submission.determination.message|bleach }} {% endblock %} diff --git a/opentech/apply/activity/tests/factories.py b/opentech/apply/activity/tests/factories.py index 89c491bff1ab994b2c2386cd4cfdb5f7f61b9e1a..30c09ab31a5971a7300269b9af699093afeca13d 100644 --- a/opentech/apply/activity/tests/factories.py +++ b/opentech/apply/activity/tests/factories.py @@ -1,6 +1,8 @@ +import uuid + import factory -from opentech.apply.activity.models import Activity, Event, INTERNAL, MESSAGES, REVIEWER +from opentech.apply.activity.models import Activity, Event, INTERNAL, Message, MESSAGES, REVIEWER from opentech.apply.funds.tests.factories import ApplicationSubmissionFactory from opentech.apply.users.tests.factories import UserFactory @@ -29,3 +31,14 @@ class EventFactory(factory.DjangoModelFactory): type = factory.Iterator([choice[0] for choice in MESSAGES.choices()]) by = factory.SubFactory(UserFactory) submission = factory.SubFactory(ApplicationSubmissionFactory) + + +class MessageFactory(factory.DjangoModelFactory): + class Meta: + model = Message + + type = 'Email' + content = factory.Faker('sentence') + recipient = factory.Faker('email') + event = factory.SubFactory(EventFactory) + external_id = factory.LazyFunction(lambda: '<{}>'.format(uuid.uuid4())) diff --git a/opentech/apply/activity/tests/test_messaging.py b/opentech/apply/activity/tests/test_messaging.py index a94e3d19bf770d1c96f05111d49781c9b9f06663..77b3ca5924aa1f91523f6d970eb6301711355019 100644 --- a/opentech/apply/activity/tests/test_messaging.py +++ b/opentech/apply/activity/tests/test_messaging.py @@ -1,3 +1,5 @@ +import hashlib +import hmac import json from unittest.mock import Mock, patch @@ -20,7 +22,7 @@ from ..messaging import ( MESSAGES, SlackAdapter, ) -from .factories import CommentFactory, EventFactory +from .factories import CommentFactory, EventFactory, MessageFactory class TestAdapter(AdapterBase): @@ -65,6 +67,7 @@ class TestBaseAdapter(AdapterMixin, TestCase): patched_class = patch.object(TestAdapter, 'send_message') self.mock_adapter = patched_class.start() self.adapter = TestAdapter() + self.adapter.send_message.return_value = 'dummy_message' self.addCleanup(patched_class.stop) def test_can_send_a_message(self): @@ -337,7 +340,7 @@ class TestEmailAdapter(AdapterMixin, TestCase): self.adapter_process(MESSAGES.NEW_SUBMISSION) self.assertEqual(Message.objects.count(), 1) sent_message = Message.objects.first() - self.assertEqual(sent_message.status, 'Emails sent: 1') + self.assertEqual(sent_message.status, 'sent') def test_email_failed(self): with patch('django.core.mail.backends.locmem.EmailBackend.send_messages', side_effect=Exception('An error occurred')): @@ -346,3 +349,73 @@ class TestEmailAdapter(AdapterMixin, TestCase): self.assertEqual(Message.objects.count(), 1) sent_message = Message.objects.first() self.assertEqual(sent_message.status, 'Error: An error occurred') + + +@override_settings( + SEND_MESSAGES=True, + EMAIL_BACKEND='anymail.backends.test.EmailBackend', +) +class TestAnyMailBehaviour(AdapterMixin, TestCase): + adapter = EmailAdapter() + TEST_API_KEY = 'TEST_API_KEY' + + # from: https://github.com/anymail/django-anymail/blob/7d8dbdace92d8addfcf0a517be0aaf481da11952/tests/test_mailgun_webhooks.py#L19 + def mailgun_sign(self, data, api_key=TEST_API_KEY): + """Add a Mailgun webhook signature to data dict""" + # Modifies the dict in place + data.setdefault('timestamp', '1234567890') + data.setdefault('token', '1234567890abcdef1234567890abcdef') + data['signature'] = hmac.new( + key=api_key.encode('ascii'), + msg='{timestamp}{token}'.format(**data).encode('ascii'), + digestmod=hashlib.sha256, + ).hexdigest() + + return data + + def test_email_new_submission(self): + submission = ApplicationSubmissionFactory() + self.adapter_process(MESSAGES.NEW_SUBMISSION, submission=submission) + + self.assertEqual(len(mail.outbox), 1) + self.assertEqual(mail.outbox[0].to, [submission.user.email]) + message = Message.objects.first() + self.assertEqual(message.status, 'sent') + # Anymail test Backend uses the index of the email as id: '0' + self.assertEqual(message.external_id, '0') + + @override_settings(ANYMAIL_MAILGUN_API_KEY=TEST_API_KEY) + def test_webhook_updates_status(self): + message = MessageFactory() + response = self.client.post( + '/activity/anymail/mailgun/tracking/', + data=self.mailgun_sign({ + 'event': 'delivered', + 'Message-Id': message.external_id + }), + secure=True, + json=True, + ) + self.assertEqual(response.status_code, 200) + message.refresh_from_db() + self.assertTrue('delivered' in message.status) + + @override_settings(ANYMAIL_MAILGUN_API_KEY=TEST_API_KEY) + def test_webhook_adds_reject_reason(self): + message = MessageFactory() + response = self.client.post( + '/activity/anymail/mailgun/tracking/', + data=self.mailgun_sign({ + 'event': 'dropped', + 'reason': 'hardfail', + 'code': 607, + 'description': 'Marked as spam', + 'Message-Id': message.external_id + }), + secure=True, + json=True, + ) + self.assertEqual(response.status_code, 200) + message.refresh_from_db() + self.assertTrue('rejected' in message.status) + self.assertTrue('spam' in message.status) diff --git a/opentech/apply/activity/tests/test_tasks.py b/opentech/apply/activity/tests/test_tasks.py new file mode 100644 index 0000000000000000000000000000000000000000..b6aede89d7e74c4ab4886d7b956c5196e26e62b6 --- /dev/null +++ b/opentech/apply/activity/tests/test_tasks.py @@ -0,0 +1,20 @@ +from unittest.mock import patch + +from django.test import TestCase + +from ..tasks import send_mail + +from .factories import MessageFactory + + +class TestSendEmail(TestCase): + @patch('opentech.apply.activity.tasks.EmailMessage', autospec=True) + def test_args_passed_to_django(self, email_mock): + kwargs = { + 'subject': 'subject', + 'body': 'body', + 'from_email': 'from_email', + 'to': 'to', + } + send_mail(*kwargs, log=MessageFactory()) + email_mock.assert_called_once_with(**kwargs) diff --git a/opentech/apply/activity/urls.py b/opentech/apply/activity/urls.py new file mode 100644 index 0000000000000000000000000000000000000000..cdad5c3af33c762c1c1758d709d10609ac7b4078 --- /dev/null +++ b/opentech/apply/activity/urls.py @@ -0,0 +1,9 @@ +from django.urls import include, path + + +app_name = 'activity' + + +urlpatterns = [ + path('anymail/', include('anymail.urls')), +] diff --git a/opentech/apply/determinations/models.py b/opentech/apply/determinations/models.py index 57d234e006b9838a871e0afd23430433b987d92f..2ad79b26a53892fa7c03bb478eb2ec990c481233 100644 --- a/opentech/apply/determinations/models.py +++ b/opentech/apply/determinations/models.py @@ -53,7 +53,7 @@ class Determination(models.Model): updated_at = models.DateTimeField(verbose_name=_('Update time'), auto_now=True) @property - def clean_message(self): + def stripped_message(self): return bleach.clean(self.message, tags=[], strip=True) @property diff --git a/opentech/apply/determinations/views.py b/opentech/apply/determinations/views.py index d39cfac32069faa2d15653d4ee0e839efac86b6e..a8154435f3767e20d2e93d12a719dcde3c09287a 100644 --- a/opentech/apply/determinations/views.py +++ b/opentech/apply/determinations/views.py @@ -84,7 +84,7 @@ class DeterminationCreateOrUpdateView(CreateOrUpdateView): if self.object.outcome == NEEDS_MORE_INFO: # We keep a record of the message sent to the user in the comment Activity.comments.create( - message=self.object.clean_message, + message=self.object.stripped_message, user=self.request.user, submission=self.submission, ) diff --git a/opentech/apply/urls.py b/opentech/apply/urls.py index af5565ba19061a894a56d245bf6634df642edf83..e730c8025c66f734737c7d20369e1cdfbd58fff1 100644 --- a/opentech/apply/urls.py +++ b/opentech/apply/urls.py @@ -6,6 +6,7 @@ from .dashboard import urls as dashboard_urls urlpatterns = [ path('apply/', include('opentech.apply.funds.urls', 'apply')), + path('activity/', include('opentech.apply.activity.urls', 'activity')), path('account/', include(users_urls)), path('dashboard/', include(dashboard_urls)), path('hijack/', include('hijack.urls', 'hijack')), diff --git a/opentech/apply/utils/options.py b/opentech/apply/utils/options.py index b2ef62f9a2960566c36e255a3d486c09f0101c44..2b8ccc214b6ed9f99bd55460848d02d205576874 100644 --- a/opentech/apply/utils/options.py +++ b/opentech/apply/utils/options.py @@ -3,6 +3,7 @@ from tinymce.widgets import TinyMCE MCE_ATTRIBUTES = { 'elementpath': False, 'branding': False, + 'entity_encoding': 'raw', 'toolbar1': 'undo redo | styleselect | bold italic | bullist numlist | link', 'style_formats': [ {'title': 'Headers', 'items': [ diff --git a/opentech/settings/base.py b/opentech/settings/base.py index c4da25f92ef4553be9dbb4358242a49b94d39b46..1d04aab1a00d01c84136fa7c7aa50235e552608e 100644 --- a/opentech/settings/base.py +++ b/opentech/settings/base.py @@ -55,6 +55,7 @@ INSTALLED_APPS = [ 'wagtail.admin', 'wagtail.core', + 'anymail', 'modelcluster', 'taggit', 'django_extensions', @@ -356,3 +357,18 @@ HIJACK_DECORATOR = 'opentech.apply.users.decorators.superuser_decorator' 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) + + +# Email and Celery config +EMAIL_BACKEND = 'anymail.backends.mailgun.EmailBackend' + +if 'MAILGUN_API_KEY' in env: + MAILGUN_API_KEY = env.get('MAILGUN_API_KEY') + +if 'ANYMAIL_WEBHOOK_SECRET' in env: + ANYMAIL_WEBHOOK_SECRET = env.get('ANYMAIL_WEBHOOK_SECRET') + +if 'REDIS_URL' in env: + CELERY_BROKER_URL = env.get('REDIS_URL') +else: + CELERY_TASK_ALWAYS_EAGER = True diff --git a/requirements.txt b/requirements.txt index beb571c642a3e18eba3c76d4ec83563eaed9a745..0ea9b0b33fa436dc7538579fffaa11c5336c4588 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,8 @@ stellar==0.4.3 django-tinymce4-lite==1.7.0 uwsgidecorators==1.1.0 django-hijack==2.1.9 +django-anymail==3.0 +celery==4.2.1 factory_boy==2.9.2 # wagtail_factories - waiting on merge and release form master branch