Add postage for send-one-off letters.

The postage is set to europe or rest-of-world for international letters, otherwise the template postage is used.

Also set international for letters.
This commit is contained in:
Rebecca Law
2020-07-29 14:52:18 +01:00
parent ed5e73d548
commit 10fe7d9fe8
11 changed files with 216 additions and 66 deletions

View File

@@ -4,6 +4,7 @@ from base64 import urlsafe_b64encode
from botocore.exceptions import ClientError as BotoClientError from botocore.exceptions import ClientError as BotoClientError
from flask import current_app from flask import current_app
from notifications_utils.postal_address import PostalAddress
from notifications_utils.statsd_decorators import statsd from notifications_utils.statsd_decorators import statsd
from notifications_utils.letter_timings import LETTER_PROCESSING_DEADLINE from notifications_utils.letter_timings import LETTER_PROCESSING_DEADLINE
@@ -43,8 +44,8 @@ from app.models import (
NOTIFICATION_VALIDATION_FAILED, NOTIFICATION_VALIDATION_FAILED,
NOTIFICATION_VIRUS_SCAN_FAILED, NOTIFICATION_VIRUS_SCAN_FAILED,
POSTAGE_TYPES, POSTAGE_TYPES,
RESOLVE_POSTAGE_FOR_FILE_NAME RESOLVE_POSTAGE_FOR_FILE_NAME,
) INTERNATIONAL_POSTAGE_TYPES)
from app.cronitor import cronitor 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): 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()} update_dict = {'status': status, 'billable_units': billable_units, 'updated_at': datetime.utcnow()}
if postage:
update_dict.update({'postage': postage, 'international': True})
if recipient_address: if recipient_address:
update_dict['to'] = recipient_address update_dict['to'] = recipient_address
return dao_update_notifications_by_reference( return dao_update_notifications_by_reference(

View File

@@ -27,7 +27,7 @@ from app.models import (
LETTER_TYPE, LETTER_TYPE,
NOTIFICATION_CREATED, NOTIFICATION_CREATED,
Notification, Notification,
) INTERNATIONAL_POSTAGE_TYPES)
from app.dao.notifications_dao import ( from app.dao.notifications_dao import (
dao_create_notification, dao_create_notification,
dao_delete_notifications_by_id, dao_delete_notifications_by_id,
@@ -148,6 +148,7 @@ def persist_notification(
notification.normalised_to = format_email_address(notification.to) notification.normalised_to = format_email_address(notification.to)
elif notification_type == LETTER_TYPE: elif notification_type == LETTER_TYPE:
notification.postage = postage or template_postage notification.postage = postage or template_postage
notification.international = True if postage in INTERNATIONAL_POSTAGE_TYPES else False
notification.normalised_to = ''.join(notification.to.split()).lower() 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 # if simulated create a Notification model to return but do not persist the Notification to the dB

View File

@@ -1,3 +1,4 @@
from notifications_utils.postal_address import PostalAddress
from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.orm.exc import NoResultFound
from flask import current_app from flask import current_app
from notifications_utils import SMS_CHAR_COUNT_LIMIT 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, INTERNATIONAL_SMS_TYPE, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE,
KEY_TYPE_TEST, KEY_TYPE_TEAM, KEY_TYPE_TEST, KEY_TYPE_TEAM,
ServicePermission, ServicePermission,
) INTERNATIONAL_LETTERS)
from app.service.utils import service_allowed_to_send_to 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 import redis_store
from app.notifications.process_notifications import create_content_for_notification from app.notifications.process_notifications import create_content_for_notification
from app.utils import get_public_notify_type_text 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 {}'\ message = 'letter_contact_id {} does not exist in database for service id {}'\
.format(letter_contact_id, service_id) .format(letter_contact_id, service_id)
raise BadRequestError(message=message) 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

View File

@@ -13,8 +13,8 @@ from app.notifications.validators import (
check_service_has_permission, check_service_has_permission,
check_service_over_daily_message_limit, check_service_over_daily_message_limit,
validate_and_format_recipient, validate_and_format_recipient,
validate_template validate_template,
) validate_address)
from app.notifications.process_notifications import ( from app.notifications.process_notifications import (
persist_notification, persist_notification,
send_notification_to_queue send_notification_to_queue
@@ -75,7 +75,11 @@ def send_one_off_notification(service_id, post_data):
notification_type=template.template_type, notification_type=template.template_type,
allow_whitelisted_recipients=False, allow_whitelisted_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)
validate_created_by(service, post_data['created_by']) validate_created_by(service, post_data['created_by'])
sender_id = post_data.get('sender_id', None) sender_id = post_data.get('sender_id', None)
@@ -98,6 +102,7 @@ def send_one_off_notification(service_id, post_data):
created_by_id=post_data['created_by'], 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), reference=create_one_off_reference(template.template_type),
postage=postage
) )
queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None queue_name = QueueNames.PRIORITY if template.process_type == PRIORITY else None

View File

@@ -5,7 +5,6 @@ from datetime import datetime
from boto.exception import SQSError from boto.exception import SQSError
from flask import request, jsonify, current_app, abort 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 notifications_utils.recipients import try_validate_and_format_phone_number
from gds_metrics import Histogram from gds_metrics import Histogram
@@ -36,7 +35,6 @@ from app.models import (
NOTIFICATION_SENDING, NOTIFICATION_SENDING,
NOTIFICATION_DELIVERED, NOTIFICATION_DELIVERED,
NOTIFICATION_PENDING_VIRUS_CHECK, NOTIFICATION_PENDING_VIRUS_CHECK,
INTERNATIONAL_LETTERS,
Notification) Notification)
from app.notifications.process_letter_notifications import ( from app.notifications.process_letter_notifications import (
create_letter_notification create_letter_notification
@@ -51,15 +49,18 @@ from app.notifications.validators import (
check_service_email_reply_to_id, check_service_email_reply_to_id,
check_service_has_permission, check_service_has_permission,
check_service_sms_sender_id, check_service_sms_sender_id,
validate_address,
validate_and_format_recipient, validate_and_format_recipient,
validate_template, validate_template,
) )
from app.schema_validation import validate 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 ( 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 ( from app.v2.notifications.notification_schemas import (
post_sms_request, post_sms_request,
post_email_request, post_email_request,
@@ -339,7 +340,7 @@ def process_letter_notification(
template=template, template=template,
reply_to_text=reply_to_text) reply_to_text=reply_to_text)
postage = validate_address(service, letter_data) postage = validate_address(service, letter_data['personalisation'])
test_key = api_key.key_type == KEY_TYPE_TEST test_key = api_key.key_type == KEY_TYPE_TEST
@@ -389,37 +390,6 @@ def process_letter_notification(
return resp 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: @ ( ) = [ ] ” \\ / ,'
)
if address.postage == 'united-kingdom':
return None # use postage from template
else:
return address.postage
def process_precompiled_letter_notifications(*, letter_data, api_key, service, template, reply_to_text): def process_precompiled_letter_notifications(*, letter_data, api_key, service, template, reply_to_text):
try: try:
status = NOTIFICATION_PENDING_VIRUS_CHECK status = NOTIFICATION_PENDING_VIRUS_CHECK

View File

@@ -646,6 +646,54 @@ def test_process_sanitised_letter_with_valid_letter(
assert file_contents == 'sanitised_pdf_content' 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 @mock_s3
@pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEST]) @pytest.mark.parametrize('key_type', [KEY_TYPE_NORMAL, KEY_TYPE_TEST])
def test_process_sanitised_letter_with_invalid_letter(sample_letter_notification, key_type): def test_process_sanitised_letter_with_invalid_letter(sample_letter_notification, key_type):

View File

@@ -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.dao.api_key_dao import save_model_api_key
from app.errors import InvalidRequest from app.errors import InvalidRequest
from app.models import Template from app.models import Template
from app.service.send_notification import send_one_off_notification
from app.v2.errors import RateLimitError from app.v2.errors import RateLimitError
from tests import create_authorization_header 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() notifications = Notification.query.all()
assert len(notifications) == 1 assert len(notifications) == 1
assert notifications[0].reply_to_text == expected_reply_to 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

View File

@@ -541,3 +541,23 @@ def test_persist_notification_with_billable_units_stores_correct_info(
persisted_notification = Notification.query.all()[0] persisted_notification = Notification.query.all()[0]
assert persisted_notification.billable_units == 3 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,
template_postage='second'
)
persisted_notification = Notification.query.get(notification.id)
assert persisted_notification.postage == postage
assert persisted_notification.international

View File

@@ -5,7 +5,12 @@ from notifications_utils import SMS_CHAR_COUNT_LIMIT
import app import app
from app.dao import templates_dao 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.process_notifications import create_content_for_notification
from app.notifications.validators import ( from app.notifications.validators import (
check_content_char_count, check_content_char_count,
@@ -20,6 +25,7 @@ from app.notifications.validators import (
check_service_letter_contact_id, check_service_letter_contact_id,
check_reply_to, check_reply_to,
service_can_send_to_recipient, service_can_send_to_recipient,
validate_address,
validate_and_format_recipient, validate_and_format_recipient,
validate_template, 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_contact_link=sample_service.contact_link,
service_id=sample_service.id 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

View File

@@ -100,6 +100,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_sms(
created_by_id=str(service.created_by_id), created_by_id=str(service.created_by_id),
reply_to_text='testing', reply_to_text='testing',
reference=None, reference=None,
postage=None
) )
@@ -161,6 +162,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_email(
created_by_id=str(service.created_by_id), created_by_id=str(service.created_by_id),
reply_to_text=None, reply_to_text=None,
reference=None, reference=None,
postage=None
) )
@@ -188,8 +190,8 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter(
'to': 'First Last', 'to': 'First Last',
'personalisation': { 'personalisation': {
'name': 'foo', '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', 'postcode': 'SW1A 1AA',
}, },
'created_by': str(service.created_by_id) 'created_by': str(service.created_by_id)
@@ -210,6 +212,7 @@ def test_send_one_off_notification_calls_persist_correctly_for_letter(
created_by_id=str(service.created_by_id), created_by_id=str(service.created_by_id),
reply_to_text=None, reply_to_text=None,
reference='this-is-random-in-real-life', reference='this-is-random-in-real-life',
postage=None
) )
@@ -362,6 +365,12 @@ def test_send_one_off_letter_notification_should_use_template_reply_to_text(samp
data = { data = {
'to': 'user@example.com', 'to': 'user@example.com',
'template_id': str(sample_letter_template.id), '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) 'created_by': str(sample_letter_template.service.created_by_id)
} }
@@ -383,6 +392,12 @@ def test_send_one_off_letter_should_not_make_pdf_in_research_mode(sample_letter_
data = { data = {
'to': 'A. Name', 'to': 'A. Name',
'template_id': str(sample_letter_template.id), '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) 'created_by': str(sample_letter_template.service.created_by_id)
} }

View File

@@ -175,29 +175,30 @@ def test_post_letter_notification_stores_country(
'Germany' 'Germany'
) )
assert notification.postage == 'europe' assert notification.postage == 'europe'
assert notification.international
def test_post_letter_notification_international_sets_rest_of_world( def test_post_letter_notification_international_sets_rest_of_world(
client, notify_db_session, mocker client, notify_db_session, mocker
): ):
service = create_service(service_permissions=[LETTER_TYPE, INTERNATIONAL_LETTERS]) service = create_service(service_permissions=[LETTER_TYPE, INTERNATIONAL_LETTERS])
template = create_template(service, template_type="letter") template = create_template(service, template_type="letter")
mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async') mocker.patch('app.celery.tasks.letters_pdf_tasks.get_pdf_for_templated_letter.apply_async')
data = { data = {
'template_id': str(template.id), 'template_id': str(template.id),
'personalisation': { 'personalisation': {
'address_line_1': 'Prince Harry', 'address_line_1': 'Prince Harry',
'address_line_2': 'Toronto', 'address_line_2': 'Toronto',
'address_line_5': 'Canada', 'address_line_5': 'Canada',
}
} }
}
resp_json = letter_request(client, data, service_id=service.id) resp_json = letter_request(client, data, service_id=service.id)
assert validate(resp_json, post_letter_response) == resp_json assert validate(resp_json, post_letter_response) == resp_json
notification = Notification.query.one() notification = Notification.query.one()
assert notification.postage == 'rest-of-world' assert notification.postage == 'rest-of-world'
@pytest.mark.parametrize('permissions, personalisation, expected_error', ( @pytest.mark.parametrize('permissions, personalisation, expected_error', (