diff --git a/app/main/views/send.py b/app/main/views/send.py index 856403105..2fdb5d620 100644 --- a/app/main/views/send.py +++ b/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//send/.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//send//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} diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 315bc32eb..d60bef88f 100644 --- a/app/templates/views/check.html +++ b/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 %} diff --git a/app/templates/views/send.html b/app/templates/views/send.html index 7b0dcf027..2d62b9f2a 100644 --- a/app/templates/views/send.html +++ b/app/templates/views/send.html @@ -40,7 +40,7 @@ {% endif %}

- to + {{ recipient_column }} {{ template.placeholders_as_markup|join(" ") }}

diff --git a/app/utils.py b/app/utils.py index 176ff26cb..0fd38f808 100644 --- a/app/utils.py +++ b/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): diff --git a/requirements.txt b/requirements.txt index 2c746a497..45cd1b2d5 100644 --- a/requirements.txt +++ b/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 diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index e638a5481..7371318d4 100644 --- a/tests/app/main/views/test_send.py +++ b/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)