From f5ea77ffa06e355101974a0eb9f24afbed325d4a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 30 Nov 2018 16:35:00 +0000 Subject: [PATCH 1/2] Add reference to one off letters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Letters should always have a reference, because that’s what DVLA use to tell us when they’ve sent a letter. If a letter has a reference of `None` then DVLA say they’ve sent a letter with a reference of `'None'`. This means we can never reconcile the letter, which means it stays in `created`, which means it never gets billed. We don’t think this has affected any real letters yet, just ones that we’ve sent as tests. --- app/service/send_notification.py | 10 +- .../service/test_send_one_off_notification.py | 97 ++++++++++++++++++- 2 files changed, 103 insertions(+), 4 deletions(-) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index b80baf770..62e61283c 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -1,5 +1,6 @@ from sqlalchemy.orm.exc import NoResultFound +from app import create_random_identifier from app.config import QueueNames from app.dao.notifications_dao import _update_notification_status from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id @@ -37,6 +38,12 @@ def validate_created_by(service, created_by_id): raise BadRequestError(message=message) +def create_one_off_reference(template_type): + if template_type != LETTER_TYPE: + return None + return create_random_identifier() + + def send_one_off_notification(service_id, post_data): service = dao_fetch_service_by_id(service_id) template = dao_get_template_by_id_and_service_id( @@ -77,7 +84,8 @@ def send_one_off_notification(service_id, post_data): api_key_id=None, key_type=KEY_TYPE_NORMAL, created_by_id=post_data['created_by'], - reply_to_text=reply_to + reply_to_text=reply_to, + reference=create_one_off_reference(template.template_type), ) queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None diff --git a/tests/app/service/test_send_one_off_notification.py b/tests/app/service/test_send_one_off_notification.py index f8ffdf28a..0c20611dc 100644 --- a/tests/app/service/test_send_one_off_notification.py +++ b/tests/app/service/test_send_one_off_notification.py @@ -10,7 +10,9 @@ from app.config import QueueNames from app.dao.service_whitelist_dao import dao_add_and_commit_whitelisted_contacts from app.service.send_notification import send_one_off_notification from app.models import ( + EMAIL_TYPE, KEY_TYPE_NORMAL, + LETTER_TYPE, MOBILE_TYPE, PRIORITY, SMS_TYPE, @@ -64,13 +66,17 @@ def test_send_one_off_notification_calls_celery_correctly(persist_mock, celery_m ) -def test_send_one_off_notification_calls_persist_correctly( +def test_send_one_off_notification_calls_persist_correctly_for_sms( persist_mock, celery_mock, notify_db_session ): service = create_service() - template = create_template(service=service, content="Hello (( Name))\nYour thing is due soon") + template = create_template( + service=service, + template_type=SMS_TYPE, + content="Hello (( Name))\nYour thing is due soon", + ) post_data = { 'template_id': str(template.id), @@ -91,7 +97,92 @@ def test_send_one_off_notification_calls_persist_correctly( api_key_id=None, key_type=KEY_TYPE_NORMAL, created_by_id=str(service.created_by_id), - reply_to_text='testing' + reply_to_text='testing', + reference=None, + ) + + +def test_send_one_off_notification_calls_persist_correctly_for_email( + persist_mock, + celery_mock, + notify_db_session +): + service = create_service() + template = create_template( + service=service, + template_type=EMAIL_TYPE, + subject="Test subject", + content="Hello (( Name))\nYour thing is due soon", + ) + + post_data = { + 'template_id': str(template.id), + 'to': 'test@example.com', + 'personalisation': {'name': 'foo'}, + 'created_by': str(service.created_by_id) + } + + send_one_off_notification(service.id, post_data) + + persist_mock.assert_called_once_with( + template_id=template.id, + template_version=template.version, + recipient=post_data['to'], + service=template.service, + personalisation={'name': 'foo'}, + notification_type=EMAIL_TYPE, + api_key_id=None, + key_type=KEY_TYPE_NORMAL, + created_by_id=str(service.created_by_id), + reply_to_text=None, + reference=None, + ) + + +def test_send_one_off_notification_calls_persist_correctly_for_letter( + mocker, + persist_mock, + celery_mock, + notify_db_session +): + mocker.patch( + 'app.service.send_notification.create_random_identifier', + return_value='this-is-random-in-real-life', + ) + service = create_service() + template = create_template( + service=service, + template_type=LETTER_TYPE, + subject="Test subject", + content="Hello (( Name))\nYour thing is due soon", + ) + + post_data = { + 'template_id': str(template.id), + 'to': 'First Last', + 'personalisation': { + 'name': 'foo', + 'address line 1': 'First Last', + 'address line 2': '1 Example Street', + 'postcode': 'SW1A 1AA', + }, + 'created_by': str(service.created_by_id) + } + + send_one_off_notification(service.id, post_data) + + persist_mock.assert_called_once_with( + template_id=template.id, + template_version=template.version, + recipient=post_data['to'], + service=template.service, + personalisation=post_data['personalisation'], + notification_type=LETTER_TYPE, + api_key_id=None, + key_type=KEY_TYPE_NORMAL, + created_by_id=str(service.created_by_id), + reply_to_text=None, + reference='this-is-random-in-real-life', ) From 3cfeadcae873b6e45c73d3b637dff256d190ad12 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 3 Dec 2018 11:33:59 +0000 Subject: [PATCH 2/2] Refactor if statement to be positive --- app/service/send_notification.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 62e61283c..a00d151a4 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -39,9 +39,9 @@ def validate_created_by(service, created_by_id): def create_one_off_reference(template_type): - if template_type != LETTER_TYPE: - return None - return create_random_identifier() + if template_type == LETTER_TYPE: + return create_random_identifier() + return None def send_one_off_notification(service_id, post_data):