From ba0d3305937ade88eaccfc534ce28a38e59c49ff Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 28 Apr 2020 11:16:11 +0100 Subject: [PATCH] Allow countries in last line of addresses For services that have permission to send international letters we should not reject letters that are addressed to another country. We should still reject letters that are badly-addressed. --- app/v2/notifications/post_notifications.py | 12 ++++- requirements-app.txt | 2 +- .../test_post_letter_notifications.py | 46 ++++++++++++++++++- 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 399940d44..acd37f1a8 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -36,6 +36,7 @@ from app.models import ( NOTIFICATION_SENDING, NOTIFICATION_DELIVERED, NOTIFICATION_PENDING_VIRUS_CHECK, + INTERNATIONAL_LETTERS, Notification) from app.notifications.process_letter_notifications import ( create_letter_notification @@ -345,7 +346,10 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text template=template, reply_to_text=reply_to_text) - address = PostalAddress.from_personalisation(letter_data['personalisation']) + address = PostalAddress.from_personalisation( + letter_data['personalisation'], + allow_international_letters=api_key.service.has_permission(INTERNATIONAL_LETTERS), + ) if not address.has_enough_lines: raise ValidationError( @@ -357,7 +361,11 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text message=f'Address must be no more than {PostalAddress.MAX_LINES} lines' ) - if not address.postcode: + if not address.has_valid_last_line: + if address.allow_international_letters: + raise ValidationError( + message=f'Last line of address must be a real UK postcode or another country' + ) raise ValidationError( message='Must be a real UK postcode' ) diff --git a/requirements-app.txt b/requirements-app.txt index eda2bd02a..6686f123d 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -26,6 +26,6 @@ notifications-python-client==5.5.1 # PaaS awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@38.0.0#egg=notifications-utils==38.0.0 +git+https://github.com/alphagov/notifications-utils.git@38.1.0#egg=notifications-utils==38.1.0 gds-metrics==0.2.0 diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index 509e728a9..323688543 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -18,7 +18,8 @@ from app.models import ( NOTIFICATION_SENDING, NOTIFICATION_DELIVERED, NOTIFICATION_PENDING_VIRUS_CHECK, - SMS_TYPE + SMS_TYPE, + INTERNATIONAL_LETTERS, ) from app.schema_validation import validate from app.v2.errors import RateLimitError @@ -144,8 +145,49 @@ def test_post_letter_notification_formats_postcode( assert notification.personalisation["postcode"] == ' Sw1 1aa ' -def test_post_letter_notification_throws_error_for_bad_postcode( +def test_post_letter_notification_stores_country( client, notify_db_session, mocker +): + service = create_service(service_permissions=[LETTER_TYPE, INTERNATIONAL_LETTERS]) + template = create_template(service, template_type="letter") + mocker.patch('app.celery.tasks.letters_pdf_tasks.create_letters_pdf.apply_async') + data = { + 'template_id': str(template.id), + 'personalisation': { + 'address_line_1': 'Kaiser Wilhelm II', + 'address_line_2': 'Kronprinzenpalais', + 'address_line_5': ' deutschland ', + } + } + + resp_json = letter_request(client, data, service_id=service.id) + + assert validate(resp_json, post_letter_response) == resp_json + notification = Notification.query.one() + # In the personalisation we store what the client gives us + assert notification.personalisation["address_line_1"] == 'Kaiser Wilhelm II' + assert notification.personalisation["address_line_2"] == 'Kronprinzenpalais' + assert notification.personalisation["address_line_5"] == ' deutschland ' + # In the to field we store the whole address with the canonical country + assert notification.to == ( + 'Kaiser Wilhelm II\n' + 'Kronprinzenpalais\n' + 'Germany' + ) + + +@pytest.mark.parametrize('permissions, expected_error', ( + ( + [LETTER_TYPE], + 'Must be a real UK postcode', + ), + ( + [LETTER_TYPE, INTERNATIONAL_LETTERS], + 'Last line of address must be a real UK postcode or another country', + ), +)) +def test_post_letter_notification_throws_error_for_bad_postcode( + client, notify_db_session, mocker, permissions, expected_error ): service = create_service(service_permissions=[LETTER_TYPE]) template = create_template(service, template_type="letter", postage="first")