diff --git a/app/__init__.py b/app/__init__.py index e85140659..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 @@ -62,11 +63,10 @@ def create_app(config_name, config_overrides=None): application.session_interface = ItsdangerousSessionInterface() - application.add_template_filter(placeholders) - application.add_template_filter(replace_placeholders) 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) @@ -123,16 +123,6 @@ def convert_to_boolean(value): return value -def placeholders(value): - if not value: - return value - return Markup(re.sub( - r"\(\(([^\)]+)\)\)", # anything that looks like ((registration number)) - lambda match: "{}".format(match.group(1)), - value - )) - - def nl2br(value): _paragraph_re = re.compile(r'(?:\r\n|\r|\n){2,}') @@ -141,16 +131,6 @@ def nl2br(value): return Markup(result) -def replace_placeholders(template, values): - if not template: - return template - return Markup(re.sub( - r"\(\(([^\)]+)\)\)", # anything that looks like ((registration number)) - lambda match: values.get(match.group(1), ''), - template - )) - - def syntax_highlight_json(code): return Markup(highlight(code, JavascriptLexer(), HtmlFormatter(noclasses=True))) @@ -161,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/app.scss b/app/assets/stylesheets/app.scss index dcb2a9f2b..e4a629217 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -40,6 +40,10 @@ a { } } +.form-control-1-1 { + width: 100%; +} + .form-control-5em { width: 100%; 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/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index 650509f24..1b40c5fa8 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -36,6 +36,13 @@ } + &-missing { + color: $error-colour; + font-weight: bold; + border-left: 5px solid $error-colour; + padding-left: 7px; + } + } } diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index f04d61fce..eb420be44 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -49,6 +49,7 @@ $path: '/static/images/'; @import 'components/api-key'; @import 'views/job'; +@import 'views/edit-template'; // TODO: break this up @import 'app'; diff --git a/app/assets/stylesheets/views/edit-template.scss b/app/assets/stylesheets/views/edit-template.scss new file mode 100644 index 000000000..3635b067f --- /dev/null +++ b/app/assets/stylesheets/views/edit-template.scss @@ -0,0 +1,9 @@ +.edit-template { + + &-placeholder-hint { + display: block; + padding-top: 20px; + color: $secondary-text-colour; + } + +} 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 72d47bcd2..d34f2b187 100644 --- a/app/main/dao/templates_dao.py +++ b/app/main/dao/templates_dao.py @@ -1,25 +1,31 @@ -from flask import url_for +from flask import url_for, abort from app import notifications_api_client -from app.main.utils import BrowsableItem +from app.utils import BrowsableItem +from notifications_python_client.errors import HTTPError -def insert_service_template(name, type_, content, service_id): +def insert_service_template(name, content, service_id): return notifications_api_client.create_service_template( - name, type_, content, service_id) + name, 'sms', content, service_id) -def update_service_template(id_, name, type_, content, service_id): +def update_service_template(id_, name, content, service_id): return notifications_api_client.update_service_template( - id_, name, type_, content, service_id) + id_, name, 'sms', content, service_id) def get_service_templates(service_id): return notifications_api_client.get_service_templates(service_id) -def get_service_template(service_id, template_id): - return notifications_api_client.get_service_template( - service_id, template_id) +def get_service_template_or_404(service_id, template_id): + try: + return notifications_api_client.get_service_template(service_id, template_id) + except HTTPError as e: + if e.status_code == 404: + abort(404) + else: + raise e def delete_service_template(service_id, template_id): diff --git a/app/main/forms.py b/app/main/forms.py index d97784863..ddab3fe01 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 @@ -188,7 +188,6 @@ class TemplateForm(Form): name = StringField( u'Template name', validators=[DataRequired(message="Template name cannot be empty")]) - template_type = RadioField(u'Template type', choices=[('sms', 'SMS')]) template_content = TextAreaField( u'Message', diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index d19e29cd3..dd5e45e04 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -8,6 +8,7 @@ from flask import ( ) from flask_login import login_required from notifications_python_client.errors import HTTPError +from utils.template import Template from app import job_api_client from app.main import main @@ -38,7 +39,6 @@ def view_jobs(service_id): def view_job(service_id, job_id): try: job = job_api_client.get_job(service_id, job_id)['data'] - template = templates_dao.get_service_template(service_id, job['template'])['data'] messages = [] return render_template( 'views/job.html', @@ -55,7 +55,9 @@ def view_job(service_id, job_id): cost=u'£0.00', uploaded_file_name=job['original_file_name'], uploaded_file_time=job['created_at'], - template=template, + template=Template( + templates_dao.get_service_template_or_404(service_id, job['template'])['data'] + ), service_id=service_id ) except HTTPError as e: diff --git a/app/main/views/sms.py b/app/main/views/sms.py index c7c7f75c1..958a549e0 100644 --- a/app/main/views/sms.py +++ b/app/main/views/sms.py @@ -1,6 +1,8 @@ import csv import uuid import botocore +import re +import io from datetime import date @@ -15,9 +17,10 @@ from flask import ( current_app ) -from flask_login import login_required +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, NeededByTemplateError, NoPlaceholderForDataError from app.main import main from app.main.forms import CsvUploadForm @@ -27,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 ) @@ -35,22 +38,19 @@ from app.main.utils import ( @main.route("/services//sms/send", methods=['GET']) def choose_sms_template(service_id): - try: - templates = templates_dao.get_service_templates(service_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e - - return render_template('views/choose-sms-template.html', - templates=templates, - service_id=service_id) + return render_template( + 'views/choose-sms-template.html', + templates=[ + Template(template) for template in templates_dao.get_service_templates(service_id)['data'] + ], + service_id=service_id + ) @main.route("/services//sms/send/", methods=['GET', 'POST']) @login_required def send_sms(service_id, template_id): + form = CsvUploadForm() if form.validate_on_submit(): try: @@ -63,24 +63,43 @@ def send_sms(service_id, template_id): service_id=service_id, upload_id=upload_id)) except ValueError as e: - message = 'There was a problem uploading: {}'.format( - csv_file.filename) - flash(message) + 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)) - try: - template = templates_dao.get_service_template(service_id, template_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e + template = Template( + templates_dao.get_service_template_or_404(service_id, template_id)['data'] + ) - return render_template('views/send-sms.html', - template=template, - form=form, - service_id=service_id) + example_data = [dict( + phone=current_user.mobile_number, + **{ + header: "test {}".format(header) for header in template.placeholders + } + )] + + return render_template( + 'views/send-sms.html', + template=template, + column_headers=['phone'] + template.placeholders_as_markup, + placeholders=template.placeholders, + example_data=example_data, + form=form, + service_id=service_id + ) + + +@main.route("/services//sms/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) + output = io.StringIO() + writer = csv.writer(output) + writer.writerow(['phone'] + 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'}) @main.route("/services//sms/check/", @@ -89,22 +108,28 @@ def send_sms(service_id, template_id): def check_sms(service_id, upload_id): if request.method == 'GET': - 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'] - original_file_name = upload_data.get('original_file_name') template_id = upload_data.get('template_id') - template = templates_dao.get_service_template(service_id, template_id)['data'] + raw_template = templates_dao.get_service_template_or_404(service_id, template_id)['data'] + upload_result = _get_rows(contents, raw_template) + template = Template( + 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, - message_template=template['content'], - original_file_name=original_file_name, - template_id=template_id, - service_id=service_id + template=template, + 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() ) elif request.method == 'POST': upload_data = session['upload_data'] @@ -133,23 +158,20 @@ 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, + skipinitialspace=True + ) + 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} + Template(raw_template, values=row, drop_values={'phone'}).replaced + except (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 33d93e93a..a8453f9e5 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1,13 +1,15 @@ from flask import request, render_template, redirect, url_for, flash, abort from flask_login import login_required +from notifications_python_client.errors import HTTPError +from utils.template import Template + from app.main import main from app.main.forms import TemplateForm from app import job_api_client from app.main.dao.services_dao import get_service_by_id from app.main.dao import templates_dao as tdao from app.main.dao import services_dao as sdao -from notifications_python_client.errors import HTTPError @main.route("/services//templates") @@ -15,7 +17,6 @@ from notifications_python_client.errors import HTTPError def manage_service_templates(service_id): try: jobs = job_api_client.get_job(service_id)['data'] - templates = tdao.get_service_templates(service_id)['data'] except HTTPError as e: if e.status_code == 404: abort(404) @@ -25,7 +26,11 @@ def manage_service_templates(service_id): 'views/manage-templates.html', service_id=service_id, has_jobs=bool(jobs), - templates=[tdao.TemplatesBrowsableItem(x) for x in templates]) + templates=[ + Template(template) + for template in tdao.get_service_templates(service_id)['data'] + ] + ) @main.route("/services//templates/add", methods=['GET', 'POST']) @@ -43,7 +48,7 @@ def add_service_template(service_id): if form.validate_on_submit(): tdao.insert_service_template( - form.name.data, form.template_type.data, form.template_content.data, service_id) + form.name.data, form.template_content.data, service_id) return redirect(url_for( '.manage_service_templates', service_id=service_id)) return render_template( @@ -56,20 +61,13 @@ def add_service_template(service_id): @main.route("/services//templates/", methods=['GET', 'POST']) @login_required def edit_service_template(service_id, template_id): - try: - template = tdao.get_service_template(service_id, template_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e - + template = tdao.get_service_template_or_404(service_id, template_id)['data'] template['template_content'] = template['content'] form = TemplateForm(**template) if form.validate_on_submit(): tdao.update_service_template( - template_id, form.name.data, form.template_type.data, + template_id, form.name.data, form.template_content.data, service_id) return redirect(url_for('.manage_service_templates', service_id=service_id)) @@ -84,21 +82,14 @@ def edit_service_template(service_id, template_id): @main.route("/services//templates//delete", methods=['GET', 'POST']) @login_required def delete_service_template(service_id, template_id): - try: - template = tdao.get_service_template(service_id, template_id)['data'] - except HTTPError as e: - if e.status_code == 404: - abort(404) - else: - raise e - - template['template_content'] = template['content'] - form = TemplateForm(**template) + template = tdao.get_service_template_or_404(service_id, template_id)['data'] if request.method == 'POST': tdao.delete_service_template(service_id, template_id) return redirect(url_for('.manage_service_templates', service_id=service_id)) + template['template_content'] = template['content'] + form = TemplateForm(**template) flash('Are you sure you want to delete ‘{}’?'.format(form.name.data), 'delete') return render_template( 'views/edit-template.html', diff --git a/app/templates/components/email-message.html b/app/templates/components/email-message.html index 2a8f65eb8..6d9920121 100644 --- a/app/templates/components/email-message.html +++ b/app/templates/components/email-message.html @@ -10,10 +10,10 @@ {% endif %} {% endmacro %} diff --git a/app/templates/components/sms-message.html b/app/templates/components/sms-message.html index 044f3f5c7..490254485 100644 --- a/app/templates/components/sms-message.html +++ b/app/templates/components/sms-message.html @@ -11,7 +11,7 @@ {% endif %}
- {{ body|placeholders }} + {{ body }}
{% if recipient %}

diff --git a/app/templates/views/check-sms.html b/app/templates/views/check-sms.html index 2a61a83f2..c41c4759f 100644 --- a/app/templates/views/check-sms.html +++ b/app/templates/views/check-sms.html @@ -2,6 +2,7 @@ {% from "components/sms-message.html" import sms_message %} {% from "components/table.html" import list_table, field %} {% from "components/placeholder.html" import placeholder %} +{% from "components/file-upload.html" import file_upload %} {% from "components/page-footer.html" import page_footer %} {% block page_title %} @@ -10,40 +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" }} +

-
-
- {{ sms_message( - message_template|replace_placeholders(upload_result.valid[0]) - )}} -
+
+
+ {% if template.missing_data or template.additional_data %} + {{ sms_message(template.formatted_as_markup)}} + {% else %} + {{ sms_message(template.replaced)}} + {% endif %}
+
-
+ {% if upload_result.valid %} + - - Back + + Back
+ {% else %} +
+ {{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=['Phone number'] - ) %} + {% 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 %} - {% 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/choose-sms-template.html b/app/templates/views/choose-sms-template.html index e12ed204f..383d34834 100644 --- a/app/templates/views/choose-sms-template.html +++ b/app/templates/views/choose-sms-template.html @@ -17,7 +17,7 @@
{% for template in templates %}
- {{ sms_message(template.content, name=template.name) }} + {{ sms_message(template.formatted_as_markup, name=template.name) }}