From a43e6a8885529cb2635d82efb07d1c736f2c19ce Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Thu, 15 Nov 2018 17:14:03 +0000 Subject: [PATCH] Make deletion confirmation banner messages consistent across our app Also introduce a way to provide context to a banner / flash message that will be displayed in plain font style. --- app/main/views/api_keys.py | 5 +++- app/main/views/templates.py | 7 ++--- app/templates/components/banner.html | 7 ++++- app/templates/flash_messages.html | 7 +++-- app/templates/views/api/keys.html | 30 ++++++-------------- app/templates/views/templates/template.html | 14 --------- tests/app/main/views/test_api_integration.py | 2 +- tests/app/main/views/test_templates.py | 6 ++-- 8 files changed, 28 insertions(+), 50 deletions(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index e59aaf770..90c5feb2a 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -130,9 +130,12 @@ def create_api_key(service_id): def revoke_api_key(service_id, key_id): key_name = current_service.get_api_key(key_id)['name'] if request.method == 'GET': + flash([ + "Are you sure you want to revoke this API key?", + "‘{}’ will no longer let you connect to GOV.UK Notify.".format(key_name) + ], 'revoke') return render_template( 'views/api/keys.html', - revoke_key=key_name, ) elif request.method == 'POST': api_key_api_client.revoke_api_key(service_id=service_id, key_id=key_id) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 058ef0771..488cc1446 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -441,7 +441,7 @@ def delete_template_folder(service_id, template_folder_id): else: abort(500, e) - flash("Are you sure you want to delete the '{}' folder?".format(template_folder_name), 'delete') + flash("Are you sure you want to delete the ‘{}’ folder?".format(template_folder_name), 'delete') return render_template( 'views/templates/manage-template-folder.html', form=form, @@ -628,12 +628,9 @@ def delete_service_template(service_id, template_id): else: raise e + flash(["Are you sure you want to delete ‘{}’?".format(template['name']), message], 'delete') return render_template( 'views/templates/template.html', - template_delete_confirmation_message=( - 'Are you sure you want to delete {}?'.format(template['name']), - message, - ), template=get_template( template, current_service, diff --git a/app/templates/components/banner.html b/app/templates/components/banner.html index b344309aa..c352b247a 100644 --- a/app/templates/components/banner.html +++ b/app/templates/components/banner.html @@ -1,4 +1,4 @@ -{% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None) %} +{% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None, context=None) %}
{{ subhead }} {%- endif -%} {{ body }} + {% if context %} +

+ {{ context }} +

+ {% endif %} {% if delete_button %}
diff --git a/app/templates/flash_messages.html b/app/templates/flash_messages.html index 0107749a1..a33947d48 100644 --- a/app/templates/flash_messages.html +++ b/app/templates/flash_messages.html @@ -4,10 +4,11 @@ {% for category, message in messages %}
{{ banner( - message, + message if message is string else message[0], 'default' if ((category == 'default') or (category == 'default_with_tick')) else 'dangerous', - delete_button="Yes, {}".format(category) if category in ['delete', 'suspend', 'resume', 'remove'] else None, - with_tick=True if category == 'default_with_tick' else False + delete_button="Yes, {}".format(category) if category in ['delete', 'suspend', 'resume', 'remove', 'revoke'] else None, + with_tick=True if category == 'default_with_tick' else False, + context=message[1] if message is not string )}}
{% endfor %} diff --git a/app/templates/views/api/keys.html b/app/templates/views/api/keys.html index 5bbfc0e28..66b36d62a 100644 --- a/app/templates/views/api/keys.html +++ b/app/templates/views/api/keys.html @@ -10,30 +10,16 @@ {% block maincolumn_content %} - {% if revoke_key %} -
- {% call banner_wrapper(type='dangerous', subhead='Are you sure you want to revoke this API key?') %} -

- ‘{{ revoke_key }}’ will no longer let you connect to GOV.UK Notify. -

- - - - - {% endcall %} +
+
+

+ API keys +

- {% else %} -
-
-

- API keys -

-
-
- Create an API key -
+
+ Create an API key
- {% endif %} +
{% call(item, row_number) list_table( diff --git a/app/templates/views/templates/template.html b/app/templates/views/templates/template.html index aaf0f08fa..8bde511c6 100644 --- a/app/templates/views/templates/template.html +++ b/app/templates/views/templates/template.html @@ -27,20 +27,6 @@ {% endcall %}
- {% elif template_delete_confirmation_message %} -
- {% call banner_wrapper(type='dangerous', subhead=template_delete_confirmation_message[0]) %} - {% if template_delete_confirmation_message[1] %} -

- {{ template_delete_confirmation_message[1] }} -

- {% endif %} -
- - -
- {% endcall %} -
{% else %}

{{ template.name }}

{% endif %} diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index a0e62694a..f187fec1f 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -310,7 +310,7 @@ def test_should_show_confirm_revoke_api_key( assert normalize_spaces(page.select('.banner-dangerous')[0].text) == ( 'Are you sure you want to revoke this API key? ' '‘some key name’ will no longer let you connect to GOV.UK Notify. ' - 'Confirm' + 'Yes, revoke' ) assert mock_get_api_keys.call_args_list == [ call( diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 1a57db7e5..5f4664143 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -1281,7 +1281,7 @@ def test_should_show_delete_template_page_with_time_block( template_id=fake_uuid, _test_page_title=False, ) - assert page.h1.text == 'Are you sure you want to delete Two week reminder?' + assert "Are you sure you want to delete ‘Two week reminder’?" in page.select('.banner-dangerous')[0].text assert normalize_spaces(page.select('.banner-dangerous p')[0].text) == ( 'It was last used 10 minutes ago' ) @@ -1310,7 +1310,7 @@ def test_should_show_delete_template_page_with_time_block_for_empty_notification template_id=fake_uuid, _test_page_title=False, ) - assert page.h1.text == 'Are you sure you want to delete Two week reminder?' + assert "Are you sure you want to delete ‘Two week reminder’?" in page.select('.banner-dangerous')[0].text assert normalize_spaces(page.select('.banner-dangerous p')[0].text) == ( 'It was last used more than seven days ago' ) @@ -1336,7 +1336,7 @@ def test_should_show_delete_template_page_with_never_used_block( template_id=fake_uuid, _test_page_title=False, ) - assert page.h1.text == 'Are you sure you want to delete Two week reminder?' + assert "Are you sure you want to delete ‘Two week reminder’?" in page.select('.banner-dangerous')[0].text assert not page.select('.banner-dangerous p') assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == ( 'service one: Template content with & entity'