Replace placeholders before sending a message

This commit replaces placeholders in a template with the user’s data, using
the Template class from utils
(https://github.com/alphagov/notifications-utils/tree/master/utils#template)

It also extracts the personalisation data from the CSV, taking account of the
different column headings that SMS and email CSVs will have.

At the point of creating the task to send an individual messages, validation of
the placeholders matching the template is assumed to have been done either:
- in processing the CSV in the admin app
- in the endpoint for the API call

No exceptions should be raised at this point.

app/celery/tasks.py

@@ -9,8 +9,9 @@ from app.models import Notification
 from flask import current_app
 from sqlalchemy.exc import SQLAlchemyError
 from app.aws import s3
-from app.csv import get_recipient_from_csv
 from datetime import datetime
+from utils.template import Template
+from utils.process_csv import get_rows_from_csv, get_recipient_from_row, first_column_heading
 
 
 @notify_celery.task(name="process-job")
@@ -21,13 +22,14 @@ def process_job(job_id):
     dao_update_job(job)
 
     file = s3.get_job_from_s3(job.bucket_name, job_id)
-    recipients = get_recipient_from_csv(file)
 
-    for recipient in recipients:
+    for row in get_rows_from_csv(file):
+
         encrypted = encryption.encrypt({
             'template': job.template_id,
             'job': str(job.id),
-            'to': recipient
+            'to': get_recipient_from_row(row, job.template.template_type),
+            'personalisation': row
         })
 
         if job.template.template_type == 'sms':
@@ -62,7 +64,10 @@ def process_job(job_id):
 @notify_celery.task(name="send-sms")
 def send_sms(service_id, notification_id, encrypted_notification, created_at):
     notification = encryption.decrypt(encrypted_notification)
-    template = dao_get_template_by_id(notification['template'])
+    template = Template(
+        dao_get_template_by_id(notification['template']),
+        values=notification.get('personalisation', {})
+    )
 
     client = firetext_client
 
@@ -78,12 +83,14 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
             created_at=created_at,
             sent_at=sent_at,
             sent_by=client.get_name()
-
         )
         dao_create_notification(notification_db_object)
 
         try:
-            client.send_sms(notification['to'], template.content)
+            client.send_sms(
+                notification['to'],
+                template.replaced
+            )
         except FiretextClientException as e:
             current_app.logger.debug(e)
             notification_db_object.status = 'failed'
@@ -99,7 +106,10 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
 @notify_celery.task(name="send-email")
 def send_email(service_id, notification_id, subject, from_address, encrypted_notification, created_at):
     notification = encryption.decrypt(encrypted_notification)
-    template = dao_get_template_by_id(notification['template'])
+    template = Template(
+        dao_get_template_by_id(notification['template']),
+        values=notification.get('personalisation', {})
+    )
 
     client = aws_ses_client
 
@@ -123,7 +133,7 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not
                 from_address,
                 notification['to'],
                 subject,
-                template.content
+                template.replaced
             )
         except AwsSesClientException as e:
             current_app.logger.debug(e)

app/notifications/rest.py

@@ -147,7 +147,7 @@ def send_notification(notification_type):
             }
         ), 404
 
-    template_object = Template({'content': template.content}, notification.get('personalisation', {}))
+    template_object = Template(template.__dict__, notification.get('personalisation', {}))
     if template_object.missing_data:
         return jsonify(
             result="error",

requirements.txt

@@ -19,4 +19,4 @@ twilio==4.6.0
 
 git+https://github.com/alphagov/notifications-python-client.git@0.2.6#egg=notifications-python-client==0.2.6
 
-git+https://github.com/alphagov/notifications-utils.git@0.2.1#egg=notifications-utils==0.2.1
+git+https://github.com/alphagov/notifications-utils.git@1.0.0#egg=notifications-utils==1.0.0

test_csv_files/email.csv

@@ -1,2 +1,2 @@
-to
+email address
 test@test.com

test_csv_files/empty.csv

@@ -1 +1 @@
-to
+phone number

test_csv_files/multiple_email.csv

@@ -1,4 +1,4 @@
-to
+email address
 test1@test.com
 test2@test.com
 test3@test.com
@@ -9,4 +9,3 @@ test7@test.com
 test8@test.com
 test9@test.com
 test0@test.com
-

test_csv_files/multiple_sms.csv

@@ -1,4 +1,4 @@
-to
+phone number
 +441234123121
 +441234123122
 +441234123123

test_csv_files/sms.csv

@@ -1,2 +1,2 @@
-to
+phone number
 +441234123123

tests/app/celery/test_tasks.py

@@ -85,10 +85,11 @@ def test_should_process_all_sms_job(sample_job, mocker):
     assert job.status == 'finished'
 
 
-def test_should_send_template_to_correct_sms_provider_and_persist(sample_template, mocker):
+def test_should_send_template_to_correct_sms_provider_and_persist(sample_template_with_placeholders, mocker):
     notification = {
-        "template": sample_template.id,
+        "template": sample_template_with_placeholders.id,
-        "to": "+441234123123"
+        "to": "+441234123123",
+        "personalisation": {"name": "Jo"}
     }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.firetext_client.send_sms')
@@ -97,17 +98,19 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
     notification_id = uuid.uuid4()
     now = datetime.utcnow()
     send_sms(
-        sample_template.service_id,
+        sample_template_with_placeholders.service_id,
         notification_id,
         "encrypted-in-reality",
         now
     )
 
-    firetext_client.send_sms.assert_called_once_with("+441234123123", sample_template.content)
+    firetext_client.send_sms.assert_called_once_with("+441234123123", "Hello Jo")
-    persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id)
+    persisted_notification = notifications_dao.get_notification(
+        sample_template_with_placeholders.service_id, notification_id
+    )
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == '+441234123123'
-    assert persisted_notification.template_id == sample_template.id
+    assert persisted_notification.template_id == sample_template_with_placeholders.id
     assert persisted_notification.status == 'sent'
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_at > now
@@ -145,10 +148,11 @@ def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sa
     assert persisted_notification.sent_by == 'firetext'
 
 
-def test_should_use_email_template_and_persist(sample_email_template, mocker):
+def test_should_use_email_template_and_persist(sample_email_template_with_placeholders, mocker):
     notification = {
-        "template": sample_email_template.id,
+        "template": sample_email_template_with_placeholders.id,
-        "to": "my_email@my_email.com"
+        "to": "my_email@my_email.com",
+        "personalisation": {"name": "Jo"}
     }
     mocker.patch('app.encryption.decrypt', return_value=notification)
     mocker.patch('app.aws_ses_client.send_email')
@@ -157,7 +161,7 @@ def test_should_use_email_template_and_persist(sample_email_template, mocker):
     notification_id = uuid.uuid4()
     now = datetime.utcnow()
     send_email(
-        sample_email_template.service_id,
+        sample_email_template_with_placeholders.service_id,
         notification_id,
         'subject',
         'email_from',
@@ -168,12 +172,12 @@ def test_should_use_email_template_and_persist(sample_email_template, mocker):
         "email_from",
         "my_email@my_email.com",
         "subject",
-        sample_email_template.content
+        "Hello Jo"
     )
-    persisted_notification = notifications_dao.get_notification(sample_email_template.service_id, notification_id)
+    persisted_notification = notifications_dao.get_notification(sample_email_template_with_placeholders.service_id, notification_id)
     assert persisted_notification.id == notification_id
     assert persisted_notification.to == 'my_email@my_email.com'
-    assert persisted_notification.template_id == sample_email_template.id
+    assert persisted_notification.template_id == sample_email_template_with_placeholders.id
     assert persisted_notification.created_at == now
     assert persisted_notification.sent_at > now
     assert persisted_notification.status == 'sent'

tests/app/conftest.py

@@ -174,6 +174,11 @@ def sample_email_template(
     return template
 
 
+@pytest.fixture(scope='function')
+def sample_email_template_with_placeholders(notify_db, notify_db_session):
+    return sample_email_template(notify_db, notify_db_session, content="Hello ((name))")
+
+
 @pytest.fixture(scope='function')
 def sample_api_key(notify_db,
                    notify_db_session,