Merge branch 'master' into email-registered-users

.travis.yml

@@ -1,12 +1,16 @@
 sudo: false
 language: python
-cache: pip
+cache:
+  pip: true
+  directories:
+    - ~/.pip-accel
 python:
 - '3.4'
 addons:
   postgresql: '9.3'
 install:
-- pip install -r requirements_for_test.txt
+- pip install pip-accel
+- pip-accel install -r requirements_for_test.txt
 before_script:
 - psql -c 'create database test_notification_api;' -U postgres
 script:

app/celery/provider_tasks.py

@@ -20,9 +20,8 @@ from notifications_utils.recipients import (
 )
 
 from app.dao.templates_dao import dao_get_template_by_id
-from notifications_utils.template import (
+from notifications_utils.template import Template
-    Template
+from notifications_utils.renderers import HTMLEmail, PlainTextEmail, SMSMessage
-)
 
 from app.models import SMS_TYPE, EMAIL_TYPE, KEY_TYPE_TEST
 
@@ -62,7 +61,7 @@ def send_sms_to_provider(self, service_id, notification_id):
         template = Template(
             template_model.__dict__,
             values={} if not notification.personalisation else notification.personalisation,
-            prefix=service.name
+            renderer=SMSMessage(prefix=service.name)
         )
         try:
             if service.research_mode or notification.key_type == KEY_TYPE_TEST:
@@ -129,9 +128,18 @@ def send_email_to_provider(self, service_id, notification_id):
     notification = get_notification_by_id(notification_id)
     if notification.status == 'created':
         try:
-            template = Template(
+            template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__
-                dao_get_template_by_id(notification.template_id, notification.template_version).__dict__,
+
-                values=notification.personalisation
+            html_email = Template(
+                template_dict,
+                values=notification.personalisation,
+                renderer=HTMLEmail()
+            )
+
+            plain_text_email = Template(
+                template_dict,
+                values=notification.personalisation,
+                renderer=PlainTextEmail()
             )
 
             if service.research_mode or notification.key_type == KEY_TYPE_TEST:
@@ -145,9 +153,9 @@ def send_email_to_provider(self, service_id, notification_id):
                 reference = provider.send_email(
                     from_address,
                     notification.to,
-                    template.replaced_subject,
+                    plain_text_email.replaced_subject,
-                    body=template.replaced_govuk_escaped,
+                    body=plain_text_email.replaced,
-                    html_body=template.as_HTML_email,
+                    html_body=html_email.replaced,
                     reply_to_address=service.reply_to_email_address,
                 )
 

app/notifications/rest.py

@@ -9,6 +9,7 @@ from flask import (
 )
 from notifications_utils.recipients import allowed_to_send_to, first_column_heading
 from notifications_utils.template import Template
+from notifications_utils.renderers import PassThrough
 from app.clients.email.aws_ses import get_aws_responses
 from app import api_user, encryption, create_uuid, DATETIME_FORMAT, DATE_FORMAT, statsd_client
 from app.models import KEY_TYPE_TEAM
@@ -240,7 +241,11 @@ def send_notification(notification_type):
     if errors:
         raise InvalidRequest(errors, status_code=400)
 
-    template_object = Template(template.__dict__, notification.get('personalisation', {}))
+    template_object = Template(
+        template.__dict__,
+        notification.get('personalisation', {}),
+        renderer=PassThrough()
+    )
     if template_object.missing_data:
         message = 'Missing personalisation: {}'.format(", ".join(template_object.missing_data))
         errors = {'template': [message]}

app/schemas.py

@@ -24,6 +24,8 @@ from notifications_utils.recipients import (
     validate_and_format_phone_number
 )
 
+from notifications_utils.renderers import PassThrough
+
 from app import ma
 from app import models
 from app.dao.permissions_dao import permission_dao
@@ -272,7 +274,11 @@ class NotificationStatusSchema(BaseSchema):
     @post_dump
     def handle_template_merge(self, in_data):
         from notifications_utils.template import Template
-        template = Template(in_data['template'], in_data['personalisation'])
+        template = Template(
+            in_data['template'],
+            in_data['personalisation'],
+            renderer=PassThrough()
+        )
         in_data['body'] = template.replaced
         if in_data['template']['template_type'] == 'email':
             in_data['subject'] = template.replaced_subject

requirements.txt

@@ -23,4 +23,4 @@ statsd==3.2.1
 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0
 
 
-git+https://github.com/alphagov/notifications-utils.git@6.3.2#egg=notifications-utils==6.3.2
+git+https://github.com/alphagov/notifications-utils.git@8.2.0#egg=notifications-utils==8.2.0

tests/app/celery/test_provider_tasks.py

@@ -80,10 +80,11 @@ def test_should_return_highest_priority_active_provider(notify_db, notify_db_ses
 
 
 def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
-        notify_db,
+    notify_db,
-        notify_db_session,
+    notify_db_session,
-        sample_template_with_placeholders,
+    sample_template_with_placeholders,
-        mocker):
+    mocker
+):
     db_notification = sample_notification(notify_db, notify_db_session, template=sample_template_with_placeholders,
                                           to_field="+447234123123", personalisation={"name": "Jo"},
                                           status='created')
@@ -101,7 +102,7 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
 
     mmg_client.send_sms.assert_called_once_with(
         to=format_phone_number(validate_phone_number("+447234123123")),
-        content="Sample service: Hello Jo",
+        content="Sample service: Hello Jo\nYour thing is due soon",
         reference=str(db_notification.id),
         sender=None
     )
@@ -110,7 +111,49 @@ def test_should_send_personalised_template_to_correct_sms_provider_and_persist(
     assert notification.status == 'sending'
     assert notification.sent_at <= datetime.utcnow()
     assert notification.sent_by == 'mmg'
-    assert notification.content_char_count == 24
+    assert notification.content_char_count == len("Sample service: Hello Jo\nYour thing is due soon")
+    assert notification.personalisation == {"name": "Jo"}
+
+
+def test_should_send_personalised_template_to_correct_email_provider_and_persist(
+    notify_db,
+    notify_db_session,
+    sample_email_template_with_placeholders,
+    mocker
+):
+    db_notification = sample_notification(
+        notify_db=notify_db, notify_db_session=notify_db_session,
+        template=sample_email_template_with_placeholders,
+        to_field="jo.smith@example.com",
+        personalisation={'name': 'Jo'}
+    )
+
+    mocker.patch('app.aws_ses_client.send_email', return_value='reference')
+    mocker.patch('app.aws_ses_client.get_name', return_value="ses")
+    mocker.patch('app.statsd_client.incr')
+    mocker.patch('app.statsd_client.timing_with_dates')
+    mocker.patch('app.statsd_client.timing')
+
+    send_email_to_provider(
+        db_notification.service_id,
+        db_notification.id
+    )
+
+    app.aws_ses_client.send_email.assert_called_once_with(
+        '"Sample service" <sample.service@test.notify.com>',
+        'jo.smith@example.com',
+        'Jo',
+        body='Hello Jo\nThis is an email from GOV.\u200bUK',
+        html_body=ANY,
+        reply_to_address=None
+    )
+    assert '<!DOCTYPE html' in app.aws_ses_client.send_email.call_args[1]['html_body']
+
+    notification = Notification.query.filter_by(id=db_notification.id).one()
+
+    assert notification.status == 'sending'
+    assert notification.sent_at <= datetime.utcnow()
+    assert notification.sent_by == 'ses'
     assert notification.personalisation == {"name": "Jo"}
 
 
@@ -141,7 +184,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
 
     mmg_client.send_sms.assert_called_once_with(
         to=format_phone_number(validate_phone_number("+447234123123")),
-        content="Sample service: This is a template",
+        content="Sample service: This is a template:\nwith a newline",
         reference=str(db_notification.id),
         sender=None
     )
@@ -151,7 +194,7 @@ def test_send_sms_should_use_template_version_from_notification_not_latest(
     assert persisted_notification.template_id == sample_template.id
     assert persisted_notification.template_version == version_on_notification
     assert persisted_notification.template_version != sample_template.version
-    assert persisted_notification.content_char_count == len("Sample service: This is a template")
+    assert persisted_notification.content_char_count == len("Sample service: This is a template:\nwith a newline")
     assert persisted_notification.status == 'sending'
     assert not persisted_notification.personalisation
 
@@ -332,7 +375,7 @@ def test_should_send_sms_sender_from_service_if_present(
 
     mmg_client.send_sms.assert_called_once_with(
         to=format_phone_number(validate_phone_number("+447234123123")),
-        content="Sample service: This is a template",
+        content="Sample service: This is a template:\nwith a newline",
         reference=str(db_notification.id),
         sender=sample_service.sms_sender
     )

tests/app/conftest.py

@@ -154,7 +154,7 @@ def sample_template(notify_db,
                     notify_db_session,
                     template_name="Template Name",
                     template_type="sms",
-                    content="This is a template",
+                    content="This is a template:\nwith a newline",
                     archived=False,
                     subject_line='Subject',
                     user=None,
@@ -185,7 +185,7 @@ def sample_template(notify_db,
 
 @pytest.fixture(scope='function')
 def sample_template_with_placeholders(notify_db, notify_db_session):
-    return sample_template(notify_db, notify_db_session, content="Hello ((name))")
+    return sample_template(notify_db, notify_db_session, content="Hello ((name))\nYour thing is due soon")
 
 
 @pytest.fixture(scope='function')
@@ -223,7 +223,7 @@ def sample_email_template_with_placeholders(notify_db, notify_db_session):
     return sample_email_template(
         notify_db,
         notify_db_session,
-        content="Hello ((name))",
+        content="Hello ((name))\nThis is an email from GOV.UK",
         subject_line="((name))")
 
 

tests/app/notifications/rest/test_send_notification.py

@@ -125,7 +125,7 @@ def test_send_notification_with_placeholders_replaced(notify_api, sample_email_t
             )
             assert response.status_code == 201
             assert encryption.decrypt(app.celery.tasks.send_email.apply_async.call_args[0][0][2]) == data
-            assert response_data['body'] == 'Hello Jo'
+            assert response_data['body'] == 'Hello Jo\nThis is an email from GOV.UK'
             assert response_data['subject'] == 'Jo'
 
 

tests/app/notifications/test_rest.py

@@ -34,7 +34,7 @@ def test_get_sms_notification_by_id(notify_api, sample_notification):
             }
             assert notification['to'] == '+447700900855'
             assert notification['service'] == str(sample_notification.service_id)
-            assert notification['body'] == "This is a template"  # sample_template.content
+            assert notification['body'] == "This is a template:\nwith a newline"
             assert not notification.get('subject')
 
 
@@ -172,7 +172,7 @@ def test_get_all_notifications(notify_api, sample_notification):
 
             assert notifications['notifications'][0]['to'] == '+447700900855'
             assert notifications['notifications'][0]['service'] == str(sample_notification.service_id)
-            assert notifications['notifications'][0]['body'] == "This is a template"  # sample_template.content
+            assert notifications['notifications'][0]['body'] == "This is a template:\nwith a newline"
 
 
 def test_normal_api_key_returns_notifications_created_from_jobs_and_from_api(
@@ -553,7 +553,7 @@ def test_get_notification_by_id_returns_merged_template_content(notify_db,
 
             notification = json.loads(response.get_data(as_text=True))['data']['notification']
             assert response.status_code == 200
-            assert notification['body'] == 'Hello world'
+            assert notification['body'] == 'Hello world\nYour thing is due soon'
             assert 'subject' not in notification
 
 
@@ -576,7 +576,7 @@ def test_get_notification_by_id_returns_merged_template_content_for_email(
 
         notification = json.loads(response.get_data(as_text=True))['data']['notification']
         assert response.status_code == 200
-        assert notification['body'] == 'Hello world'
+        assert notification['body'] == 'Hello world\nThis is an email from GOV.UK'
         assert notification['subject'] == 'world'
 
 
@@ -610,8 +610,8 @@ def test_get_notifications_for_service_returns_merged_template_content(notify_ap
 
             resp = json.loads(response.get_data(as_text=True))
             assert len(resp['notifications']) == 2
-            assert resp['notifications'][0]['body'] == 'Hello merged with first'
+            assert resp['notifications'][0]['body'] == 'Hello merged with first\nYour thing is due soon'
-            assert resp['notifications'][1]['body'] == 'Hello merged with second'
+            assert resp['notifications'][1]['body'] == 'Hello merged with second\nYour thing is due soon'
 
 
 def _create_auth_header_from_key(api_key):