From 579ae72abba0769341347576d6e367f8d0c23f49 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 27 Sep 2019 16:52:09 +0100 Subject: [PATCH 01/10] Do not allow to send a letter template longer than 10 pages --- app/main/views/templates.py | 1 + app/templates/views/templates/_template.html | 2 +- tests/app/main/views/test_templates.py | 24 ++++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 7a5923e13..3a62d61fb 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -74,6 +74,7 @@ def view_template(service_id, template_id): ), template_postage=template["postage"], user_has_template_permission=user_has_template_permission, + page_count=get_page_count_for_letter(template), ) diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index a6d907a23..c46259faa 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -15,7 +15,7 @@
{% if template.template_type == 'letter' %} - {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) %} + {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) and page_count < 11 %}
Send diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index b9e0c041e..b6b2ceb52 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -521,6 +521,30 @@ def test_view_non_letter_template_does_not_display_postage( assert "Postage" not in page.text +def test_view_letter_template_does_not_display_send_button_if_template_over_10_pages_long( + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, + single_letter_contact_block, + mock_has_jobs, + active_user_with_permissions, + mocker, + fake_uuid, +): + mocker.patch('app.main.views.templates.get_page_count_for_letter', return_value=11) + client_request.login(active_user_with_permissions) + mock_get_service_letter_template(mocker, postage="second") + page = client_request.get( + 'main.view_template', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _test_page_title=False, + ) + + assert "Send" not in page.text + + def test_edit_letter_template_postage_page_displays_correctly( client_request, service_one, From 0adf80f2947423997a7346cd9f1a7422fdc89104 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 27 Sep 2019 17:54:44 +0100 Subject: [PATCH 02/10] Do not allow to send uploaded pdf letters longer than 10 pages --- app/main/views/uploads.py | 2 ++ tests/app/main/views/test_uploads.py | 33 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 31d3a4f46..b54f754f4 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -79,6 +79,8 @@ def upload_letter(service_id): raise ex else: status = 'valid' + if page_count > 10: + status = 'invalid' file_contents = base64.b64decode(response.json()['file'].encode()) upload_letter_to_s3( file_contents, diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index e58031e0f..8fdd1ec94 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -204,6 +204,39 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): assert not page.find('button', {'type': 'submit'}) +def test_post_upload_letter_with_letter_that_is_too_long(mocker, client_request): + letter_template = {'template_type': 'letter', + 'reply_to_text': '', + 'postage': 'second', + 'subject': 'hi', + 'content': 'my letter'} + + mocker.patch('uuid.uuid4', return_value='fake-uuid') + mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) + mocker.patch( + 'app.main.views.uploads.sanitise_letter', + return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'}) + ) + mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mock_page_count = mocker.patch('app.main.views.uploads.pdf_page_count', return_value=11) + mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) + + with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: + page = client_request.post( + 'main.upload_letter', + service_id=SERVICE_ONE_ID, + _data={'file': file}, + _follow_redirects=True, + ) + + assert mock_page_count.called + + assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' + assert page.find("div", {"class": "banner-dangerous bottom-gutter"}) + assert "This letter is too long" in page.text + assert not page.find('button', {'type': 'submit'}) + + def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client_request): letter_template = {'template_type': 'letter', 'reply_to_text': '', From c690434b1ff1ccae7054d3b3ac013abbaf77ccb4 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 1 Oct 2019 14:31:36 +0100 Subject: [PATCH 03/10] One off letter flow does not allow to send letter longer than 10 pages --- app/main/views/send.py | 4 +- app/templates/views/notifications/check.html | 2 +- tests/app/main/views/test_send.py | 43 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index c6d720be1..1b42eca71 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -850,6 +850,7 @@ def _check_notification(service_id, template_id, exception=None): email_reply_to = get_email_reply_to_address_from_session() elif db_template['template_type'] == 'sms': sms_sender = get_sms_sender_from_session() + page_count = get_page_count_for_letter(db_template) template = get_template( db_template, current_service, @@ -862,7 +863,7 @@ def _check_notification(service_id, template_id, exception=None): template_id=template_id, filetype='png', ), - page_count=get_page_count_for_letter(db_template), + page_count=page_count, ) back_link = get_back_link(service_id, template, len(fields_to_fill_in(template))) @@ -881,6 +882,7 @@ def _check_notification(service_id, template_id, exception=None): template=template, back_link=back_link, help=get_help_argument(), + page_count=page_count, **(get_template_error_dict(exception) if exception else {}), ) diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html index bdf55ecf4..b6ef62927 100644 --- a/app/templates/views/notifications/check.html +++ b/app/templates/views/notifications/check.html @@ -66,7 +66,7 @@ help='3' if help else 0 )}}" class='page-footer'> - {% if not error %} + {% if not error and page_count < 11 %} {% endif %} {% if template.template_type == 'letter' %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index c63c9fe29..1597495b3 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -3066,6 +3066,49 @@ def test_send_one_off_letter_errors_in_trial_mode( assert page.select_one('a[download]').text == 'Download as a PDF' +def test_send_one_off_letter_errors_if_letter_longer_than_10_pages( + client_request, + mocker, + mock_get_live_service, + mock_get_service_letter_template, + mock_has_permissions, + fake_uuid, + mock_get_users_by_service, + mock_get_service_statistics, + mock_get_job_doesnt_exist, + mock_s3_set_metadata, +): + + mocker.patch( + 'app.main.views.send.get_page_count_for_letter', + return_value=11, + ) + + with client_request.session_transaction() as session: + session['recipient'] = None + session['placeholders'] = { + 'address_line_1': 'First Last', + 'address_line_2': '123 Street', + 'postcode': 'SW1 1AA', + } + + page = client_request.get( + 'main.check_notification', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _test_page_title=False, + ) + + assert normalize_spaces(page.select('.banner-dangerous')) == normalize_spaces( + 'This letter is too long ' + 'Letters must be 10 pages or less' + ) + + assert len(page.select('.letter img')) == 10 + + assert not page.select('[type=submit]') + + def test_check_messages_shows_over_max_row_error( client_request, mock_get_users_by_service, From 028d156dc7949a6b25d0e1c2257cfc7e00f917d1 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 1 Oct 2019 15:34:46 +0100 Subject: [PATCH 04/10] Do not allow to send job of letters if letters longer than 10 pages --- app/main/views/send.py | 22 +++++++------ app/templates/views/check/ok.html | 2 +- tests/app/main/views/test_send.py | 52 +++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 11 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 1b42eca71..a51dae2c9 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -522,6 +522,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ email_reply_to = get_email_reply_to_address_from_session() elif db_template['template_type'] == 'sms': sms_sender = get_sms_sender_from_session() + page_count = get_page_count_for_letter(db_template) template = get_template( db_template, current_service, @@ -536,7 +537,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ ) if not letters_as_pdf else None, email_reply_to=email_reply_to, sms_sender=sms_sender, - page_count=get_page_count_for_letter(db_template), + page_count=page_count, ) recipients = RecipientCSV( contents, @@ -589,7 +590,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ preview_row=preview_row, sent_previously=job_api_client.has_sent_previously( service_id, template.id, db_template['version'], request.args.get('original_file_name', '') - ) + ), + page_count=page_count ) @@ -601,12 +603,12 @@ def check_messages(service_id, template_id, upload_id, row_index=2): data = _check_messages(service_id, template_id, upload_id, row_index) if ( - data['recipients'].too_many_rows or - not data['count_of_recipients'] or - not data['recipients'].has_recipient_columns or - data['recipients'].duplicate_recipient_column_headers or - data['recipients'].missing_column_headers or - data['sent_previously'] + data['recipients'].too_many_rows + or not data['count_of_recipients'] + or not data['recipients'].has_recipient_columns + or data['recipients'].duplicate_recipient_column_headers + or data['recipients'].missing_column_headers + or data['sent_previously'] ): return render_template('views/check/column-errors.html', **data) @@ -614,8 +616,8 @@ def check_messages(service_id, template_id, upload_id, row_index=2): return render_template('views/check/row-errors.html', **data) if ( - data['errors'] or - data['trying_to_send_letters_in_trial_mode'] + data['errors'] + or data['trying_to_send_letters_in_trial_mode'] ): return render_template('views/check/column-errors.html', **data) diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html index 693208bf9..b6374ee00 100644 --- a/app/templates/views/check/ok.html +++ b/app/templates/views/check/ok.html @@ -39,7 +39,7 @@ wrapping_class='bottom-gutter-2-3' ) }} {% endif %} - {% if template.template_type != 'letter' or not request.args.from_test %} + {% if (template.template_type != 'letter' or not request.args.from_test) and page_count < 11 %} {% else %} Download as a PDF diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 1597495b3..2f8c0e980 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -2735,6 +2735,58 @@ def test_check_messages_shows_trial_mode_error_for_letters( assert page.select_one('.table-field-index a').text == '3' +@pytest.mark.parametrize('number_of_rows, expected_error_message', [ + (1, 'This letter is'), + (11, 'These letters are'), # TODO: Pluralise too many pages error message for multiple letters +]) +def test_check_messages_does_not_allow_to_send_letter_longer_than_10_pages( + client_request, + api_user_active, + mock_get_service_letter_template, + mock_has_permissions, + mock_get_users_by_service, + mock_get_service_statistics, + mock_get_job_doesnt_exist, + mock_get_jobs, + mock_s3_set_metadata, + fake_uuid, + mocker, + mock_get_live_service, + number_of_rows, + expected_error_message, +): + mocker.patch('app.main.views.send.s3download', return_value='\n'.join( + ['address_line_1,address_line_2,postcode,'] + + ['First Last, 123 Street, SW1 1AA'] * number_of_rows + )) + mocker.patch( + 'app.main.views.send.get_page_count_for_letter', + return_value=11, + ) + + with client_request.session_transaction() as session: + session['file_uploads'] = { + fake_uuid: { + 'template_id': '', + } + } + + page = client_request.get( + 'main.check_messages', + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + upload_id=fake_uuid, + _test_page_title=False, + ) + + error = page.select('.banner-dangerous') + + assert normalize_spaces(error[0].text) == 'This letter is too long Letters must be 10 pages or less' + + assert len(page.select('.letter img')) == 10 # if letter longer than 10 pages, only 10 first pages are displayed + assert not page.select('[type=submit]') + + def test_check_messages_shows_data_errors_before_trial_mode_errors_for_letters( mocker, client_request, From b42c7c4c9ff25e46cb1d7e9046d3f70f3bea79a9 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 1 Oct 2019 16:03:35 +0100 Subject: [PATCH 05/10] Refactor page_count checks to avoid magic numbers --- app/config.py | 1 + app/main/views/send.py | 4 +++- app/main/views/templates.py | 11 ++++++++++- app/main/views/uploads.py | 2 +- app/templates/views/check/ok.html | 2 +- app/templates/views/notifications/check.html | 2 +- app/templates/views/templates/_template.html | 2 +- 7 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/config.py b/app/config.py index aa4f0eaeb..903e53b83 100644 --- a/app/config.py +++ b/app/config.py @@ -53,6 +53,7 @@ class Config(object): EMAIL_2FA_EXPIRY_SECONDS = 1800 # 30 Minutes HEADER_COLOUR = '#FFBF47' # $yellow HTTP_PROTOCOL = 'http' + LETTER_MAX_PAGES = 10 MAX_FAILED_LOGIN_COUNT = 10 NOTIFY_APP_NAME = 'admin' NOTIFY_LOG_LEVEL = 'DEBUG' diff --git a/app/main/views/send.py b/app/main/views/send.py index a51dae2c9..8bc75281a 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -591,7 +591,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ sent_previously=job_api_client.has_sent_previously( service_id, template.id, db_template['version'], request.args.get('original_file_name', '') ), - page_count=page_count + page_count=page_count, + letter_max_pages=current_app.config['LETTER_MAX_PAGES'], ) @@ -885,6 +886,7 @@ def _check_notification(service_id, template_id, exception=None): back_link=back_link, help=get_help_argument(), page_count=page_count, + letter_max_pages=current_app.config['LETTER_MAX_PAGES'], **(get_template_error_dict(exception) if exception else {}), ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 3a62d61fb..7ba58b014 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -2,7 +2,15 @@ from datetime import datetime, timedelta from string import ascii_uppercase from dateutil.parser import parse -from flask import abort, flash, redirect, render_template, request, url_for +from flask import ( + abort, + current_app, + flash, + redirect, + render_template, + request, + url_for, +) from flask_login import current_user from markupsafe import Markup from notifications_python_client.errors import HTTPError @@ -75,6 +83,7 @@ def view_template(service_id, template_id): template_postage=template["postage"], user_has_template_permission=user_has_template_permission, page_count=get_page_count_for_letter(template), + letter_max_pages=current_app.config['LETTER_MAX_PAGES'], ) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index b54f754f4..4d8614b77 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -79,7 +79,7 @@ def upload_letter(service_id): raise ex else: status = 'valid' - if page_count > 10: + if page_count > current_app.config['LETTER_MAX_PAGES']: status = 'invalid' file_contents = base64.b64decode(response.json()['file'].encode()) upload_letter_to_s3( diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html index b6374ee00..047725baf 100644 --- a/app/templates/views/check/ok.html +++ b/app/templates/views/check/ok.html @@ -39,7 +39,7 @@ wrapping_class='bottom-gutter-2-3' ) }} {% endif %} - {% if (template.template_type != 'letter' or not request.args.from_test) and page_count < 11 %} + {% if (template.template_type != 'letter' or not request.args.from_test) and (not page_count or page_count <= letter_max_pages) %} {% else %} Download as a PDF diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html index b6ef62927..dc5ea647f 100644 --- a/app/templates/views/notifications/check.html +++ b/app/templates/views/notifications/check.html @@ -66,7 +66,7 @@ help='3' if help else 0 )}}" class='page-footer'> - {% if not error and page_count < 11 %} + {% if not error and (not page_count or page_count <= letter_max_pages) %} {% endif %} {% if template.template_type == 'letter' %} diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index c46259faa..af1496616 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -15,7 +15,7 @@
{% if template.template_type == 'letter' %} - {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) and page_count < 11 %} + {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) and page_count <= letter_max_pages %}
Send From 12ec2870af032938f43b608280a51d3b292305a5 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Tue, 1 Oct 2019 17:16:15 +0100 Subject: [PATCH 06/10] Move letter too long banner message over from utils, also refactor --- app/config.py | 1 - app/main/views/send.py | 7 +++---- app/main/views/templates.py | 14 +++----------- app/main/views/uploads.py | 6 +++--- app/templates/components/banner.html | 9 ++++++--- app/templates/partials/check/letter-too-long.html | 6 ++++++ app/templates/views/check/ok.html | 9 +++++++-- app/templates/views/notifications/check.html | 10 +++++++++- app/templates/views/templates/_template.html | 8 +++++++- app/templates/views/uploads/preview.html | 9 +++++++-- app/utils.py | 8 ++++++++ tests/app/main/views/test_send.py | 10 ++-------- tests/app/main/views/test_templates.py | 1 + tests/app/main/views/test_uploads.py | 3 +-- 14 files changed, 63 insertions(+), 38 deletions(-) create mode 100644 app/templates/partials/check/letter-too-long.html diff --git a/app/config.py b/app/config.py index 903e53b83..aa4f0eaeb 100644 --- a/app/config.py +++ b/app/config.py @@ -53,7 +53,6 @@ class Config(object): EMAIL_2FA_EXPIRY_SECONDS = 1800 # 30 Minutes HEADER_COLOUR = '#FFBF47' # $yellow HTTP_PROTOCOL = 'http' - LETTER_MAX_PAGES = 10 MAX_FAILED_LOGIN_COUNT = 10 NOTIFY_APP_NAME = 'admin' NOTIFY_LOG_LEVEL = 'DEBUG' diff --git a/app/main/views/send.py b/app/main/views/send.py index 8bc75281a..a11a70ba7 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -54,6 +54,7 @@ from app.utils import ( get_errors_for_csv, get_help_argument, get_template, + is_letter_too_long, should_skip_template_page, unicode_truncate, user_has_permissions, @@ -591,8 +592,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ sent_previously=job_api_client.has_sent_previously( service_id, template.id, db_template['version'], request.args.get('original_file_name', '') ), - page_count=page_count, - letter_max_pages=current_app.config['LETTER_MAX_PAGES'], + letter_too_long=is_letter_too_long(page_count), ) @@ -885,8 +885,7 @@ def _check_notification(service_id, template_id, exception=None): template=template, back_link=back_link, help=get_help_argument(), - page_count=page_count, - letter_max_pages=current_app.config['LETTER_MAX_PAGES'], + letter_too_long=is_letter_too_long(page_count), **(get_template_error_dict(exception) if exception else {}), ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 7ba58b014..5bb34deb4 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -2,15 +2,7 @@ from datetime import datetime, timedelta from string import ascii_uppercase from dateutil.parser import parse -from flask import ( - abort, - current_app, - flash, - redirect, - render_template, - request, - url_for, -) +from flask import abort, flash, redirect, render_template, request, url_for from flask_login import current_user from markupsafe import Markup from notifications_python_client.errors import HTTPError @@ -41,6 +33,7 @@ from app.template_previews import TemplatePreview, get_page_count_for_letter from app.utils import ( email_or_sms_not_enabled, get_template, + is_letter_too_long, should_skip_template_page, user_has_permissions, user_is_platform_admin, @@ -82,8 +75,7 @@ def view_template(service_id, template_id): ), template_postage=template["postage"], user_has_template_permission=user_has_template_permission, - page_count=get_page_count_for_letter(template), - letter_max_pages=current_app.config['LETTER_MAX_PAGES'], + letter_too_long=is_letter_too_long(get_page_count_for_letter(template)), ) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 4d8614b77..1d010a224 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -26,7 +26,7 @@ from app.s3_client.s3_letter_upload_client import ( upload_letter_to_s3, ) from app.template_previews import TemplatePreview, sanitise_letter -from app.utils import get_template, user_has_permissions +from app.utils import get_template, is_letter_too_long, user_has_permissions MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @@ -79,8 +79,6 @@ def upload_letter(service_id): raise ex else: status = 'valid' - if page_count > current_app.config['LETTER_MAX_PAGES']: - status = 'invalid' file_contents = base64.b64decode(response.json()['file'].encode()) upload_letter_to_s3( file_contents, @@ -111,6 +109,7 @@ def uploaded_letter_preview(service_id, file_id): metadata = get_letter_metadata(service_id, file_id) original_filename = metadata.get('filename') page_count = metadata.get('page_count') + letter_too_long = is_letter_too_long(int(page_count)) status = metadata.get('status') template_dict = service_api_client.get_precompiled_template(service_id) @@ -132,6 +131,7 @@ def uploaded_letter_preview(service_id, file_id): template=template, status=status, file_id=file_id, + letter_too_long=letter_too_long, ) diff --git a/app/templates/components/banner.html b/app/templates/components/banner.html index 3b30cf1cb..1a272fa55 100644 --- a/app/templates/components/banner.html +++ b/app/templates/components/banner.html @@ -1,12 +1,15 @@ {% from "components/form.html" import form_wrapper %} -{% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None, context=None, action=None) %} +{% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None, context=None, action=None, id=None) %}
{% if subhead -%}

{{ subhead }}

@@ -26,6 +29,6 @@
{% endmacro %} -{% macro banner_wrapper(type=None, with_tick=False, delete_button=None, subhead=None, action=None) %} - {{ banner(caller()|safe, type=type, with_tick=with_tick, delete_button=delete_button, subhead=subhead, action=action) }} +{% macro banner_wrapper(type=None, with_tick=False, delete_button=None, subhead=None, action=None, id=None) %} + {{ banner(caller()|safe, type=type, with_tick=with_tick, delete_button=delete_button, subhead=subhead, action=action, id=id) }} {% endmacro %} diff --git a/app/templates/partials/check/letter-too-long.html b/app/templates/partials/check/letter-too-long.html new file mode 100644 index 000000000..58e257f8f --- /dev/null +++ b/app/templates/partials/check/letter-too-long.html @@ -0,0 +1,6 @@ +

+ This letter is too long +

+

+ Letters must be {{ letter_max_pages }} pages or fewer +

diff --git a/app/templates/views/check/ok.html b/app/templates/views/check/ok.html index 047725baf..d726395a9 100644 --- a/app/templates/views/check/ok.html +++ b/app/templates/views/check/ok.html @@ -25,10 +25,15 @@ back_link=back_link ) }} + {% if letter_too_long %} + {% call banner_wrapper(type='dangerous', id='letter-too-long') %} + {% include "partials/check/letter-too-long.html" %} + {% endcall %} + {% endif %} + {{ skip_to_file_contents() }} {{ template|string }} -
@@ -39,7 +44,7 @@ wrapping_class='bottom-gutter-2-3' ) }} {% endif %} - {% if (template.template_type != 'letter' or not request.args.from_test) and (not page_count or page_count <= letter_max_pages) %} + {% if (template.template_type != 'letter' or not request.args.from_test) and not letter_too_long %} {% else %} Download as a PDF diff --git a/app/templates/views/notifications/check.html b/app/templates/views/notifications/check.html index dc5ea647f..9fc2ff20a 100644 --- a/app/templates/views/notifications/check.html +++ b/app/templates/views/notifications/check.html @@ -49,6 +49,14 @@ {% include "partials/check/message-too-long.html" %} {% endcall %}
+ {% elif letter_too_long %} + {% set error = 'letter-too-long' %} + {{ govuk_back_link(back_link) }} +
+ {% call banner_wrapper(type='dangerous', id='letter-too-long') %} + {% include "partials/check/letter-too-long.html" %} + {% endcall %} +
{% else %} {{ page_header( 'Preview of ‘{}’'.format(template.name), @@ -66,7 +74,7 @@ help='3' if help else 0 )}}" class='page-footer'> - {% if not error and (not page_count or page_count <= letter_max_pages) %} + {% if not error %} {% endif %} {% if template.template_type == 'letter' %} diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index af1496616..f1112724b 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -1,4 +1,5 @@ {% from 'components/message-count-label.html' import message_count_label %} +{% from "components/banner.html" import banner_wrapper %}
{% if template._template.archived %} @@ -15,7 +16,12 @@
{% if template.template_type == 'letter' %} - {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) and page_count <= letter_max_pages %} + {% if letter_too_long %} + {% call banner_wrapper(type='dangerous', id='letter-too-long') %} + {% include "partials/check/letter-too-long.html" %} + {% endcall %} + {% endif %} + {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) and not letter_too_long %}
Send diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index 5642540e0..b841dca52 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -1,5 +1,6 @@ {% extends "withnav_template.html" %} {% from "components/page-header.html" import page_header %} +{% from "components/banner.html" import banner_wrapper %} {% block service_page_title %} {{ original_filename }} @@ -16,12 +17,16 @@ Validation failed

{% endif %} - + {% if letter_too_long %} + {% call banner_wrapper(type='dangerous', id='letter-too-long') %} + {% include "partials/check/letter-too-long.html" %} + {% endcall %} + {% endif %}
{{ template|string }}
- {% if status == 'valid' %} + {% if status == 'valid' and not letter_too_long %}
Date: Tue, 8 Oct 2019 14:56:00 +0100 Subject: [PATCH 07/10] Check page count of actual notification not of template But for jobs we are only checking preview row, otherwise it would be too slow. We will check other row when creating the pdf --- app/main/views/send.py | 13 +++++++++---- app/main/views/templates.py | 2 ++ app/main/views/uploads.py | 8 +++++++- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index a11a70ba7..e8805e80e 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -48,6 +48,7 @@ from app.s3_client.s3_csv_client import ( ) from app.template_previews import TemplatePreview, get_page_count_for_letter from app.utils import ( + LETTER_MAX_PAGES, PermanentRedirect, Spreadsheet, email_or_sms_not_enabled, @@ -523,7 +524,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ email_reply_to = get_email_reply_to_address_from_session() elif db_template['template_type'] == 'sms': sms_sender = get_sms_sender_from_session() - page_count = get_page_count_for_letter(db_template) + template = get_template( db_template, current_service, @@ -538,7 +539,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ ) if not letters_as_pdf else None, email_reply_to=email_reply_to, sms_sender=sms_sender, - page_count=page_count, + page_count=get_page_count_for_letter(db_template), ) recipients = RecipientCSV( contents, @@ -569,6 +570,8 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ elif preview_row > 2: abort(404) + page_count = get_page_count_for_letter(db_template, template.values) + return dict( recipients=recipients, template=template, @@ -593,6 +596,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ service_id, template.id, db_template['version'], request.args.get('original_file_name', '') ), letter_too_long=is_letter_too_long(page_count), + letter_max_pages=LETTER_MAX_PAGES, ) @@ -853,7 +857,6 @@ def _check_notification(service_id, template_id, exception=None): email_reply_to = get_email_reply_to_address_from_session() elif db_template['template_type'] == 'sms': sms_sender = get_sms_sender_from_session() - page_count = get_page_count_for_letter(db_template) template = get_template( db_template, current_service, @@ -866,7 +869,7 @@ def _check_notification(service_id, template_id, exception=None): template_id=template_id, filetype='png', ), - page_count=page_count, + page_count=get_page_count_for_letter(db_template), ) back_link = get_back_link(service_id, template, len(fields_to_fill_in(template))) @@ -881,11 +884,13 @@ def _check_notification(service_id, template_id, exception=None): raise PermanentRedirect(back_link) template.values = get_recipient_and_placeholders_from_session(template.template_type) + page_count = get_page_count_for_letter(db_template, template.values) return dict( template=template, back_link=back_link, help=get_help_argument(), letter_too_long=is_letter_too_long(page_count), + letter_max_pages=LETTER_MAX_PAGES, **(get_template_error_dict(exception) if exception else {}), ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 5bb34deb4..afb57deb7 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -31,6 +31,7 @@ from app.models.service import Service from app.models.template_list import TemplateList, TemplateLists from app.template_previews import TemplatePreview, get_page_count_for_letter from app.utils import ( + LETTER_MAX_PAGES, email_or_sms_not_enabled, get_template, is_letter_too_long, @@ -76,6 +77,7 @@ def view_template(service_id, template_id): template_postage=template["postage"], user_has_template_permission=user_has_template_permission, letter_too_long=is_letter_too_long(get_page_count_for_letter(template)), + letter_max_pages=LETTER_MAX_PAGES, ) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 1d010a224..346226843 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -26,7 +26,12 @@ from app.s3_client.s3_letter_upload_client import ( upload_letter_to_s3, ) from app.template_previews import TemplatePreview, sanitise_letter -from app.utils import get_template, is_letter_too_long, user_has_permissions +from app.utils import ( + LETTER_MAX_PAGES, + get_template, + is_letter_too_long, + user_has_permissions, +) MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @@ -132,6 +137,7 @@ def uploaded_letter_preview(service_id, file_id): status=status, file_id=file_id, letter_too_long=letter_too_long, + letter_max_pages=LETTER_MAX_PAGES, ) From c524b0bbf51122c58af2583bbc8120692d88a96f Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Oct 2019 10:43:08 +0100 Subject: [PATCH 08/10] Rebase and set upload status to invalid if too many pages --- app/main/views/uploads.py | 5 ++++- tests/app/main/views/test_uploads.py | 11 ++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index 346226843..a2afd2929 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -83,7 +83,10 @@ def upload_letter(service_id): else: raise ex else: - status = 'valid' + if is_letter_too_long(page_count): + status = 'invalid' + else: + status = 'valid' file_contents = base64.b64decode(response.json()['file'].encode()) upload_letter_to_s3( file_contents, diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 529e79033..7888cd94c 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -217,9 +217,11 @@ def test_post_upload_letter_with_letter_that_is_too_long(mocker, client_request) 'app.main.views.uploads.sanitise_letter', return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'}) ) - mocker.patch('app.main.views.uploads.upload_letter_to_s3') + mock_upload = mocker.patch('app.main.views.uploads.upload_letter_to_s3') mock_page_count = mocker.patch('app.main.views.uploads.pdf_page_count', return_value=11) mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) + mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ + 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '11', 'status': 'invalid'}) with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: page = client_request.post( @@ -230,6 +232,13 @@ def test_post_upload_letter_with_letter_that_is_too_long(mocker, client_request) ) assert mock_page_count.called + mock_upload.assert_called_once_with( + b'The sanitised content', + file_location='service-596364a0-858e-42c8-9062-a8fe822260eb/fake-uuid.pdf', + filename='tests/test_pdf_files/one_page_pdf.pdf', + page_count=11, + status='invalid' + ) assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' assert page.select('#letter-too-long') From 2ed1e382b4b828cfae97464f6653ba7cd613b85f Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Oct 2019 14:56:18 +0100 Subject: [PATCH 09/10] Move letter length check to utils repo so template-preview can use it, too Update requirements --- app/main/views/send.py | 9 +++--- app/main/views/templates.py | 6 ++-- app/main/views/uploads.py | 15 ++------- app/templates/views/uploads/preview.html | 9 ++---- app/utils.py | 7 ---- requirements-app.txt | 2 +- requirements.txt | 16 ++++----- tests/app/main/views/test_uploads.py | 41 ------------------------ 8 files changed, 20 insertions(+), 85 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index e8805e80e..968452852 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -15,8 +15,9 @@ from flask import ( ) from flask_login import current_user from notifications_python_client.errors import HTTPError -from notifications_utils import SMS_CHAR_COUNT_LIMIT +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.recipients import ( RecipientCSV, first_column_headings, @@ -48,14 +49,12 @@ from app.s3_client.s3_csv_client import ( ) from app.template_previews import TemplatePreview, get_page_count_for_letter from app.utils import ( - LETTER_MAX_PAGES, PermanentRedirect, Spreadsheet, email_or_sms_not_enabled, get_errors_for_csv, get_help_argument, get_template, - is_letter_too_long, should_skip_template_page, unicode_truncate, user_has_permissions, @@ -596,7 +595,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ service_id, template.id, db_template['version'], request.args.get('original_file_name', '') ), letter_too_long=is_letter_too_long(page_count), - letter_max_pages=LETTER_MAX_PAGES, + letter_max_pages=LETTER_MAX_PAGE_COUNT, ) @@ -890,7 +889,7 @@ def _check_notification(service_id, template_id, exception=None): back_link=back_link, help=get_help_argument(), letter_too_long=is_letter_too_long(page_count), - letter_max_pages=LETTER_MAX_PAGES, + letter_max_pages=LETTER_MAX_PAGE_COUNT, **(get_template_error_dict(exception) if exception else {}), ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index afb57deb7..e4c8bf159 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -6,7 +6,9 @@ from flask import abort, flash, redirect, render_template, request, url_for from flask_login import current_user from markupsafe import Markup from notifications_python_client.errors import HTTPError +from notifications_utils import LETTER_MAX_PAGE_COUNT from notifications_utils.formatters import nl2br +from notifications_utils.pdf import is_letter_too_long from notifications_utils.recipients import first_column_headings from app import ( @@ -31,10 +33,8 @@ from app.models.service import Service from app.models.template_list import TemplateList, TemplateLists from app.template_previews import TemplatePreview, get_page_count_for_letter from app.utils import ( - LETTER_MAX_PAGES, email_or_sms_not_enabled, get_template, - is_letter_too_long, should_skip_template_page, user_has_permissions, user_is_platform_admin, @@ -77,7 +77,7 @@ def view_template(service_id, template_id): template_postage=template["postage"], user_has_template_permission=user_has_template_permission, letter_too_long=is_letter_too_long(get_page_count_for_letter(template)), - letter_max_pages=LETTER_MAX_PAGES, + letter_max_pages=LETTER_MAX_PAGE_COUNT, ) diff --git a/app/main/views/uploads.py b/app/main/views/uploads.py index a2afd2929..31d3a4f46 100644 --- a/app/main/views/uploads.py +++ b/app/main/views/uploads.py @@ -26,12 +26,7 @@ from app.s3_client.s3_letter_upload_client import ( upload_letter_to_s3, ) from app.template_previews import TemplatePreview, sanitise_letter -from app.utils import ( - LETTER_MAX_PAGES, - get_template, - is_letter_too_long, - user_has_permissions, -) +from app.utils import get_template, user_has_permissions MAX_FILE_UPLOAD_SIZE = 2 * 1024 * 1024 # 2MB @@ -83,10 +78,7 @@ def upload_letter(service_id): else: raise ex else: - if is_letter_too_long(page_count): - status = 'invalid' - else: - status = 'valid' + status = 'valid' file_contents = base64.b64decode(response.json()['file'].encode()) upload_letter_to_s3( file_contents, @@ -117,7 +109,6 @@ def uploaded_letter_preview(service_id, file_id): metadata = get_letter_metadata(service_id, file_id) original_filename = metadata.get('filename') page_count = metadata.get('page_count') - letter_too_long = is_letter_too_long(int(page_count)) status = metadata.get('status') template_dict = service_api_client.get_precompiled_template(service_id) @@ -139,8 +130,6 @@ def uploaded_letter_preview(service_id, file_id): template=template, status=status, file_id=file_id, - letter_too_long=letter_too_long, - letter_max_pages=LETTER_MAX_PAGES, ) diff --git a/app/templates/views/uploads/preview.html b/app/templates/views/uploads/preview.html index b841dca52..5642540e0 100644 --- a/app/templates/views/uploads/preview.html +++ b/app/templates/views/uploads/preview.html @@ -1,6 +1,5 @@ {% extends "withnav_template.html" %} {% from "components/page-header.html" import page_header %} -{% from "components/banner.html" import banner_wrapper %} {% block service_page_title %} {{ original_filename }} @@ -17,16 +16,12 @@ Validation failed

{% endif %} - {% if letter_too_long %} - {% call banner_wrapper(type='dangerous', id='letter-too-long') %} - {% include "partials/check/letter-too-long.html" %} - {% endcall %} - {% endif %} +
{{ template|string }}
- {% if status == 'valid' and not letter_too_long %} + {% if status == 'valid' %}
=1.4,<1.5 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default itsdangerous==0.24 # pyup: <1.0.0 -git+https://github.com/alphagov/notifications-utils.git@34.1.0#egg=notifications-utils==34.1.0 +git+https://github.com/alphagov/notifications-utils.git@35.0.0#egg=notifications-utils==35.0.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.3.0-alpha#egg=govuk-frontend-jinja==0.3.0-alpha diff --git a/requirements.txt b/requirements.txt index 9f5399ffd..b05f94dc2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,18 +25,18 @@ awscli-cwlogs>=1.4,<1.5 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default itsdangerous==0.24 # pyup: <1.0.0 -git+https://github.com/alphagov/notifications-utils.git@34.1.0#egg=notifications-utils==34.1.0 +git+https://github.com/alphagov/notifications-utils.git@35.0.0#egg=notifications-utils==35.0.0 git+https://github.com/alphagov/govuk-frontend-jinja.git@v0.3.0-alpha#egg=govuk-frontend-jinja==0.3.0-alpha ## The following requirements were added by pip freeze: -awscli==1.16.251 +awscli==1.16.255 bleach==3.1.0 -boto3==1.6.16 -botocore==1.12.241 +boto3==1.9.221 +botocore==1.12.245 certifi==2019.9.11 chardet==3.0.4 Click==7.0 -colorama==0.3.9 +colorama==0.4.1 dnspython==1.16.0 docopt==0.6.2 docutils==0.15.2 @@ -46,7 +46,7 @@ future==0.17.1 greenlet==0.4.15 idna==2.8 jdcal==1.4.1 -Jinja2==2.10.1 +Jinja2==2.10.3 jmespath==0.9.4 lml==0.0.9 lxml==4.4.1 @@ -55,14 +55,14 @@ mistune==0.8.4 monotonic==1.5 openpyxl==2.5.14 orderedset==2.0.1 -phonenumbers==8.10.13 +phonenumbers==8.10.17 pyasn1==0.4.7 pyexcel-ezodf==0.3.4 PyJWT==1.7.1 PyPDF2==1.26.0 python-dateutil==2.8.0 python-json-logger==0.1.11 -PyYAML==4.2b1 +PyYAML==5.1.2 redis==3.3.8 requests==2.22.0 rsa==3.4.2 diff --git a/tests/app/main/views/test_uploads.py b/tests/app/main/views/test_uploads.py index 7888cd94c..e58031e0f 100644 --- a/tests/app/main/views/test_uploads.py +++ b/tests/app/main/views/test_uploads.py @@ -204,47 +204,6 @@ def test_post_upload_letter_with_invalid_file(mocker, client_request): assert not page.find('button', {'type': 'submit'}) -def test_post_upload_letter_with_letter_that_is_too_long(mocker, client_request): - letter_template = {'template_type': 'letter', - 'reply_to_text': '', - 'postage': 'second', - 'subject': 'hi', - 'content': 'my letter'} - - mocker.patch('uuid.uuid4', return_value='fake-uuid') - mocker.patch('app.main.views.uploads.antivirus_client.scan', return_value=True) - mocker.patch( - 'app.main.views.uploads.sanitise_letter', - return_value=Mock(content='The sanitised content', json=lambda: {'file': 'VGhlIHNhbml0aXNlZCBjb250ZW50'}) - ) - mock_upload = mocker.patch('app.main.views.uploads.upload_letter_to_s3') - mock_page_count = mocker.patch('app.main.views.uploads.pdf_page_count', return_value=11) - mocker.patch('app.main.views.uploads.service_api_client.get_precompiled_template', return_value=letter_template) - mocker.patch('app.main.views.uploads.get_letter_metadata', return_value={ - 'filename': 'tests/test_pdf_files/one_page_pdf.pdf', 'page_count': '11', 'status': 'invalid'}) - - with open('tests/test_pdf_files/one_page_pdf.pdf', 'rb') as file: - page = client_request.post( - 'main.upload_letter', - service_id=SERVICE_ONE_ID, - _data={'file': file}, - _follow_redirects=True, - ) - - assert mock_page_count.called - mock_upload.assert_called_once_with( - b'The sanitised content', - file_location='service-596364a0-858e-42c8-9062-a8fe822260eb/fake-uuid.pdf', - filename='tests/test_pdf_files/one_page_pdf.pdf', - page_count=11, - status='invalid' - ) - - assert page.find('h1').text == 'tests/test_pdf_files/one_page_pdf.pdf' - assert page.select('#letter-too-long') - assert not page.find('button', {'type': 'submit'}) - - def test_post_upload_letter_shows_letter_preview_for_invalid_file(mocker, client_request): letter_template = {'template_type': 'letter', 'reply_to_text': '', From 4b5a131072b1eec8dd840318d382110e198a98e5 Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 16 Oct 2019 13:02:11 +0100 Subject: [PATCH 10/10] Harmonise content of error message with the document laid out by our content designer --- app/main/views/send.py | 2 ++ app/main/views/templates.py | 5 ++++- app/templates/partials/check/letter-too-long.html | 6 ++++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/main/views/send.py b/app/main/views/send.py index 968452852..c3415c78b 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -596,6 +596,7 @@ def _check_messages(service_id, template_id, upload_id, preview_row, letters_as_ ), letter_too_long=is_letter_too_long(page_count), letter_max_pages=LETTER_MAX_PAGE_COUNT, + page_count=page_count ) @@ -890,6 +891,7 @@ def _check_notification(service_id, template_id, exception=None): help=get_help_argument(), letter_too_long=is_letter_too_long(page_count), letter_max_pages=LETTER_MAX_PAGE_COUNT, + page_count=page_count, **(get_template_error_dict(exception) if exception else {}), ) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index e4c8bf159..7e8fc6019 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -60,6 +60,8 @@ def view_template(service_id, template_id): '.send_one_off', service_id=service_id, template_id=template_id )) + page_count = get_page_count_for_letter(template) + return render_template( 'views/templates/template.html', template=get_template( @@ -76,8 +78,9 @@ def view_template(service_id, template_id): ), template_postage=template["postage"], user_has_template_permission=user_has_template_permission, - letter_too_long=is_letter_too_long(get_page_count_for_letter(template)), + letter_too_long=is_letter_too_long(page_count), letter_max_pages=LETTER_MAX_PAGE_COUNT, + page_count=page_count ) diff --git a/app/templates/partials/check/letter-too-long.html b/app/templates/partials/check/letter-too-long.html index 58e257f8f..578759bc7 100644 --- a/app/templates/partials/check/letter-too-long.html +++ b/app/templates/partials/check/letter-too-long.html @@ -1,6 +1,8 @@

- This letter is too long + Your letter is too long

- Letters must be {{ letter_max_pages }} pages or fewer + Letters must be {{ letter_max_pages }} pages or less. +
+ Your letter is {{ page_count }} pages long.