Merge pull request #2943 from alphagov/international-postage

Set international postage and international flag for international letters

app/celery/letters_pdf_tasks.py

@@ -4,6 +4,7 @@ from base64 import urlsafe_b64encode
 
 from botocore.exceptions import ClientError as BotoClientError
 from flask import current_app
+from notifications_utils.postal_address import PostalAddress
 
 from notifications_utils.statsd_decorators import statsd
 from notifications_utils.letter_timings import LETTER_PROCESSING_DEADLINE
@@ -43,8 +44,8 @@ from app.models import (
     NOTIFICATION_VALIDATION_FAILED,
     NOTIFICATION_VIRUS_SCAN_FAILED,
     POSTAGE_TYPES,
-    RESOLVE_POSTAGE_FOR_FILE_NAME
+    RESOLVE_POSTAGE_FOR_FILE_NAME,
-)
+    INTERNATIONAL_POSTAGE_TYPES)
 from app.cronitor import cronitor
 
 
@@ -393,8 +394,16 @@ def process_virus_scan_error(filename):
 
 
 def update_letter_pdf_status(reference, status, billable_units, recipient_address=None):
-
+    postage = None
+    if recipient_address:
+        # fix allow_international_letters
+        postage = PostalAddress(raw_address=recipient_address.replace(',', '\n'),
+                                allow_international_letters=True
+                                ).postage
+        postage = postage if postage in INTERNATIONAL_POSTAGE_TYPES else None
     update_dict = {'status': status, 'billable_units': billable_units, 'updated_at': datetime.utcnow()}
+    if postage:
+        update_dict.update({'postage': postage, 'international': True})
     if recipient_address:
         update_dict['to'] = recipient_address
     return dao_update_notifications_by_reference(

app/celery/tasks.py

@@ -354,7 +354,7 @@ def save_letter(
         saved_notification = persist_notification(
             template_id=notification['template'],
             template_version=notification['template_version'],
-            template_postage=template.postage,
+            postage=template.postage,
             recipient=recipient,
             service=service,
             personalisation=notification['personalisation'],

app/notifications/process_letter_notifications.py

@@ -14,11 +14,11 @@ def create_letter_notification(
     reply_to_text=None,
     billable_units=None,
     updated_at=None,
+    postage=None
 ):
     notification = persist_notification(
         template_id=template.id,
         template_version=template.version,
-        template_postage=template.postage,
         # we only accept addresses_with_underscores from the API (from CSV we also accept dashes, spaces etc)
         recipient=PostalAddress.from_personalisation(letter_data['personalisation']).normalised,
         service=service,
@@ -33,7 +33,9 @@ def create_letter_notification(
         status=status,
         reply_to_text=reply_to_text,
         billable_units=billable_units,
-        postage=letter_data.get('postage'),
+        # letter_data.get('postage') is only set for precompiled letters (if international it is set after sanitise)
+        # letters from a template will pass in 'europe' or 'rest-of-world' if None then use postage from template
+        postage=postage or letter_data.get('postage') or template.postage,
         updated_at=updated_at
     )
     return notification

app/notifications/process_notifications.py

@@ -27,7 +27,7 @@ from app.models import (
     LETTER_TYPE,
     NOTIFICATION_CREATED,
     Notification,
-)
+    INTERNATIONAL_POSTAGE_TYPES)
 from app.dao.notifications_dao import (
     dao_create_notification,
     dao_delete_notifications_by_id,
@@ -107,7 +107,6 @@ def persist_notification(
     reply_to_text=None,
     billable_units=None,
     postage=None,
-    template_postage=None,
     document_download_count=None,
     updated_at=None
 ):
@@ -147,7 +146,8 @@ def persist_notification(
     elif notification_type == EMAIL_TYPE:
         notification.normalised_to = format_email_address(notification.to)
     elif notification_type == LETTER_TYPE:
-        notification.postage = postage or template_postage
+        notification.postage = postage
+        notification.international = postage in INTERNATIONAL_POSTAGE_TYPES
         notification.normalised_to = ''.join(notification.to.split()).lower()
 
     # if simulated create a Notification model to return but do not persist the Notification to the dB

app/notifications/rest.py

@@ -120,7 +120,7 @@ def send_notification(notification_type):
     simulated = simulated_recipient(notification_form['to'], notification_type)
     notification_model = persist_notification(template_id=template.id,
                                               template_version=template.version,
-                                              template_postage=template.postage,
+                                              postage=template.postage,
                                               recipient=request.get_json()['to'],
                                               service=authenticated_service,
                                               personalisation=notification_form.get('personalisation', None),

app/notifications/validators.py

@@ -1,3 +1,4 @@
+from notifications_utils.postal_address import PostalAddress
 from sqlalchemy.orm.exc import NoResultFound
 from flask import current_app
 from notifications_utils import SMS_CHAR_COUNT_LIMIT
@@ -14,9 +15,9 @@ from app.models import (
     INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE,
     KEY_TYPE_TEST, KEY_TYPE_TEAM,
     ServicePermission,
-)
+    INTERNATIONAL_LETTERS)
 from app.service.utils import service_allowed_to_send_to
-from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError
+from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError, ValidationError
 from app import redis_store
 from app.notifications.process_notifications import create_content_for_notification
 from app.utils import get_public_notify_type_text
@@ -214,3 +215,34 @@ def check_service_letter_contact_id(service_id, letter_contact_id, notification_
             message = 'letter_contact_id {} does not exist in database for service id {}'\
                 .format(letter_contact_id, service_id)
             raise BadRequestError(message=message)
+
+
+def validate_address(service, letter_data):
+    address = PostalAddress.from_personalisation(
+        letter_data,
+        allow_international_letters=(INTERNATIONAL_LETTERS in str(service.permissions)),
+    )
+    if not address.has_enough_lines:
+        raise ValidationError(
+            message=f'Address must be at least {PostalAddress.MIN_LINES} lines'
+        )
+    if address.has_too_many_lines:
+        raise ValidationError(
+            message=f'Address must be no more than {PostalAddress.MAX_LINES} lines'
+        )
+    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'
+        )
+    if address.has_invalid_characters:
+        raise ValidationError(
+            message='Address lines must not start with any of the following characters: @ ( ) = [ ] ” \\ / ,'
+        )
+    if address.postage == 'united-kingdom':
+        return None  # use postage from template
+    else:
+        return address.postage

app/service/send_notification.py

@@ -13,8 +13,8 @@ from app.notifications.validators import (
     check_service_has_permission,
     check_service_over_daily_message_limit,
     validate_and_format_recipient,
-    validate_template
+    validate_template,
-)
+    validate_address)
 from app.notifications.process_notifications import (
     persist_notification,
     send_notification_to_queue
@@ -75,7 +75,13 @@ def send_one_off_notification(service_id, post_data):
         notification_type=template.template_type,
         allow_guest_list_recipients=False,
     )
-
+    postage = None
+    if template.template_type == LETTER_TYPE:
+        # Validate address and set postage to europe|rest-of-world if international letter,
+        # otherwise persist_notification with use template postage
+        postage = validate_address(service, personalisation)
+        if not postage:
+            postage = template.postage
     validate_created_by(service, post_data['created_by'])
 
     sender_id = post_data.get('sender_id', None)
@@ -88,7 +94,6 @@ def send_one_off_notification(service_id, post_data):
     notification = persist_notification(
         template_id=template.id,
         template_version=template.version,
-        template_postage=template.postage,
         recipient=post_data['to'],
         service=service,
         personalisation=personalisation,
@@ -98,6 +103,7 @@ def send_one_off_notification(service_id, post_data):
         created_by_id=post_data['created_by'],
         reply_to_text=reply_to,
         reference=create_one_off_reference(template.template_type),
+        postage=postage
     )
 
     queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None
@@ -173,7 +179,6 @@ def send_pdf_letter_notification(service_id, post_data):
         notification_id=post_data['file_id'],
         template_id=template.id,
         template_version=template.version,
-        template_postage=template.postage,
         recipient=urllib.parse.unquote(post_data['recipient_address']),
         service=service,
         personalisation=personalisation,
@@ -184,7 +189,7 @@ def send_pdf_letter_notification(service_id, post_data):
         client_reference=post_data['filename'],
         created_by_id=post_data['created_by'],
         billable_units=billable_units,
-        postage=post_data['postage'],
+        postage=post_data['postage'] or template.postage,
     )
 
     upload_filename = get_letter_pdf_filename(

app/v2/notifications/post_notifications.py

@@ -5,7 +5,6 @@ from datetime import datetime
 
 from boto.exception import SQSError
 from flask import request, jsonify, current_app, abort
-from notifications_utils.postal_address import PostalAddress
 from notifications_utils.recipients import try_validate_and_format_phone_number
 from gds_metrics import Histogram
 
@@ -36,7 +35,6 @@ 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
@@ -51,15 +49,18 @@ from app.notifications.validators import (
     check_service_email_reply_to_id,
     check_service_has_permission,
     check_service_sms_sender_id,
+    validate_address,
     validate_and_format_recipient,
     validate_template,
 )
 from app.schema_validation import validate
-from app.v2.errors import BadRequestError, ValidationError
+from app.v2.errors import BadRequestError
-from app.v2.notifications import v2_notification_blueprint
 from app.v2.notifications.create_response import (
-    create_post_sms_response_from_notification, create_post_email_response_from_notification,
+    create_post_email_response_from_notification,
-    create_post_letter_response_from_notification)
+    create_post_sms_response_from_notification,
+    create_post_letter_response_from_notification
+)
+from app.v2.notifications import v2_notification_blueprint
 from app.v2.notifications.notification_schemas import (
     post_sms_request,
     post_email_request,
@@ -339,7 +340,7 @@ def process_letter_notification(
                                                         template=template,
                                                         reply_to_text=reply_to_text)
 
-    validate_address(service, letter_data)
+    postage = validate_address(service, letter_data['personalisation'])
 
     test_key = api_key.key_type == KEY_TYPE_TEST
 
@@ -362,7 +363,8 @@ def process_letter_notification(
                                               api_key=api_key,
                                               status=status,
                                               reply_to_text=reply_to_text,
-                                              updated_at=updated_at
+                                              updated_at=updated_at,
+                                              postage=postage
                                               )
 
     get_pdf_for_templated_letter.apply_async(
@@ -388,33 +390,6 @@ def process_letter_notification(
     return resp
 
 
-def validate_address(service, letter_data):
-    address = PostalAddress.from_personalisation(
-        letter_data['personalisation'],
-        allow_international_letters=(INTERNATIONAL_LETTERS in service.permissions),
-    )
-    if not address.has_enough_lines:
-        raise ValidationError(
-            message=f'Address must be at least {PostalAddress.MIN_LINES} lines'
-        )
-    if address.has_too_many_lines:
-        raise ValidationError(
-            message=f'Address must be no more than {PostalAddress.MAX_LINES} lines'
-        )
-    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'
-        )
-    if address.has_invalid_characters:
-        raise ValidationError(
-            message='Address lines must not start with any of the following characters: @ ( ) = [ ] ” \\ / ,'
-        )
-
-
 def process_precompiled_letter_notifications(*, letter_data, api_key, service, template, reply_to_text):
     try:
         status = NOTIFICATION_PENDING_VIRUS_CHECK

tests/app/celery/test_letters_pdf_tasks.py

@@ -646,6 +646,54 @@ def test_process_sanitised_letter_with_valid_letter(
     assert file_contents == 'sanitised_pdf_content'
 
 
+@mock_s3
+@pytest.mark.parametrize('address, expected_postage, expected_international',
+                         [('Lady Lou, 123 Main Street, SW1 1AA', 'second', False),
+                          ('Lady Lou, 123 Main Street, France', 'europe', True),
+                          ('Lady Lou, 123 Main Street, New Zealand', 'rest-of-world', True),
+                          ])
+def test_process_sanitised_letter_sets_postage_international(
+    sample_letter_notification,
+    expected_postage,
+    expected_international,
+    address
+):
+    filename = 'NOTIFY.{}'.format(sample_letter_notification.reference)
+
+    scan_bucket_name = current_app.config['LETTERS_SCAN_BUCKET_NAME']
+    template_preview_bucket_name = current_app.config['LETTER_SANITISE_BUCKET_NAME']
+    destination_bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME']
+    conn = boto3.resource('s3', region_name='eu-west-1')
+    conn.create_bucket(Bucket=scan_bucket_name)
+    conn.create_bucket(Bucket=template_preview_bucket_name)
+    conn.create_bucket(Bucket=destination_bucket_name)
+
+    s3 = boto3.client('s3', region_name='eu-west-1')
+    s3.put_object(Bucket=scan_bucket_name, Key=filename, Body=b'original_pdf_content')
+    s3.put_object(Bucket=template_preview_bucket_name, Key=filename, Body=b'sanitised_pdf_content')
+
+    sample_letter_notification.status = NOTIFICATION_PENDING_VIRUS_CHECK
+    sample_letter_notification.billable_units = 1
+    sample_letter_notification.created_at = datetime(2018, 7, 1, 12)
+
+    encrypted_data = encryption.encrypt({
+        'page_count': 2,
+        'message': None,
+        'invalid_pages': None,
+        'validation_status': 'passed',
+        'filename': filename,
+        'notification_id': str(sample_letter_notification.id),
+        'address': address
+    })
+    process_sanitised_letter(encrypted_data)
+
+    assert sample_letter_notification.status == 'created'
+    assert sample_letter_notification.billable_units == 1
+    assert sample_letter_notification.to == address
+    assert sample_letter_notification.postage == expected_postage
+    assert sample_letter_notification.international == expected_international
+
+
 @mock_s3
 @pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEST])
 def test_process_sanitised_letter_with_invalid_letter(sample_letter_notification, key_type):

tests/app/notifications/rest/test_send_notification.py

@@ -18,6 +18,7 @@ from app.dao.services_dao import dao_update_service
 from app.dao.api_key_dao import save_model_api_key
 from app.errors import InvalidRequest
 from app.models import Template
+from app.service.send_notification import send_one_off_notification
 from app.v2.errors import RateLimitError
 
 from tests import create_authorization_header
@@ -1214,3 +1215,29 @@ def test_post_notification_should_set_reply_to_text(client, sample_service, mock
     notifications = Notification.query.all()
     assert len(notifications) == 1
     assert notifications[0].reply_to_text == expected_reply_to
+
+
+@pytest.mark.parametrize('last_line_of_address, expected_postage, expected_international',
+                         [('France', 'europe', True),
+                          ('Canada', 'rest-of-world', True),
+                          ('SW1 1AA', 'second', False)])
+def test_send_notification_should_send_international_letters(
+    sample_letter_template, mocker, last_line_of_address, expected_postage, expected_international
+):
+    deliver_mock = mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async')
+    data = {
+        'template_id': sample_letter_template.id,
+        'personalisation': {
+            'address_line_1': 'Jane',
+            'address_line_2': 'Rue Vert',
+            'address_line_3': last_line_of_address
+        },
+        'to': 'Jane',
+        'created_by': sample_letter_template.service.created_by_id
+    }
+
+    notification_id = send_one_off_notification(sample_letter_template.service_id, data)
+    assert deliver_mock.called
+    notification = Notification.query.get(notification_id['id'])
+    assert notification.postage == expected_postage
+    assert notification.international == expected_international

tests/app/notifications/test_process_notification.py

@@ -489,8 +489,7 @@ def test_persist_email_notification_stores_normalised_email(
     [
         ("second", "first", "second"),
         ("first", "first", "first"),
-        ("first", "second", "first"),
+        ("first", "second", "first")
-        (None, "second", "second")
     ]
 )
 def test_persist_letter_notification_finds_correct_postage(
@@ -506,7 +505,6 @@ def test_persist_letter_notification_finds_correct_postage(
     persist_notification(
         template_id=template.id,
         template_version=template.version,
-        template_postage=template.postage,
         recipient="Jane Doe, 10 Downing Street, London",
         service=sample_service_full_permissions,
         personalisation=None,
@@ -536,8 +534,26 @@ def test_persist_notification_with_billable_units_stores_correct_info(
         api_key_id=None,
         key_type="normal",
         billable_units=3,
-        template_postage=template.postage
     )
     persisted_notification = Notification.query.all()[0]
 
     assert persisted_notification.billable_units == 3
+
+
+@pytest.mark.parametrize('postage', ['europe', 'rest-of-world'])
+def test_persist_notification_for_international_letter(sample_letter_template, postage):
+    notification = persist_notification(
+        template_id=sample_letter_template.id,
+        template_version=sample_letter_template.version,
+        recipient="123 Main Street",
+        service=sample_letter_template.service,
+        personalisation=None,
+        notification_type=sample_letter_template.template_type,
+        api_key_id=None,
+        key_type="normal",
+        billable_units=3,
+        postage=postage,
+    )
+    persisted_notification = Notification.query.get(notification.id)
+    assert persisted_notification.postage == postage
+    assert persisted_notification.international

tests/app/notifications/test_validators.py

@@ -5,7 +5,12 @@ from notifications_utils import SMS_CHAR_COUNT_LIMIT
 
 import app
 from app.dao import templates_dao
-from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE
+from app.models import (
+    EMAIL_TYPE,
+    INTERNATIONAL_LETTERS,
+    LETTER_TYPE,
+    SMS_TYPE,
+)
 from app.notifications.process_notifications import create_content_for_notification
 from app.notifications.validators import (
     check_content_char_count,
@@ -20,6 +25,7 @@ from app.notifications.validators import (
     check_service_letter_contact_id,
     check_reply_to,
     service_can_send_to_recipient,
+    validate_address,
     validate_and_format_recipient,
     validate_template,
 )
@@ -619,3 +625,19 @@ def test_check_if_service_can_send_files_by_email_passes_if_contact_link_set(sam
         service_contact_link=sample_service.contact_link,
         service_id=sample_service.id
     )
+
+
+@pytest.mark.parametrize('key, address_line_3, expected_postage',
+                         [('address_line_3', 'SW1 1AA', None),
+                          ('address_line_5', 'CANADA', 'rest-of-world'),
+                          ('address_line_3', 'GERMANY', 'europe')
+                          ])
+def test_validate_address(notify_db_session, key, address_line_3, expected_postage):
+    service = create_service(service_permissions=[LETTER_TYPE, INTERNATIONAL_LETTERS])
+    data = {
+        'address_line_1': 'Prince Harry',
+        'address_line_2': 'Toronto',
+        key: address_line_3,
+    }
+    postage = validate_address(service, data)
+    assert postage == expected_postage

tests/app/service/test_send_one_off_notification.py

@@ -90,7 +90,6 @@ def test_send_one_off_notification_calls_persist_correctly_for_sms(
     persist_mock.assert_called_once_with(
         template_id=template.id,
         template_version=template.version,
-        template_postage=None,
         recipient=post_data['to'],
         service=template.service,
         personalisation={'name': 'foo'},
@@ -100,6 +99,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_sms(
         created_by_id=str(service.created_by_id),
         reply_to_text='testing',
         reference=None,
+        postage=None
     )
 
 
@@ -151,7 +151,6 @@ def test_send_one_off_notification_calls_persist_correctly_for_email(
     persist_mock.assert_called_once_with(
         template_id=template.id,
         template_version=template.version,
-        template_postage=None,
         recipient=post_data['to'],
         service=template.service,
         personalisation={'name': 'foo'},
@@ -161,6 +160,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_email(
         created_by_id=str(service.created_by_id),
         reply_to_text=None,
         reference=None,
+        postage=None
     )
 
 
@@ -188,8 +188,8 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter(
         'to': 'First Last',
         'personalisation': {
             'name': 'foo',
-            'address line 1': 'First Last',
+            'address_line_1': 'First Last',
-            'address line 2': '1 Example Street',
+            'address_line_2': '1 Example Street',
             'postcode': 'SW1A 1AA',
         },
         'created_by': str(service.created_by_id)
@@ -200,7 +200,6 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter(
     persist_mock.assert_called_once_with(
         template_id=template.id,
         template_version=template.version,
-        template_postage='first',
         recipient=post_data['to'],
         service=template.service,
         personalisation=post_data['personalisation'],
@@ -210,6 +209,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter(
         created_by_id=str(service.created_by_id),
         reply_to_text=None,
         reference='this-is-random-in-real-life',
+        postage='first'
     )
 
 
@@ -362,6 +362,12 @@ def test_send_one_off_letter_notification_should_use_template_reply_to_text(samp
     data = {
         'to': 'user@example.com',
         'template_id': str(sample_letter_template.id),
+        'personalisation': {
+            'name': 'foo',
+            'address_line_1': 'First Last',
+            'address_line_2': '1 Example Street',
+            'address_line_3': 'SW1A 1AA',
+        },
         'created_by': str(sample_letter_template.service.created_by_id)
     }
 
@@ -383,6 +389,12 @@ def test_send_one_off_letter_should_not_make_pdf_in_research_mode(sample_letter_
     data = {
         'to': 'A. Name',
         'template_id': str(sample_letter_template.id),
+        'personalisation': {
+            'name': 'foo',
+            'address_line_1': 'First Last',
+            'address_line_2': '1 Example Street',
+            'address_line_3': 'SW1A 1AA',
+        },
         'created_by': str(sample_letter_template.service.created_by_id)
     }
 

tests/app/v2/notifications/test_post_letter_notifications.py

@@ -174,6 +174,31 @@ def test_post_letter_notification_stores_country(
         'Kronprinzenpalais\n'
         'Germany'
     )
+    assert notification.postage == 'europe'
+    assert notification.international
+
+
+def test_post_letter_notification_international_sets_rest_of_world(
+    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.get_pdf_for_templated_letter.apply_async')
+    data = {
+        'template_id': str(template.id),
+        'personalisation': {
+            'address_line_1': 'Prince Harry',
+            'address_line_2': 'Toronto',
+            'address_line_5': 'Canada',
+        }
+    }
+
+    resp_json = letter_request(client, data, service_id=service.id)
+
+    assert validate(resp_json, post_letter_response) == resp_json
+    notification = Notification.query.one()
+
+    assert notification.postage == 'rest-of-world'
 
 
 @pytest.mark.parametrize('permissions, personalisation, expected_error', (