From 94bc8191aec3fc5ab1a71fd815fd83cc236ae91c Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Thu, 20 Jul 2017 09:10:23 +0100
Subject: [PATCH] Remove irrelevant stuff from error check pages
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If you have errors in your file then there’s stuff you’re not going to
see on the page. So this doesn’t need to be in the Jinja templates that
are only used when there are errors.

Basically the conditional stuff is moving up to the level above these
templates.
---
 app/main/views/send.py                       |   8 +
 app/templates/views/check/column-errors.html |  66 +-------
 app/templates/views/check/row-errors.html    | 164 +++----------------
 3 files changed, 32 insertions(+), 206 deletions(-)

diff --git a/app/main/views/send.py b/app/main/views/send.py
index 9cdd56419..ba92533d4 100644
--- a/app/main/views/send.py
+++ b/app/main/views/send.py
@@ -450,6 +450,14 @@ def check_messages(service_id, template_type, upload_id):
 
     data = _check_messages(service_id, template_type, upload_id)
 
+    if (
+        data['recipients'].too_many_rows or
+        not data['count_of_recipients'] or
+        not data['recipients'].has_recipient_columns or
+        data['recipients'].missing_column_headers
+    ):
+        return render_template('views/check/column-errors.html', **data)
+
     if data['row_errors']:
         return render_template('views/check/row-errors.html', **data)
 
diff --git a/app/templates/views/check/column-errors.html b/app/templates/views/check/column-errors.html
index ebb207edd..d331eb1a2 100644
--- a/app/templates/views/check/column-errors.html
+++ b/app/templates/views/check/column-errors.html
@@ -14,7 +14,7 @@
 {% endmacro %}
 
 {% block service_page_title %}
-  {{ "Error" if errors else "Preview of {}".format(template.name) }}
+  Error
 {% endblock %}
 
 {% block maincolumn_content %}
@@ -91,34 +91,6 @@
       {% endcall %}
     </div>
 
-  {% elif row_errors %}
-
-    <div class="bottom-gutter">
-      {% call banner_wrapper(type='dangerous') %}
-        {% if row_errors|length == 1 %}
-          <h1 class='banner-title'>
-            There is a problem with your data
-          </h1>
-          <p>
-            You need to {{ row_errors[0] }}
-          </p>
-        {% else %}
-          <h1 class='banner-title'>
-            There are some problems with your data
-          </h1>
-          <p>
-            You need to:
-          </p>
-          <ul class="list-bullet">
-            {% for error in row_errors %}
-              <li>{{ error }}</li>
-            {% endfor %}
-          </ul>
-        {% endif %}
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
-
   {% elif not recipients.allowed_to_send_to %}
     {% with
       count_of_recipients=count_of_recipients,
@@ -128,44 +100,14 @@
     {% endwith %}
   {% elif recipients.more_rows_than_can_send %}
     {% include "partials/check/too-many-messages.html" %}
-  {% else %}
-
-    <h1 class="heading-large">
-      Preview of {{ template.name }}
-    </h1>
-    {{ skip_to_file_contents() }}
-
-  {% endif %}
-
-  {% if not errors %}
-    {{ template|string }}
   {% endif %}
 
   <div class="bottom-gutter-3-2">
-  {% if errors %}
     {% if request.args.from_test %}
       <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
     {% else %}
       {{file_upload(form.file, button_text='Re-upload your file')}}
     {% endif %}
-  {% else %}
-    <form method="post" enctype="multipart/form-data" action="{{url_for('main.start_job', service_id=current_service.id, upload_id=upload_id)}}" class='page-footer'>
-      <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
-      <input type="hidden" name="help" value="{{ '3' if help else 0 }}" />
-      {% if choose_time_form and template.template_type != 'letter' %}
-        {{ radio_select(
-          choose_time_form.scheduled_for,
-          wrapping_class='bottom-gutter-2-3'
-        ) }}
-      {% endif %}
-      {% if template.template_type != 'letter' or not request.args.from_test %}
-      <input type="submit" class="button" value="Send {{ count_of_recipients }} {{ message_count_label(count_of_recipients, template.template_type, suffix='') }}" />
-      {% else %}
-        <a href="{{ url_for('main.check_messages_preview', service_id=current_service.id, template_type=template.template_type, upload_id=upload_id, filetype='pdf') }}" download="download" class="button">Download as a printable PDF</a>
-      {% endif %}
-      <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
-    </form>
-  {% endif %}
   </div>
 
   {% if not request.args.from_test %}
@@ -227,9 +169,7 @@
     </p>
   {% endif %}
 
-  {% if errors %}
-    <h2 class="heading-medium">Preview of {{ template.name }}</h2>
-    {{ template|string }}
-  {% endif %}
+  <h2 class="heading-medium">Preview of {{ template.name }}</h2>
+  {{ template|string }}
 
 {% endblock %}
diff --git a/app/templates/views/check/row-errors.html b/app/templates/views/check/row-errors.html
index ebb207edd..8071e7804 100644
--- a/app/templates/views/check/row-errors.html
+++ b/app/templates/views/check/row-errors.html
@@ -14,158 +14,43 @@
 {% endmacro %}
 
 {% block service_page_title %}
-  {{ "Error" if errors else "Preview of {}".format(template.name) }}
+  Error
 {% endblock %}
 
 {% block maincolumn_content %}
 
-  {% if recipients.too_many_rows %}
-
-    <div class="bottom-gutter">
-      {% call banner_wrapper(type='dangerous') %}
+  <div class="bottom-gutter">
+    {% call banner_wrapper(type='dangerous') %}
+      {% if row_errors|length == 1 %}
         <h1 class='banner-title'>
-          Your file has too many rows
+          There is a problem with your data
         </h1>
         <p>
-          Notify can process up to
-          {{ "{:,}".format(recipients.max_rows) }} rows at once. Your
-          file has {{ "{:,}".format(recipients|length) }} rows.
+          You need to {{ row_errors[0] }}
         </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
-
-  {% elif not count_of_recipients %}
-
-    <div class="bottom-gutter">
-      {% call banner_wrapper(type='dangerous') %}
+      {% else %}
         <h1 class='banner-title'>
-          Your file is missing some rows
+          There are some problems with your data
         </h1>
         <p>
-          It needs at least one row of data, and {{ recipients.missing_column_headers | sort() | formatted_list(
-            prefix='a column called',
-            prefix_plural='columns called'
-          ) }}.
+          You need to:
         </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
-
-  {% elif not recipients.has_recipient_columns %}
-
-    <div class="bottom-gutter">
-      {% call banner_wrapper(type='dangerous') %}
-        <h1 class='banner-title'>
-          Your file needs {{ recipients.recipient_column_headers | formatted_list(
-            prefix='a column called',
-            prefix_plural='columns called'
-          ) }}
-        </h1>
-        <p>
-          Right now it has {{ recipients.column_headers | formatted_list(
-            prefix='one column, called ',
-            prefix_plural='columns called '
-          ) }}.
-        </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
-
-  {% elif recipients.missing_column_headers %}
-
-    <div class="bottom-gutter">
-      {% call banner_wrapper(type='dangerous') %}
-        <h1 class='banner-title'>
-          The columns in your file need to match the double brackets in
-          your template
-        </h1>
-        <p>
-          Your file is missing {{ recipients.missing_column_headers | formatted_list(
-            conjunction='and',
-            prefix='a column called ',
-            prefix_plural='columns called '
-          ) }}.
-        </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
-
-  {% elif row_errors %}
-
-    <div class="bottom-gutter">
-      {% call banner_wrapper(type='dangerous') %}
-        {% if row_errors|length == 1 %}
-          <h1 class='banner-title'>
-            There is a problem with your data
-          </h1>
-          <p>
-            You need to {{ row_errors[0] }}
-          </p>
-        {% else %}
-          <h1 class='banner-title'>
-            There are some problems with your data
-          </h1>
-          <p>
-            You need to:
-          </p>
-          <ul class="list-bullet">
-            {% for error in row_errors %}
-              <li>{{ error }}</li>
-            {% endfor %}
-          </ul>
-        {% endif %}
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
-
-  {% elif not recipients.allowed_to_send_to %}
-    {% with
-      count_of_recipients=count_of_recipients,
-      template_type_label=recipients.recipient_column_headers[0]
-    %}
-      {% include "partials/check/not-allowed-to-send-to.html" %}
-    {% endwith %}
-  {% elif recipients.more_rows_than_can_send %}
-    {% include "partials/check/too-many-messages.html" %}
-  {% else %}
-
-    <h1 class="heading-large">
-      Preview of {{ template.name }}
-    </h1>
-    {{ skip_to_file_contents() }}
-
-  {% endif %}
-
-  {% if not errors %}
-    {{ template|string }}
-  {% endif %}
+        <ul class="list-bullet">
+          {% for error in row_errors %}
+            <li>{{ error }}</li>
+          {% endfor %}
+        </ul>
+      {% endif %}
+      {{ skip_to_file_contents() }}
+    {% endcall %}
+  </div>
 
   <div class="bottom-gutter-3-2">
-  {% if errors %}
     {% if request.args.from_test %}
       <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
     {% else %}
       {{file_upload(form.file, button_text='Re-upload your file')}}
     {% endif %}
-  {% else %}
-    <form method="post" enctype="multipart/form-data" action="{{url_for('main.start_job', service_id=current_service.id, upload_id=upload_id)}}" class='page-footer'>
-      <input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
-      <input type="hidden" name="help" value="{{ '3' if help else 0 }}" />
-      {% if choose_time_form and template.template_type != 'letter' %}
-        {{ radio_select(
-          choose_time_form.scheduled_for,
-          wrapping_class='bottom-gutter-2-3'
-        ) }}
-      {% endif %}
-      {% if template.template_type != 'letter' or not request.args.from_test %}
-      <input type="submit" class="button" value="Send {{ count_of_recipients }} {{ message_count_label(count_of_recipients, template.template_type, suffix='') }}" />
-      {% else %}
-        <a href="{{ url_for('main.check_messages_preview', service_id=current_service.id, template_type=template.template_type, upload_id=upload_id, filetype='pdf') }}" download="download" class="button">Download as a printable PDF</a>
-      {% endif %}
-      <a href="{{ back_link }}" class="page-footer-back-link">Back</a>
-    </form>
-  {% endif %}
   </div>
 
   {% if not request.args.from_test %}
@@ -208,12 +93,7 @@
 
   {% endif %}
 
-
-  {% if recipients.too_many_rows %}
-    <p class="table-show-more-link">
-      Can’t show the contents of this file
-    </p>
-  {% elif count_of_displayed_recipients < count_of_recipients %}
+  {% if count_of_displayed_recipients < count_of_recipients %}
     <p class="table-show-more-link">
       {% if row_errors and not recipients.missing_column_headers %}
         Only showing the first {{ count_of_displayed_recipients }} rows with errors
@@ -227,9 +107,7 @@
     </p>
   {% endif %}
 
-  {% if errors %}
-    <h2 class="heading-medium">Preview of {{ template.name }}</h2>
-    {{ template|string }}
-  {% endif %}
+  <h2 class="heading-medium">Preview of {{ template.name }}</h2>
+  {{ template|string }}
 
 {% endblock %}