From 9d37040d49ca9afa56133009db27209b0466dc15 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 5 May 2016 08:38:55 +0100 Subject: [PATCH 1/4] Add recipient to message previews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When previewing a template on the send page, having the recipient appear as a placeholder should help reinforce the relationship between the columns in the CSV and the placeholders. Then, when previewing a message, having the template populated with the first recipient’s email address/phone number should reinforce that relationship again. --- .../stylesheets/components/sms-message.scss | 2 +- app/main/views/send.py | 2 ++ app/templates/components/email-message.html | 25 ++++++++++++++++- app/templates/components/sms-message.html | 27 ++++++++++++++----- app/templates/views/check.html | 8 ++++-- 5 files changed, 54 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/components/sms-message.scss b/app/assets/stylesheets/components/sms-message.scss index 32d57ebd9..21dc3227c 100644 --- a/app/assets/stylesheets/components/sms-message.scss +++ b/app/assets/stylesheets/components/sms-message.scss @@ -31,7 +31,7 @@ .sms-message-recipient { @include copy-19; color: $secondary-text-colour; - margin: -20px 0 $gutter 0; + margin: 10px 0 5px 0; } .sms-message-name { diff --git a/app/main/views/send.py b/app/main/views/send.py index f18bc8c19..cf9bebc22 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -234,12 +234,14 @@ def check_messages(service_id, template_type, upload_id): with suppress(StopIteration): template.values = next(recipients.rows) + first_recipient = template.values.get(recipients.recipient_column_header, '') session['upload_data']['notification_count'] = len(list(recipients.rows)) session['upload_data']['valid'] = not recipients.has_errors return render_template( 'views/check.html', recipients=recipients, + first_recipient=first_recipient, template=template, page_heading=get_page_headings(template.template_type), errors=get_errors_for_csv(recipients, template.template_type), diff --git a/app/templates/components/email-message.html b/app/templates/components/email-message.html index 9df0f3d26..297a97a4d 100644 --- a/app/templates/components/email-message.html +++ b/app/templates/components/email-message.html @@ -1,4 +1,13 @@ -{% macro email_message(subject, body, name=None, edit_link=None, from_name=None, from_address=None) %} +{% macro email_message( + subject, + body, + name=None, + edit_link=None, + from_name=None, + from_address=None, + recipient=None, + show_placeholder_for_recipient=False +) %} {% if name %}

{% if edit_link %} @@ -20,6 +29,20 @@ {% endif %} + {% if recipient is not none %} + + To + + {% if show_placeholder_for_recipient %} + + email address + + {% else %} + {{ recipient }} + {% endif %} + + + {% endif %} {% if subject %} Subject diff --git a/app/templates/components/sms-message.html b/app/templates/components/sms-message.html index eb809d409..5ae59f21a 100644 --- a/app/templates/components/sms-message.html +++ b/app/templates/components/sms-message.html @@ -1,4 +1,12 @@ -{% macro sms_message(body, recipient=None, name=None, id=None, edit_link=None, from=None) %} +{% macro sms_message( + body, + recipient=None, + name=None, + id=None, + edit_link=None, + from=None, + show_placeholder_for_recipient=False +) %} {% if name %}

{% if edit_link %} @@ -8,6 +16,18 @@ {% endif %}

{% endif %} + {% if recipient is not none %} +

+ To: + {% if show_placeholder_for_recipient %} + + phone number + + {% else %} + {{ recipient }} + {% endif %} +

+ {% endif %}
{% if from %} @@ -16,11 +36,6 @@ {% endif %} {{ body|nl2br }}
- {% if recipient %} -

- {{ recipient }} -

- {% endif %} {% if id %}

Template ID: {{ id }} diff --git a/app/templates/views/check.html b/app/templates/views/check.html index 5cbcbc4ee..c368bca9c 100644 --- a/app/templates/views/check.html +++ b/app/templates/views/check.html @@ -77,13 +77,17 @@ template.formatted_subject_as_markup if errors else template.replaced_subject, template.formatted_as_markup if errors else template.replaced, from_address='{}@notifications.service.gov.uk'.format(current_service.email_from), - from_name=current_service.name + from_name=current_service.name, + recipient=first_recipient, + show_placeholder_for_recipient=errors )}} {% elif 'sms' == template.template_type %}

{{ sms_message( - template.formatted_as_markup if errors else template.replaced + template.formatted_as_markup if errors else template.replaced, + recipient=first_recipient, + show_placeholder_for_recipient=errors )}}
From 687ccad018655bac531e288cb365143f918554a4 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 5 May 2016 08:39:36 +0100 Subject: [PATCH 2/4] Make send a test teach you how placeholders work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit does two main things: - adds textboxes to the send yourself a test page, so you can replace ((name)) with ‘Chris’, or whatever your name is - rejigs the send page a bit to make it clearer what the CSV upload is for and how to use it The idea being that, since most users go into ‘send yourself a test’ first, it teaches them about how placeholders work by making them do the replacing themselves. --- .../stylesheets/components/file-upload.scss | 5 + .../stylesheets/components/page-footer.scss | 6 +- app/main/views/send.py | 103 +++++++++++------- app/templates/components/file-upload.html | 7 +- app/templates/views/check.html | 10 +- app/templates/views/send-test.html | 59 ++++++++++ app/templates/views/send.html | 64 +++++++---- tests/app/main/views/test_send.py | 56 ++++++++-- tests/conftest.py | 12 ++ 9 files changed, 245 insertions(+), 77 deletions(-) create mode 100644 app/templates/views/send-test.html diff --git a/app/assets/stylesheets/components/file-upload.scss b/app/assets/stylesheets/components/file-upload.scss index 2b9a596ca..dc33ad3f9 100644 --- a/app/assets/stylesheets/components/file-upload.scss +++ b/app/assets/stylesheets/components/file-upload.scss @@ -50,6 +50,11 @@ display: none; } + &-alternate-link { + display: inline-block; + line-height: 35px; + } + } } diff --git a/app/assets/stylesheets/components/page-footer.scss b/app/assets/stylesheets/components/page-footer.scss index bb627a396..ee41ec9fb 100644 --- a/app/assets/stylesheets/components/page-footer.scss +++ b/app/assets/stylesheets/components/page-footer.scss @@ -5,7 +5,6 @@ &-back-link { @include button($grey-1); display: inline-block; - margin-left: 10px; } &-delete-link { @@ -36,6 +35,11 @@ margin-top: $gutter; } + .button, + .button-destructive { + margin-right: 10px; + } + .button-destructive { @include button($error-colour); padding: 0.52632em 0.78947em 0.26316em 0.78947em; diff --git a/app/main/views/send.py b/app/main/views/send.py index cf9bebc22..d9a81a02c 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -50,20 +50,26 @@ def get_page_headings(template_type): }[template_type] -def get_example_csv_rows(template, number_of_rows=2): +def get_example_csv_fields(column_headers, use_example_as_example, submitted_fields): + print("="*40) + print(list(column_headers)) + if use_example_as_example: + return ["example" for header in column_headers] + elif submitted_fields: + return [submitted_fields.get(header) for header in column_headers] + else: + return list(column_headers) + + +def get_example_csv_rows(template, use_example_as_example=True, submitted_fields=False): return [ - [ - { - 'email': current_user.email_address, - 'sms': validate_and_format_phone_number( - current_user.mobile_number, human_readable=True - ) - }[template.template_type] - ] + [ - "{} {}".format(header, i) for header in template.placeholders - ] - for i in range(1, number_of_rows + 1) - ] + { + 'email': 'test@example.com' if use_example_as_example else current_user.email_address, + 'sms': '07700 900321' if use_example_as_example else validate_and_format_phone_number( + current_user.mobile_number, human_readable=True + ) + }[template.template_type] + ] + get_example_csv_fields(template.placeholders, use_example_as_example, submitted_fields) @main.route("/services//send/", methods=['GET']) @@ -131,7 +137,7 @@ def send_messages(service_id, template_id): 'views/send.html', template=template, recipient_column=first_column_heading[template.template_type], - example=get_example_csv_rows(template), + example=[get_example_csv_rows(template, use_example_as_example=True)], form=form ) @@ -139,43 +145,58 @@ def send_messages(service_id, template_id): @main.route("/services//send/.csv", methods=['GET']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters', 'manage_templates', any_=True) -def get_example_csv(service_id, template_id, number_of_rows=2): +def get_example_csv(service_id, template_id): template = Template(service_api_client.get_service_template(service_id, template_id)['data']) with io.StringIO() as output: writer = csv.writer(output) - writer.writerows( - [ - [first_column_heading[template.template_type]] + - list(template.placeholders) - ] + - get_example_csv_rows(template, number_of_rows=number_of_rows) - ) + writer.writerows([ + [first_column_heading[template.template_type]] + list(template.placeholders), + get_example_csv_rows(template) + ]) return output.getvalue(), 200, { 'Content-Type': 'text/csv; charset=utf-8', 'Content-Disposition': 'inline; filename="{}.csv"'.format(template.name) } -@main.route("/services//send//to-self", methods=['GET']) +@main.route("/services//send//test", methods=['GET', 'POST']) @login_required @user_has_permissions('send_texts', 'send_emails', 'send_letters') -def send_message_to_self(service_id, template_id): - template = Template(service_api_client.get_service_template(service_id, template_id)['data']) +def send_test(service_id, template_id): - filedata = { - 'file_name': 'Test run', - 'data': get_example_csv(service_id, template_id, number_of_rows=1)[0] - } + template = Template( + service_api_client.get_service_template(service_id, template_id)['data'], + prefix=current_service['name'] + ) - upload_id = str(uuid.uuid4()) + if len(template.placeholders) == 0 or request.method == 'POST': + with io.StringIO() as output: + writer = csv.writer(output) + writer.writerows([ + [first_column_heading[template.template_type]] + list(template.placeholders), + get_example_csv_rows(template, use_example_as_example=False, submitted_fields=request.form) + ]) + filedata = { + 'file_name': 'Test run', + 'data': output.getvalue() + } + 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_messages', + upload_id=upload_id, + service_id=service_id, + template_type=template.template_type, + from_test=True + )) - 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_messages', - upload_id=upload_id, - service_id=service_id, - template_type=template.template_type)) + return render_template( + 'views/send-test.html', + template=template, + recipient_column=first_column_heading[template.template_type], + example=[get_example_csv_rows(template, use_example_as_example=False)] + ) @main.route("/services//send//from-api", methods=['GET']) @@ -232,6 +253,11 @@ def check_messages(service_id, template_type, upload_id): ) if current_service['restricted'] else None ) + if request.args.get('from_test'): + back_link = url_for('.send_test', service_id=service_id, template_id=template.id) + else: + back_link = url_for('.send_messages', service_id=service_id, template_id=template.id) + with suppress(StopIteration): template.values = next(recipients.rows) first_recipient = template.values.get(recipients.recipient_column_header, '') @@ -256,7 +282,8 @@ def check_messages(service_id, template_type, upload_id): send_button_text=get_send_button_text(template.template_type, session['upload_data']['notification_count']), upload_id=upload_id, form=CsvUploadForm(), - statistics=statistics + statistics=statistics, + back_link=back_link ) diff --git a/app/templates/components/file-upload.html b/app/templates/components/file-upload.html index 6dcb22a2e..89f4ea25d 100644 --- a/app/templates/components/file-upload.html +++ b/app/templates/components/file-upload.html @@ -1,4 +1,4 @@ -{% macro file_upload(field, button_text="Choose file") %} +{% macro file_upload(field, button_text="Choose file", alternate_link=None, alternate_link_text=None) %}