Merge pull request #3411 from alphagov/whole-row-errors

Show errors that aren’t specific to a single spreadsheet column across the whole table row

app/templates/components/table.html

@@ -65,12 +65,12 @@
   {% endif %}
 {%- endmacro %}
 
-{% macro field(align='left', status='', border=True) -%}
+{% macro field(align='left', status='', border=True, colspan=None) -%}
 
     {% set field_alignment = 'table-field-right-aligned' if align == 'right' else 'table-field-left-aligned' %}
     {% set border = '' if border else 'table-field-noborder' %}
 
-    <td class="{{ [field_alignment, border]|join(' ') }}">
+    <td class="{{ [field_alignment, border]|join(' ') }}" {% if colspan %}colspan="{{ colspan }}"{% endif %}>
       <div class="{{ 'table-field-status-' + status if status }}">{{ caller() }}</div>
     </td>
 {%- endmacro %}
@@ -81,8 +81,8 @@
   </th>
 {%- endmacro %}
 
-{% macro index_field(text=None) -%}
-  <td class="table-field-index">
+{% macro index_field(text=None, rowspan=None) -%}
+  <td class="table-field-index" {% if rowspan %}rowspan="{{ rowspan }}"{% endif %}>
     {{ text if text != None else caller()  }}
   </td>
 {%- endmacro %}

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

@@ -1,7 +1,7 @@
 {% 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/table.html" import mapping_table, row, field, text_field, index_field, hidden_field_heading %}
 {% from "components/file-upload.html" import file_upload %}
 {% from "components/back-link/macro.njk" import govukBackLink %}
 {% from "components/message-count-label.html" import message_count_label %}
@@ -59,38 +59,61 @@
   </div>
 
   <div class="fullscreen-content" data-module="fullscreen-table">
-    {% call(item, row_number) list_table(
-      recipients.displayed_rows,
+    {% call(item, row_number) mapping_table(
       caption=original_file_name,
       caption_visible=False,
       field_headings=[
         '<span class="govuk-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.has_errors %}table-field-error{% endif %}">
-          {{ item.index + 2 }}
-        </span>
-      {% endcall %}
-      {% for column in recipients.column_headers %}
-        {% if item[column].error and not recipients.missing_column_headers %}
-          {% call field() %}
-            <span>
-              <span class="table-field-error-label">{{ item[column].error }}</span>
-              {{ item[column].data if item[column].data != None }}
-            </span>
+      {% for item in recipients.displayed_rows %}
+        {% if item.has_error_spanning_multiple_cells %}
+          {% call row() %}
+            {% call index_field(rowspan=2) %}
+              <span class="table-field-error">
+                {{ item.index + 2 }}
+              </span>
+            {% endcall %}
+            {% call field(colspan=recipients.column_headers|length) %}
+              <span class="table-field-error-label">
+                {% if item.message_empty %}
+                  No content for this message
+                {% elif item.message_too_long %}
+                  Message is too long
+                {% endif %}
+              </span>
+            {% endcall %}
           {% endcall %}
-        {% elif item[column].ignore %}
-          {{ text_field(item[column].data or '', status='default') }}
-        {% else %}
-          {{ text_field(item[column].data or '') }}
         {% endif %}
+        {% call row(item.id) %}
+          {% if not item.has_error_spanning_multiple_cells %}
+            {% call index_field() %}
+              <span class="{% if item.has_errors %}table-field-error{% endif %}">
+                {{ item.index + 2 }}
+              </span>
+            {% endcall %}
+          {% endif %}
+          {% for column in recipients.column_headers %}
+            {% if item[column].error and not recipients.missing_column_headers %}
+              {% call field() %}
+                <span>
+                  <span class="table-field-error-label">{{ item[column].error }}</span>
+                  {{ item[column].data if item[column].data != None }}
+                </span>
+              {% endcall %}
+            {% elif item[column].ignore %}
+              {{ text_field(item[column].data or '', status='default') }}
+            {% else %}
+              {{ text_field(item[column].data or '') }}
+            {% endif %}
+          {% endfor %}
+          {% if item[None].data %}
+            {% for column in item[None].data %}
+              {{ text_field(column, status='default') }}
+            {% endfor %}
+          {% endif %}
+        {% endcall %}
       {% endfor %}
-      {% if item[None].data %}
-        {% for column in item[None].data %}
-          {{ text_field(column, status='default') }}
-        {% endfor %}
-      {% endif %}
     {% endcall %}
   </div>
   {% if count_of_displayed_recipients < count_of_recipients %}

tests/app/main/views/test_send.py

@@ -475,9 +475,85 @@ def test_upload_csv_file_with_empty_message_shows_check_page_with_errors(
         assert 'file_uploads' not in session
 
     assert response.status_code == 200
-    content = response.get_data(as_text=True)
-    assert 'There’s a problem with invalid.csv' in content
-    assert 'check you have content for the empty message in 1 row' in content
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert normalize_spaces(
+        page.select_one('.banner-dangerous').text
+    ) == (
+        'There’s a problem with invalid.csv '
+        'You need to check you have content for the empty message in 1 row. '
+        'Skip to file contents'
+    )
+    assert [
+        normalize_spaces(row.text) for row in page.select('tbody tr')
+    ] == [
+        '3 No content for this message',
+        '+447700900986 no',
+    ]
+    assert normalize_spaces(page.select_one('.table-field-index').text) == '3'
+    assert page.select_one('.table-field-index')['rowspan'] == '2'
+    assert normalize_spaces(page.select('tbody tr td')[0].text) == '3'
+    assert normalize_spaces(page.select('tbody tr td')[1].text) == (
+        'No content for this message'
+    )
+    assert page.select('tbody tr td')[1]['colspan'] == '2'
+
+
+def test_upload_csv_file_with_very_long_placeholder_shows_check_page_with_errors(
+    logged_in_client,
+    service_one,
+    mocker,
+    mock_get_service_template_with_placeholders,
+    mock_s3_upload,
+    mock_get_users_by_service,
+    mock_get_service_statistics,
+    mock_get_job_doesnt_exist,
+    mock_get_jobs,
+    fake_uuid,
+):
+    big_placeholder = ' '.join(['not ok'] * 102)
+    mocker.patch(
+        'app.main.views.send.s3download',
+        return_value=f"""
+            phone number, name
+            +447700900986, {big_placeholder}
+            +447700900987, {big_placeholder}
+        """
+    )
+
+    response = logged_in_client.post(
+        url_for('main.send_messages', service_id=service_one['id'], template_id=fake_uuid),
+        data={'file': (BytesIO(''.encode('utf-8')), 'invalid.csv')},
+        content_type='multipart/form-data',
+        follow_redirects=True
+    )
+
+    with logged_in_client.session_transaction() as session:
+        assert 'file_uploads' not in session
+
+    assert response.status_code == 200
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    assert normalize_spaces(
+        page.select_one('.banner-dangerous').text
+    ) == (
+        'There’s a problem with invalid.csv '
+        'You need to shorten the messages in 2 rows. '
+        'Skip to file contents'
+    )
+    assert [
+        normalize_spaces(row.text) for row in page.select('tbody tr')
+    ] == [
+        '2 Message is too long',
+        f'+447700900986 {big_placeholder}',
+        '3 Message is too long',
+        f'+447700900987 {big_placeholder}',
+    ]
+    assert normalize_spaces(page.select_one('.table-field-index').text) == '2'
+    assert page.select_one('.table-field-index')['rowspan'] == '2'
+    assert normalize_spaces(page.select('tbody tr td')[0].text) == '2'
+    assert normalize_spaces(page.select('tbody tr td')[1].text) == (
+        'Message is too long'
+    )
+    assert page.select('tbody tr td')[1]['colspan'] == '2'
 
 
 @pytest.mark.parametrize('file_contents, expected_error,', [