Accept CSVs with 'email address' or 'phone number'

CSV files currently have ‘to’ as the recipient column. This is changing in
https://github.com/alphagov/notifications-api/pull/109

The admin app also has to validate that the CSV files have the right columns,
because the API expects any CSV that it’s given to have been checked (also we
want things to actually work).

This commit is the minimum code change needed. In the future it should reuse
the same code as the API for processing CSV files. This will need more thinking.

app/main/views/send.py

@@ -27,6 +27,7 @@ from app.main.dao import templates_dao
 from app.main.dao import services_dao
 from app import job_api_client
 from app.utils import validate_recipient, InvalidPhoneError, InvalidEmailError
+from utils.process_csv import first_column_heading
 
 page_headings = {
     'email': 'Send emails',
@@ -97,11 +98,12 @@ def send_messages(service_id, template_id):
         templates_dao.get_service_template_or_404(service_id, template_id)['data'],
         prefix=service['name']
     )
+    recipient_column = first_column_heading[template.template_type]
 
     return render_template(
         'views/send.html',
         template=template,
-        column_headers=['to'] + template.placeholders_as_markup,
+        recipient_column=first_column_heading[template.template_type],
         form=form,
         service=service,
         service_id=service_id
@@ -111,29 +113,36 @@ def send_messages(service_id, template_id):
 @main.route("/services/<service_id>/send/<template_id>.csv", methods=['GET'])
 @login_required
 def get_example_csv(service_id, template_id):
-    template = templates_dao.get_service_template_or_404(service_id, template_id)['data']
-    placeholders = list(Template(template).placeholders)
+    template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data'])
     output = io.StringIO()
     writer = csv.writer(output)
-    writer.writerow(['to'] + placeholders)
+    writer.writerow(
+        [first_column_heading[template.template_type]] +
+        list(template.placeholders)
+    )
     writer.writerow([
         {
             'email': current_user.email_address,
             'sms': current_user.mobile_number
-        }[template['template_type']]
-    ] + ["test {}".format(header) for header in placeholders])
+        }[template.template_type]
+    ] + ["test {}".format(header) for header in template.placeholders])
     return output.getvalue(), 200, {'Content-Type': 'text/csv; charset=utf-8'}
 
 
 @main.route("/services/<service_id>/send/<template_id>/to-self", methods=['GET'])
 @login_required
 def send_message_to_self(service_id, template_id):
-    template = templates_dao.get_service_template_or_404(service_id, template_id)['data']
-    placeholders = list(Template(template).placeholders)
+    template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data'])
     output = io.StringIO()
     writer = csv.writer(output)
-    writer.writerow(['to'] + placeholders)
-    writer.writerow([current_user.mobile_number] + ["test {}".format(header) for header in placeholders])
+    writer.writerow(
+        [first_column_heading[template.template_type]] +
+        list(template.placeholders)
+    )
+    writer.writerow(
+        [current_user.mobile_number] +
+        ["test {}".format(header) for header in template.placeholders]
+    )
     filedata = {
         'file_name': 'Test run',
         'data': output.getvalue().splitlines()
@@ -166,7 +175,7 @@ def check_messages(service_id, upload_id):
         template = Template(
             raw_template,
             values=upload_result['rows'][0] if upload_result['valid'] else {},
-            drop_values={'to'},
+            drop_values={first_column_heading[raw_template['template_type']]},
             prefix=service['name']
         )
         return render_template(
@@ -174,7 +183,7 @@ def check_messages(service_id, upload_id):
             upload_result=upload_result,
             template=template,
             page_heading=page_headings[template.template_type],
-            column_headers=['to'] + list(template.placeholders_as_markup),
+            column_headers=[first_column_heading[template.template_type]] + list(template.placeholders_as_markup),
             original_file_name=upload_data.get('original_file_name'),
             service_id=service_id,
             service=service,
@@ -236,10 +245,13 @@ def _get_rows(contents, raw_template):
         rows.append(row)
         try:
             validate_recipient(
-                row.get('to', ''),
-                template_type=raw_template['template_type']
+                row, template_type=raw_template['template_type']
             )
-            Template(raw_template, values=row, drop_values={'to'}).replaced
+            Template(
+                raw_template,
+                values=row,
+                drop_values={first_column_heading[raw_template['template_type']]}
+            ).replaced
         except (InvalidEmailError, InvalidPhoneError, NeededByTemplateError, NoPlaceholderForDataError):
             valid = False
     return {"valid": valid, "rows": rows}

app/templates/views/check.html

@@ -61,13 +61,17 @@
     caption=original_file_name,
     field_headings=column_headers
   ) %}
-    {% if item.to or ''|valid_phone_number %}
+    {% if item.get('phone number', '')|valid_phone_number %}
       {% call field() %}
-        {{ item.to }}
+        {{ item['phone number'] }}
+      {% endcall %}
+    {% elif item.get('email address') %}
+      {% call field() %}
+        {{ item['email address'] }}
       {% endcall %}
     {% else %}
       {% call field(status='missing') %}
-        {{ item.to }}
+        {{ item['phone number'] }}
       {% endcall %}
     {% endif %}
     {% for column in template.placeholders %}

app/templates/views/send.html

@@ -40,7 +40,7 @@
         {% endif %}
       </p>
       <p class="bottom-gutter-2-3">
-        <span class='placeholder'>to</span>
+        <span class='placeholder'>{{ recipient_column }}</span>
         {{ template.placeholders_as_markup|join(" ") }}
       </p>
       <p>

app/utils.py

@@ -3,6 +3,8 @@ import re
 from functools import wraps
 from flask import (abort, session)
 
+from utils.process_csv import get_recipient_from_row
+
 
 class BrowsableItem(object):
     """
@@ -87,11 +89,11 @@ def validate_email_address(email_address):
     raise InvalidEmailError('Not a valid email address')
 
 
-def validate_recipient(recipient, template_type):
+def validate_recipient(row, template_type):
     return {
         'email': validate_email_address,
         'sms': validate_phone_number
-    }[template_type](recipient)
+    }[template_type](get_recipient_from_row(row, template_type))
 
 
 def user_has_permissions(*permissions):

requirements.txt

@@ -14,4 +14,4 @@ Pygments==2.0.2
 
 git+https://github.com/alphagov/notifications-python-client.git@0.2.8#egg=notifications-python-client==0.2.8
 
-git+https://github.com/alphagov/notifications-utils.git@0.1.2#egg=notifications-utils==0.1.2
+git+https://github.com/alphagov/notifications-utils.git@1.0.0#egg=notifications-utils==1.0.0

tests/app/main/views/test_send.py

@@ -67,7 +67,7 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(
     mock_s3_upload
 ):
 
-    contents = 'to,name\n+44 123,test1\n+44 456,test2'
+    contents = 'phone number,name\n+44 123,test1\n+44 456,test2'
     file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv')
     mocker.patch('app.main.views.send.s3download', return_value=contents)
 
@@ -98,14 +98,14 @@ def test_upload_csvfile_removes_empty_lines_and_trailing_commas(
     mock_s3_upload
 ):
 
-    contents = 'to,name,,,\n++44 7700 900981,test1,,,\n+44 7700 900981,test2,,,\n ,,, \n ,,, \t \t \n'
+    contents = 'phone number,name,,,\n++44 7700 900981,test1,,,\n+44 7700 900981,test2,,,\n ,,, \n ,,, \t \t \n'
     file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv')
 
-    expected_data = {'data': ['to,name', '++44 7700 900981,test1', '+44 7700 900981,test2'],
+    expected_data = {'data': ['phone number,name', '++44 7700 900981,test1', '+44 7700 900981,test2'],
                      'file_name': 'invalid.csv'}
 
     mocker.patch('app.main.views.send.s3download',
-                 return_value='to,name\n++44 7700 900981,test1\n+44 7700 900981,test2')
+                 return_value='phone number,name\n++44 7700 900981,test1\n+44 7700 900981,test2')
 
     with app_.test_request_context():
         with app_.test_client() as client:
@@ -130,8 +130,8 @@ def test_send_test_message_to_self(
     mock_s3_upload
 ):
 
-    expected_data = {'data': ['to', '+4412341234'], 'file_name': 'Test run'}
-    mocker.patch('app.main.views.send.s3download', return_value='to\r\n+4412341234')
+    expected_data = {'data': ['phone number', '+4412341234'], 'file_name': 'Test run'}
+    mocker.patch('app.main.views.send.s3download', return_value='phone number\r\n+4412341234')
 
     with app_.test_request_context():
         with app_.test_client() as client:
@@ -161,7 +161,7 @@ def test_download_example_csv(
                 follow_redirects=True
             )
         assert response.status_code == 200
-        assert response.get_data(as_text=True) == 'to\r\n+4412341234\r\n'
+        assert response.get_data(as_text=True) == 'phone number\r\n+4412341234\r\n'
         assert 'text/csv' in response.headers['Content-Type']
 
 
@@ -175,7 +175,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(
     mock_s3_upload
 ):
 
-    contents = 'to\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986'  # noqa
+    contents = 'phone number\n+44 7700 900981\n+44 7700 900982\n+44 7700 900983\n+44 7700 900984\n+44 7700 900985\n+44 7700 900986'  # noqa
     file_data = (BytesIO(contents.encode('utf-8')), 'valid.csv')
     mocker.patch('app.main.views.send.s3download', return_value=contents)