From befe93ec0b8062d97d76fa97c70dfc46366ebd2d Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 24 Jul 2017 14:50:56 +0100 Subject: [PATCH] Make sure confirmation/danger banners have a H1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes errors on all pages have a `

` element, which is important for accessibility. It means a bit of rewriting the messages, but I think they’re better for it. --- app/main/views/templates.py | 14 +++-- app/templates/components/banner.html | 2 +- app/templates/views/templates/template.html | 14 +++++ tests/app/main/views/test_templates.py | 70 +++++++++------------ 4 files changed, 53 insertions(+), 47 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index f2b0f5727..180c4fc5e 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -428,22 +428,24 @@ def delete_service_template(service_id, template_id): last_used_notification = template_statistics_client.get_template_statistics_for_template( service_id, template['id'] ) - message = '{} was last used {} ago'.format( - last_used_notification['template']['name'], + message = 'It was last used {} ago.'.format( get_human_readable_delta( parse(last_used_notification['created_at']).replace(tzinfo=None), - datetime.utcnow()) + datetime.utcnow() + ) ) except HTTPError as e: if e.status_code == 404: - message = '{} has never been used'.format(template['name']) + message = 'It’s never been used.'.format(template['name']) else: raise e - flash('{}. Are you sure you want to delete it?'.format(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 a21984b25..d797525d4 100644 --- a/app/templates/components/banner.html +++ b/app/templates/components/banner.html @@ -7,7 +7,7 @@ {% endif %} > {% if subhead -%} - {{ subhead }}  +

{{ subhead }}

{%- endif -%} {{ body }} {% if delete_button %} diff --git a/app/templates/views/templates/template.html b/app/templates/views/templates/template.html index fb98e8a90..a2638582d 100644 --- a/app/templates/views/templates/template.html +++ b/app/templates/views/templates/template.html @@ -29,6 +29,20 @@ {% endif %} + {% if template_delete_confirmation_message %} +
+ {% call banner_wrapper(type='dangerous', subhead=template_delete_confirmation_message[0]) %} +

+ {{ template_delete_confirmation_message[1] }} +

+
+ + +
+ {% endcall %} +
+ {% endif %} +

{{ template.name }}

diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index dfc9f8df8..5c8fe8b44 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -679,16 +679,10 @@ def test_should_redirect_when_saving_a_template_email( def test_should_show_delete_template_page_with_time_block( - logged_in_client, - api_user_active, - mock_login, - mock_get_service, + client_request, mock_get_service_template, - mock_get_user, - mock_get_user_by_email, - mock_has_permissions, - fake_uuid, mocker, + fake_uuid ): with freeze_time('2012-01-01 12:00:00'): template = template_json('1234', '1234', "Test template", "sms", "Something very interesting") @@ -698,30 +692,25 @@ def test_should_show_delete_template_page_with_time_block( return_value=notification) with freeze_time('2012-01-01 12:10:00'): - service_id = fake_uuid - template_id = fake_uuid - response = logged_in_client.get(url_for( + page = client_request.get( '.delete_service_template', - service_id=service_id, - template_id=template_id)) - content = response.get_data(as_text=True) - assert response.status_code == 200 - assert 'Test template was last used 10 minutes ago. Are you sure you want to delete it?' in content - assert 'Are you sure' in content - assert 'Two week reminder' in content - assert 'Template <em>content</em> with & entity' in content - mock_get_service_template.assert_called_with(service_id, template_id) + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _test_page_title=False, + ) + assert page.h1.text == 'Are you sure you want to delete Two week reminder?' + assert normalize_spaces(page.select('.banner-dangerous p')[0].text) == ( + 'It was last used 10 minutes ago.' + ) + assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == ( + 'service one: Template content with & entity' + ) + mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid) def test_should_show_delete_template_page_with_never_used_block( - logged_in_client, - api_user_active, - mock_login, - mock_get_service, + client_request, mock_get_service_template, - mock_get_user, - mock_get_user_by_email, - mock_has_permissions, fake_uuid, mocker, ): @@ -729,20 +718,20 @@ def test_should_show_delete_template_page_with_never_used_block( 'app.template_statistics_client.get_template_statistics_for_template', side_effect=HTTPError(response=Mock(status_code=404), message="Default message") ) - service_id = fake_uuid - template_id = fake_uuid - response = logged_in_client.get(url_for( + page = client_request.get( '.delete_service_template', - service_id=service_id, - template_id=template_id)) - - content = response.get_data(as_text=True) - assert response.status_code == 200 - assert 'Two week reminder has never been used. Are you sure you want to delete it?' in content - assert 'Are you sure' in content - assert 'Two week reminder' in content - assert 'Template <em>content</em> with & entity' in content - mock_get_service_template.assert_called_with(service_id, template_id) + service_id=SERVICE_ONE_ID, + template_id=fake_uuid, + _test_page_title=False, + ) + assert page.h1.text == 'Are you sure you want to delete Two week reminder?' + assert normalize_spaces(page.select('.banner-dangerous p')[0].text) == ( + 'It’s never been used.' + ) + assert normalize_spaces(page.select('.sms-message-wrapper')[0].text) == ( + 'service one: Template content with & entity' + ) + mock_get_service_template.assert_called_with(SERVICE_ONE_ID, fake_uuid) def test_should_redirect_when_deleting_a_template( @@ -1094,6 +1083,7 @@ def test_should_show_message_before_redacting_template( 'main.redact_template', service_id=SERVICE_ONE_ID, template_id=fake_uuid, + _test_page_title=False, ) assert (