From 264fbed04e26bc93881351eb045909e5780b4b70 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 3 Apr 2020 13:13:57 +0100 Subject: [PATCH] Refactor postcode validation to use utils MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t need to reformat the postcode here once template preview takes care of it when rendering the PDF. It’s better (and less code) to store what people give us, so we give them back the same thing. --- app/celery/tasks.py | 9 +-------- app/v2/notifications/post_notifications.py | 10 +++------- requirements-app.txt | 2 +- requirements.txt | 10 +++++----- tests/app/celery/test_tasks.py | 2 +- .../v2/notifications/test_post_letter_notifications.py | 4 +++- 6 files changed, 14 insertions(+), 23 deletions(-) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index ce7f906e7..4a71d7bf2 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -3,10 +3,7 @@ from datetime import datetime from collections import namedtuple, defaultdict from flask import current_app -from notifications_utils.recipients import ( - format_postcode_for_printing, - RecipientCSV -) +from notifications_utils.recipients import RecipientCSV from notifications_utils.statsd_decorators import statsd from notifications_utils.timezones import convert_utc_to_bst from requests import ( @@ -346,10 +343,6 @@ def save_letter( # we store the recipient as just the first item of the person's address recipient = notification['personalisation']['addressline1'] - notification['personalisation']['postcode'] = format_postcode_for_printing( - notification['personalisation']['postcode'] - ) - service = dao_fetch_service_by_id(service_id) template = dao_get_template_by_id(notification['template'], version=notification['template_version']) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index c0c586dd1..bd86fb2a7 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -5,9 +5,8 @@ from datetime import datetime from boto.exception import SQSError from flask import request, jsonify, current_app, abort -from notifications_utils.recipients import ( - format_postcode_for_printing, is_a_real_uk_postcode, try_validate_and_format_phone_number -) +from notifications_utils.postal_address import PostalAddress +from notifications_utils.recipients import try_validate_and_format_phone_number from notifications_utils.template import WithSubjectTemplate from app import ( @@ -349,8 +348,7 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text template=template, reply_to_text=reply_to_text) - postcode = letter_data['personalisation']['postcode'] - if not is_a_real_uk_postcode(postcode): + if not PostalAddress.from_personalisation(letter_data['personalisation']).postcode: raise ValidationError(message='Must be a real UK postcode') test_key = api_key.key_type == KEY_TYPE_TEST @@ -359,8 +357,6 @@ def process_letter_notification(*, letter_data, api_key, template, reply_to_text status = NOTIFICATION_CREATED if not test_key else NOTIFICATION_SENDING queue = QueueNames.CREATE_LETTERS_PDF if not test_key else QueueNames.RESEARCH_MODE - letter_data['personalisation']['postcode'] = format_postcode_for_printing(postcode) - notification = create_letter_notification(letter_data=letter_data, template=template, api_key=api_key, diff --git a/requirements-app.txt b/requirements-app.txt index ea084ac8c..f2aacc001 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -27,4 +27,4 @@ notifications-python-client==5.5.1 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@36.9.3#egg=notifications-utils==36.9.3 +git+https://github.com/alphagov/notifications-utils.git@36.12.0#egg=notifications-utils==36.12.0 diff --git a/requirements.txt b/requirements.txt index 3e301bb2e..2b3051305 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,21 +29,21 @@ notifications-python-client==5.5.1 awscli-cwlogs>=1.4,<1.5 -git+https://github.com/alphagov/notifications-utils.git@36.9.3#egg=notifications-utils==36.9.3 +git+https://github.com/alphagov/notifications-utils.git@36.12.0#egg=notifications-utils==36.12.0 ## The following requirements were added by pip freeze: alembic==1.4.2 amqp==1.4.9 anyjson==0.3.3 attrs==19.3.0 -awscli==1.18.32 +awscli==1.18.37 bcrypt==3.1.7 billiard==3.3.0.23 bleach==3.1.4 boto==2.49.0 boto3==1.10.38 -botocore==1.15.32 -certifi==2019.11.28 +botocore==1.15.37 +certifi==2020.4.5.1 chardet==3.0.4 click==7.1.1 colorama==0.4.3 @@ -81,5 +81,5 @@ smartypants==2.0.1 statsd==3.3.0 urllib3==1.25.8 webencodings==0.5.1 -Werkzeug==1.0.0 +Werkzeug==1.0.1 zipp==3.1.0 diff --git a/tests/app/celery/test_tasks.py b/tests/app/celery/test_tasks.py index d1a60cf75..b0e021c07 100644 --- a/tests/app/celery/test_tasks.py +++ b/tests/app/celery/test_tasks.py @@ -1048,7 +1048,7 @@ def test_save_letter_saves_letter_to_database_with_formatted_postcode(mocker, no notification_db = Notification.query.one() assert notification_db.id == notification_id - assert notification_db.personalisation["postcode"] == "SE16 4SA" + assert notification_db.personalisation["postcode"] == "se1 64sa" def test_save_letter_saves_letter_to_database_right_reply_to(mocker, notify_db_session): diff --git a/tests/app/v2/notifications/test_post_letter_notifications.py b/tests/app/v2/notifications/test_post_letter_notifications.py index fc80ecef7..14c7fb4b6 100644 --- a/tests/app/v2/notifications/test_post_letter_notifications.py +++ b/tests/app/v2/notifications/test_post_letter_notifications.py @@ -139,7 +139,9 @@ def test_post_letter_notification_formats_postcode( assert validate(resp_json, post_letter_response) == resp_json notification = Notification.query.one() - assert notification.personalisation["postcode"] == "SW1 1AA" + # We store what the client gives us, and only reformat it when + # generating the PDF + assert notification.personalisation["postcode"] == ' Sw1 1aa ' def test_post_letter_notification_throws_error_for_bad_postcode(