darkhelm/notifications-admin
1
0
Fork 0
mirror of https://github.com/GSA/notifications-admin.git synced 2026-07-17 21:20:32 -04:00
Code Issues Packages Projects Releases Wiki Activity

Refactor to use PostalAddress helper from utils

Browse Source 
Since we’re doing normalisation and line-count-checking of addresses in
multiple places it makes sense for that code to be shared. Which is
what happened here:
https://github.com/alphagov/notifications-utils/pull/713

This commit refactors the admin code to make use of the new utils code.

Note about placeholders:
- they now go into the session as `address_line_1` instead of `address
  line 1` because this is the format the API uses, so should be
  considered canonical
- they are now fetched from the session in a way that isn’t sensitive
  to case or underscores (using the `Columns` class)
- the API doesn’t care about case or underscores vs spaces in
  placeholder names because it’s checking an instance of `Template` to
  see if all the required placeholders are present (see
  401c8e41d6/app/notifications/process_notifications.py (L40))
This commit is contained in:
Chris Hill-Scott
2020-04-02 17:21:27 +01:00
parent 32eef2f203
commit 26f702ebce
5 changed files with 54 additions and 89 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
app/main/forms.py
View File

@@ -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):

33
app/main/views/send.py
View File

@@ -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/<uuid:service_id>/send/<uuid:template_id>/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):

2
requirements-app.txt
View File

@@ -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

8
requirements.txt
View File

@@ -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

41
tests/app/main/views/test_send.py
View File

@@ -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',
}),
])