From 16d1526c46cb0364dfb7121231553736886dfbb8 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Tue, 2 Apr 2019 17:04:25 +0100 Subject: [PATCH 1/2] bump utils to 31.2.4 brings in dev logging filter --- requirements-app.txt | 2 +- requirements.txt | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/requirements-app.txt b/requirements-app.txt index e44d51732..3dadce190 100644 --- a/requirements-app.txt +++ b/requirements-app.txt @@ -23,4 +23,4 @@ awscli-cwlogs>=1.4,<1.5 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default itsdangerous==0.24 # pyup: <1.0.0 -git+https://github.com/alphagov/notifications-utils.git@31.2.2#egg=notifications-utils==31.2.2 +git+https://github.com/alphagov/notifications-utils.git@31.2.4#egg=notifications-utils==31.2.4 diff --git a/requirements.txt b/requirements.txt index e773ded81..ab2ecc76b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,13 +25,13 @@ awscli-cwlogs>=1.4,<1.5 # Putting upgrade on hold due to v1.0.0 using sha512 instead of sha1 by default itsdangerous==0.24 # pyup: <1.0.0 -git+https://github.com/alphagov/notifications-utils.git@31.2.2#egg=notifications-utils==31.2.2 +git+https://github.com/alphagov/notifications-utils.git@31.2.4#egg=notifications-utils==31.2.4 ## The following requirements were added by pip freeze: -awscli==1.16.130 +awscli==1.16.136 bleach==3.1.0 boto3==1.6.16 -botocore==1.12.120 +botocore==1.12.126 certifi==2019.3.9 chardet==3.0.4 Click==7.0 @@ -48,7 +48,7 @@ jdcal==1.4 Jinja2==2.10 jmespath==0.9.4 lml==0.0.9 -lxml==4.3.2 +lxml==4.3.3 MarkupSafe==1.1.1 mistune==0.8.4 monotonic==1.5 @@ -61,7 +61,7 @@ PyJWT==1.7.1 PyPDF2==1.26.0 python-dateutil==2.8.0 python-json-logger==0.1.10 -PyYAML==3.12 +PyYAML==3.13 redis==3.2.1 requests==2.21.0 rsa==3.4.2 From 249b80762a69a4b05bfb3103bdc6fa057dbc4284 Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 4 Apr 2019 17:55:37 +0100 Subject: [PATCH 2/2] add count of folders visible per user to the team members page Shows a count of how many folders that user can see - this doesn't do anything smart with parent folder stuff, it's just "how many checkboxes are ticked on the edit page". * doesn't show if service has no folders * doesn't show if service hasn't got folder permissions enabled --- app/models/user.py | 17 ++++++ app/templates/views/manage-users.html | 14 +++++ tests/app/main/views/test_manage_users.py | 67 +++++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/app/models/user.py b/app/models/user.py index bf5d4feaa..b74c8c2af 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -169,6 +169,19 @@ class User(UserMixin): return self.id in template_folder.get("users_with_permission", []) + def template_folders_for_service(self, service=None): + """ + Returns list of template folders that a user can view for a given service + """ + if not service.has_permission('edit_folder_permissions'): + return service.all_template_folders + + return [ + template_folder + for template_folder in service.all_template_folders + if self.id in template_folder.get("users_with_permission", []) + ] + def belongs_to_service(self, service_id): return str(service_id) in self.services @@ -268,6 +281,10 @@ class InvitedUser(object): data['permissions'] = sorted(self.permissions) return data + def template_folders_for_service(self, service=None): + # only used on the manage users page to display the count, so okay to not be fully fledged for now + return [{'id': x} for x in self.folder_permissions] + class InvitedOrgUser(object): diff --git a/app/templates/views/manage-users.html b/app/templates/views/manage-users.html index 645f67f1d..d97e1a269 100644 --- a/app/templates/views/manage-users.html +++ b/app/templates/views/manage-users.html @@ -57,6 +57,20 @@ {% endif %} {% endif %} + {# only show if the service has folders #} + {% if current_service.has_permission('edit_folder_permissions') and current_service.all_template_folders %} +
+ {% set folder_count = user.template_folders_for_service(current_service) | length %} + + {% if folder_count == 0 %} + Cannot see any folders + {% elif folder_count != current_service.all_template_folders | length %} + Can see {{ folder_count }} folder{% if folder_count > 1 %}s{% endif %} + {% else %} + Can see all folders + {% endif%} +
+ {% endif %} {% if current_user.has_permissions('manage_service') %}
  • diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 5e8df8324..5450e647d 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -878,6 +878,73 @@ def test_no_permission_manage_users_page( assert "Team members" not in resp_text +@pytest.mark.parametrize('folders_user_can_see, expected_message', [ + (3, 'Can see all folders'), + (2, 'Can see 2 folders'), + (1, 'Can see 1 folder'), + (0, 'Cannot see any folders'), +]) +def test_manage_user_page_shows_how_many_folders_user_can_view( + client_request, + service_one, + mock_get_template_folders, + mock_get_users_by_service, + mock_get_invites_for_service, + api_user_active, + folders_user_can_see, + expected_message +): + service_one['permissions'] = ['edit_folder_permissions'] + mock_get_template_folders.return_value = [ + {'id': 'folder-id-1', 'name': 'f1', 'parent_id': None, 'users_with_permission': []}, + {'id': 'folder-id-2', 'name': 'f2', 'parent_id': None, 'users_with_permission': []}, + {'id': 'folder-id-3', 'name': 'f3', 'parent_id': None, 'users_with_permission': []}, + ] + for i in range(folders_user_can_see): + mock_get_template_folders.return_value[i]['users_with_permission'].append(api_user_active.id) + + page = client_request.get('main.manage_users', service_id=service_one['id']) + + user_div = page.select_one("h3[title='notify@digital.cabinet-office.gov.uk']").parent + assert user_div.select_one('.tick-cross-list-hint:last-child').text.strip() == expected_message + + +def test_manage_user_page_doesnt_show_folder_hint_if_service_has_no_folders( + client_request, + service_one, + mock_get_template_folders, + mock_get_users_by_service, + mock_get_invites_for_service, + api_user_active, +): + service_one['permissions'] = ['edit_folder_permissions'] + mock_get_template_folders.return_value = [] + + page = client_request.get('main.manage_users', service_id=service_one['id']) + + user_div = page.select_one("h3[title='notify@digital.cabinet-office.gov.uk']").parent + assert user_div.find('.tick-cross-list-hint:last-child') is None + + +def test_manage_user_page_doesnt_show_folder_hint_if_service_cant_edit_folder_permissions( + client_request, + service_one, + mock_get_template_folders, + mock_get_users_by_service, + mock_get_invites_for_service, + api_user_active +): + service_one['permissions'] = [] + mock_get_template_folders.return_value = [ + {'id': 'folder-id-1', 'name': 'f1', 'parent_id': None, 'users_with_permission': [api_user_active.id]}, + ] + + page = client_request.get('main.manage_users', service_id=service_one['id']) + + user_div = page.select_one("h3[title='notify@digital.cabinet-office.gov.uk']").parent + assert user_div.find('.tick-cross-list-hint:last-child') is None + + def test_remove_user_from_service( client_request, active_user_with_permissions,