Merge pull request #213 from alphagov/accept-phone-or-email-column

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

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']
+    template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data'])
-    placeholders = list(Template(template).placeholders)
     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']]
+        }[template.template_type]
-    ] + ["test {}".format(header) for header in placeholders])
+    ] + ["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']
+    template = Template(templates_dao.get_service_template_or_404(service_id, template_id)['data'])
-    placeholders = list(Template(template).placeholders)
     output = io.StringIO()
     writer = csv.writer(output)
-    writer.writerow(['to'] + placeholders)
+    writer.writerow(
-    writer.writerow([current_user.mobile_number] + ["test {}".format(header) for header in placeholders])
+        [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', ''),
+                row, template_type=raw_template['template_type']
-                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.3.1#egg=notifications-python-client==0.3.1
 
-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'}
+    expected_data = {'data': ['phone number', '+4412341234'], 'file_name': 'Test run'}
-    mocker.patch('app.main.views.send.s3download', return_value='to\r\n+4412341234')
+    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)