Merge pull request #1921 from alphagov/duplicate-cols-error

Catch duplicate recipient columns in spreadsheets

app/main/views/send.py

@@ -581,6 +581,7 @@ def check_messages(service_id, template_type, upload_id, row_index=2):
         data['recipients'].too_many_rows or
         not data['count_of_recipients'] or
         not data['recipients'].has_recipient_columns or
+        data['recipients'].duplicate_recipient_column_headers or
         data['recipients'].missing_column_headers
     ):
         return render_template('views/check/column-errors.html', **data)

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

@@ -69,6 +69,21 @@
           ) }}.
         </p>
 
+      {% elif recipients.duplicate_recipient_column_headers %}
+
+        <h1 class='banner-title' data-module="track-error" data-error-type="Duplicate recipient columns" data-error-label="{{ upload_id }}">
+          Your file has more than one column called {{ (
+            recipients.duplicate_recipient_column_headers
+          ) | formatted_list(
+            conjunction='or',
+            prefix='',
+            prefix_plural=''
+          ) }}
+        </h1>
+        <p>
+          Delete or rename one of these columns and try again.
+        </p>
+
       {% elif recipients.missing_column_headers %}
 
         <h1 class='banner-title' data-module="track-error" data-error-type="Missing placeholder columns" data-error-label="{{ upload_id }}">
@@ -128,6 +143,8 @@
 
   {% if not request.args.from_test %}
 
+    {% set column_headers = recipients._raw_column_headers if recipients.duplicate_recipient_column_headers else recipients.column_headers %}
+
     <h2 class="heading-medium" id="{{ file_contents_header_id }}">{{ original_file_name }}</h2>
 
     <div class="fullscreen-content" data-module="fullscreen-table">
@@ -137,14 +154,14 @@
         caption_visible=False,
         field_headings=[
           '<span class="visually-hidden">Row in file</span><span aria-hidden="true">1</span>'|safe
-        ] + recipients.column_headers
+        ] + column_headers
       ) %}
         {% call index_field() %}
           <span>
             {{ item.index + 2 }}
           </span>
         {% endcall %}
-        {% for column in recipients.column_headers %}
+        {% for column in column_headers %}
           {% if item['columns'][column].error and not recipients.missing_column_headers %}
             {% call field() %}
               <span>

requirements.txt

@@ -18,4 +18,4 @@ notifications-python-client==4.7.2
 # PaaS
 awscli-cwlogs>=1.4,<1.5
 
-git+https://github.com/alphagov/notifications-utils.git@23.8.0#egg=notifications-utils==23.8.0
+git+https://github.com/alphagov/notifications-utils.git@23.8.1#egg=notifications-utils==23.8.1

tests/app/main/views/test_send.py

@@ -421,6 +421,17 @@ def test_upload_csvfile_with_errors_shows_check_page_with_errors(
             'Skip to file contents'
         )
     ),
+    (
+        """
+            phone number, phone number, PHONE_NUMBER
+            +447700900111,+447700900222,+447700900333,
+        """,
+        (
+            'Your file has more than one column called ‘phone number’ or ‘PHONE_NUMBER’ '
+            'Delete or rename one of these columns and try again. '
+            'Skip to file contents'
+        )
+    ),
     (
         """
             phone number, name
@@ -614,6 +625,40 @@ def test_upload_valid_csv_shows_preview_and_table(
         assert normalize_spaces(str(page.select('table tbody td')[index + 1])) == cell
 
 
+def test_show_all_columns_if_there_are_duplicate_recipient_columns(
+    client_request,
+    mocker,
+    mock_get_live_service,
+    mock_get_service_template_with_placeholders,
+    mock_get_users_by_service,
+    mock_get_detailed_service_for_today,
+    fake_uuid,
+):
+
+    with client_request.session_transaction() as session:
+        session['upload_data'] = {'template_id': fake_uuid}
+
+    mocker.patch('app.main.views.send.s3download', return_value="""
+        phone number, phone_number, PHONENUMBER
+        07700900001,  07700900002,  07700900003
+    """)
+
+    page = client_request.get(
+        'main.check_messages',
+        service_id=SERVICE_ONE_ID,
+        template_type='sms',
+        upload_id=fake_uuid,
+        _test_page_title=False,
+    )
+
+    assert normalize_spaces(page.select_one('thead').text) == (
+        'Row in file1 phone number phone_number PHONENUMBER'
+    )
+    assert normalize_spaces(page.select_one('tbody').text) == (
+        '2 07700900003 07700900003 07700900003'
+    )
+
+
 @pytest.mark.parametrize('row_index, expected_status', [
     (0, 404),
     (1, 404),