From c31264d4c9b1b757146e60f2361dc6b68e1cde53 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 12 Jun 2020 09:00:08 +0100 Subject: [PATCH 1/7] =?UTF-8?q?Rename=20=E2=80=98whitelist=E2=80=99=20to?= =?UTF-8?q?=20=E2=80=98guest=20list=E2=80=99=20in=20UI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit changes all the places where a user would see the term ‘whitelist’ in the content of page to say guestlist instead. We’re removing the term ‘whitelist’ for two reasons. The first reason is that we agree with the National Cyber Security Centre say: > It's fairly common to say whitelisting and blacklisting to describe > desirable and undesirable things in cyber security. For instance, when > talking about which applications you will allow or deny on your > corporate network; or deciding which bad passwords you want your users > not to be able to use. > However, there's an issue with the terminology. It only makes sense if > you equate white with 'good, permitted, safe' and black with 'bad, > dangerous, forbidden'. There are some obvious problems with this. So > in the name of helping to stamp out racism in cyber security, we will > avoid this casually pejorative wording on our website in the future. > No, it's not the biggest issue in the world - but to borrow a slogan > from elsewhere: every little helps. – https://www.ncsc.gov.uk/blog-post/terminology-its-not-black-and-white The second reason is that we’ve observed some users think that they have to put recipients in the whitelist even when they’re already with in the team. We think that the term ‘whitelist’ might be reinforcing this mental model because of how ‘whitelists’ might work in other applications. We considered the following alternatives or concepts: - Development - Recipients - Sandbox - Extended team - Smoke test recipients - Allowed - Nominated - Bonus - Additional - Safe - Team list - Trusted contacts - Designated people - Guest list - Team key list We also considered not giving it a name, and explaining it as a nuance of how the team key works. After mocking this up it felt more disjoined. We think it’s still useful for the thing to have a name so that it’s easy to refer to between the docs and the UI. We like the term ‘guest list’ because: - of how it sits with team members – members and guests in the abstract - a guest list is a concept that a lot of people will be familiar with – a list of people who can access a thing - ‘guest’ is very different to ‘recipient’ – we want to mitigate any confusion between this and the (emergency) contact lists --- app/main/views/api_keys.py | 4 ++-- app/templates/views/api/index.html | 2 +- app/templates/views/api/keys.html | 2 +- app/templates/views/api/whitelist.html | 8 ++++---- tests/app/main/views/test_api_integration.py | 8 ++++---- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 8ea786659..8c120411d 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -61,7 +61,7 @@ def whitelist(service_id): 'email_addresses': list(filter(None, form.email_addresses.data)), 'phone_numbers': list(filter(None, form.phone_numbers.data)) }) - flash('Whitelist updated', 'default_with_tick') + flash('Guest list updated', 'default_with_tick') return redirect(url_for('.api_integration', service_id=service_id)) if not form.errors: form.populate(**service_api_client.get_whitelist(service_id)) @@ -85,7 +85,7 @@ def create_api_key(service_id): form = CreateKeyForm(current_service.api_keys) form.key_type.choices = [ (KEY_TYPE_NORMAL, 'Live – sends to anyone'), - (KEY_TYPE_TEAM, 'Team and whitelist – limits who you can send to'), + (KEY_TYPE_TEAM, 'Team and guest list – limits who you can send to'), (KEY_TYPE_TEST, 'Test – pretends to send messages'), ] disabled_options, option_hints = [], {} diff --git a/app/templates/views/api/index.html b/app/templates/views/api/index.html index c8cf54694..f5633863f 100644 --- a/app/templates/views/api/index.html +++ b/app/templates/views/api/index.html @@ -19,7 +19,7 @@ API keys
- Whitelist + Guest list
Callbacks diff --git a/app/templates/views/api/keys.html b/app/templates/views/api/keys.html index b9d0ba773..72b9be879 100644 --- a/app/templates/views/api/keys.html +++ b/app/templates/views/api/keys.html @@ -33,7 +33,7 @@ {% if item.key_type == 'normal' %} Live – sends to anyone {% elif item.key_type == 'team' %} - Team and whitelist – limits who you can send to + Team and guest list – limits who you can send to {% elif item.key_type == 'test' %} Test – pretends to send messages {% endif %} diff --git a/app/templates/views/api/whitelist.html b/app/templates/views/api/whitelist.html index e8fa33977..55745283d 100644 --- a/app/templates/views/api/whitelist.html +++ b/app/templates/views/api/whitelist.html @@ -8,7 +8,7 @@ {% from "components/form.html" import form_wrapper %} {% block service_page_title %} - Whitelist + Guest list {% endblock %} {% block maincolumn_content %} @@ -16,7 +16,7 @@ {% if form.email_addresses.errors or form.phone_numbers.errors %} {% call banner_wrapper(type='dangerous') %}

- There was a problem with your whitelist + There was a problem with your guest list

Fix these errors:

- Guest list + Guest list
Callbacks diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index 5cf6083b3..dd15da606 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -421,7 +421,7 @@ def test_should_show_whitelist_page( mock_get_whitelist, ): page = client_request.get( - 'main.whitelist', + 'main.guest_list', service_id=SERVICE_ONE_ID, ) textboxes = page.find_all('input', {'type': 'text'}) @@ -443,7 +443,7 @@ def test_should_update_whitelist( ]) client_request.post( - 'main.whitelist', + 'main.guest_list', service_id=SERVICE_ONE_ID, _data=data, ) @@ -459,7 +459,7 @@ def test_should_validate_whitelist_items( ): page = client_request.post( - 'main.whitelist', + 'main.guest_list', service_id=SERVICE_ONE_ID, _data=OrderedDict([ ('email_addresses-1', 'abc'), From 16cc64082269f106654653b991f5a43e57fd4ecf Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 12 Jun 2020 09:05:26 +0100 Subject: [PATCH 4/7] =?UTF-8?q?Rename=20API=20client=20methods=20to=20remo?= =?UTF-8?q?ve=20term=20=E2=80=98whitelist=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See c31264d4c for rationale. To avoid confusion the codebase should use the same terminology as the UI. --- app/main/views/api_keys.py | 4 ++-- app/notify_client/service_api_client.py | 4 ++-- tests/app/main/views/test_api_integration.py | 10 +++++----- tests/app/notify_client/test_service_api_client.py | 2 +- tests/conftest.py | 10 +++++----- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index fd9aa949b..21117997e 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -57,14 +57,14 @@ def api_documentation(service_id): def guest_list(service_id): form = GuestList() if form.validate_on_submit(): - service_api_client.update_whitelist(service_id, { + service_api_client.update_guest_list(service_id, { 'email_addresses': list(filter(None, form.email_addresses.data)), 'phone_numbers': list(filter(None, form.phone_numbers.data)) }) flash('Guest list updated', 'default_with_tick') return redirect(url_for('.api_integration', service_id=service_id)) if not form.errors: - form.populate(**service_api_client.get_whitelist(service_id)) + form.populate(**service_api_client.get_guest_list(service_id)) return render_template( 'views/api/whitelist.html', form=form diff --git a/app/notify_client/service_api_client.py b/app/notify_client/service_api_client.py index 7378e2de1..1747a171f 100644 --- a/app/notify_client/service_api_client.py +++ b/app/notify_client/service_api_client.py @@ -326,11 +326,11 @@ class ServiceAPIClient(NotifyAdminAPIClient): def get_monthly_notification_stats(self, service_id, year): return self.get(url='/service/{}/notifications/monthly?year={}'.format(service_id, year)) - def get_whitelist(self, service_id): + def get_guest_list(self, service_id): return self.get(url='/service/{}/whitelist'.format(service_id)) @cache.delete('service-{service_id}') - def update_whitelist(self, service_id, data): + def update_guest_list(self, service_id, data): return self.put(url='/service/{}/whitelist'.format(service_id), data=data) def get_inbound_sms(self, service_id, user_number=''): diff --git a/tests/app/main/views/test_api_integration.py b/tests/app/main/views/test_api_integration.py index dd15da606..c3ecce6c4 100644 --- a/tests/app/main/views/test_api_integration.py +++ b/tests/app/main/views/test_api_integration.py @@ -418,7 +418,7 @@ def test_should_show_whitelist_page( api_user_active, mock_get_service, mock_has_permissions, - mock_get_whitelist, + mock_get_guest_list, ): page = client_request.get( 'main.guest_list', @@ -433,7 +433,7 @@ def test_should_show_whitelist_page( def test_should_update_whitelist( client_request, - mock_update_whitelist, + mock_update_guest_list, ): data = OrderedDict([ ('email_addresses-1', 'test@example.com'), @@ -448,14 +448,14 @@ def test_should_update_whitelist( _data=data, ) - mock_update_whitelist.assert_called_once_with(SERVICE_ONE_ID, { + mock_update_guest_list.assert_called_once_with(SERVICE_ONE_ID, { 'email_addresses': ['test@example.com', 'test@example.com'], 'phone_numbers': ['07900900000', '+1800-555-555']}) def test_should_validate_whitelist_items( client_request, - mock_update_whitelist, + mock_update_guest_list, ): page = client_request.post( @@ -477,7 +477,7 @@ def test_should_validate_whitelist_items( assert jump_links[1].string.strip() == 'Enter valid phone numbers' assert jump_links[1]['href'] == '#phone_numbers' - assert mock_update_whitelist.called is False + assert mock_update_guest_list.called is False @pytest.mark.parametrize('endpoint', [ diff --git a/tests/app/notify_client/test_service_api_client.py b/tests/app/notify_client/test_service_api_client.py index 1b5703d56..d3de2c0cf 100644 --- a/tests/app/notify_client/test_service_api_client.py +++ b/tests/app/notify_client/test_service_api_client.py @@ -393,7 +393,7 @@ def test_returns_value_from_cache( (service_api_client, 'suspend_service', [SERVICE_ONE_ID], {}), (service_api_client, 'resume_service', [SERVICE_ONE_ID], {}), (service_api_client, 'remove_user_from_service', [SERVICE_ONE_ID, ''], {}), - (service_api_client, 'update_whitelist', [SERVICE_ONE_ID, {}], {}), + (service_api_client, 'update_guest_list', [SERVICE_ONE_ID, {}], {}), (service_api_client, 'create_service_inbound_api', [SERVICE_ONE_ID] + [''] * 3, {}), (service_api_client, 'update_service_inbound_api', [SERVICE_ONE_ID] + [''] * 4, {}), (service_api_client, 'add_reply_to_email_address', [SERVICE_ONE_ID, ''], {}), diff --git a/tests/conftest.py b/tests/conftest.py index 25b5b083d..b819c457e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2827,22 +2827,22 @@ def mock_update_email_branding(mocker): @pytest.fixture(scope='function') -def mock_get_whitelist(mocker): - def _get_whitelist(service_id): +def mock_get_guest_list(mocker): + def _get_guest_list(service_id): return { 'email_addresses': ['test@example.com'], 'phone_numbers': ['07900900000'] } return mocker.patch( - 'app.service_api_client.get_whitelist', side_effect=_get_whitelist + 'app.service_api_client.get_guest_list', side_effect=_get_guest_list ) @pytest.fixture(scope='function') -def mock_update_whitelist(mocker): +def mock_update_guest_list(mocker): return mocker.patch( - 'app.service_api_client.update_whitelist' + 'app.service_api_client.update_guest_list' ) From e721c73119451f4e07bdcdc044a7259662b5ad34 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 12 Jun 2020 09:06:22 +0100 Subject: [PATCH 5/7] =?UTF-8?q?Rename=20Jinja=20template=20to=20remove=20t?= =?UTF-8?q?erm=20=E2=80=98whitelist=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See c31264d4c for rationale. To avoid confusion the codebase should use the same terminology as the UI. --- app/main/views/api_keys.py | 2 +- app/templates/views/api/{whitelist.html => guest-list.html} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename app/templates/views/api/{whitelist.html => guest-list.html} (100%) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 21117997e..48c6be905 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -66,7 +66,7 @@ def guest_list(service_id): if not form.errors: form.populate(**service_api_client.get_guest_list(service_id)) return render_template( - 'views/api/whitelist.html', + 'views/api/guest-list.html', form=form ) diff --git a/app/templates/views/api/whitelist.html b/app/templates/views/api/guest-list.html similarity index 100% rename from app/templates/views/api/whitelist.html rename to app/templates/views/api/guest-list.html From 8bc5fa5bb07e3864f315d2bb11447efa0d60f604 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 12 Jun 2020 09:07:29 +0100 Subject: [PATCH 6/7] =?UTF-8?q?Rename=20URL=20to=20remove=20term=20?= =?UTF-8?q?=E2=80=98whitelist=E2=80=99?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See c31264d4c for rationale. To avoid confusion the codebase should use the same terminology as the UI. --- app/main/views/api_keys.py | 3 ++- app/navigation.py | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 48c6be905..8fe5e1c88 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -52,7 +52,8 @@ def api_documentation(service_id): return redirect(url_for('.documentation'), code=301) -@main.route("/services//api/whitelist", methods=['GET', 'POST']) +@main.route("/services//api/whitelist", methods=['GET', 'POST'], endpoint='old_guest_list') +@main.route("/services//api/guest-list", methods=['GET', 'POST']) @user_has_permissions('manage_api_keys') def guest_list(service_id): form = GuestList() diff --git a/app/navigation.py b/app/navigation.py index 97a30d178..0368112d4 100644 --- a/app/navigation.py +++ b/app/navigation.py @@ -349,6 +349,7 @@ class HeaderNavigation(Navigation): 'no_cookie.view_template_version_preview', 'view_template_versions', 'guest_list', + 'old_guest_list', 'who_can_use_notify', 'who_its_for', } @@ -481,6 +482,7 @@ class MainNavigation(Navigation): 'received_text_messages_callback', 'revoke_api_key', 'guest_list', + 'old_guest_list', }, } @@ -976,6 +978,7 @@ class CaseworkNavigation(Navigation): 'no_cookie.view_template_version_preview', 'view_template_versions', 'guest_list', + 'old_guest_list', 'who_can_use_notify', 'who_its_for', } @@ -1286,6 +1289,7 @@ class OrgNavigation(Navigation): 'no_cookie.view_template_version_preview', 'view_template_versions', 'guest_list', + 'old_guest_list', 'who_can_use_notify', 'who_its_for', } From 0f0b2dad3f645232eb24f970cc75da8c3f473405 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 12 Jun 2020 09:15:23 +0100 Subject: [PATCH 7/7] Correct whitelist to non-government in test name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See c31264d4c for why ‘whitelist’ should be avoided. The use of whitelist here was not referring to the user-maintained list, but to mean ‘not a government’ email address. This commit renames these tests to make that difference clear. --- tests/app/main/views/test_add_service.py | 4 ++-- tests/app/main/views/test_forgot_password.py | 2 +- tests/app/main/views/test_manage_users.py | 4 ++-- tests/conftest.py | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 582460468..96147d4db 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -314,7 +314,7 @@ def test_should_return_form_errors_with_duplicate_service_name_regardless_of_cas ) -def test_non_whitelist_user_cannot_access_create_service_page( +def test_non_government_user_cannot_access_create_service_page( client_request, mock_get_non_govuser, api_nongov_user_active, @@ -327,7 +327,7 @@ def test_non_whitelist_user_cannot_access_create_service_page( ) -def test_non_whitelist_user_cannot_create_service( +def test_non_government_user_cannot_create_service( client_request, mock_get_non_govuser, api_nongov_user_active, diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index c28c14430..c75f83210 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -15,7 +15,7 @@ def test_should_render_forgot_password(client): @pytest.mark.parametrize('email_address', [ 'test@user.gov.uk', - 'someuser@notonwhitelist.com' + 'someuser@notgovernment.com' ]) def test_should_redirect_to_password_reset_sent_for_valid_email( client, diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 36095e59b..eeca85f48 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -684,7 +684,7 @@ def test_should_show_folder_permission_form_if_service_has_folder_permissions_en @pytest.mark.parametrize('email_address, gov_user', [ ('test@example.gov.uk', True), - ('test@nonwhitelist.com', False) + ('test@example.com', False) ]) def test_invite_user( client_request, @@ -735,7 +735,7 @@ def test_invite_user( ]) @pytest.mark.parametrize('email_address, gov_user', [ ('test@example.gov.uk', True), - ('test@nonwhitelist.com', False) + ('test@example.com', False) ]) def test_invite_user_with_email_auth_service( client_request, diff --git a/tests/conftest.py b/tests/conftest.py index b819c457e..daaeec0cb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1152,7 +1152,7 @@ def api_nongov_user_active(fake_uuid): 'id': fake_uuid, 'name': 'Test User', 'password': 'somepassword', - 'email_address': 'someuser@notonwhitelist.com', + 'email_address': 'someuser@example.com', 'mobile_number': '07700 900762', 'state': 'active', 'failed_login_count': 0, @@ -1414,7 +1414,7 @@ def mock_register_user(mocker, api_user_pending): @pytest.fixture(scope='function') def mock_get_non_govuser(mocker, api_user_active): - api_user_active['email_address'] = 'someuser@notonwhitelist.com' + api_user_active['email_address'] = 'someuser@example.com' def _get_user(id_): api_user_active['id'] = id_