From 8302b2b667eee34386ffdcb224d8c1b9b926c367 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Dec 2020 13:56:24 +0000 Subject: [PATCH 1/2] Validate length of broadcast content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Depends on: - [ ] https://github.com/alphagov/notifications-utils/pull/826/files Adds error messages for when the content of a broadcast template is too long. The error message is explicit when this is cause by non-GSM characters. We may not want to expose this complexity to our users, but it’s useful for now while we’re testing things out. --- app/main/forms.py | 2 ++ app/main/validators.py | 23 +++++++++++++++++++ requirements-app.txt | 2 +- requirements.txt | 2 +- tests/app/main/views/test_templates.py | 31 ++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 2 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index b276f8215..823137497 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -40,6 +40,7 @@ from wtforms.validators import URL, DataRequired, Length, Optional, Regexp from app import format_thousands from app.main.validators import ( + BroadcastLength, CommonlyUsedPassword, CsvFileValidator, DoesNotStartWithDoubleZero, @@ -1324,6 +1325,7 @@ class BroadcastTemplateForm(SMSTemplateForm): def validate_template_content(self, field): OnlySMSCharacters(template_type='broadcast')(None, field) NoPlaceholders()(None, field) + BroadcastLength()(None, field) class LetterAddressForm(StripWhitespaceForm): diff --git a/app/main/validators.py b/app/main/validators.py index 104b4a932..8e091b0f7 100644 --- a/app/main/validators.py +++ b/app/main/validators.py @@ -7,6 +7,7 @@ from notifications_utils.recipients import ( validate_email_address, ) from notifications_utils.sanitise_text import SanitiseSMS +from notifications_utils.template import BroadcastMessageTemplate from wtforms import ValidationError from app.main._commonly_used_passwords import commonly_used_passwords @@ -120,6 +121,28 @@ class NoPlaceholders: raise ValidationError(self.message) +class BroadcastLength: + + def __call__(self, form, field): + template = BroadcastMessageTemplate({ + 'template_type': 'broadcast', + 'content': field.data, + }) + + if template.content_too_long: + non_gsm_characters = list(sorted(template.non_gsm_characters)) + if non_gsm_characters: + raise ValidationError( + f'Content must be {template.max_content_count:,.0f} ' + f'characters or fewer because it contains ' + f'{formatted_list(non_gsm_characters, conjunction="and", before_each="", after_each="")}' + ) + raise ValidationError( + f'Content must be {template.max_content_count:,.0f} ' + f'characters or fewer' + ) + + class LettersNumbersFullStopsAndUnderscoresOnly: regex = re.compile(r'^[a-zA-Z0-9\s\._]+$') diff --git a/requirements-app.txt b/requirements-app.txt index da31bf0d3..a18e37063 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -24,7 +24,7 @@ Shapely==1.7.1 awscli-cwlogs>=1.4,<1.5 itsdangerous==1.1.0 -git+https://github.com/alphagov/notifications-utils.git@43.5.8#egg=notifications-utils==43.5.8 +git+https://github.com/alphagov/notifications-utils.git@43.6.0#egg=notifications-utils==43.6.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.1-alpha#egg=govuk-frontend-jinja==0.5.1-alpha # gds-metrics requires prometheseus 0.2.0, override that requirement as later versions bring significant performance gains diff --git a/requirements.txt b/requirements.txt index 2f41e058e..f824f7542 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,7 +26,7 @@ Shapely==1.7.1 awscli-cwlogs>=1.4,<1.5 itsdangerous==1.1.0 -git+https://github.com/alphagov/notifications-utils.git@43.5.8#egg=notifications-utils==43.5.8 +git+https://github.com/alphagov/notifications-utils.git@43.6.0#egg=notifications-utils==43.6.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.5.1-alpha#egg=govuk-frontend-jinja==0.5.1-alpha # gds-metrics requires prometheseus 0.2.0, override that requirement as later versions bring significant performance gains diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 5560622c8..c7aac09a7 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1865,6 +1865,37 @@ def test_should_not_update_too_big_template( assert "Content has a character count greater than the limit of 459" in page.text +@pytest.mark.parametrize('content, expected_error', ( + (("ŴŶ" * 308), ( + 'Content must be 615 characters or fewer because it contains Ŵ and Ŷ' + )), + (("ab" * 698), ( + 'Content must be 1,395 characters or fewer' + )), +)) +def test_should_not_create_too_big_template_for_broadcasts( + client_request, + service_one, + content, + expected_error, +): + service_one['permissions'] = ['broadcast'] + page = client_request.post( + '.add_service_template', + service_id=SERVICE_ONE_ID, + template_type='broadcast', + _data={ + 'name': 'New name', + 'template_content': content, + 'template_type': 'broadcast', + 'service': SERVICE_ONE_ID, + 'process_type': 'normal' + }, + _expected_status=200, + ) + assert normalize_spaces(page.select_one('.error-message').text) == expected_error + + def test_should_redirect_when_saving_a_template_email( client_request, mock_get_service_email_template, From 810dc03628ab8e2c7cfc89b01829f1a72a7d1dd9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 24 Dec 2020 14:10:37 +0000 Subject: [PATCH 2/2] Make broadcast template editor expand to fit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s fiddly having to scroll within a small textbox to see all the content. Let’s make the box expand to fit the contents like we do elsewhere. This was removed by accident when we stopped highlighting placeholders in broadcast templates in https://github.com/alphagov/notifications-admin/pull/3672/files --- app/templates/views/edit-broadcast-template.html | 2 +- tests/app/main/views/test_templates.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/templates/views/edit-broadcast-template.html b/app/templates/views/edit-broadcast-template.html index 8c5d7cfe9..21b0374d3 100644 --- a/app/templates/views/edit-broadcast-template.html +++ b/app/templates/views/edit-broadcast-template.html @@ -24,7 +24,7 @@ }) }}
- {{ textbox(form.template_content, highlight_placeholders=False, width='1-1', rows=5) }} + {{ textbox(form.template_content, highlight_placeholders=False, autosize=True, width='1-1', rows=5) }} {{ sticky_page_footer('Save') }}
diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index c7aac09a7..a45fcb06e 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -480,6 +480,7 @@ def test_broadcast_template_doesnt_highlight_placeholders( service_id=SERVICE_ONE_ID, template_id=fake_uuid, ) + assert page.select_one('textarea')['data-module'] == 'enhanced-textbox' assert page.select_one('textarea')['data-highlight-placeholders'] == 'false'