Merge pull request #142 from alphagov/allow-csvs-with-extra-personalisation

Accept CSV files with additional columns

app/celery/tasks.py

@@ -22,19 +22,24 @@ def process_job(job_id):
     job.status = 'in progress'
     dao_update_job(job)
 
+    template = Template(
+        dao_get_template_by_id(job.template_id).__dict__
+    )
+
     for recipient, personalisation in RecipientCSV(
         s3.get_job_from_s3(job.bucket_name, job_id),
-        template_type=job.template.template_type
+        template_type=template.template_type,
+        placeholders=template.placeholders
     ).recipients_and_personalisation:
 
         encrypted = encryption.encrypt({
-            'template': job.template_id,
+            'template': template.id,
             'job': str(job.id),
             'to': recipient,
             'personalisation': personalisation
         })
 
-        if job.template.template_type == 'sms':
+        if template.template_type == 'sms':
             send_sms.apply_async((
                 str(job.service_id),
                 str(create_uuid()),
@@ -43,11 +48,11 @@ def process_job(job_id):
                 queue='bulk-sms'
             )
 
-        if job.template.template_type == 'email':
+        if template.template_type == 'email':
             send_email.apply_async((
                 str(job.service_id),
                 str(create_uuid()),
-                job.template.subject,
+                template.subject,
                 "{}@{}".format(job.service.email_from, current_app.config['NOTIFY_EMAIL_DOMAIN']),
                 encrypted,
                 datetime.utcnow().strftime(DATETIME_FORMAT)),
@@ -101,8 +106,7 @@ 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.send_sms(
@@ -169,8 +173,7 @@ def send_email(service_id, notification_id, subject, from_address, encrypted_not
             try:
                 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.send_email(

requirements.txt

@@ -21,4 +21,4 @@ monotonic==0.3
 
 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@2.0.0#egg=notifications-utils==2.0.0
+git+https://github.com/alphagov/notifications-utils.git@2.0.1#egg=notifications-utils==2.0.1

test_csv_files/multiple_sms.csv

@@ -1,11 +1,11 @@
-phone number
+phone number,name
-+441234123121
++441234123121,chris
-+441234123122
++441234123122,chris
-+441234123123
++441234123123,chris
-+441234123124
++441234123124,chris
-+441234123125
++441234123125,chris
-+441234123126
++441234123126,chris
-+441234123127
++441234123127,chris
-+441234123128
++441234123128,chris
-+441234123129
++441234123129,chris
-+441234123120
++441234123120,chris

test_csv_files/sms.csv

@@ -1,2 +1,2 @@
-phone number
+phone number, ignore this column
-+441234123123
++441234123123, nope

tests/app/celery/test_tasks.py

@@ -22,12 +22,13 @@ from freezegun import freeze_time
 from tests.app.conftest import (
     sample_service,
     sample_user,
-    sample_template
+    sample_template,
+    sample_template_with_placeholders
 )
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
-def test_should_process_sms_job(sample_job, mocker):
+def test_should_process_sms_job(sample_job, sample_template, mocker):
     mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('sms'))
     mocker.patch('app.celery.tasks.send_sms.apply_async')
     mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
@@ -36,6 +37,8 @@ def test_should_process_sms_job(sample_job, mocker):
     process_job(sample_job.id)
 
     s3.get_job_from_s3.assert_called_once_with(sample_job.bucket_name, sample_job.id)
+    assert encryption.encrypt.call_args[0][0]['to'] == '+441234123123'
+    assert encryption.encrypt.call_args[0][0]['personalisation'] == {}
     tasks.send_sms.apply_async.assert_called_once_with(
         (str(sample_job.service_id),
          "uuid",
@@ -47,7 +50,7 @@ def test_should_process_sms_job(sample_job, mocker):
     assert job.status == 'finished'
 
 
-def test_should_not_create_send_task_for_empty_file(sample_job, mocker):
+def test_should_not_create_send_task_for_empty_file(sample_job, sample_template, mocker):
     mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('empty'))
     mocker.patch('app.celery.tasks.send_sms.apply_async')
 
@@ -60,7 +63,7 @@ def test_should_not_create_send_task_for_empty_file(sample_job, mocker):
 
 
 @freeze_time("2016-01-01 11:09:00.061258")
-def test_should_process_email_job(sample_email_job, mocker):
+def test_should_process_email_job(sample_email_job, sample_template, mocker):
     mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('email'))
     mocker.patch('app.celery.tasks.send_email.apply_async')
     mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
@@ -69,6 +72,8 @@ def test_should_process_email_job(sample_email_job, mocker):
     process_job(sample_email_job.id)
 
     s3.get_job_from_s3.assert_called_once_with(sample_email_job.bucket_name, sample_email_job.id)
+    assert encryption.encrypt.call_args[0][0]['to'] == 'test@test.com'
+    assert encryption.encrypt.call_args[0][0]['personalisation'] == {}
     tasks.send_email.apply_async.assert_called_once_with(
         (str(sample_email_job.service_id),
          "uuid",
@@ -82,17 +87,22 @@ def test_should_process_email_job(sample_email_job, mocker):
     assert job.status == 'finished'
 
 
-def test_should_process_all_sms_job(sample_job, mocker):
+def test_should_process_all_sms_job(sample_job, sample_job_with_placeholdered_template, mocker):
     mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=load_example_csv('multiple_sms'))
     mocker.patch('app.celery.tasks.send_sms.apply_async')
     mocker.patch('app.encryption.encrypt', return_value="something_encrypted")
     mocker.patch('app.celery.tasks.create_uuid', return_value="uuid")
 
-    process_job(sample_job.id)
+    process_job(sample_job_with_placeholdered_template.id)
 
-    s3.get_job_from_s3.assert_called_once_with(sample_job.bucket_name, sample_job.id)
+    s3.get_job_from_s3.assert_called_once_with(
+        sample_job_with_placeholdered_template.bucket_name,
+        sample_job_with_placeholdered_template.id
+    )
+    assert encryption.encrypt.call_args[0][0]['to'] == '+441234123120'
+    assert encryption.encrypt.call_args[0][0]['personalisation'] == {'name': 'chris'}
     tasks.send_sms.apply_async.call_count == 10
-    job = jobs_dao.dao_get_job_by_id(sample_job.id)
+    job = jobs_dao.dao_get_job_by_id(sample_job_with_placeholdered_template.id)
     assert job.status == 'finished'
 
 

tests/app/conftest.py

@@ -220,6 +220,20 @@ def sample_job(notify_db,
     return job
 
 
+@pytest.fixture(scope='function')
+def sample_job_with_placeholdered_template(
+    notify_db,
+    notify_db_session,
+    service=None
+):
+    return sample_job(
+        notify_db,
+        notify_db_session,
+        service=service,
+        template=sample_template_with_placeholders(notify_db, notify_db_session)
+    )
+
+
 @pytest.fixture(scope='function')
 def sample_email_job(notify_db,
                      notify_db_session,