diff --git a/app/__init__.py b/app/__init__.py index c0c431c0b..b28fc9adc 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -19,6 +19,7 @@ from app.notify_client.job_api_client import JobApiClient from app.notify_client.status_api_client import StatusApiClient from app.its_dangerous_session import ItsdangerousSessionInterface from app.asset_fingerprinter import AssetFingerprinter +from app.utils import validate_phone_number, InvalidPhoneError import app.proxy_fix from config import configs from utils import logging @@ -65,6 +66,7 @@ def create_app(config_name, config_overrides=None): application.add_template_filter(nl2br) application.add_template_filter(format_datetime) application.add_template_filter(syntax_highlight_json) + application.add_template_filter(valid_phone_number) application.after_request(useful_headers_after_request) register_errorhandlers(application) @@ -139,6 +141,14 @@ def format_datetime(date): return native.strftime('%A %d %B %Y at %H:%M') +def valid_phone_number(phone_number): + try: + validate_phone_number(phone_number) + return True + except InvalidPhoneError: + return False + + # https://www.owasp.org/index.php/List_of_useful_HTTP_headers def useful_headers_after_request(response): response.headers.add('X-Frame-Options', 'deny') diff --git a/app/assets/stylesheets/components/page-footer.scss b/app/assets/stylesheets/components/page-footer.scss index 1df6405f1..7dfe8ece0 100644 --- a/app/assets/stylesheets/components/page-footer.scss +++ b/app/assets/stylesheets/components/page-footer.scss @@ -1,6 +1,6 @@ .page-footer { - margin-bottom: 50px; + margin-bottom: 30px; &-back-link { @include button($grey-1); diff --git a/app/main/dao/services_dao.py b/app/main/dao/services_dao.py index 5c6fd25dc..52c217679 100644 --- a/app/main/dao/services_dao.py +++ b/app/main/dao/services_dao.py @@ -1,6 +1,6 @@ from flask import url_for from app import notifications_api_client -from app.main.utils import BrowsableItem +from app.utils import BrowsableItem def insert_new_service(service_name, user_id): diff --git a/app/main/dao/templates_dao.py b/app/main/dao/templates_dao.py index 4eb05ed9d..032c45fbf 100644 --- a/app/main/dao/templates_dao.py +++ b/app/main/dao/templates_dao.py @@ -1,6 +1,6 @@ from flask import url_for, abort from app import notifications_api_client -from app.main.utils import BrowsableItem +from app.utils import BrowsableItem def insert_service_template(name, type_, content, service_id): diff --git a/app/main/forms.py b/app/main/forms.py index d97784863..cf1aa76c8 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -14,7 +14,7 @@ from wtforms.validators import DataRequired, Email, Length, Regexp from app.main.validators import Blacklist, CsvFileValidator -from app.main.utils import ( +from app.utils import ( validate_phone_number, format_phone_number, InvalidPhoneError diff --git a/app/main/views/sms.py b/app/main/views/sms.py index d191063ff..2e4154af3 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -20,7 +20,7 @@ from flask import ( from flask_login import login_required, current_user from werkzeug import secure_filename from notifications_python_client.errors import HTTPError -from utils.template import Template +from utils.template import Template, NeededByTemplateError, NoPlaceholderForDataError from app.main import main from app.main.forms import CsvUploadForm @@ -30,7 +30,7 @@ from app.main.uploader import ( ) from app.main.dao import templates_dao from app import job_api_client -from app.main.utils import ( +from app.utils import ( validate_phone_number, InvalidPhoneError ) @@ -68,7 +68,7 @@ def send_sms(service_id, template_id): return redirect(url_for('.send_sms', service_id=service_id, template_id=template_id)) template = Template( - templates_dao.get_service_template_or_404(service_id, template_id)['data'] + templates_dao.get_service_template_or_404(service_id, template_id)['data'] ) example_data = [dict( @@ -111,19 +111,22 @@ def check_sms(service_id, upload_id): contents = s3download(service_id, upload_id) if not contents: flash('There was a problem reading your upload file') - upload_result = _get_numbers(contents) 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'] + upload_result = _get_rows(contents, raw_template) template = Template( - templates_dao.get_service_template_or_404(service_id, template_id)['data'], - values=upload_result['valid'][0] if upload_result['valid'] else {}, + raw_template, + values=upload_result['rows'][0] if upload_result['valid'] else {}, drop_values={'phone'} ) return render_template( 'views/check-sms.html', upload_result=upload_result, template=template, - column_headers=['phone number'] + template.placeholders_as_markup, + column_headers=['phone number'] + list( + template.placeholders if upload_result['valid'] else template.placeholders_as_markup + ), original_file_name=upload_data.get('original_file_name'), service_id=service_id, form=CsvUploadForm() @@ -155,36 +158,19 @@ def _get_filedata(file): return {'file_name': file.filename, 'data': lines} -def _format_filename(filename): - d = date.today() - basename, extenstion = filename.split('.') - formatted_name = '{}_{}.csv'.format(basename, d.strftime('%Y%m%d')) - return secure_filename(formatted_name) - - -def _get_numbers(contents): +def _get_rows(contents, raw_template): reader = csv.DictReader( contents.split('\n'), lineterminator='\n', - quoting=csv.QUOTE_NONE) - valid, rejects = [], [] - for i, row in enumerate(reader): + quoting=csv.QUOTE_NONE + ) + valid = True + rows = [] + for row in reader: + rows.append(row) try: validate_phone_number(row['phone']) - valid.append(row) - except InvalidPhoneError: - rejects.append({"line_number": i+2, "phone": row['phone']}) - return {"valid": valid, "rejects": rejects} - - -def _get_column_headers(template_content, marked_up=False): - headers = re.findall( - r"\(\(([^\)]+)\)\)", # anything that looks like ((registration number)) - template_content - ) - if marked_up: - return [ - Markup("{}".format(header)) - for header in headers - ] - return headers + Template(raw_template, values=row, drop_values={'phone'}).replaced + except (InvalidPhoneError, NeededByTemplateError, NoPlaceholderForDataError): + valid = False + return {"valid": valid, "rows": rows} diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 87656b2af..c41c4759f 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -11,74 +11,73 @@ {% block maincolumn_content %} -

Check and confirm

- {% if upload_result.rejects %} -

The following numbers are invalid

- {% for rejected in upload_result.rejects %} -

Line {{rejected.line_number}}: {{rejected.phone }} - {% endfor %} -

Go back and resolve errors

+ {% if template.additional_data %} + {{ banner( + "Remove these columns from your CSV file:" + ", ".join(template.missing_data), + type="dangerous" + ) }} + {% elif not upload_result.valid %} + {{ banner( + "Your CSV file contained missing or invalid data", + type="dangerous" + ) }} + {% endif %} - {% else %} +

+ {{ "Check and confirm" if upload_result.valid else "Send text messages" }} +

-
-
- {% if template.missing_data or template.additional_data %} - {{ sms_message(template.formatted_as_markup)}} - {% else %} - {{ sms_message(template.replaced)}} - {% endif %} -
+
+
+ {% if template.missing_data or template.additional_data %} + {{ sms_message(template.formatted_as_markup)}} + {% else %} + {{ sms_message(template.replaced)}} + {% endif %}
+
- {% if template.missing_data %} - {{ banner( - "Add these columns to your CSV file: " + ", ".join(template.missing_data), - type="dangerous" + {% if upload_result.valid %} +
+ + + Back +
+ {% else %} +
+ {{file_upload(form.file, button_text='Choose a CSV file')}} + {{ page_footer( + "Upload" ) }} - {% elif template.additional_data %} - {{ banner( - "Remove these columns from your CSV file:" + ", ".join(template.missing_data), - type="dangerous" - ) }} - {% else %} - - - - Back -
- {% endif %} + + {% endif %} - {% if template.missing_data or template.additional_data %} -
- {{file_upload(form.file, button_text='Choose a CSV file')}} - {{ page_footer( - "Upload" - ) }} -
- {% endif %} - - {% call(item) list_table( - upload_result.valid, - caption=original_file_name, - field_headings=column_headers - ) %} + {% call(item) list_table( + upload_result.rows, + caption=original_file_name, + field_headings=column_headers + ) %} + {% if item.phone|valid_phone_number %} {% call field() %} {{ item.phone }} {% endcall %} - {% for column in template.placeholders %} - {% if item.get(column) %} - {% call field() %} - {{ item.get(column) }} - {% endcall %} - {% else %} - {% call field(status='missing') %} - missing - {% endcall %} - {% endif %} - {% endfor %} - {% endcall %} + {% else %} + {% call field(status='missing') %} + {{ item.phone }} + {% endcall %} + {% endif %} + {% for column in template.placeholders %} + {% if item.get(column) %} + {% call field() %} + {{ item.get(column) }} + {% endcall %} + {% else %} + {% call field(status='missing') %} + missing + {% endcall %} + {% endif %} + {% endfor %} + {% endcall %} - {% endif %} {% endblock %} diff --git a/app/templates/views/send-sms.html b/app/templates/views/send-sms.html index 74c01d724..15671cc60 100644 --- a/app/templates/views/send-sms.html +++ b/app/templates/views/send-sms.html @@ -22,13 +22,6 @@ {{file_upload(form.file, button_text='Choose a CSV file')}} - {{ banner( - 'You can only send messages to yourself until you request to go live'.format( - url_for('.service_request_to_go_live', service_id=service_id) - )|safe, - type='important' - ) }} - {{ page_footer( "Continue to preview" ) }} @@ -36,10 +29,10 @@ {% if column_headers %} {% call(item) list_table( example_data, - caption='Preview', + caption='Example', field_headings=column_headers, field_headings_visible=True, - caption_visible=False, + caption_visible=True, empty_message="Your data here" ) %} {% call field() %} diff --git a/app/main/utils.py b/app/utils.py similarity index 98% rename from app/main/utils.py rename to app/utils.py index 9b023c91d..88a127a23 100644 --- a/app/main/utils.py +++ b/app/utils.py @@ -1,6 +1,3 @@ -from flask import current_app - - class BrowsableItem(object): """ Maps for the template browse-list. diff --git a/tests/app/main/test_phone_validation.py b/tests/app/main/test_phone_validation.py index f1b54c5fb..62595d5da 100644 --- a/tests/app/main/test_phone_validation.py +++ b/tests/app/main/test_phone_validation.py @@ -1,4 +1,4 @@ -from app.main.utils import ( +from app.utils import ( validate_phone_number, InvalidPhoneError ) diff --git a/tests/app/main/views/test_sms.py b/tests/app/main/views/test_sms.py index 57af2f99e..d3609afb4 100644 --- a/tests/app/main/views/test_sms.py +++ b/tests/app/main/views/test_sms.py @@ -63,10 +63,10 @@ def test_upload_csvfile_with_invalid_phone_shows_check_page_with_errors(app_, follow_redirects=True) assert response.status_code == 200 content = response.get_data(as_text=True) - assert 'The following numbers are invalid' in content + assert 'Your CSV file contained missing or invalid data' in content assert '+44 123' in content assert '+44 456' in content - assert 'Go back and resolve errors' in content + assert 'Choose a CSV file' in content @moto.mock_s3