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' %} - +
{{ caller() }}
{%- endmacro %} @@ -81,8 +81,8 @@ {%- endmacro %} -{% macro index_field(text=None) -%} - +{% macro index_field(text=None, rowspan=None) -%} + {{ text if text != None else caller() }} {%- endmacro %} diff --git a/app/templates/views/check/row-errors.html b/app/templates/views/check/row-errors.html index 69b6bc373..3ddc43b29 100644 --- a/app/templates/views/check/row-errors.html +++ b/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 @@
- {% call(item, row_number) list_table( - recipients.displayed_rows, + {% call(item, row_number) mapping_table( caption=original_file_name, caption_visible=False, field_headings=[ 'Row in file'|safe ] + recipients.column_headers ) %} - {% call index_field() %} - - {{ item.index + 2 }} - - {% endcall %} - {% for column in recipients.column_headers %} - {% if item[column].error and not recipients.missing_column_headers %} - {% call field() %} - - {{ item[column].error }} - {{ item[column].data if item[column].data != None }} - + {% for item in recipients.displayed_rows %} + {% if item.has_error_spanning_multiple_cells %} + {% call row() %} + {% call index_field(rowspan=2) %} + + {{ item.index + 2 }} + + {% endcall %} + {% call field(colspan=recipients.column_headers|length) %} + + {% if item.message_empty %} + No content for this message + {% elif item.message_too_long %} + Message is too long + {% endif %} + + {% 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() %} + + {{ item.index + 2 }} + + {% endcall %} + {% endif %} + {% for column in recipients.column_headers %} + {% if item[column].error and not recipients.missing_column_headers %} + {% call field() %} + + {{ item[column].error }} + {{ item[column].data if item[column].data != None }} + + {% 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 %}
{% if count_of_displayed_recipients < count_of_recipients %} diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index 57e7e4122..89e851f11 100644 --- a/tests/app/main/views/test_send.py +++ b/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,', [