From 2ed1e382b4b828cfae97464f6653ba7cd613b85f Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Wed, 9 Oct 2019 14:56:18 +0100 Subject: [PATCH] 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': '',