From b202af716ddec68e8d63c0ed9354beabf37c79bc Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Wed, 2 Mar 2016 12:27:07 +0000
Subject: [PATCH] Fix bug where sending messages failed

When building the template it was looking for a placeholder called
((phone number)). This caused it to fail because the template it had did not
match the personalisation it was being given.

`Template` has an optional parameter for specifying personalisation values that
should be ignored. The recipient of a message is an example of such a value.

This commit passes that extra parameter, which fixes that bug.
---
 app/celery/tasks.py            |  6 +++--
 tests/app/celery/test_tasks.py | 49 ++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/app/celery/tasks.py b/app/celery/tasks.py
index b12768c58..873159e7c 100644
--- a/app/celery/tasks.py
+++ b/app/celery/tasks.py
@@ -69,7 +69,8 @@ def send_sms(service_id, notification_id, encrypted_notification, created_at):
     template = Template(
         dao_get_template_by_id(notification['template']).__dict__,
         values=notification.get('personalisation', {}),
-        prefix=service.name
+        prefix=service.name,
+        drop_values={first_column_heading['sms']}
     )
 
     client = firetext_client
@@ -111,7 +112,8 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not
     notification = encryption.decrypt(encrypted_notification)
     template = Template(
         dao_get_template_by_id(notification['template']).__dict__,
-        values=notification.get('personalisation', {})
+        values=notification.get('personalisation', {}),
+        drop_values={first_column_heading['email']}
     )
 
     client = aws_ses_client
diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py
index 661c09f78..9f5fea5ed 100644
--- a/tests/app/celery/test_tasks.py
+++ b/tests/app/celery/test_tasks.py
@@ -118,6 +118,27 @@ def test_should_send_template_to_correct_sms_provider_and_persist(sample_templat
     assert not persisted_notification.job_id
 
 
+def test_should_send_sms_without_personalisation(sample_template, mocker):
+    notification = {
+        "template": sample_template.id,
+        "to": "+441234123123"
+    }
+    mocker.patch('app.encryption.decrypt', return_value=notification)
+    mocker.patch('app.firetext_client.send_sms')
+    mocker.patch('app.firetext_client.get_name', return_value="firetext")
+
+    notification_id = uuid.uuid4()
+    now = datetime.utcnow()
+    send_sms(
+        sample_template.service_id,
+        notification_id,
+        "encrypted-in-reality",
+        now
+    )
+
+    firetext_client.send_sms.assert_called_once_with("+441234123123", "Sample service: This is a template")
+
+
 def test_should_send_template_to_correct_sms_provider_and_persist_with_job_id(sample_job, mocker):
     notification = {
         "template": sample_job.template.id,
@@ -186,6 +207,34 @@ def test_should_use_email_template_and_persist(sample_email_template_with_placeh
     assert persisted_notification.sent_by == 'ses'
 
 
+def test_should_use_email_template_and_persist_without_personalisation(
+    sample_email_template, mocker
+):
+    mocker.patch('app.encryption.decrypt', return_value={
+        "template": sample_email_template.id,
+        "to": "my_email@my_email.com",
+    })
+    mocker.patch('app.aws_ses_client.send_email')
+    mocker.patch('app.aws_ses_client.get_name', return_value='ses')
+
+    notification_id = uuid.uuid4()
+    now = datetime.utcnow()
+    send_email(
+        sample_email_template.service_id,
+        notification_id,
+        'subject',
+        'email_from',
+        "encrypted-in-reality",
+        now)
+
+    aws_ses_client.send_email.assert_called_once_with(
+        "email_from",
+        "my_email@my_email.com",
+        "subject",
+        "This is a template"
+    )
+
+
 def test_should_persist_notification_as_failed_if_sms_client_fails(sample_template, mocker):
     notification = {
         "template": sample_template.id,