Merge pull request #1424 from alphagov/row-error-cleanup

Tidy up some things around row-level errors in CSV files

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

@@ -23,14 +23,14 @@
     {% 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
+          There is a problem with {{ original_file_name }}
         </h1>
         <p>
           You need to {{ row_errors[0] }}
         </p>
       {% else %}
-        <h1 class='banner-title'>
-          There are some problems with your data
+        <h1 class='banner-title' data-module="track-error" data-error-type="Bad rows" data-error-label="{{ upload_id }}">
+          There are some problems with {{ original_file_name }}
         </h1>
         <p>
           You need to:
@@ -46,52 +46,42 @@
   </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 %}
+    {{ file_upload(form.file, button_text='Re-upload your file') }}
   </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 %}
+  {% 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 %}
-
-  {% endif %}
+    {% 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 %}
 
   {% if count_of_displayed_recipients < count_of_recipients %}
     <p class="table-show-more-link">
@@ -107,7 +97,4 @@
     </p>
   {% endif %}
 
-  <h2 class="heading-medium">Preview of {{ template.name }}</h2>
-  {{ template|string }}
-
 {% endblock %}

tests/app/main/views/test_send.py

@@ -140,7 +140,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
     for response in [initial_upload, reupload]:
         assert response.status_code == 200
         content = response.get_data(as_text=True)
-        assert 'There is a problem with your data' in content
+        assert 'There is a problem with invalid.csv' in content
         assert '+447700900986' in content
         assert 'Missing' in content
         assert 'Re-upload your file' in content
@@ -193,7 +193,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
             +447700900986, example
         """,
         (
-            'There is a problem with your data '
+            'There is a problem with invalid.csv '
             'You need to enter missing data in 1 row '
             'Skip to file contents'
         )
@@ -206,7 +206,7 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
             +447700900986, example
         """,
         (
-            'There is a problem with your data '
+            'There is a problem with invalid.csv '
             'You need to enter missing data in 1 row '
             'Skip to file contents'
         )
@@ -1283,7 +1283,7 @@ def test_check_messages_should_revalidate_file_when_uploading_file(
         follow_redirects=True
     )
     assert response.status_code == 200
-    assert 'There is a problem with your data' in response.get_data(as_text=True)
+    assert 'There is a problem with invalid.csv' in response.get_data(as_text=True)
 
 
 @pytest.mark.parametrize('route, response_code', [
@@ -1637,7 +1637,11 @@ def test_non_ascii_characters_in_letter_recipients_file_shows_error(
     )
 
     with logged_in_client.session_transaction() as session:
-        session['upload_data'] = {'template_id': fake_uuid}
+        session['upload_data'] = {
+            'template_id': fake_uuid,
+            'original_file_name': 'unicode.csv',
+        }
+
     response = logged_in_client.get(url_for(
         'main.check_messages',
         service_id=fake_uuid,
@@ -1650,7 +1654,7 @@ def test_non_ascii_characters_in_letter_recipients_file_shows_error(
     assert ' '.join(
         page.find('div', class_='banner-dangerous').text.split()
     ) == (
-            'There is a problem with your data '
+            'There is a problem with unicode.csv '
             'You need to fix 1 address '
             'Skip to file contents'
         )