From e2d1401b54828138ec2fbe5dcadcca2d91d60a83 Mon Sep 17 00:00:00 2001
From: Chris Hill-Scott <me@quis.cc>
Date: Sat, 11 Apr 2020 11:42:13 +0100
Subject: [PATCH] Show error across the whole row
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Errors with messages being too long or empty aren’t specific to a single
cell of the uploaded spreadsheet, they’re the results of combining all
the cells with the template.

Previously we could only show errors against a specific cell. This
commit makes it possible to add a super-row which spans all the cells,
into which we can put errors.

The index (header) column then spans both these rows, to show that they
are both associated with the same row of input.

Depends on:
- [x] https://github.com/alphagov/notifications-utils/pull/719
---
 app/templates/components/table.html       |  8 +++---
 app/templates/views/check/row-errors.html | 30 +++++++++++++++++++----
 tests/app/main/views/test_send.py         | 10 +++++++-
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/app/templates/components/table.html b/app/templates/components/table.html
index 4eba1c54e..f8b03e248 100644
--- a/app/templates/components/table.html
+++ b/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 %}
diff --git a/app/templates/views/check/row-errors.html b/app/templates/views/check/row-errors.html
index b29aa88d9..3ddc43b29 100644
--- a/app/templates/views/check/row-errors.html
+++ b/app/templates/views/check/row-errors.html
@@ -67,12 +67,32 @@
       ] + recipients.column_headers
     ) %}
       {% for item in recipients.displayed_rows %}
-        {% call row(item.id) %}
-          {% call index_field() %}
-            <span class="{% if item.has_errors %}table-field-error{% endif %}">
-              {{ item.index + 2 }}
-            </span>
+        {% 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 %}
+        {% 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() %}
diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py
index 72812158d..09c7ce4b7 100644
--- a/tests/app/main/views/test_send.py
+++ b/tests/app/main/views/test_send.py
@@ -486,8 +486,16 @@ def test_upload_csv_file_with_empty_message_shows_check_page_with_errors(
     assert [
         normalize_spaces(row.text) for row in page.select('tbody tr')
     ] == [
-        '3 +447700900986 no',
+        '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'
 
 
 @pytest.mark.parametrize('file_contents, expected_error,', [