Make send the send flow generic

This commit parameterises all methods in the send view so that they can send
either emails or SMS messages.

It works out what kind of message it is sending from the `template_type`
property of the template object.

This means that the `Template` util class needs to know about these properties,
which means that this commit depends on:
https://github.com/alphagov/notifications-utils/pull/2

This commit does _not_ add tests for sending emails. The existing tests for
sending SMS still pass, but actually sending emails is outside the scope of
this story.
This commit is contained in:
Chris Hill-Scott
2016-02-22 17:17:18 +00:00
parent aaa6317371
commit 1e46922876
13 changed files with 174 additions and 103 deletions

View File

@@ -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;
}

View File

@@ -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):

View File

@@ -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/<service_id>/send/<template_type>", 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/<service_id>/send/<int:template_id>", 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/<service_id>/send/<template_id>/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/<service_id>/check/<upload_id>",
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('Weve 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}

View File

@@ -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,

View File

@@ -42,7 +42,7 @@
<form method="post" enctype="multipart/form-data">
<input type="hidden" name="csrf_token" value="{{ csrf_token() }}" />
<input type="submit" class="button" value="{{ "Send {} text message{}".format(upload_result.rows|count, '' if upload_result.rows|count == 1 else 's') }}" />
<a href="{{url_for('.send_sms', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
<a href="{{url_for('.send_messages', service_id=service_id, template_id=template.id)}}" class="page-footer-back-link">Back</a>
</form>
{% else %}
{{file_upload(form.file, button_text='Upload a CSV file')}}

View File

@@ -21,8 +21,6 @@
</div>
<div class="column-one-third">
<div class="sms-message-use-links">
<a href="{{ url_for(".send_sms", service_id=service_id, template_id=template.id) }}">Add recipients</a>
<a href="{{ url_for(".send_sms_to_self", service_id=service_id, template_id=template.id) }}">Send yourself a test</a>
<a href="{{ url_for(".edit_service_template", service_id=service_id, template_id=template.id) }}">Edit template</a>
</div>
</div>

View File

@@ -30,8 +30,8 @@
</div>
<div class="column-one-third">
<div class="sms-message-use-links">
<a href="{{ url_for(".send_sms", service_id=service_id, template_id=template.id) }}">Add recipients</a>
<a href="{{ url_for(".send_sms_to_self", service_id=service_id, template_id=template.id) }}">Send yourself a test</a>
<a href="{{ url_for(".send_messages", service_id=service_id, template_id=template.id) }}">Add recipients</a>
<a href="{{ url_for(".send_message_to_self", service_id=service_id, template_id=template.id) }}">Send yourself a test</a>
<a href="{{ url_for(".edit_service_template", service_id=service_id, template_id=template.id) }}">Edit template</a>
</div>
</div>

View File

@@ -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 @@
<div class="grid-row">
<div class="column-two-thirds">
{{ 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 well only send to your mobile number until you <a href="{}">request to go live</a>'|safe,
'You can upload real data, but well only send to your mobile number until you <a href="{}">request to go live</a>'.format(
url_for('.service_request_to_go_live', service_id=service_id)
)|safe,
'info'
)}}
</div>
</div>

View File

@@ -36,7 +36,7 @@
</ol>
""".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 @@
"""
<a href='{}'>Send yourself a text message</a>
""".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"

View File

@@ -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)