diff --git a/app/assets/stylesheets/app.scss b/app/assets/stylesheets/app.scss index b47b576be..9a1b05c57 100644 --- a/app/assets/stylesheets/app.scss +++ b/app/assets/stylesheets/app.scss @@ -153,3 +153,24 @@ details summary { text-decoration: underline; margin-bottom: $gutter-half; } + +.spreadsheet { + + th, + .table-field-index { + background: $grey-4; + font-weight: bold; + text-align: center; + } + + th, td { + padding-left: 10px; + padding-right: 10px; + border: 1px solid $border-colour; + } + + td { + border-top: 0; + } + +} diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 915206649..fceec018a 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -1,12 +1,16 @@ -from flask import request, render_template, redirect, url_for, flash, abort +import string + +from flask import request, render_template, redirect, url_for, flash, abort, session from flask_login import login_required -from notifications_utils.template import Template +from notifications_utils.template import Template, TemplateChange +from notifications_utils.recipients import first_column_heading from notifications_python_client.errors import HTTPError from app.main import main from app.utils import user_has_permissions from app.main.forms import SMSTemplateForm, EmailTemplateForm +from app.main.views.send import get_example_csv_rows from app import service_api_client, current_service @@ -109,6 +113,28 @@ def edit_service_template(service_id, template_id): form = form_objects[template['template_type']](**template) if form.validate_on_submit(): + subject = form.subject.data if getattr(form, 'subject', None) else None + new_template = Template({ + 'name': form.name.data, + 'content': form.template_content.data, + 'subject': subject, + 'template_type': template['template_type'], + 'id': template['id'] + }) + template_change = Template(template).compare_to(new_template) + if template_change.has_different_placeholders and not request.form.get('confirm'): + return render_template( + 'views/templates/breaking-change.html', + template_change=template_change, + new_template=new_template, + column_headings=list(string.ascii_uppercase[:len(new_template.placeholders) + 1]), + example_rows=[ + [first_column_heading[new_template.template_type]] + list(new_template.placeholders), + get_example_csv_rows(new_template), + get_example_csv_rows(new_template) + ], + form=form + ) try: service_api_client.update_service_template( template_id, @@ -116,7 +142,7 @@ def edit_service_template(service_id, template_id): template['template_type'], form.template_content.data, service_id, - form.subject.data if getattr(form, 'subject', None) else None + subject ) except HTTPError as e: if e.status_code == 400: diff --git a/app/templates/views/templates/breaking-change.html b/app/templates/views/templates/breaking-change.html new file mode 100644 index 000000000..b6bfd33b3 --- /dev/null +++ b/app/templates/views/templates/breaking-change.html @@ -0,0 +1,72 @@ +{% extends "withnav_template.html" %} +{% from "components/banner.html" import banner_wrapper %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/table.html" import list_table, text_field, index_field, index_field_heading %} + +{% macro list_of_placeholders(placeholders, oxford_comma=False) %} + {% for placeholder in placeholders %} + {% if loop.last and loop.length > 1 %}{% if oxford_comma %},{% endif %} and {% endif %} + (({{ placeholder }})){% if not loop.last and loop.length != 2 %},{% endif %} + {% endfor %} +{% endmacro %} + +{% block page_title %} + GOV.UK Notify +{% endblock %} + +{% block maincolumn_content %} + +

Confirm changes

+ +
+ {% if template_change.placeholders_removed %} +

+ You removed + {{ list_of_placeholders(template_change.placeholders_removed) }} +

+ {% endif %} + {% if template_change.placeholders_added %} +

+ You added {{ list_of_placeholders(template_change.placeholders_added) }} +

+ {% endif %} +
+ +

+ When you send messages using this template you’ll need + {{ new_template.placeholders|length + 1 }} columns of data: +

+ +
+ {% call(item, row_number) list_table( + example_rows, + caption="Example", + caption_visible=False, + field_headings=[''] + column_headings + ) %} + {% if 1 == row_number %} + {{ index_field('') }} + {% else %} + {{ index_field(row_number - 1) }} + {% endif %} + {% for column in item %} + {{ text_field(column) }} + {% endfor %} + {% endcall %} +
+ +

Developers, you’ll need to update your API calls

+ +
+ + + + + {{ page_footer( + 'Save changes to template', + back_link=url_for(".edit_service_template", service_id=current_service.id, template_id=new_template.id), + back_link_text="Back" + ) }} +
+ +{% endblock %} diff --git a/requirements.txt b/requirements.txt index acc96b2f0..b04c2c4ad 100644 --- a/requirements.txt +++ b/requirements.txt @@ -18,4 +18,4 @@ pytz==2016.4 git+https://github.com/alphagov/notifications-python-client.git@1.0.0#egg=notifications-python-client==1.0.0 -git+https://github.com/alphagov/notifications-utils.git@5.3.0#egg=notifications-utils==5.3.0 +git+https://github.com/alphagov/notifications-utils.git@5.4.0#egg=notifications-utils==5.4.0 diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index fb8166ab3..defd07af7 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1,5 +1,6 @@ import json import uuid +from bs4 import BeautifulSoup from tests import validate_route_permission from flask import url_for @@ -67,6 +68,47 @@ def test_should_redirect_when_saving_a_template(app_, template_id, name, 'sms', content, service_id, None) +def test_should_show_interstitial_when_making_breaking_change( + app_, + api_user_active, + mock_login, + mock_get_service_template, + mock_update_service_template, + mock_get_user, + mock_get_service, + mock_get_user_by_email, + mock_has_permissions, + fake_uuid +): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(api_user_active) + service_id = fake_uuid + template_id = fake_uuid + response = client.post( + url_for('.edit_service_template', service_id=service_id, template_id=template_id), + data={ + 'id': template_id, + 'name': "new name", + 'template_content': "hello ((name))", + 'template_type': 'sms', + 'service': service_id + } + ) + + assert response.status_code == 200 + page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') + assert page.h1.string.strip() == "Confirm changes" + + for key, value in { + 'name': 'new name', + 'subject': '', + 'template_content': 'hello ((name))', + 'confirm': 'true' + }.items(): + assert page.find('input', {'name': key})['value'] == value + + def test_should_not_create_too_big_template(app_, api_user_active, mock_login,