From 411abb136a1d39d6bb769574e8e20d5bb4aa829c Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 24 Mar 2022 12:26:06 +0000 Subject: [PATCH 1/3] Make broadcast user fixtures realistic A broadcast service user will have the permission on creation [^1]. [^1]: https://github.com/alphagov/notifications-api/blob/b145a299351255c5755ca547629b7936cb7fcf4a/app/dao/broadcast_service_dao.py#L69 --- tests/conftest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 69bb391a5..84bebc261 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3604,6 +3604,7 @@ def create_active_user_create_broadcasts_permissions(with_unique_id=False): 'create_broadcasts', 'reject_broadcasts', 'cancel_broadcasts', + 'view_activity', # added automatically by API ]}, auth_type='webauthn_auth', ) @@ -3617,6 +3618,7 @@ def create_active_user_approve_broadcasts_permissions(with_unique_id=False): 'approve_broadcasts', 'reject_broadcasts', 'cancel_broadcasts', + 'view_activity', # added automatically by API ]}, auth_type='webauthn_auth', ) From dd85cf076d259dd6709bb7c687ac881d2e4e6f8f Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 24 Mar 2022 12:28:14 +0000 Subject: [PATCH 2/3] Fix test for broadcast service with realistic user Previously the user had permissions like "manage_users", which can't currently be set in the UI. --- tests/app/test_navigation.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 8477a9581..01e3769bf 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -521,13 +521,20 @@ def test_navigation_urls( def test_navigation_for_services_with_broadcast_permission( + mocker, client_request, service_one, mock_get_service_templates, mock_get_template_folders, mock_get_api_keys, + active_user_create_broadcasts_permission, ): service_one['permissions'] += ['broadcast'] + mocker.patch( + 'app.user_api_client.get_user', + return_value=active_user_create_broadcasts_permission + ) + page = client_request.get('main.choose_template', service_id=SERVICE_ONE_ID) assert [ a['href'] for a in page.select('.navigation a') @@ -537,7 +544,6 @@ def test_navigation_for_services_with_broadcast_permission( '/services/{}/rejected-alerts'.format(SERVICE_ONE_ID), '/services/{}/templates'.format(SERVICE_ONE_ID), '/services/{}/users'.format(SERVICE_ONE_ID), - '/services/{}/service-settings'.format(SERVICE_ONE_ID), ] From b6bc598e8c5ca290b5bc7818b14c4d0f01ee17a9 Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 24 Mar 2022 12:29:14 +0000 Subject: [PATCH 3/3] Make it easy to see API keys for broadcast service This made it easier to debug a problem with the functional tests due to the fixtures not working correctly [^1]. It's a platform admin only convenience over knowing the page URL. We may want to expose the top-level "/api-integration" page but that will require more work to show which broadcasts were sent with which key - currently it's oriented around "messages". For now I think it's useful to see what keys a service has. [^1]: https://github.com/alphagov/notifications-functional-tests/pull/411#pullrequestreview-920069799 --- app/templates/main_nav.html | 8 ++++++-- tests/app/test_navigation.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 092825c73..5969d20c9 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -25,8 +25,12 @@ {% if current_user.has_permissions('manage_api_keys', 'manage_service') %}
  • Settings
    • {% endif %} - {% if current_user.has_permissions('manage_api_keys') and not current_service.has_permission('broadcast') %} -
  • API integration
    • + {% if current_user.has_permissions('manage_api_keys') %} + {% if current_service.has_permission('broadcast') %} +
  • API integration
    • + {% else %} +
  • API integration
    • + {% endif %} {% endif %} {% elif current_user.has_permissions(allow_org_user=True) %}
  • Usage
    • diff --git a/tests/app/test_navigation.py b/tests/app/test_navigation.py index 01e3769bf..81a6ea634 100644 --- a/tests/app/test_navigation.py +++ b/tests/app/test_navigation.py @@ -547,6 +547,35 @@ def test_navigation_for_services_with_broadcast_permission( ] +def test_navigation_for_services_with_broadcast_permission_platform_admin( + mocker, + client_request, + service_one, + mock_get_service_templates, + mock_get_template_folders, + mock_get_api_keys, + platform_admin_user, +): + service_one['permissions'] += ['broadcast'] + mocker.patch( + 'app.user_api_client.get_user', + return_value=platform_admin_user, + ) + + page = client_request.get('main.choose_template', service_id=SERVICE_ONE_ID) + assert [ + a['href'] for a in page.select('.navigation a') + ] == [ + '/services/{}/current-alerts'.format(SERVICE_ONE_ID), + '/services/{}/past-alerts'.format(SERVICE_ONE_ID), + '/services/{}/rejected-alerts'.format(SERVICE_ONE_ID), + '/services/{}/templates'.format(SERVICE_ONE_ID), + '/services/{}/users'.format(SERVICE_ONE_ID), + '/services/{}/service-settings'.format(SERVICE_ONE_ID), + '/services/{}/api/keys'.format(SERVICE_ONE_ID), + ] + + def test_caseworkers_get_caseworking_navigation( client_request, mocker,