Merge pull request #1374 from alphagov/split-the-check

Split the ‘check’ template into three separate templates, add error tracking

app/assets/javascripts/errorTracking.js

@@ -0,0 +1,22 @@
+(function(Modules) {
+  "use strict";
+
+  Modules.TrackError = function() {
+
+    this.start = function(component) {
+
+      if (!ga) return;
+
+      ga(
+        'send',
+        'event',
+        'Error',
+        $(component).data('error-type'),
+        $(component).data('error-label')
+      );
+
+    };
+
+  };
+
+})(window.GOVUK.Modules);

app/main/views/send.py

@@ -447,10 +447,24 @@ def _check_messages(service_id, template_type, upload_id, letters_as_pdf=False):
 @login_required
 @user_has_permissions('send_texts', 'send_emails', 'send_letters')
 def check_messages(service_id, template_type, upload_id):
-    return render_template(
+
-        'views/check.html',
+    data = _check_messages(service_id, template_type, upload_id)
-        **_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)
+
+    if data['errors']:
+        return render_template('views/check/column-errors.html', **data)
+
+    return render_template('views/check/ok.html', **data)
 
 
 @main.route("/services/<service_id>/<template_type>/check/<upload_id>.<filetype>", methods=['GET'])

app/templates/partials/check/not-allowed-to-send-to.html

@@ -1,25 +1,12 @@
-{% from "components/banner.html" import banner_wrapper %}
+<h1 class='banner-title' data-module="track-error" data-error-type="Trial mode: bad recipients" data-error-label="{{ upload_id }}">
-
+  You can’t send to
-{% macro skip_to_file_contents() %}
+  {{ 'this' if count_of_recipients == 1 else 'these' }}
-  <p class="visually-hidden">
+  {{ template_type_label }}
-    <a href="#{{ file_contents_header_id }}">Skip to file contents</a>
+  {%- if count_of_recipients != 1 -%}
-  </p>
+    {{ 'es' if 'email address' == template_type_label else 's' }}
-{% endmacro %}
+  {%- endif %}
-
+</h1>
-<div class="bottom-gutter">
+<p>
-  {% call banner_wrapper(type='dangerous') %}
+  In <a href="{{ url_for('.trial_mode') }}">trial mode</a> you can only
-    <h1 class='banner-title'>
+  send to yourself and members of your team
-      You can’t send to
+</p>
-      {{ 'this' if count_of_recipients == 1 else 'these' }}
-      {{ template_type_label }}
-      {%- if count_of_recipients != 1 -%}
-        {{ 'es' if 'email address' == template_type_label else 's' }}
-      {%- endif %}
-    </h1>
-    <p>
-      In <a href="{{ url_for('.trial_mode') }}">trial mode</a> you can only
-      send to yourself and members of your team
-    </p>
-    {{ skip_to_file_contents() }}
-  {% endcall %}
-</div>

app/templates/partials/check/too-many-messages.html

@@ -1,46 +1,40 @@
-{% from "components/banner.html" import banner_wrapper %}
+<h1 class='banner-title' data-module="track-error" data-error-type="Trial mode: too many recipients" data-error-label="{{ upload_id }}">
-
+  {% if original_file_name %}
-<div class="bottom-gutter">
+    Too many recipients
-  {% call banner_wrapper(type='dangerous') %}
+  {% else %}
-    <h1 class='banner-title'>
+    Daily limit reached
-      {% if original_file_name %}
+  {% endif %}
-        Too many recipients
+</h1>
-      {% else %}
+<p>
-        Daily limit reached
+  You can only send {{ current_service.message_limit }} messages per day
-      {% endif %}
+  {%- if current_service.restricted %}
-    </h1>
+    in <a href="{{ url_for('.trial_mode')}}">trial mode</a>
-    <p>
+  {%- endif -%}
-      You can only send {{ current_service.message_limit }} messages per day
+  .
-      {%- if current_service.restricted %}
+</p>
-        in <a href="{{ url_for('.trial_mode')}}">trial mode</a>
+{% if original_file_name %}
-      {%- endif -%}
+  <p>
-      .
+    {% if current_service.message_limit != remaining_messages %}
-    </p>
+      You can still send {{ remaining_messages }} messages today, but
-    {% if original_file_name %}
-      <p>
-        {% if current_service.message_limit != remaining_messages %}
-          You can still send {{ remaining_messages }} messages today, but
-        {% endif %}
-        ‘{{ original_file_name }}’ contains
-        {{ count_of_recipients }}
-        {% if count_of_recipients == 1 -%}
-          {%- if template.template_type == 'email' -%}
-            email address
-          {%- elif template.template_type == 'sms' -%}
-            phone number
-          {%- elif template.template_type == 'letter' -%}
-            address
-          {%- endif -%}
-        {%- else -%}
-          {%- if template.template_type == 'email' -%}
-            email addresses
-          {%- elif template.template_type == 'sms' -%}
-            phone numbers
-          {%- elif template.template_type == 'letter' -%}
-            addresses
-          {%- endif -%}
-        {%- endif -%}.
-      </p>
     {% endif %}
-  {% endcall %}
+    ‘{{ original_file_name }}’ contains
-</div>
+    {{ count_of_recipients }}
+    {% if count_of_recipients == 1 -%}
+      {%- if template.template_type == 'email' -%}
+        email address
+      {%- elif template.template_type == 'sms' -%}
+        phone number
+      {%- elif template.template_type == 'letter' -%}
+        address
+      {%- endif -%}
+    {%- else -%}
+      {%- if template.template_type == 'email' -%}
+        email addresses
+      {%- elif template.template_type == 'sms' -%}
+        phone numbers
+      {%- elif template.template_type == 'letter' -%}
+        addresses
+      {%- endif -%}
+    {%- endif -%}.
+  </p>
+{% endif %}

app/templates/views/check/column-errors.html

@@ -14,16 +14,17 @@
 {% 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">
+      {% if recipients.too_many_rows %}
-      {% call banner_wrapper(type='dangerous') %}
+
-        <h1 class='banner-title'>
+        <h1 class='banner-title' data-module="track-error" data-error-type="Too many rows" data-error-label="{{ upload_id }}">
           Your file has too many rows
         </h1>
         <p>
@@ -31,15 +32,10 @@
           {{ "{:,}".format(recipients.max_rows) }} rows at once. Your
           file has {{ "{:,}".format(recipients|length) }} rows.
         </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
 
-  {% elif not count_of_recipients %}
+      {% elif not count_of_recipients %}
 
-    <div class="bottom-gutter">
+        <h1 class='banner-title' data-module="track-error" data-error-type="No rows" data-error-label="{{ upload_id }}">
-      {% call banner_wrapper(type='dangerous') %}
-        <h1 class='banner-title'>
           Your file is missing some rows
         </h1>
         <p>
@@ -48,15 +44,10 @@
             prefix_plural='columns called'
           ) }}.
         </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
 
-  {% elif not recipients.has_recipient_columns %}
+      {% elif not recipients.has_recipient_columns %}
 
-    <div class="bottom-gutter">
+        <h1 class='banner-title' data-module="track-error" data-error-type="Missing recipient columns" data-error-label="{{ upload_id }}">
-      {% 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'
@@ -68,15 +59,10 @@
             prefix_plural='columns called '
           ) }}.
         </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
 
-  {% elif recipients.missing_column_headers %}
+      {% elif recipients.missing_column_headers %}
 
-    <div class="bottom-gutter">
+        <h1 class='banner-title' data-module="track-error" data-error-type="Missing placeholder columns" data-error-label="{{ upload_id }}">
-      {% call banner_wrapper(type='dangerous') %}
-        <h1 class='banner-title'>
           The columns in your file need to match the double brackets in
           your template
         </h1>
@@ -87,85 +73,29 @@
             prefix_plural='columns called '
           ) }}.
         </p>
-        {{ skip_to_file_contents() }}
-      {% endcall %}
-    </div>
 
-  {% elif row_errors %}
+      {% 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" %}
+      {% endif %}
 
-    <div class="bottom-gutter">
+      {{ skip_to_file_contents() }}
-      {% 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 %}
+    {% endcall %}
-    {% with
+  </div>
-      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 %}
 
   <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 %}
@@ -177,7 +107,7 @@
       caption=original_file_name,
       caption_visible=False,
       field_headings=[
-        '<span class="visually-hidden">Row in file</span><span aria-hidden="true" class="{}">1</span>'.format("table-field-invisible-error" if errors else "")|safe
+        '<span class="visually-hidden">Row in file</span><span aria-hidden="true">1</span>'|safe
       ] + recipients.column_headers
     ) %}
       {% call index_field() %}
@@ -227,9 +157,7 @@
     </p>
   {% endif %}
 
-  {% if errors %}
+  <h2 class="heading-medium">Preview of {{ template.name }}</h2>
-    <h2 class="heading-medium">Preview of {{ template.name }}</h2>
+  {{ template|string }}
-    {{ template|string }}
-  {% endif %}
 
 {% endblock %}

app/templates/views/check/ok.html

@@ -0,0 +1,91 @@
+{% extends "withnav_template.html" %}
+{% from "components/banner.html" import banner_wrapper %}
+{% from "components/radios.html" import radio_select %}
+{% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %}
+{% from "components/file-upload.html" import file_upload %}
+{% from "components/page-footer.html" import page_footer %}
+{% from "components/message-count-label.html" import message_count_label %}
+
+{% set file_contents_header_id = 'file-preview' %}
+{% macro skip_to_file_contents() %}
+  <p class="visually-hidden">
+    <a href="#{{ file_contents_header_id }}">Skip to file contents</a>
+  </p>
+{% endmacro %}
+
+{% block service_page_title %}
+  {{ "Preview of {}".format(template.name) }}
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <h1 class="heading-large">
+    Preview of {{ template.name }}
+  </h1>
+  {{ skip_to_file_contents() }}
+
+  {{ template|string }}
+
+  <div class="bottom-gutter-3-2">
+    <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>
+  </div>
+
+  {% if not request.args.from_test %}
+
+    <h2 class="heading-medium" id="{{ file_contents_header_id }}">{{ original_file_name }}</h2>
+
+    {% call(item, row_number) list_table(
+      recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
+      caption=original_file_name,
+      caption_visible=False,
+      field_headings=[
+        '<span class="visually-hidden">Row in file</span><span aria-hidden="true">1</span>'|safe
+      ] + recipients.column_headers
+    ) %}
+      {% call index_field() %}
+        <span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}">
+          {{ item.index + 2 }}
+        </span>
+      {% endcall %}
+      {% for column in recipients.column_headers %}
+        {% if item['columns'][column].ignore %}
+          {{ text_field(item['columns'][column].data or '', status='default') }}
+        {% else %}
+          {{ text_field(item['columns'][column].data or '') }}
+        {% endif %}
+      {% endfor %}
+      {% if item['columns'].get(None) %}
+        {% for column in item['columns'][None].data %}
+          {{ text_field(column, status='default') }}
+        {% endfor %}
+      {% endif %}
+    {% endcall %}
+
+  {% endif %}
+
+  {% 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
+      {% else %}
+        Only showing the first {{ count_of_displayed_recipients }} rows
+      {% endif %}
+    </p>
+  {% endif %}
+
+{% endblock %}

app/templates/views/check/row-errors.html

@@ -0,0 +1,113 @@
+{% extends "withnav_template.html" %}
+{% from "components/banner.html" import banner_wrapper %}
+{% from "components/radios.html" import radio_select %}
+{% from "components/table.html" import list_table, field, text_field, index_field, hidden_field_heading %}
+{% from "components/file-upload.html" import file_upload %}
+{% from "components/page-footer.html" import page_footer %}
+{% from "components/message-count-label.html" import message_count_label %}
+
+{% set file_contents_header_id = 'file-preview' %}
+{% macro skip_to_file_contents() %}
+  <p class="visually-hidden">
+    <a href="#{{ file_contents_header_id }}">Skip to file contents</a>
+  </p>
+{% endmacro %}
+
+{% block service_page_title %}
+  Error
+{% endblock %}
+
+{% block maincolumn_content %}
+
+  <div class="bottom-gutter">
+    {% call banner_wrapper(type='dangerous') %}
+      {% if row_errors|length == 1 %}
+        <h1 class='banner-title' data-module="track-error" data-error-type="Bad rows" data-error-label="{{ upload_id }}">
+          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>
+
+  <div class="bottom-gutter-3-2">
+    {% 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 %}
+  </div>
+
+  {% if not request.args.from_test %}
+
+    <h2 class="heading-medium" id="{{ file_contents_header_id }}">{{ original_file_name }}</h2>
+
+    {% call(item, row_number) list_table(
+      recipients.initial_annotated_rows_with_errors if row_errors and not recipients.missing_column_headers else recipients.initial_annotated_rows,
+      caption=original_file_name,
+      caption_visible=False,
+      field_headings=[
+        '<span class="visually-hidden">Row in file</span><span aria-hidden="true" class="table-field-invisible-error">1</span>'|safe
+      ] + recipients.column_headers
+    ) %}
+      {% call index_field() %}
+        <span class="{% if item.index in recipients.rows_with_errors %}table-field-error{% endif %}">
+          {{ item.index + 2 }}
+        </span>
+      {% endcall %}
+      {% for column in recipients.column_headers %}
+        {% if item['columns'][column].error and not recipients.missing_column_headers %}
+          {% call field() %}
+            <span>
+              <span class="table-field-error-label">{{ item['columns'][column].error }}</span>
+              {{ item['columns'][column].data if item['columns'][column].data != None }}
+            </span>
+          {% endcall %}
+        {% elif item['columns'][column].ignore %}
+          {{ text_field(item['columns'][column].data or '', status='default') }}
+        {% else %}
+          {{ text_field(item['columns'][column].data or '') }}
+        {% endif %}
+      {% endfor %}
+      {% if item['columns'].get(None) %}
+        {% for column in item['columns'][None].data %}
+          {{ text_field(column, status='default') }}
+        {% endfor %}
+      {% endif %}
+    {% endcall %}
+
+  {% endif %}
+
+  {% 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
+      {% else %}
+        Only showing the first {{ count_of_displayed_recipients }} rows
+      {% endif %}
+    </p>
+  {% elif row_errors and not recipients.missing_column_headers %}
+    <p class="table-show-more-link">
+      Only showing rows with errors
+    </p>
+  {% endif %}
+
+  <h2 class="heading-medium">Preview of {{ template.name }}</h2>
+  {{ template|string }}
+
+{% endblock %}

gulpfile.babel.js

@@ -66,6 +66,7 @@ gulp.task('javascripts', () => gulp
     paths.src + 'javascripts/updateContent.js',
     paths.src + 'javascripts/listEntry.js',
     paths.src + 'javascripts/liveSearch.js',
+    paths.src + 'javascripts/errorTracking.js',
     paths.src + 'javascripts/main.js'
   ])
   .pipe(plugins.prettyerror())