diff --git a/opentech/apply/activity/admin.py b/opentech/apply/activity/admin.py
index e979c14a171a19eadae6a2a239806d5762c67c3c..9b9141baf43121b1d1ba3d04fde51aa917078c1d 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')
+ readonly_fields = ('type', 'recipient', 'content', 'status')
can_delete = False
def has_add_permission(self, request):
diff --git a/opentech/apply/activity/messaging.py b/opentech/apply/activity/messaging.py
index b92445ff6693037828271e9463a9b5bcaf4a4d99..f570de8c06f949fa4a27d6c4a7a3d73114175372 100644
--- a/opentech/apply/activity/messaging.py
+++ b/opentech/apply/activity/messaging.py
@@ -70,14 +70,12 @@ class AdapterBase:
for recipient in self.recipients(message_type, **kwargs):
if settings.SEND_MESSAGES or self.always_send:
- self.send_message(message, recipient=recipient, **kwargs)
- from.models import Message
- Message.objects.create(
- type=self.adapter_type,
- content=message,
- recipient=recipient,
- event=event,
- )
+ status = self.send_message(message, recipient=recipient, **kwargs)
+ else:
+ status = 'Message not sent as SEND_MESSAGES==FALSE'
+
+ if status:
+ self.log_message(message, recipient, event, status)
if not settings.SEND_MESSAGES:
if recipient:
@@ -86,7 +84,19 @@ 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(
+ type=self.adapter_type,
+ content=message,
+ recipient=recipient,
+ event=event,
+ status=status,
+ )
+
def send_message(self, message, **kwargs):
+ # Process the message, should return the result of the send
+ # Returning None will not record this action
raise NotImplementedError()
@@ -182,7 +192,12 @@ class SlackAdapter(AdapterBase):
def send_message(self, message, recipient, **kwargs):
if not self.destination or not self.target_room:
- return
+ errors = list()
+ if not self.destination:
+ errors.append('Destination URL')
+ if not self.target_room:
+ errors.append('Room ID')
+ return 'Missing configuration: {}'.format(', '.join(errors))
message = ' '.join([recipient, message]).strip()
@@ -190,7 +205,7 @@ class SlackAdapter(AdapterBase):
"room": self.target_room,
"message": message,
}
- requests.post(self.destination, json=data)
+ return requests.post(self.destination, json=data)
class EmailAdapter(AdapterBase):
@@ -215,7 +230,8 @@ class EmailAdapter(AdapterBase):
def notify_comment(self, **kwargs):
comment = kwargs['comment']
- if not comment.private:
+ submission = kwargs['submission']
+ if not comment.private and not comment.user == submission.user:
return self.render_message('messages/email/comment.html', **kwargs)
def recipients(self, message_type, submission, **kwargs):
@@ -234,7 +250,7 @@ class EmailAdapter(AdapterBase):
return render_to_string(template, kwargs)
def send_message(self, message, submission, subject, recipient, **kwargs):
- send_mail(
+ return send_mail(
subject,
message,
submission.page.specific.from_address,
diff --git a/opentech/apply/activity/migrations/0007_message_status.py b/opentech/apply/activity/migrations/0007_message_status.py
new file mode 100644
index 0000000000000000000000000000000000000000..64789fcf701a87958f36728fd6e26b2a7462bb49
--- /dev/null
+++ b/opentech/apply/activity/migrations/0007_message_status.py
@@ -0,0 +1,19 @@
+# Generated by Django 2.0.2 on 2018-07-30 14:09
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('activity', '0006_message'),
+ ]
+
+ operations = [
+ migrations.AddField(
+ model_name='message',
+ name='status',
+ field=models.TextField(default=''),
+ preserve_default=False,
+ ),
+ ]
diff --git a/opentech/apply/activity/models.py b/opentech/apply/activity/models.py
index 41b970b558188f18ce2c0ca0d7c4bc5487bc1ec1..7970cf9eb5fe928833f5bf5a38eb4da0466c5227 100644
--- a/opentech/apply/activity/models.py
+++ b/opentech/apply/activity/models.py
@@ -120,3 +120,4 @@ class Message(models.Model):
content = models.TextField()
recipient = models.CharField(max_length=250)
event = models.ForeignKey(Event, on_delete=models.CASCADE)
+ status = models.TextField()
diff --git a/opentech/apply/activity/templates/messages/email/applicant_base.html b/opentech/apply/activity/templates/messages/email/applicant_base.html
index 4405cab7b2cb888abf87d6418b4a630f830063d6..a629768ef022ed0c9e8d3ac9750ac05068778e1e 100644
--- a/opentech/apply/activity/templates/messages/email/applicant_base.html
+++ b/opentech/apply/activity/templates/messages/email/applicant_base.html
@@ -1,6 +1,5 @@
-{% block salutation %}
-Dear {{ submission.user.get_full_name|default:"applicant" }},
-{% endblock %}
+{% extends "messages/email/base.html" %}
+{% block salutation %}Dear {{ submission.user.get_full_name|default:"applicant" }},{% endblock %}
{% block more_info %}
You can access your application here: {{ request.scheme }}://{{ request.get_host }}{{ submission.get_absolute_url }}
diff --git a/opentech/apply/activity/templates/messages/email/comment.html b/opentech/apply/activity/templates/messages/email/comment.html
index 845e7fe7598a0f7a421da871e13bcf2d6f849304..c092483e37e1ed63db7ece92aa2fd89938793761 100644
--- a/opentech/apply/activity/templates/messages/email/comment.html
+++ b/opentech/apply/activity/templates/messages/email/comment.html
@@ -1,7 +1,5 @@
{% extends "messages/email/applicant_base.html" %}
-{% block content %}
-There has been a new comment on your application: {{ submission.title }}
+{% block content %}There has been a new comment on your application: {{ submission.title }}
-{{ comment.message }}
-{% endblock %}
+{{ comment.user }}: {{ comment.message }}{% endblock %}
diff --git a/opentech/apply/activity/templates/messages/email/ready_to_review.html b/opentech/apply/activity/templates/messages/email/ready_to_review.html
index bef3718e392b9469c8841a78749538bb794f52d6..b7ec4973950cb081dfefb2b01bb89fd0830d00e1 100644
--- a/opentech/apply/activity/templates/messages/email/ready_to_review.html
+++ b/opentech/apply/activity/templates/messages/email/ready_to_review.html
@@ -1,5 +1,5 @@
{% extends "messages/email/base.html" %}
-{% block salutation %}Dear Reviewer{% endblock %}
+{% block salutation %}Dear Reviewer,{% endblock %}
{% block content %}
A new proposal has been added to your review list.
diff --git a/opentech/apply/activity/tests/factories.py b/opentech/apply/activity/tests/factories.py
new file mode 100644
index 0000000000000000000000000000000000000000..89c491bff1ab994b2c2386cd4cfdb5f7f61b9e1a
--- /dev/null
+++ b/opentech/apply/activity/tests/factories.py
@@ -0,0 +1,31 @@
+import factory
+
+from opentech.apply.activity.models import Activity, Event, INTERNAL, MESSAGES, REVIEWER
+from opentech.apply.funds.tests.factories import ApplicationSubmissionFactory
+from opentech.apply.users.tests.factories import UserFactory
+
+
+class CommentFactory(factory.DjangoModelFactory):
+ class Meta:
+ model = Activity
+
+ class Params:
+ internal = factory.Trait(visibility=INTERNAL)
+ reviewers = factory.Trait(visibility=REVIEWER)
+
+ submission = factory.SubFactory(ApplicationSubmissionFactory)
+ user = factory.SubFactory(UserFactory)
+ message = factory.Faker('sentence')
+
+ @classmethod
+ def _get_manager(cls, model_class):
+ return model_class.comments
+
+
+class EventFactory(factory.DjangoModelFactory):
+ class Meta:
+ model = Event
+
+ type = factory.Iterator([choice[0] for choice in MESSAGES.choices()])
+ by = factory.SubFactory(UserFactory)
+ submission = factory.SubFactory(ApplicationSubmissionFactory)
diff --git a/opentech/apply/activity/tests/test_messaging.py b/opentech/apply/activity/tests/test_messaging.py
index f05467316f50a02b8f48ada2356b6fe983c46c61..1b22b3317c8e1b559d3ad56ec74bb2f41188bfbc 100644
--- a/opentech/apply/activity/tests/test_messaging.py
+++ b/opentech/apply/activity/tests/test_messaging.py
@@ -20,6 +20,7 @@ from ..messaging import (
MESSAGES,
SlackAdapter,
)
+from .factories import CommentFactory, EventFactory
class TestAdapter(AdapterBase):
@@ -36,9 +37,30 @@ class TestAdapter(AdapterBase):
def recipients(self, message_type, **kwargs):
return [message_type]
+ def log_message(self, message, recipient, event, status):
+ pass
+
+
+class AdapterMixin:
+ adapter = None
+
+ def process_kwargs(self, **kwargs):
+ if 'user' not in kwargs:
+ kwargs['user'] = UserFactory()
+ if 'submission' not in kwargs:
+ kwargs['submission'] = ApplicationSubmissionFactory()
+ if 'request' not in kwargs:
+ kwargs['request'] = None
+
+ return kwargs
+
+ def adapter_process(self, message_type, **kwargs):
+ kwargs = self.process_kwargs(**kwargs)
+ self.adapter.process(message_type, event=EventFactory(submission=kwargs['submission']), **kwargs)
+
@override_settings(SEND_MESSAGES=True)
-class TestBaseAdapter(TestCase):
+class TestBaseAdapter(AdapterMixin, TestCase):
def setUp(self):
patched_class = patch.object(TestAdapter, 'send_message')
self.mock_adapter = patched_class.start()
@@ -47,55 +69,61 @@ class TestBaseAdapter(TestCase):
def test_can_send_a_message(self):
message_type = MESSAGES.UPDATE_LEAD
- self.adapter.process(message_type)
+ self.adapter_process(message_type)
- self.adapter.send_message.assert_called_once_with(message_type.value, recipient=message_type)
+ self.adapter.send_message.assert_called_once()
+ self.assertEqual(self.adapter.send_message.call_args[0], (message_type.value,))
def test_doesnt_send_a_message_if_not_configured(self):
- self.adapter.process('this_is_not_a_message_type')
+ self.adapter_process('this_is_not_a_message_type')
self.adapter.send_message.assert_not_called()
def test_calls_method_if_avaliable(self):
method_name = 'new_method'
return_message = 'Returned message'
- setattr(self.adapter, method_name, lambda: return_message)
+ setattr(self.adapter, method_name, lambda **kw: return_message)
self.adapter.messages[method_name] = method_name
- self.adapter.process(method_name)
+ self.adapter_process(method_name)
- self.adapter.send_message.assert_called_once_with(return_message, recipient=method_name)
+ self.adapter.send_message.assert_called_once()
+ self.assertEqual(self.adapter.send_message.call_args[0], (return_message,))
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_process(message_type, **kwargs)
- self.adapter.send_message.assert_called_once_with(message_type.value, recipient=message_type, **kwargs)
+ self.adapter.send_message.assert_called_once()
+ for key in kwargs:
+ self.assertTrue(key in self.adapter.send_message.call_args[1])
def test_that_message_is_formatted(self):
message_type = MESSAGES.UPDATE_LEAD
message = 'message value'
with patch.dict(self.adapter.messages, {message_type: '{message_to_format}'}):
- self.adapter.process(message_type, message_to_format=message)
+ self.adapter_process(message_type, message_to_format=message)
- self.adapter.send_message.assert_called_once_with(message, message_to_format=message, recipient=message_type)
+ self.adapter.send_message.assert_called_once()
+ self.assertEqual(self.adapter.send_message.call_args[0], (message,))
def test_can_include_extra_kwargs(self):
message_type = MESSAGES.UPDATE_LEAD
with patch.dict(self.adapter.messages, {message_type: '{extra}'}):
with patch.object(self.adapter, 'extra_kwargs', return_value={'extra': 'extra'}):
- self.adapter.process(message_type)
+ self.adapter_process(message_type)
- self.adapter.send_message.assert_called_once_with('extra', extra='extra', recipient=message_type)
+ self.adapter.send_message.assert_called_once()
+ self.assertTrue('extra' in self.adapter.send_message.call_args[1])
@override_settings(SEND_MESSAGES=False)
def test_django_messages_used(self):
request = make_request()
- self.adapter.process(MESSAGES.UPDATE_LEAD, request=request)
+ self.adapter_process(MESSAGES.UPDATE_LEAD, request=request)
messages = list(get_messages(request))
self.assertEqual(len(messages), 1)
@@ -107,7 +135,11 @@ class TestMessageBackend(TestCase):
def setUp(self):
self.mocked_adapter = Mock(AdapterBase)
self.backend = MessengerBackend
- self.kwargs = {'request': None, 'user': None, 'submission': None}
+ self.kwargs = {
+ 'request': None,
+ 'user': UserFactory(),
+ 'submission': ApplicationSubmissionFactory(),
+ }
def test_message_sent_to_adapter(self):
adapter = self.mocked_adapter()
@@ -115,7 +147,7 @@ class TestMessageBackend(TestCase):
messenger(MESSAGES.UPDATE_LEAD, **self.kwargs)
- adapter.process.assert_called_once_with(MESSAGES.UPDATE_LEAD, **self.kwargs)
+ adapter.process.assert_called_once_with(MESSAGES.UPDATE_LEAD, Event.objects.first(), **self.kwargs)
def test_message_sent_to_all_adapter(self):
adapters = [self.mocked_adapter(), self.mocked_adapter()]
@@ -135,8 +167,8 @@ class TestMessageBackend(TestCase):
messenger(MESSAGES.UPDATE_LEAD, **self.kwargs)
self.assertEqual(Event.objects.count(), 1)
- self.assertEqual(Event.objects.first().type, MESSAGES.UPDATE_LEAD)
- self.assertEqual(Event.objects.first().get_type_display, MESSAGES.UPDATE_LEAD.value)
+ self.assertEqual(Event.objects.first().type, MESSAGES.UPDATE_LEAD.name)
+ self.assertEqual(Event.objects.first().get_type_display(), MESSAGES.UPDATE_LEAD.value)
self.assertEqual(Event.objects.first().by, user)
@@ -240,20 +272,33 @@ class TestSlackAdapter(TestCase):
@override_settings(SEND_MESSAGES=True)
-class TestEmailAdapter(TestCase):
+class TestEmailAdapter(AdapterMixin, TestCase):
+ adapter = EmailAdapter()
+
def test_email_new_submission(self):
- adapter = EmailAdapter()
submission = ApplicationSubmissionFactory()
- adapter.process(MESSAGES.NEW_SUBMISSION, submission=submission)
+ self.adapter_process(MESSAGES.NEW_SUBMISSION, submission=submission)
self.assertEqual(len(mail.outbox), 1)
self.assertEqual(mail.outbox[0].to, [submission.user.email])
+ def test_no_email_private_comment(self):
+ comment = CommentFactory(internal=True)
+
+ self.adapter_process(MESSAGES.COMMENT, comment=comment, submission=comment.submission)
+ self.assertEqual(len(mail.outbox), 0)
+
+ def test_no_email_own_comment(self):
+ application = ApplicationSubmissionFactory()
+ comment = CommentFactory(user=application.user, submission=application)
+
+ self.adapter_process(MESSAGES.COMMENT, comment=comment, user=comment.user, submission=comment.submission)
+ self.assertEqual(len(mail.outbox), 0)
+
def test_reviewers_email(self):
- adapter = EmailAdapter()
reviewers = ReviewerFactory.create_batch(4)
submission = ApplicationSubmissionFactory(status='external_review', reviewers=reviewers, workflow_stages=2)
- adapter.process(MESSAGES.READY_FOR_REVIEW, submission=submission)
+ self.adapter_process(MESSAGES.READY_FOR_REVIEW, submission=submission)
self.assertEqual(len(mail.outbox), 4)
self.assertTrue(mail.outbox[0].subject, 'ready to review')
diff --git a/opentech/apply/stream_forms/testing/factories.py b/opentech/apply/stream_forms/testing/factories.py
index 50d5f7d774c8c767f71d738e923213cb11f1beed..1d03a5670b7a3c7978245cb81a10ae1a8698949b 100644
--- a/opentech/apply/stream_forms/testing/factories.py
+++ b/opentech/apply/stream_forms/testing/factories.py
@@ -1,3 +1,4 @@
+from collections import defaultdict
import json
import uuid
@@ -136,8 +137,10 @@ class MultiFileFieldBlockFactory(UploadableMediaFactory):
class StreamFieldUUIDFactory(wagtail_factories.StreamFieldFactory):
- def generate(self, *args, **kwargs):
- blocks = super().generate(*args, **kwargs)
+ def generate(self, step, params):
+ if not params:
+ params = self.build_form(params)
+ blocks = super().generate(step, params)
ret_val = list()
# Convert to JSON so we can add id before create
for block_name, value in blocks:
@@ -145,3 +148,14 @@ class StreamFieldUUIDFactory(wagtail_factories.StreamFieldFactory):
value = block.get_prep_value(value)
ret_val.append({'type': block_name, 'value': value, 'id': str(uuid.uuid4())})
return json.dumps(ret_val)
+
+ def build_form(self, data):
+ extras = defaultdict(dict)
+
+ form_fields = {}
+ for i, field in enumerate(self.factories):
+ form_fields[f'{i}__{field}__'] = ''
+ for attr, value in extras[field].items():
+ form_fields[f'{i}__{field}__{attr}'] = value
+
+ return form_fields