diff --git a/app/assets/stylesheets/components/banner.scss b/app/assets/stylesheets/components/banner.scss index c62bfe2c6..77b706779 100644 --- a/app/assets/stylesheets/components/banner.scss +++ b/app/assets/stylesheets/components/banner.scss @@ -71,7 +71,7 @@ color: $text-colour; background-image: file-url('icon-important-2x.png'); background-size: 34px 34px; - background-position: 0 0px; + background-position: 0 0; background-repeat: no-repeat; padding: 7px 0 5px 50px; } diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py index b7ce664f8..7d07b7d0e 100644 --- a/app/main/dao/templates_dao.py +++ b/app/main/dao/templates_dao.py @@ -4,14 +4,14 @@ from app.utils import BrowsableItem from notifications_python_client.errors import HTTPError -def insert_service_template(name, content, service_id, subject=None): +def insert_service_template(name, type_, content, service_id, subject=None): return notifications_api_client.create_service_template( - name, 'sms' if subject is None else 'email', content, service_id, subject) + name, type_, content, service_id, subject) -def update_service_template(id_, name, content, service_id, subject=None): +def update_service_template(id_, name, type_, content, service_id, subject=None): return notifications_api_client.update_service_template( - id_, name, 'sms', content, service_id) + id_, name, type_, content, service_id) def get_service_templates(service_id): diff --git a/app/main/views/send.py b/app/main/views/send.py index 63b947218..91a41fc5f 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -28,15 +28,21 @@ from app.main.uploader import ( s3download ) 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_phone_number, - InvalidPhoneError -) +from app.utils import validate_recipient, InvalidPhoneError, InvalidEmailError + +first_column_header = { + 'email': 'email', + 'sms': 'phone' +} @main.route("/services//send/", methods=['GET']) def choose_template(service_id, template_type): + + services_dao.get_service_by_id_or_404(service_id) + if template_type not in ['email', 'sms']: abort(404) try: @@ -59,7 +65,7 @@ def choose_template(service_id, template_type): @main.route("/services//send/", methods=['GET', 'POST']) @login_required -def send_sms(service_id, template_id): +def send_messages(service_id, template_id): form = CsvUploadForm() if form.validate_on_submit(): @@ -69,23 +75,25 @@ def send_sms(service_id, template_id): upload_id = str(uuid.uuid4()) s3upload(upload_id, service_id, filedata, current_app.config['AWS_REGION']) session['upload_data'] = {"template_id": template_id, "original_file_name": filedata['file_name']} - return redirect(url_for('.check_sms', + return redirect(url_for('.check_messages', service_id=service_id, upload_id=upload_id)) except ValueError as e: flash('There was a problem uploading: {}'.format(csv_file.filename)) flash(str(e)) - return redirect(url_for('.send_sms', service_id=service_id, template_id=template_id)) + return redirect(url_for('.send_messages', service_id=service_id, template_id=template_id)) + service = services_dao.get_service_by_id_or_404(service_id) template = Template( templates_dao.get_service_template_or_404(service_id, template_id)['data'] ) return render_template( - 'views/send-sms.html', + 'views/send.html', template=template, - column_headers=['phone'] + template.placeholders_as_markup, + column_headers=[first_column_header[template.template_type]] + template.placeholders_as_markup, form=form, + service=service, service_id=service_id ) @@ -97,7 +105,7 @@ def get_example_csv(service_id, template_id): placeholders = list(Template(template).placeholders) output = io.StringIO() writer = csv.writer(output) - writer.writerow(['phone'] + placeholders) + writer.writerow([first_column_header[template['template_type']]] + placeholders) writer.writerow([current_user.mobile_number] + ["test {}".format(header) for header in placeholders]) return(output.getvalue(), 200, {'Content-Type': 'text/csv; charset=utf-8'}) @@ -105,12 +113,12 @@ def get_example_csv(service_id, template_id): @main.route("/services//send//to-self", methods=['GET']) @login_required -def send_sms_to_self(service_id, template_id): +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) output = io.StringIO() writer = csv.writer(output) - writer.writerow(['phone'] + placeholders) + writer.writerow([first_column_header[template['template_type']]] + placeholders) writer.writerow([current_user.mobile_number] + ["test {}".format(header) for header in placeholders]) filedata = { 'file_name': 'Test run', @@ -120,7 +128,7 @@ def send_sms_to_self(service_id, template_id): s3upload(upload_id, service_id, filedata, current_app.config['AWS_REGION']) session['upload_data'] = {"template_id": template_id, "original_file_name": filedata['file_name']} - return redirect(url_for('.check_sms', + return redirect(url_for('.check_messages', service_id=service_id, upload_id=upload_id)) @@ -128,27 +136,29 @@ def send_sms_to_self(service_id, template_id): @main.route("/services//check/", methods=['GET', 'POST']) @login_required -def check_sms(service_id, upload_id): +def check_messages(service_id, upload_id): + + upload_data = session['upload_data'] + template_id = upload_data.get('template_id') if request.method == 'GET': contents = s3download(service_id, upload_id) if not contents: flash('There was a problem reading your upload file') - upload_data = session['upload_data'] - template_id = upload_data.get('template_id') raw_template = templates_dao.get_service_template_or_404(service_id, template_id)['data'] + recipient_type = first_column_header[raw_template['template_type']] upload_result = _get_rows(contents, raw_template) session['upload_data']['notification_count'] = len(upload_result['rows']) template = Template( raw_template, values=upload_result['rows'][0] if upload_result['valid'] else {}, - drop_values={'phone'} + drop_values={recipient_type} ) return render_template( 'views/check-sms.html', upload_result=upload_result, template=template, - column_headers=['phone number'] + list( + column_headers=[recipient_type] + list( template.placeholders if upload_result['valid'] else template.placeholders_as_markup ), original_file_name=upload_data.get('original_file_name'), @@ -156,9 +166,7 @@ def check_sms(service_id, upload_id): form=CsvUploadForm() ) elif request.method == 'POST': - upload_data = session['upload_data'] original_file_name = upload_data.get('original_file_name') - template_id = upload_data.get('template_id') notification_count = upload_data.get('notification_count') session.pop('upload_data') try: @@ -170,9 +178,9 @@ def check_sms(service_id, upload_id): raise e flash('We’ve started sending your messages', 'default_with_tick') - return redirect(url_for('main.view_job', - service_id=service_id, - job_id=upload_id)) + return redirect( + url_for('main.view_job', service_id=service_id, job_id=upload_id) + ) def _get_filedata(file): @@ -195,8 +203,12 @@ def _get_rows(contents, raw_template): for row in reader: rows.append(row) try: - validate_phone_number(row['phone']) - Template(raw_template, values=row, drop_values={'phone'}).replaced - except (InvalidPhoneError, NeededByTemplateError, NoPlaceholderForDataError): + recipient_column = first_column_header[raw_template['template_type']] + validate_recipient( + row[recipient_column], + template_type=raw_template['template_type'] + ) + Template(raw_template, values=row, drop_values={recipient_column}).replaced + except (InvalidEmailError, InvalidPhoneError, NeededByTemplateError, NoPlaceholderForDataError): valid = False return {"valid": valid, "rows": rows} diff --git a/app/main/views/templates.py b/app/main/views/templates.py index b291e1b8c..01f3711dd 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -31,7 +31,7 @@ def add_service_template(service_id, template_type): if form.validate_on_submit(): tdao.insert_service_template( - form.name.data, form.template_content.data, service_id, form.subject.data or None + form.name.data, template['template_type'], form.template_content.data, service_id, form.subject.data or None ) return redirect( url_for('.choose_template', service_id=service_id, template_type=template_type) @@ -54,8 +54,9 @@ def edit_service_template(service_id, template_id): if form.validate_on_submit(): tdao.update_service_template( - template_id, form.name.data, - form.template_content.data, service_id) + template_id, form.name.data, template['template_type'], + form.template_content.data, service_id + ) return redirect(url_for( '.choose_template', service_id=service_id, diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 4b1354847..e0de38dca 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -42,7 +42,7 @@
- Back + Back
{% else %} {{file_upload(form.file, button_text='Upload a CSV file')}} diff --git a/app/templates/views/choose-email-template.html b/app/templates/views/choose-email-template.html index 06ed1dec6..fb5fe2b1f 100644 --- a/app/templates/views/choose-email-template.html +++ b/app/templates/views/choose-email-template.html @@ -21,8 +21,6 @@
- Add recipients - Send yourself a test Edit template
diff --git a/app/templates/views/choose-sms-template.html b/app/templates/views/choose-sms-template.html index b83217a7c..5508ca4fb 100644 --- a/app/templates/views/choose-sms-template.html +++ b/app/templates/views/choose-sms-template.html @@ -30,8 +30,8 @@
- Add recipients - Send yourself a test + Add recipients + Send yourself a test Edit template
diff --git a/app/templates/views/send-sms.html b/app/templates/views/send.html similarity index 57% rename from app/templates/views/send-sms.html rename to app/templates/views/send.html index c8a91388d..8aef83078 100644 --- a/app/templates/views/send-sms.html +++ b/app/templates/views/send.html @@ -1,5 +1,6 @@ {% extends "withnav_template.html" %} {% from "components/sms-message.html" import sms_message %} +{% from "components/email-message.html" import email_message %} {% from "components/page-footer.html" import page_footer %} {% from "components/file-upload.html" import file_upload %} {% from "components/table.html" import list_table, field %} @@ -14,14 +15,22 @@
- - {{ sms_message(template.formatted_as_markup) }} - + {% if 'sms' == template.template_type %} + {{ sms_message(template.formatted_as_markup) }} + {% elif 'email' == template.template_type %} + {{ email_message( + template.subject, + template.formatted_as_markup, + from_address='{}@notifications.service.gov.uk'.format(service.email_from), + from_name=service.name + ) }} + {% endif %} {{ banner( - 'You can upload real data, but we’ll only send to your mobile number until you request to go live'|safe, + 'You can upload real data, but we’ll only send to your mobile number until you request to go live'.format( + url_for('.service_request_to_go_live', service_id=service_id) + )|safe, 'info' )}} -
diff --git a/app/templates/views/service_dashboard.html b/app/templates/views/service_dashboard.html index 045a90695..b17c935ba 100644 --- a/app/templates/views/service_dashboard.html +++ b/app/templates/views/service_dashboard.html @@ -36,7 +36,7 @@ """.format( url_for(".add_service_template", service_id=service_id), - url_for(".choose_sms_template", service_id=service_id) + url_for(".choose_template", service_id=service_id, template_type="sms") )|safe, subhead='Get started', type="tip" @@ -46,7 +46,7 @@ """ Send yourself a text message """.format( - url_for(".choose_sms_template", service_id=service_id) + url_for(".choose_template", service_id=service_id, template_type="sms") )|safe, subhead='Next step', type="tip" diff --git a/app/utils.py b/app/utils.py index f814f4b81..9468cba77 100644 --- a/app/utils.py +++ b/app/utils.py @@ -1,3 +1,5 @@ +import re + from functools import wraps from flask import abort @@ -28,6 +30,11 @@ class BrowsableItem(object): pass +class InvalidEmailError(Exception): + def __init__(self, message): + self.message = message + + class InvalidPhoneError(Exception): def __init__(self, message): self.message = message @@ -74,6 +81,19 @@ def format_phone_number(number): return '+447{}{}{}'.format(*re.findall('...', number)) +def validate_email_address(email_address): + if re.match(r"(^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$)", email_address): + return + raise InvalidEmailError('Not a valid email address') + + +def validate_recipient(recipient, template_type): + return { + 'email': validate_email_address, + 'sms': validate_phone_number + }[template_type](recipient) + + def user_has_permissions(*permissions): def wrap(func): @wraps(func) diff --git a/requirements.txt b/requirements.txt index 49dceaccf..af95431af 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.5#egg=notifications-python-client==0.2.7 -git+https://github.com/alphagov/notifications-utils.git@0.1.0#egg=notifications-utils==0.1.0 +git+https://github.com/alphagov/notifications-utils.git@0.1.0#egg=notifications-utils==0.1.1 diff --git a/tests/app/main/views/test_send.py b/tests/app/main/views/test_send.py index c48aeed26..7e9e8dc1a 100644 --- a/tests/app/main/views/test_send.py +++ b/tests/app/main/views/test_send.py @@ -1,42 +1,56 @@ from io import BytesIO from flask import url_for +import pytest import moto +template_types = ['email', 'sms'] -def test_choose_sms_template(app_, - api_user_active, - mock_login, - mock_get_user, - mock_check_verify_code, - mock_get_service_templates, - mock_get_jobs): + +@pytest.mark.parametrize("template_type", template_types) +def test_choose_template( + template_type, + app_, + api_user_active, + mock_login, + mock_get_user, + mock_get_service, + mock_check_verify_code, + mock_get_service_templates, + mock_get_jobs +): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) - response = client.get(url_for('main.choose_template', template_type='sms', service_id=12345)) + response = client.get(url_for('main.choose_template', template_type=template_type, service_id=12345)) assert response.status_code == 200 content = response.get_data(as_text=True) - assert 'template_one' in content - assert 'template one content' in content - assert 'template_two' in content - assert 'template two content' in content + assert '{}_template_one'.format(template_type) in content + assert '{} template one content'.format(template_type) in content + assert '{}_template_two'.format(template_type) in content + assert '{} template two content'.format(template_type) in content -def test_upload_empty_csvfile_returns_to_upload_page(app_, - api_user_active, - mock_login, - mock_get_user, - mock_get_service_templates, - mock_check_verify_code, - mock_get_service_template): +def test_upload_empty_csvfile_returns_to_upload_page( + app_, + api_user_active, + mock_login, + mock_get_user, + mock_get_service, + mock_get_service_templates, + mock_check_verify_code, + mock_get_service_template +): with app_.test_request_context(): with app_.test_client() as client: client.login(api_user_active) upload_data = {'file': (BytesIO(''.encode('utf-8')), 'emtpy.csv')} - response = client.post(url_for('main.send_sms', service_id=12345, template_id=54321), - data=upload_data, follow_redirects=True) + response = client.post( + url_for('main.send_messages', service_id=12345, template_id=54321), + data=upload_data, + follow_redirects=True + ) assert response.status_code == 200 content = response.get_data(as_text=True) @@ -44,13 +58,15 @@ def test_upload_empty_csvfile_returns_to_upload_page(app_, @moto.mock_s3 -def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(app_, - mocker, - api_user_active, - mock_login, - mock_get_user, - mock_get_user_by_email, - mock_get_service_template): +def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors( + app_, + mocker, + api_user_active, + mock_login, + mock_get_user, + mock_get_user_by_email, + mock_get_service_template +): contents = 'phone\n+44 123\n+44 456' file_data = (BytesIO(contents.encode('utf-8')), 'invalid.csv') @@ -59,9 +75,11 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(app_, with app_.test_client() as client: client.login(api_user_active) upload_data = {'file': file_data} - response = client.post(url_for('main.send_sms', service_id=12345, template_id=54321), - data=upload_data, - follow_redirects=True) + response = client.post( + url_for('main.send_messages', service_id=12345, template_id=54321), + data=upload_data, + follow_redirects=True + ) assert response.status_code == 200 content = response.get_data(as_text=True) assert 'Your CSV file contained missing or invalid data' in content @@ -85,7 +103,7 @@ def test_send_test_message_to_self( with app_.test_client() as client: client.login(api_user_active) response = client.get( - url_for('main.send_sms_to_self', service_id=12345, template_id=54321), + url_for('main.send_message_to_self', service_id=12345, template_id=54321), follow_redirects=True ) assert response.status_code == 200 @@ -118,13 +136,15 @@ def test_download_example_csv( @moto.mock_s3 -def test_upload_csvfile_with_valid_phone_shows_all_numbers(app_, - mocker, - api_user_active, - mock_login, - mock_get_user, - mock_get_user_by_email, - mock_get_service_template): +def test_upload_csvfile_with_valid_phone_shows_all_numbers( + app_, + mocker, + api_user_active, + mock_login, + mock_get_user, + mock_get_user_by_email, + mock_get_service_template +): contents = 'phone\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 @@ -134,7 +154,7 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(app_, with app_.test_client() as client: client.login(api_user_active) upload_data = {'file': file_data} - response = client.post(url_for('main.send_sms', service_id=12345, template_id=54321), + response = client.post(url_for('main.send_messages', service_id=12345, template_id=54321), data=upload_data, follow_redirects=True) with client.session_transaction() as sess: @@ -154,16 +174,18 @@ def test_upload_csvfile_with_valid_phone_shows_all_numbers(app_, @moto.mock_s3 -def test_create_job_should_call_api(app_, - service_one, - api_user_active, - mock_get_user, - mock_get_user_by_email, - mock_login, - job_data, - mock_create_job, - mock_get_job, - mock_get_service_template): +def test_create_job_should_call_api( + app_, + service_one, + api_user_active, + mock_get_user, + mock_get_user_by_email, + mock_login, + job_data, + mock_create_job, + mock_get_job, + mock_get_service_template +): service_id = service_one['id'] job_id = job_data['id'] @@ -178,7 +200,7 @@ def test_create_job_should_call_api(app_, session['upload_data'] = {'original_file_name': original_file_name, 'template_id': template_id, 'notification_count': notification_count} - url = url_for('main.check_sms', service_id=service_one['id'], upload_id=job_id) + url = url_for('main.check_messages', service_id=service_one['id'], upload_id=job_id) response = client.post(url, data=job_data, follow_redirects=True) assert response.status_code == 200 diff --git a/tests/conftest.py b/tests/conftest.py index 308ee42b7..3c1de5a87 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -163,11 +163,20 @@ def mock_update_service_template(mocker): @pytest.fixture(scope='function') def mock_get_service_templates(mocker): def _create(service_id): - template_one = template_json( - 1, "template_one", "sms", "template one content", service_id) - template_two = template_json( - 2, "template_two", "sms", "template two content", service_id) - return {'data': [template_one, template_two]} + return {'data': [ + template_json( + 1, "sms_template_one", "sms", "sms template one content", service_id + ), + template_json( + 2, "sms_template_two", "sms", "sms template two content", service_id + ), + template_json( + 3, "email_template_one", "email", "email template one content", service_id + ), + template_json( + 4, "email_template_two", "email", "email template two content", service_id + ) + ]} return mocker.patch( 'app.notifications_api_client.get_service_templates',