Merge pull request #2809 from alphagov/bump-utils-38.0.0

Bump utils to 39.0.0

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,
         )

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,
+    recipient_csv = RecipientCSV(contents, template=template)
-                                 template_type=template.template_type,
-                                 placeholders=template.placeholders)
 
     return recipient_csv, template, meta_data.get("sender_id")
 

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'
         )

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

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

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,
     )
 

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',
     ]
 
 

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")