From 4f238d241afc7c73cf9d0545096def4bbf3860f0 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 19 Jan 2017 12:10:32 +0000 Subject: [PATCH] persist_letter saves address correctly to database the `to` field stores either the phone number or the email address of the recipient - it's a bit more complicated for letters, since there are address lines 1 through 6, and a postcode. In utils, they're stored alongside the personalisation, and we have to ensure that when we persist to the database we keep as much parity with utils to make our work easier. Aside from sending, the `to` field is also used to show recipients on the front end report pages - we've decided that the best thing to store here is address_line_1 - which is probably going to be either a person's name, company name, or PO box number Also, a lot of tests and test cleanup - I added create_template and create_notification functions in db.py, so if you're creating new fixtures you can use these functions, and you won't need to pass notify_db and notify_db_session around, huzzah! also removed create param from sample_notification since it's not used anywhere --- app/celery/tasks.py | 14 +++-- tests/app/celery/test_tasks.py | 96 ++++++++++++++++++++++++++++++---- tests/app/conftest.py | 51 +++++++++++++----- tests/app/db.py | 68 +++++++++++++++++++++++- 4 files changed, 200 insertions(+), 29 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 1adc449ee..96b07ed34 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -220,20 +220,24 @@ def persist_letter( created_at ): notification = encryption.decrypt(encrypted_notification) + + # we store the recipient as just the first item of the person's address + recipient = notification['personalisation']['addressline1'] + service = dao_fetch_service_by_id(service_id) try: saved_notification = persist_notification( template_id=notification['template'], template_version=notification['template_version'], - recipient=notification['to'], + recipient=recipient, service=service, - personalisation=notification.get('personalisation'), - notification_type=EMAIL_TYPE, + personalisation=notification['personalisation'], + notification_type=LETTER_TYPE, api_key_id=None, key_type=KEY_TYPE_NORMAL, created_at=created_at, - job_id=notification.get('job', None), - job_row_number=notification.get('row_number', None), + job_id=notification['job'], + job_row_number=notification['row_number'], notification_id=notification_id ) diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 66ad2f256..cc1f8df61 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1,6 +1,6 @@ import uuid from datetime import datetime -from unittest.mock import Mock, ANY +from unittest.mock import Mock, ANY, call import pytest from freezegun import freeze_time @@ -17,6 +17,7 @@ from app.celery.tasks import ( process_row, send_sms, send_email, + persist_letter, get_template_class ) from app.dao import jobs_dao, services_dao @@ -41,20 +42,16 @@ class AnyStringWith(str): mmg_error = {'Error': '40', 'Description': 'error'} -def _notification_json(template, to, personalisation=None, job_id=None, row_number=None): - notification = { +def _notification_json(template, to, personalisation=None, job_id=None, row_number=0): + return { "template": str(template.id), "template_version": template.version, "to": to, - "notification_type": template.template_type + "notification_type": template.template_type, + "personalisation": personalisation or {}, + "job": job_id and str(job_id), + "row_number": row_number } - if personalisation: - notification.update({"personalisation": personalisation}) - if job_id: - notification.update({"job": str(job_id)}) - if row_number: - notification['row_number'] = row_number - return notification def test_should_have_decorated_tasks_functions(): @@ -263,6 +260,42 @@ def test_should_process_email_job(email_job_with_placeholders, mocker): assert job.job_status == 'finished' +@freeze_time("2016-01-01 11:09:00.061258") +def test_should_process_letter_job(sample_letter_job, mocker): + csv = """address_line_1,address_line_2,address_line_3,address_line_4,postcode,name + A1,A2,A3,A4,A_POST,Alice + """ + s3_mock = mocker.patch('app.celery.tasks.s3.get_job_from_s3', return_value=csv) + mocker.patch('app.celery.tasks.send_email.apply_async') + process_row_mock = mocker.patch('app.celery.tasks.process_row') + mocker.patch('app.celery.tasks.create_uuid', return_value="uuid") + + process_job(sample_letter_job.id) + + s3_mock.assert_called_once_with( + str(sample_letter_job.service.id), + str(sample_letter_job.id) + ) + + row_call = process_row_mock.mock_calls[0][1] + + assert row_call[0] == 0 + assert row_call[1] == ['A1', 'A2', 'A3', 'A4', None, None, 'A_POST'] + assert dict(row_call[2]) == { + 'addressline1': 'A1', + 'addressline2': 'A2', + 'addressline3': 'A3', + 'addressline4': 'A4', + 'postcode': 'A_POST' + } + assert row_call[4] == sample_letter_job + assert row_call[5] == sample_letter_job.service + + assert process_row_mock.call_count == 1 + + assert sample_letter_job.job_status == 'finished' + + def test_should_process_all_sms_job(sample_job, sample_job_with_placeholdered_template, mocker): @@ -853,6 +886,47 @@ def test_send_sms_does_not_send_duplicate_and_does_not_put_in_retry_queue(sample assert not retry.called +def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker): + personalisation = { + 'addressline1': 'Foo', + 'addressline2': 'Bar', + 'addressline3': 'Baz', + 'addressline4': 'Wibble', + 'addressline5': 'Wobble', + 'addressline6': 'Wubble', + 'postcode': 'Flob', + } + notification_json = _notification_json( + template=sample_letter_job.template, + to='Foo', + personalisation=personalisation, + job_id=sample_letter_job.id, + row_number=1 + ) + notification_id = uuid.uuid4() + created_at = datetime.utcnow() + + persist_letter( + sample_letter_job.service_id, + notification_id, + encryption.encrypt(notification_json), + created_at + ) + + notification_db = Notification.query.one() + assert notification_db.id == notification_id + assert notification_db.to == 'Foo' + assert notification_db.job_id == sample_letter_job.id + assert notification_db.template_id == sample_letter_job.template.id + assert notification_db.template_version == sample_letter_job.template.version + assert notification_db.status == 'created' + assert notification_db.created_at == created_at + assert notification_db.notification_type == 'letter' + assert notification_db.sent_at is None + assert notification_db.sent_by is None + assert notification_db.personalisation == personalisation + + @pytest.mark.parametrize('template_type, expected_class', [ (SMS_TYPE, SMSMessageTemplate), (EMAIL_TYPE, WithSubjectTemplate), diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 559537744..dfbe54fc9 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -25,7 +25,7 @@ from app.models import ( NotificationStatistics, ServiceWhitelist, KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM, - MOBILE_TYPE, EMAIL_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED) + MOBILE_TYPE, EMAIL_TYPE, LETTER_TYPE, NOTIFICATION_STATUS_TYPES_COMPLETED) from app.dao.users_dao import (create_user_code, create_secret_code) from app.dao.services_dao import (dao_create_service, dao_add_user_to_service) from app.dao.templates_dao import dao_create_template @@ -35,12 +35,8 @@ from app.dao.notifications_dao import dao_create_notification from app.dao.invited_user_dao import save_invited_user from app.dao.provider_rates_dao import create_provider_rates from app.clients.sms.firetext import FiretextClient -from app.dao.provider_details_dao import ( - dao_update_provider_details, - get_provider_details_by_identifier, - get_alternative_sms_provider -) -from tests.app.db import create_user + +from tests.app.db import create_user, create_template, create_notification @pytest.yield_fixture @@ -228,6 +224,11 @@ def sample_email_template( return template +@pytest.fixture +def sample_letter_template(sample_service): + return create_template(sample_service, template_type=LETTER_TYPE) + + @pytest.fixture(scope='function') def sample_email_template_with_placeholders(notify_db, notify_db_session): return sample_email_template( @@ -363,6 +364,24 @@ def sample_email_job(notify_db, return job +@pytest.fixture +def sample_letter_job(sample_service, sample_letter_template): + data = { + 'id': uuid.uuid4(), + 'service_id': sample_service.id, + 'service': sample_service, + 'template_id': sample_letter_template.id, + 'template_version': sample_letter_template.version, + 'original_file_name': 'some.csv', + 'notification_count': 1, + 'created_at': datetime.utcnow(), + 'created_by': sample_service.created_by, + } + job = Job(**data) + dao_create_job(job) + return job + + @pytest.fixture(scope='function') def sample_notification_with_job( notify_db, @@ -377,7 +396,6 @@ def sample_notification_with_job( created_at=None, sent_at=None, billable_units=1, - create=True, personalisation=None, api_key_id=None, key_type=KEY_TYPE_NORMAL @@ -398,7 +416,6 @@ def sample_notification_with_job( created_at=created_at, sent_at=sent_at, billable_units=billable_units, - create=create, personalisation=personalisation, api_key_id=api_key_id, key_type=key_type @@ -418,7 +435,6 @@ def sample_notification(notify_db, created_at=None, sent_at=None, billable_units=1, - create=True, personalisation=None, api_key_id=None, key_type=KEY_TYPE_NORMAL, @@ -464,11 +480,22 @@ def sample_notification(notify_db, if job_row_number: data['job_row_number'] = job_row_number notification = Notification(**data) - if create: - dao_create_notification(notification) + dao_create_notification(notification) return notification +@pytest.fixture +def sample_letter_notification(sample_letter_template): + address = { + 'addressline1': 'A1', + 'addressline2': 'A2', + 'addressline3': 'A3', + 'addressline4': 'A4', + 'postcode': 'A_POST' + } + return create_notification(sample_letter_template, personalisation=address) + + @pytest.fixture(scope='function') def sample_notification_with_api_key(notify_db, notify_db_session): notification = sample_notification(notify_db, notify_db_session) diff --git a/tests/app/db.py b/tests/app/db.py index cb64f86db..2b9e97b6d 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -1,5 +1,10 @@ -from app.models import User +from datetime import datetime +import uuid + +from app.models import User, Template, Notification, SMS_TYPE, KEY_TYPE_NORMAL from app.dao.users_dao import save_model_user +from app.dao.notifications_dao import dao_create_notification +from app.dao.templates_dao import dao_create_template def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-office.gov.uk"): @@ -15,3 +20,64 @@ def create_user(mobile_number="+447700900986", email="notify@digital.cabinet-off user = User(**data) save_model_user(user) return user + + +def create_template(service, user=None, template_type=SMS_TYPE): + data = { + 'name': '{} Template Name'.format(template_type), + 'template_type': template_type, + 'content': 'Dear Sir/Madam, Hello. Yours Truly, The Government.', + 'service': service, + 'created_by': service.created_by, + } + if template_type != SMS_TYPE: + data['subject'] = 'Template subject' + template = Template(**data) + dao_create_template(template) + return template + + +def create_notification( + template, + job=None, + job_row_number=None, + to_field='+447700900855', + status='created', + reference=None, + created_at=None, + sent_at=None, + billable_units=1, + personalisation=None, + api_key_id=None, + key_type=KEY_TYPE_NORMAL, + sent_by=None, + client_reference=None +): + if created_at is None: + created_at = datetime.utcnow() + data = { + 'id': uuid.uuid4(), + 'to': to_field, + 'job_id': job.id if job else None, + 'job': job, + 'service_id': template.service_id, + 'template_id': template.id if template else None, + 'template': template, + 'template_version': template.version, + 'status': status, + 'reference': reference, + 'created_at': created_at, + 'sent_at': sent_at, + 'billable_units': billable_units, + 'personalisation': personalisation, + 'notification_type': template.template_type, + 'api_key_id': api_key_id, + 'key_type': key_type, + 'sent_by': sent_by, + 'updated_at': None, + 'client_reference': client_reference, + 'job_row_number': None + } + notification = Notification(**data) + dao_create_notification(notification) + return notification