diff --git a/app/celery/tasks.py b/app/celery/tasks.py index c9f55126b..1af2b1967 100644 --- a/app/celery/tasks.py +++ b/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) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 25dff99b5..648e7fa43 100644 --- a/app/notifications/rest.py +++ b/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", diff --git a/requirements.txt b/requirements.txt index 1d835f1df..4c00d9f14 100644 --- a/requirements.txt +++ b/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 diff --git a/test_csv_files/email.csv b/test_csv_files/email.csv index 581e64627..3d720f5e2 100644 --- a/test_csv_files/email.csv +++ b/test_csv_files/email.csv @@ -1,2 +1,2 @@ -to -test@test.com \ No newline at end of file +email address +test@test.com diff --git a/test_csv_files/empty.csv b/test_csv_files/empty.csv index bf3efa02f..64e5bd0a3 100644 --- a/test_csv_files/empty.csv +++ b/test_csv_files/empty.csv @@ -1 +1 @@ -to +phone number diff --git a/test_csv_files/multiple_email.csv b/test_csv_files/multiple_email.csv index 2e7bab603..e57e35a2a 100644 --- a/test_csv_files/multiple_email.csv +++ b/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 - diff --git a/test_csv_files/multiple_sms.csv b/test_csv_files/multiple_sms.csv index 224a1a4d7..fa51924ac 100644 --- a/test_csv_files/multiple_sms.csv +++ b/test_csv_files/multiple_sms.csv @@ -1,4 +1,4 @@ -to +phone number +441234123121 +441234123122 +441234123123 diff --git a/test_csv_files/sms.csv b/test_csv_files/sms.csv index 3776c8832..b430aaf6b 100644 --- a/test_csv_files/sms.csv +++ b/test_csv_files/sms.csv @@ -1,2 +1,2 @@ -to -+441234123123 \ No newline at end of file +phone number ++441234123123 diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 4b094344f..565d8a8ae 100644 --- a/tests/app/celery/test_tasks.py +++ b/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, - "to": "+441234123123" + "template": sample_template_with_placeholders.id, + "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) - persisted_notification = notifications_dao.get_notification(sample_template.service_id, notification_id) + firetext_client.send_sms.assert_called_once_with("+441234123123", "Hello Jo") + 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, - "to": "my_email@my_email.com" + "template": sample_email_template_with_placeholders.id, + "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' diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d640d11cf..4611e195e 100644 --- a/tests/app/conftest.py +++ b/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,