From 3fe7fd73678a2b90c492aaeaa9a14b1589ce4526 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 5 Aug 2020 10:39:59 +0100 Subject: [PATCH 01/10] Add context to 'Change details' links on /users --- app/templates/views/manage-users.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 444361dcc..b4bc7dcee 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -75,7 +75,7 @@ {% if user.status == 'pending' %} Cancel invitation {% elif user.state == 'active' and current_user.id != user.id %} - Change details + Change details for {{ user.name }} {{ user.email_address }} {% endif %} {% endif %} From 334c2677dccedef8f30fcc95e28c36c13244b2af Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 5 Aug 2020 10:47:00 +0100 Subject: [PATCH 02/10] Add context to 'Hide' button in cookie banner --- app/templates/components/cookie-banner.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/components/cookie-banner.html b/app/templates/components/cookie-banner.html index 0a80091a1..d642a37d7 100644 --- a/app/templates/components/cookie-banner.html +++ b/app/templates/components/cookie-banner.html @@ -19,7 +19,7 @@ - + {% endmacro %} From a52aa406df149d86bf5af2b5970d5a499ca7fc77 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 5 Aug 2020 10:51:37 +0100 Subject: [PATCH 03/10] Add context to template page 'Send' and 'Edit' --- app/templates/views/templates/_template.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/templates/views/templates/_template.html b/app/templates/views/templates/_template.html index ce7ed729b..ba4f384f6 100644 --- a/app/templates/views/templates/_template.html +++ b/app/templates/views/templates/_template.html @@ -39,7 +39,7 @@ {% if current_user.has_permissions('manage_templates') %} {% endif %} @@ -47,7 +47,7 @@ {% if current_user.has_permissions('send_messages', restrict_admin_usage=True) %} {% endif %} From 0933485995837e3ee64622cd6f75853e6256d68a Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 5 Aug 2020 11:15:38 +0100 Subject: [PATCH 04/10] Add context to 'Move' button on /templates Includes adding an extra parameter to the page_footer component. --- app/assets/javascripts/templateFolderForm.js | 4 +++- app/templates/components/page-footer.html | 6 ++++++ app/templates/views/templates/_move_to.html | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/templateFolderForm.js b/app/assets/javascripts/templateFolderForm.js index c7d804f4e..6e0854ade 100644 --- a/app/assets/javascripts/templateFolderForm.js +++ b/app/assets/javascripts/templateFolderForm.js @@ -246,7 +246,9 @@ this.itemsSelectedButtons = $(`
- +
- {{ page_footer('Move', button_name='operation', button_value='move-to-existing-folder') }} + {{ page_footer('Move', button_name='operation', button_value='move-to-existing-folder', button_text_hidden_suffix=' selection to folder') }}
From f608d5e979508419addd561b3dad332bc3edc856 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 5 Aug 2020 11:28:56 +0100 Subject: [PATCH 05/10] Add context to banner button text Needs flash messaging system changes. --- app/main/views/templates.py | 2 +- app/templates/components/banner.html | 9 +++++---- app/templates/flash_messages.html | 3 ++- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 384abda56..232031f8f 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -690,7 +690,7 @@ def delete_service_template(service_id, template_id): else: raise e - flash(["Are you sure you want to delete ‘{}’?".format(template['name']), message], 'delete') + flash(["Are you sure you want to delete ‘{}’?".format(template['name']), message, template['name']], 'delete') return render_template( 'views/templates/template.html', template=get_template( diff --git a/app/templates/components/banner.html b/app/templates/components/banner.html index fbf3d6115..109ff3023 100644 --- a/app/templates/components/banner.html +++ b/app/templates/components/banner.html @@ -1,7 +1,7 @@ {% from "components/form.html" import form_wrapper %} {% from "components/button/macro.njk" import govukButton %} -{% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None, context=None, action=None, id=None) %} +{% macro banner(body, type=None, with_tick=False, delete_button=None, subhead=None, context=None, action=None, id=None, thing=None) %}
{{ govukButton({ - "text": delete_button, + "text": "" if thing else delete_button, + "html": delete_button + " ‘" + thing + "’" if thing else "", "name": "delete", "classes": "govuk-button--warning govuk-!-margin-top-2", }) }} @@ -34,6 +35,6 @@
{% endmacro %} -{% macro banner_wrapper(type=None, with_tick=False, delete_button=None, subhead=None, action=None, id=None) %} - {{ banner(caller()|safe, type=type, with_tick=with_tick, delete_button=delete_button, subhead=subhead, action=action, id=id) }} +{% macro banner_wrapper(type=None, with_tick=False, delete_button=None, subhead=None, action=None, id=None, thing=None) %} + {{ banner(caller()|safe, type=type, with_tick=with_tick, delete_button=delete_button, subhead=subhead, action=action, id=id, thing=thing) }} {% endmacro %} diff --git a/app/templates/flash_messages.html b/app/templates/flash_messages.html index 0b9bdd7b3..a69c9db2b 100644 --- a/app/templates/flash_messages.html +++ b/app/templates/flash_messages.html @@ -15,7 +15,8 @@ 'default' if ((category == 'default') or (category == 'default_with_tick')) else 'dangerous', delete_button=delete_button_text, with_tick=True if category == 'default_with_tick' else False, - context=message[1] if message is not string + context=message[1] if message is not string, + thing=message[2] if message is not string and message[2] )}}
{% endfor %} From 5bc36a813dab7778accb20a391a2180a0a2f9070 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 5 Aug 2020 16:45:16 +0100 Subject: [PATCH 06/10] Add context to 'Edit' link on template page --- tests/app/main/views/test_templates.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/app/main/views/test_templates.py b/tests/app/main/views/test_templates.py index 13f859a78..75c8a6496 100644 --- a/tests/app/main/views/test_templates.py +++ b/tests/app/main/views/test_templates.py @@ -757,7 +757,7 @@ def test_view_broadcast_template( service_id=SERVICE_ONE_ID, template_id=fake_uuid, )), - ('Edit', url_for( + ('Edit this template', url_for( '.edit_service_template', service_id=SERVICE_ONE_ID, template_id=fake_uuid, From aacac232bf2fcc64177f3019b6b9b7acbb6b0b21 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 6 Aug 2020 10:14:12 +0100 Subject: [PATCH 07/10] Add context to 'Cancel invitation' links --- app/templates/views/manage-users.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index b4bc7dcee..6b41d9c41 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -73,7 +73,7 @@ {% if current_user.has_permissions('manage_service') %}
{% endif %} {% endcall %} - {{ edit_field('Change', url_for('.link_service_to_organisation', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.link_service_to_organisation', service_id=current_service.id), suffix='organisation for service') }} {% endcall %} {% call row() %} {{ text_field('Free text message allowance')}} {{ text_field('{:,}'.format(current_service.free_sms_fragment_limit)) }} - {{ edit_field('Change', url_for('.set_free_sms_allowance', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.set_free_sms_allowance', service_id=current_service.id), suffix='free text message allowance') }} {% endcall %} {% call row() %} {{ text_field('Email branding' )}} {{ text_field(current_service.email_branding_name) }} - {{ edit_field('Change', url_for('.service_set_email_branding', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.service_set_email_branding', service_id=current_service.id), suffix='email branding (admin view)') }} {% endcall %} {% call row() %} {{ text_field('Letter branding')}} {{ optional_text_field(current_service.letter_branding.name) }} - {{ edit_field('Change', url_for('.service_set_letter_branding', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.service_set_letter_branding', service_id=current_service.id), suffix='letter branding (admin view)') }} {% endcall %} {% call row() %} {{ text_field('Data retention')}} {% call field() %} {{ current_service.data_retention | join(', ', attribute='notification_type') }} {% endcall %} - {{ edit_field('Change', url_for('.data_retention', service_id=current_service.id)) }} + {{ edit_field('Change', url_for('.data_retention', service_id=current_service.id), suffix='data retention') }} {% endcall %} {% for permission in service_permissions %} @@ -365,7 +372,15 @@ {% call row() %} {{ text_field(service_permissions[permission].title)}} {{ boolean_field(current_service.has_permission(permission)) }} - {{ edit_field('Change', url_for(service_permissions[permission].endpoint or '.service_set_permission', service_id=current_service.id, permission=permission if not service_permissions[permission].endpoint else None)) }} + {{ edit_field( + 'Change', + url_for( + service_permissions[permission].endpoint or '.service_set_permission', + service_id=current_service.id, + permission=permission if not service_permissions[permission].endpoint else None + ), + suffix='your settings for ' + service_permissions[permission].title, + ) }} {% endcall %} {% endif %} {% endfor %} From c12b7cbb5d4301b877639b3b1b4e6d9b51e99344 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Wed, 5 Aug 2020 16:44:46 +0100 Subject: [PATCH 10/10] Fix tests from changes to link text --- .../test_service_setting_permissions.py | 13 +- tests/app/main/views/test_manage_users.py | 4 +- tests/app/main/views/test_service_settings.py | 148 +++++++++--------- 3 files changed, 85 insertions(+), 80 deletions(-) diff --git a/tests/app/main/views/service_settings/test_service_setting_permissions.py b/tests/app/main/views/service_settings/test_service_setting_permissions.py index d68c6bfd1..455e56538 100644 --- a/tests/app/main/views/service_settings/test_service_setting_permissions.py +++ b/tests/app/main/views/service_settings/test_service_setting_permissions.py @@ -114,13 +114,16 @@ def test_service_set_permission( @pytest.mark.parametrize('service_fields, endpoint, kwargs, text', [ - ({'restricted': True}, '.service_switch_live', {}, 'Live Off Change'), - ({'restricted': False}, '.service_switch_live', {}, 'Live On Change'), - ({'permissions': ['sms']}, '.service_set_inbound_number', {}, 'Receive inbound SMS Off Change'), + ({'restricted': True}, '.service_switch_live', {}, 'Live Off Change service status'), + ({'restricted': False}, '.service_switch_live', {}, 'Live On Change service status'), + ({'permissions': ['sms']}, '.service_set_inbound_number', {}, + 'Receive inbound SMS Off Change your settings for Receive inbound SMS'), ({'permissions': ['letter']}, - '.service_set_permission', {'permission': 'upload_letters'}, 'Uploading letters Off Change'), + '.service_set_permission', {'permission': 'upload_letters'}, + 'Uploading letters Off Change your settings for Uploading letters'), ({'permissions': ['letter']}, - '.service_set_permission', {'permission': 'international_letters'}, 'Send international letters Off Change'), + '.service_set_permission', {'permission': 'international_letters'}, + 'Send international letters Off Change your settings for Send international letters'), ]) def test_service_setting_toggles_show( mocker, diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 39a48d5ae..f60e1e4de 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -36,7 +36,7 @@ from tests.conftest import ( 'Cannot Add and edit templates ' 'Cannot Manage settings, team and usage ' 'Cannot Manage API integration ' - 'Change details' + 'Change details for ZZZZZZZZ zzzzzzz@example.gov.uk' ) ), ( @@ -1050,7 +1050,7 @@ def test_cancel_invited_user_doesnt_work_if_user_not_invited_to_this_service( 'Cannot Add and edit templates ' 'Can Manage settings, team and usage ' 'Can Manage API integration ' - 'Cancel invitation' + 'Cancel invitation for invited_user@test.gov.uk' )), ('cancelled', ( 'invited_user@test.gov.uk (cancelled invite) ' diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index bed592afa..efc8ccca9 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -55,59 +55,59 @@ def mock_get_service_settings_page_common( (create_active_user_with_permissions(), [ 'Label Value Action', - 'Service name Test Service Change', - 'Sign-in method Text message code Change', + 'Service name Test Service Change service name', + 'Sign-in method Text message code Change sign-in method', 'Label Value Action', - 'Send emails On Change', - 'Reply-to email addresses Not set Manage', - 'Email branding GOV.UK Change', - 'Send files by email contact_us@gov.uk Manage', + 'Send emails On Change your settings for sending emails', + 'Reply-to email addresses Not set Manage reply-to email addresses', + 'Email branding GOV.UK Change email branding', + 'Send files by email contact_us@gov.uk Manage sending files by email', 'Label Value Action', - 'Send text messages On Change', - 'Text message senders GOVUK Manage', - 'Start text messages with service name On Change', - 'Send international text messages Off Change', - 'Receive text messages Off Change', + 'Send text messages On Change your settings for sending text messages', + 'Text message senders GOVUK Manage text message senders', + 'Start text messages with service name On Change your settings for starting text messages with service name', + 'Send international text messages Off Change your settings for sending international text messages', + 'Receive text messages Off Change your settings for receiving text messages', 'Label Value Action', - 'Send letters Off Change', + 'Send letters Off Change your settings for sending letters', ]), (create_platform_admin_user(), [ 'Label Value Action', - 'Service name Test Service Change', - 'Sign-in method Text message code Change', + 'Service name Test Service Change service name', + 'Sign-in method Text message code Change sign-in method', 'Label Value Action', - 'Send emails On Change', - 'Reply-to email addresses Not set Manage', - 'Email branding GOV.UK Change', - 'Send files by email contact_us@gov.uk Manage', + 'Send emails On Change your settings for sending emails', + 'Reply-to email addresses Not set Manage reply-to email addresses', + 'Email branding GOV.UK Change email branding', + 'Send files by email contact_us@gov.uk Manage sending files by email', 'Label Value Action', - 'Send text messages On Change', - 'Text message senders GOVUK Manage', - 'Start text messages with service name On Change', - 'Send international text messages Off Change', - 'Receive text messages Off Change', + 'Send text messages On Change your settings for sending text messages', + 'Text message senders GOVUK Manage text message senders', + 'Start text messages with service name On Change your settings for starting text messages with service name', + 'Send international text messages Off Change your settings for sending international text messages', + 'Receive text messages Off Change your settings for receiving text messages', 'Label Value Action', - 'Send letters Off Change', + 'Send letters Off Change your settings for sending letters', 'Label Value Action', - 'Live Off Change', - 'Count in list of live services Yes Change', - 'Organisation Test organisation Central government Change', - 'Free text message allowance 250,000 Change', - 'Email branding GOV.UK Change', - 'Letter branding Not set Change', - 'Data retention email Change', - 'Receive inbound SMS Off Change', - 'Email authentication Off Change', - 'Send cell broadcasts Off Change', + 'Live Off Change service status', + 'Count in list of live services Yes Change if service is counted in list of live services', + 'Organisation Test organisation Central government Change organisation for service', + 'Free text message allowance 250,000 Change free text message allowance', + 'Email branding GOV.UK Change email branding (admin view)', + 'Letter branding Not set Change letter branding (admin view)', + 'Data retention email Change data retention', + 'Receive inbound SMS Off Change your settings for Receive inbound SMS', + 'Email authentication Off Change your settings for Email authentication', + 'Send cell broadcasts Off Change your settings for Send cell broadcasts', ]), ]) def test_should_show_overview( @@ -165,7 +165,7 @@ def test_no_go_live_link_for_service_without_organisation( organisation = find_element_by_tag_and_partial_text(page, tag='td', string='Organisation') assert normalize_spaces(organisation.find_next_siblings()[0].text) == 'Not set Central government' - assert normalize_spaces(organisation.find_next_siblings()[1].text) == 'Change' + assert normalize_spaces(organisation.find_next_siblings()[1].text) == 'Change organisation for service' def test_organisation_name_links_to_org_dashboard( @@ -194,8 +194,8 @@ def test_organisation_name_links_to_org_dashboard( @pytest.mark.parametrize('service_contact_link,expected_text', [ - ('contact.me@gov.uk', 'Send files by email contact.me@gov.uk Manage'), - (None, 'Send files by email Not set up Manage'), + ('contact.me@gov.uk', 'Send files by email contact.me@gov.uk Manage sending files by email'), + (None, 'Send files by email Not set up Manage sending files by email'), ]) def test_send_files_by_email_row_on_settings_page( client_request, @@ -230,69 +230,69 @@ def test_send_files_by_email_row_on_settings_page( @pytest.mark.parametrize('permissions, expected_rows', [ (['email', 'sms', 'inbound_sms', 'international_sms'], [ - 'Service name service one Change', - 'Sign-in method Text message code Change', + 'Service name service one Change service name', + 'Sign-in method Text message code Change sign-in method', 'Label Value Action', - 'Send emails On Change', - 'Reply-to email addresses test@example.com Manage', - 'Email branding Organisation name Change', - 'Send files by email Not set up Manage', + 'Send emails On Change your settings for sending emails', + 'Reply-to email addresses test@example.com Manage reply-to email addresses', + 'Email branding Organisation name Change email branding', + 'Send files by email Not set up Manage sending files by email', 'Label Value Action', - 'Send text messages On Change', - 'Text message senders GOVUK Manage', - 'Start text messages with service name On Change', - 'Send international text messages On Change', - 'Receive text messages On Change', + 'Send text messages On Change your settings for sending text messages', + 'Text message senders GOVUK Manage text message senders', + 'Start text messages with service name On Change your settings for starting text messages with service name', + 'Send international text messages On Change your settings for sending international text messages', + 'Receive text messages On Change your settings for receiving text messages', 'Label Value Action', - 'Send letters Off Change', + 'Send letters Off Change your settings for sending letters', ]), (['email', 'sms', 'email_auth'], [ - 'Service name service one Change', - 'Sign-in method Email link or text message code Change', + 'Service name service one Change service name', + 'Sign-in method Email link or text message code Change sign-in method', 'Label Value Action', - 'Send emails On Change', - 'Reply-to email addresses test@example.com Manage', - 'Email branding Organisation name Change', - 'Send files by email Not set up Manage', + 'Send emails On Change your settings for sending emails', + 'Reply-to email addresses test@example.com Manage reply-to email addresses', + 'Email branding Organisation name Change email branding', + 'Send files by email Not set up Manage sending files by email', 'Label Value Action', - 'Send text messages On Change', - 'Text message senders GOVUK Manage', - 'Start text messages with service name On Change', - 'Send international text messages Off Change', - 'Receive text messages Off Change', + 'Send text messages On Change your settings for sending text messages', + 'Text message senders GOVUK Manage text message senders', + 'Start text messages with service name On Change your settings for starting text messages with service name', + 'Send international text messages Off Change your settings for sending international text messages', + 'Receive text messages Off Change your settings for receiving text messages', 'Label Value Action', - 'Send letters Off Change', + 'Send letters Off Change your settings for sending letters', ]), (['letter'], [ - 'Service name service one Change', - 'Sign-in method Text message code Change', + 'Service name service one Change service name', + 'Sign-in method Text message code Change sign-in method', 'Label Value Action', - 'Send emails Off Change', + 'Send emails Off Change your settings for sending emails', 'Label Value Action', - 'Send text messages Off Change', + 'Send text messages Off Change your settings for sending text messages', 'Label Value Action', - 'Send letters On Change', - 'Sender addresses 1 Example Street Manage', - 'Letter branding Not set Change', + 'Send letters On Change your settings for sending letters', + 'Sender addresses 1 Example Street Manage sender addresses', + 'Letter branding Not set Change letter branding', ]), (['broadcast'], [ - 'Service name service one Change', - 'Sign-in method Text message code Change', + 'Service name service one Change service name', + 'Sign-in method Text message code Change sign-in method', ]), ]) @@ -2063,9 +2063,11 @@ def test_and_more_hint_appears_on_settings_with_more_than_just_a_single_sender( find_element_by_tag_and_partial_text(page, tag='tr', string=label).text ) - assert get_row(page, 'Reply-to email addresses') == "Reply-to email addresses test@example.com …and 2 more Manage" - assert get_row(page, 'Text message senders') == "Text message senders Example …and 2 more Manage" - assert get_row(page, 'Sender addresses') == "Sender addresses 1 Example Street …and 2 more Manage" + assert get_row(page, 'Reply-to email addresses') == \ + "Reply-to email addresses test@example.com …and 2 more Manage reply-to email addresses" + assert get_row(page, 'Text message senders') == \ + "Text message senders Example …and 2 more Manage text message senders" + assert get_row(page, 'Sender addresses') == "Sender addresses 1 Example Street …and 2 more Manage sender addresses" @pytest.mark.parametrize('sender_list_page, index, expected_output', [