Warn users a template change will break things

When a user adds or removes placeholders in their template we should consider
this a ‘breaking change’ and warn them accordingly.

Implementing this mostly relies on using
https://github.com/alphagov/notifications-utils/pull/37

Temporarily storing the new template until the user confirms that they want to
make the changes in done using hidden fields. This is a bit hacky, but the
complexity of making sessions interact with WTForms was just too much to handle.

This commit also changes the example spreadsheet that we show on this page to
look more like a spreadsheet.
This commit is contained in:
Chris Hill-Scott
2016-05-27 16:21:29 +01:00
parent f2cca024dd
commit 3ac76192d0
5 changed files with 165 additions and 4 deletions

View File

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

View File

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

View File

@@ -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 %}
<span class="placeholder">(({{ placeholder }}))</span>{% if not loop.last and loop.length != 2 %},{% endif %}
{% endfor %}
{% endmacro %}
{% block page_title %}
GOV.UK Notify
{% endblock %}
{% block maincolumn_content %}
<h1 class="heading-large">Confirm changes</h1>
<div class="bottom-gutter">
{% if template_change.placeholders_removed %}
<p>
You removed
{{ list_of_placeholders(template_change.placeholders_removed) }}
</p>
{% endif %}
{% if template_change.placeholders_added %}
<p>
You added {{ list_of_placeholders(template_change.placeholders_added) }}
</p>
{% endif %}
</div>
<p>
When you send messages using this template youll need
{{ new_template.placeholders|length + 1 }} columns of data:
</p>
<div class="spreadsheet">
{% 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 %}
</div>
<p>Developers, youll need to update your API calls</p>
<form method="post">
<input type="hidden" name="name" value="{{ new_template.name }}" />
<input type="hidden" name="subject" value="{{ new_template.subject or '' }}" />
<input type="hidden" name="template_content" value="{{ new_template.content }}" />
<input type="hidden" name="confirm" value="true" />
{{ 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"
) }}
</form>
{% endblock %}

View File

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

View File

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