diff --git a/app/main/forms.py b/app/main/forms.py index 850d5dee2..eca50ab26 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,4 +1,3 @@ -import re import weakref from datetime import datetime, timedelta from itertools import chain @@ -10,17 +9,13 @@ from flask_wtf import FlaskForm as Form from flask_wtf.file import FileAllowed from flask_wtf.file import FileField as FileField_wtf from notifications_utils.columns import Columns -from notifications_utils.formatters import ( - normalise_whitespace_and_newlines, - remove_whitespace_before_punctuation, - strip_whitespace, -) +from notifications_utils.formatters import strip_whitespace +from notifications_utils.postal_address import PostalAddress from notifications_utils.recipients import ( InvalidPhoneError, normalise_phone_number, validate_phone_number, ) -from notifications_utils.take import Take from wtforms import ( BooleanField, DateField, @@ -368,21 +363,10 @@ class StripWhitespaceStringField(StringField): super(StringField, self).__init__(label, **kwargs) -class StripWhitespaceTextAreaField(TextAreaField): +class PostalAddressField(TextAreaField): def process_formdata(self, valuelist): if valuelist: - self.data = Take( - valuelist[0] - ).then( - remove_whitespace_before_punctuation - ).then( - normalise_whitespace_and_newlines - ).then( - # similar to normalise_multiple_newlines but taking everything down to one `\n` instead of two - lambda value: re.compile(r'\n{2,}').sub('\n', value) - ).then( - str.strip - ) + self.data = PostalAddress(valuelist[0]).normalised class OnOffField(RadioField): @@ -765,40 +749,25 @@ class SMSTemplateForm(BaseTemplateForm): class LetterAddressForm(StripWhitespaceForm): - MIN_ADDRESS_LINES = 3 - MAX_ADDRESS_LINES = 7 - address = StripWhitespaceTextAreaField( + address = PostalAddressField( 'Address', validators=[DataRequired(message="Cannot be empty")] ) def validate_address(self, field): - lines = field.data.splitlines() - if len(lines) < self.MIN_ADDRESS_LINES: - raise ValidationError('Address must be at least 3 lines long') - if len(lines) > self.MAX_ADDRESS_LINES: - raise ValidationError('Address must be no more than 7 lines long') - @property - def as_address_lines_1_to_7_with_postcode(self): - lines = self.address.data.splitlines() - placeholders = {} + address = PostalAddress(field.data) - # set all placeholders to empty strings, or all_placeholders_in_session will always return false. - # note that it must be `address line #` with spaces, not underscores or dashes - for i in range(1, 7): - placeholders[f'address line {i}'] = '' + if not address.has_enough_lines: + raise ValidationError( + f'Address must be at least {PostalAddress.MIN_LINES} lines long' + ) - # unroll the address into lines, and place into the session in the underlying placeholder names - # postcode is required so make sure we put the last value in that - # TODO: When postcode is no longer a required field, remove this special case and just use `address line #` - address_lines, last_address_line = lines[:-1], lines[-1] - for i, line in enumerate(address_lines, start=1): - placeholders[f'address line {i}'] = line - placeholders['postcode'] = last_address_line - - return placeholders + if address.has_too_many_lines: + raise ValidationError( + f'Address must be no more than {PostalAddress.MAX_LINES} lines long' + ) class EmailTemplateForm(BaseTemplateForm): diff --git a/app/main/views/send.py b/app/main/views/send.py index a4bcf08cf..5fa68d173 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -18,6 +18,10 @@ from notifications_python_client.errors import HTTPError from notifications_utils import LETTER_MAX_PAGE_COUNT, SMS_CHAR_COUNT_LIMIT from notifications_utils.columns import Columns from notifications_utils.pdf import is_letter_too_long +from notifications_utils.postal_address import ( + PostalAddress, + address_lines_1_to_6_and_postcode_keys, +) from notifications_utils.recipients import ( RecipientCSV, first_column_headings, @@ -335,18 +339,6 @@ def get_notification_check_endpoint(service_id, template): )) -def get_letter_address_from_session(): - """ - Takes non-blank-string address values out of the placeholders, and returns a multi line string - """ - keys = [f'address line {i}' for i in range(1, 7)] + ['postcode'] - return '\n'.join([ - session['placeholders'][key] - for key in keys - if session['placeholders'].get(key) - ]) - - @main.route( "/services//send//one-off/address", methods=['GET', 'POST'] @@ -376,12 +368,14 @@ def send_one_off_letter_address(service_id, template_id): sms_sender=None ) - current_session_address = get_letter_address_from_session() + current_session_address = PostalAddress.from_personalisation( + get_normalised_placeholders_from_session() + ) - form = LetterAddressForm(address=current_session_address) + form = LetterAddressForm(address=current_session_address.normalised) if form.validate_on_submit(): - session['placeholders'].update(form.as_address_lines_1_to_7_with_postcode) + session['placeholders'].update(PostalAddress(form.address.data).as_personalisation) placeholders = fields_to_fill_in( template, @@ -482,7 +476,9 @@ def send_test_step(service_id, template_id, step_index): )) # if we're in a letter, we should show address block rather than "address line #" or "postcode" - if db_template['template_type'] == 'letter' and current_placeholder in first_column_headings['letter']: + if template.template_type == 'letter' and current_placeholder in ( + Columns.from_keys(address_lines_1_to_6_and_postcode_keys) + ): return redirect(url_for('.send_one_off_letter_address', service_id=service_id, template_id=template_id)) optional_placeholder = (current_placeholder in optional_address_columns) @@ -878,10 +874,7 @@ def fields_to_fill_in(template, prefill_current_user=False): def get_normalised_placeholders_from_session(): - return { - key: ''.join(value or []) - for key, value in session.get('placeholders', {}).items() - } + return Columns(session.get('placeholders', {})) def get_recipient_and_placeholders_from_session(template_type): diff --git a/requirements-app.txt b/requirements-app.txt index 9967d53d3..0c386fd24 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -22,5 +22,5 @@ notifications-python-client==5.5.1 awscli-cwlogs>=1.4,<1.5 itsdangerous==1.1.0 -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 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.1-alpha#egg=govuk-frontend-jinja==0.5.1-alpha diff --git a/requirements.txt b/requirements.txt index 19e54be1e..5c110203d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -24,15 +24,15 @@ notifications-python-client==5.5.1 awscli-cwlogs>=1.4,<1.5 itsdangerous==1.1.0 -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 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.1-alpha#egg=govuk-frontend-jinja==0.5.1-alpha ## The following requirements were added by pip freeze: -awscli==1.18.33 +awscli==1.18.36 bleach==3.1.4 boto3==1.10.38 -botocore==1.15.33 -certifi==2019.11.28 +botocore==1.15.36 +certifi==2020.4.5.1 chardet==3.0.4 click==7.1.1 colorama==0.4.3 diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 84398c3e3..8547bec48 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2115,7 +2115,7 @@ def test_send_test_works_as_letter_preview( assert response.get_data(as_text=True) == 'foo' assert mocked_preview.call_args[0][0].id == template_id assert type(mocked_preview.call_args[0][0]) == LetterImageTemplate - assert mocked_preview.call_args[0][0].values == {'address_line_1': 'Jo Lastname'} + assert mocked_preview.call_args[0][0].values == {'addressline1': 'Jo Lastname'} assert mocked_preview.call_args[0][1] == filetype @@ -2205,33 +2205,36 @@ def test_send_one_off_letter_address_shows_form( @pytest.mark.parametrize(['form_data', 'expected_placeholders'], [ # minimal ('\n'.join(['a', 'b', 'c']), { - 'address line 1': 'a', - 'address line 2': 'b', - 'address line 3': '', - 'address line 4': '', - 'address line 5': '', - 'address line 6': '', + 'address_line_1': 'a', + 'address_line_2': 'b', + 'address_line_3': '', + 'address_line_4': '', + 'address_line_5': '', + 'address_line_6': '', + 'address_line_7': 'c', 'postcode': 'c', }), # maximal ('\n'.join(['a', 'b', 'c', 'd', 'e', 'f', 'g']), { - 'address line 1': 'a', - 'address line 2': 'b', - 'address line 3': 'c', - 'address line 4': 'd', - 'address line 5': 'e', - 'address line 6': 'f', + 'address_line_1': 'a', + 'address_line_2': 'b', + 'address_line_3': 'c', + 'address_line_4': 'd', + 'address_line_5': 'e', + 'address_line_6': 'f', + 'address_line_7': 'g', 'postcode': 'g', }), # it ignores empty lines and strips whitespace from each line. # It also strips extra whitespace from the middle of lines. ('\n a\ta \n\n\n \n\n\n\nb b \r\nc', { - 'address line 1': 'a\ta', - 'address line 2': 'b b', - 'address line 3': '', - 'address line 4': '', - 'address line 5': '', - 'address line 6': '', + 'address_line_1': 'a\ta', + 'address_line_2': 'b b', + 'address_line_3': '', + 'address_line_4': '', + 'address_line_5': '', + 'address_line_6': '', + 'address_line_7': 'c', 'postcode': 'c', }), ])