From b42c7c4c9ff25e46cb1d7e9046d3f70f3bea79a9 Mon Sep 17 00:00:00 2001
From: Pea Tyczynska <pea.tyczynska@digital.cabinet-office.gov.uk>
Date: Tue, 1 Oct 2019 16:03:35 +0100
Subject: [PATCH] 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) %}
       <button type="submit" class="button">Send {{ count_of_recipients }} {{ message_count_label(count_of_recipients, template.template_type, suffix='') }}</button>
       {% else %}
         <a href="{{ url_for('main.check_messages_preview', service_id=current_service.id, template_id=template.id, upload_id=upload_id, filetype='pdf') }}" download class="button">Download as a PDF</a>
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'>
       <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
-      {% if not error and page_count < 11 %}
+      {% if not error and (not page_count or page_count <= letter_max_pages) %}
         <button type="submit" class="button">Send 1 {{ message_count_label(1, template.template_type, suffix='') }}</button>
       {% 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 @@
     <div class="bottom-gutter-2-3">
       <div class="grid-row">
         {% 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 %}
             <div class="column-half">
               <a href="{{ url_for(".set_sender", service_id=current_service.id, template_id=template.id) }}" class="pill-separate-item">
                 Send