diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index 495426cba..0d749e057 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -40,6 +40,7 @@ from app.letters.utils import ( get_file_names_from_error_bucket, ) from app.models import ( + INTERNATIONAL_LETTERS, KEY_TYPE_TEST, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, @@ -227,6 +228,9 @@ def sanitise_letter(self, filename): kwargs={ 'notification_id': str(notification.id), 'filename': filename, + 'allow_international_letters': notification.service.has_permission( + INTERNATIONAL_LETTERS + ), }, queue=QueueNames.SANITISE_LETTERS, ) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 4949b39b0..baed8d6ee 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -126,9 +126,7 @@ def get_recipient_csv_and_template_and_sender_id(job): template = db_template._as_utils_template() contents, meta_data = s3.get_job_and_metadata_from_s3(service_id=str(job.service_id), job_id=str(job.id)) - recipient_csv = RecipientCSV(file_data=contents, - template_type=template.template_type, - placeholders=template.placeholders) + recipient_csv = RecipientCSV(contents, template=template) return recipient_csv, template, meta_data.get("sender_id") 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 e2a8fd3be..448273766 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@37.3.0#egg=notifications-utils==37.3.0 +git+https://github.com/alphagov/notifications-utils.git@39.0.0#egg=notifications-utils==39.0.0 gds-metrics==0.2.0 diff --git a/requirements.txt b/requirements.txt index a7b247db9..99d1149a4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -28,7 +28,7 @@ notifications-python-client==5.5.1 # PaaS awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@37.3.0#egg=notifications-utils==37.3.0 +git+https://github.com/alphagov/notifications-utils.git@39.0.0#egg=notifications-utils==39.0.0 gds-metrics==0.2.0 @@ -37,17 +37,17 @@ alembic==1.4.2 amqp==1.4.9 anyjson==0.3.3 attrs==19.3.0 -awscli==1.18.45 +awscli==1.18.48 bcrypt==3.1.7 billiard==3.3.0.23 bleach==3.1.4 blinker==1.4 boto==2.49.0 boto3==1.10.38 -botocore==1.15.45 +botocore==1.15.48 certifi==2020.4.5.1 chardet==3.0.4 -click==7.1.1 +click==7.1.2 colorama==0.4.3 dnspython==1.16.0 docutils==0.15.2 @@ -74,7 +74,7 @@ pyrsistent==0.16.0 python-dateutil==2.8.1 python-editor==1.0.4 python-json-logger==0.1.11 -pytz==2019.3 +pytz==2020.1 PyYAML==5.3.1 redis==3.4.1 requests==2.23.0 diff --git a/tests/app/celery/test_letters_pdf_tasks.py b/tests/app/celery/test_letters_pdf_tasks.py index a6e43c27b..6f4e1562a 100644 --- a/tests/app/celery/test_letters_pdf_tasks.py +++ b/tests/app/celery/test_letters_pdf_tasks.py @@ -31,8 +31,10 @@ from app.celery.letters_pdf_tasks import ( from app.config import QueueNames, TaskNames from app.letters.utils import ScanErrorType from app.models import ( + INTERNATIONAL_LETTERS, KEY_TYPE_NORMAL, KEY_TYPE_TEST, + LETTER_TYPE, Notification, NOTIFICATION_CREATED, NOTIFICATION_DELIVERED, @@ -42,7 +44,7 @@ from app.models import ( NOTIFICATION_VIRUS_SCAN_FAILED, ) -from tests.app.db import create_notification, create_letter_branding +from tests.app.db import create_notification, create_letter_branding, create_service from tests.conftest import set_config_values @@ -546,16 +548,32 @@ def test_move_invalid_letter_and_update_status_logs_error_and_sets_tech_failure_ ) -def test_sanitise_letter_calls_template_preview_sanitise_task(mocker, sample_letter_notification): +@pytest.mark.parametrize('permissions, expected_international_letters_allowed', ( + ([LETTER_TYPE], False), + ([LETTER_TYPE, INTERNATIONAL_LETTERS], True), +)) +def test_sanitise_letter_calls_template_preview_sanitise_task( + mocker, + sample_letter_notification, + permissions, + expected_international_letters_allowed, +): mock_celery = mocker.patch('app.celery.letters_pdf_tasks.notify_celery.send_task') filename = 'NOTIFY.{}'.format(sample_letter_notification.reference) + sample_letter_notification.service = create_service( + service_permissions=permissions + ) sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK sanitise_letter(filename) mock_celery.assert_called_once_with( name=TaskNames.SANITISE_LETTER, - kwargs={'notification_id': str(sample_letter_notification.id), 'filename': filename}, + kwargs={ + 'notification_id': str(sample_letter_notification.id), + 'filename': filename, + 'allow_international_letters': expected_international_letters_allowed, + }, queue=QueueNames.SANITISE_LETTERS, ) diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index 94baf03be..000a078b5 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -340,7 +340,7 @@ def test_should_process_letter_job(sample_letter_job, mocker): row_call = process_row_mock.mock_calls[0][1] assert row_call[0].index == 0 - assert row_call[0].recipient == ['A1', 'A2', 'A3', 'A4', None, None, 'A_POST'] + assert row_call[0].recipient == ['A1', 'A2', 'A3', 'A4', None, None, 'A_POST', None] assert row_call[0].personalisation == { 'addressline1': 'A1', 'addressline2': 'A2', @@ -407,6 +407,7 @@ def test_process_row_sends_letter_task(template_type, research_mode, expected_fu recipient_column_headers=['to'], placeholders={'foo'}, template=template, + allow_international_letters=True, ), template, job, @@ -449,6 +450,7 @@ def test_process_row_when_sender_id_is_provided(mocker, fake_uuid): recipient_column_headers=['to'], placeholders={'foo'}, template=template, + allow_international_letters=True, ), template, job, @@ -1418,6 +1420,7 @@ def test_get_letter_template_instance(mocker, sample_job): 'address line 5', 'address line 6', 'postcode', + 'address line 7', ] 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")