Move letter length check to utils repo so template-preview can use it, too

Update requirements

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 {}),
     )
 

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,
     )
 
 

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 (
+from app.utils import get_template, user_has_permissions
-    LETTER_MAX_PAGES,
-    get_template,
-    is_letter_too_long,
-    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 = 'valid'
-                status = 'invalid'
-            else:
-                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,
     )
 
 

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
     </p>
     {% endif %}
-    {% if letter_too_long %}
+
-      {% call banner_wrapper(type='dangerous', id='letter-too-long') %}
-          {% include "partials/check/letter-too-long.html" %}
-      {% endcall %}
-    {% endif %}
     <div class="letter-sent">
       {{ template|string }}
     </div>
 
-    {% if status == 'valid' and not letter_too_long %}
+    {% if status == 'valid' %}
     <div class="js-stick-at-bottom-when-scrolling">
       <form method="post" enctype="multipart/form-data" action="{{url_for(
           'main.send_uploaded_letter',

app/utils.py

@@ -43,7 +43,6 @@ FAILURE_STATUSES = ['failed', 'temporary-failure', 'permanent-failure',
                     'technical-failure', 'virus-scan-failed', 'validation-failed']
 REQUESTED_STATUSES = SENDING_STATUSES + DELIVERED_STATUSES + FAILURE_STATUSES
 
-LETTER_MAX_PAGES = 10
 
 with open('{}/email_domains.txt'.format(
     os.path.dirname(os.path.realpath(__file__))
@@ -545,12 +544,6 @@ def get_letter_printing_statement(status, created_at):
         return 'Printed on {} at 5:30pm'.format(printed_date)
 
 
-def is_letter_too_long(page_count):
-    if not page_count:
-        return False
-    return page_count > LETTER_MAX_PAGES
-
-
 class PermanentRedirect(RequestRedirect):
     """
     In Werkzeug 0.15.0 the status code for RequestRedirect changed from 301 to 308.

requirements-app.txt

@@ -23,5 +23,5 @@ 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

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
+boto3==1.9.221
-botocore==1.12.241
+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

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': '',