From 6fa975e867e2ebfe546c87329409adf06c3fae17 Mon Sep 17 00:00:00 2001 From: Alexey Bezhan Date: Wed, 27 Feb 2019 15:45:18 +0000 Subject: [PATCH] Send updated user folder permissions to the API Integrates the folder permissions form with the updated API endpoint to store changes in the user folders. Since user folder permissions are returned in the full list of template folders for the service we need to invalidate the cache key for it each time we update user permissions. --- app/main/forms.py | 2 +- app/main/views/manage_users.py | 8 ++++ app/notify_client/user_api_client.py | 8 +++- .../views/manage-users/permissions.html | 2 +- tests/app/main/views/test_manage_users.py | 43 ++++++++++++++++++- 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index d3e410bd2..ba9f2dbbd 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -420,7 +420,7 @@ PermissionsAbstract = type("PermissionsAbstract", (StripWhitespaceForm,), { class PermissionsForm(PermissionsAbstract): def __init__(self, all_template_folders=None, *args, **kwargs): super().__init__(*args, **kwargs) - if all_template_folders: + if all_template_folders is not None: self.folder_permissions.all_template_folders = all_template_folders self.folder_permissions.choices = [ (item['id'], item['name']) for item in ([{'name': 'Templates', 'id': None}] + all_template_folders) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 56ebdcb32..8c954b767 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -87,6 +87,10 @@ def edit_user_permissions(service_id, user_id): form = PermissionsForm.from_user( user, service_id, + folder_permissions=[ + f['id'] for f in current_service.all_template_folders + if user_id in f.get('users_with_permission', []) + ], all_template_folders=current_service.all_template_folders ) @@ -94,6 +98,10 @@ def edit_user_permissions(service_id, user_id): user_api_client.set_user_permissions( user_id, service_id, permissions=form.permissions, + folder_permissions=( + form.folder_permissions.data + if current_service.has_permission('edit_folder_permissions') else None + ), ) if service_has_email_auth: user_api_client.update_user_attribute(user_id, auth_type=form.login_authentication.data) diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index 3bb007bf2..619468f4e 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -161,13 +161,17 @@ class UserApiClient(NotifyAdminAPIClient): resp = self.post('/organisations/{}/users/{}'.format(org_id, user_id), data={}) return User(resp['data'], max_failed_login_count=self.max_failed_login_count) + @cache.delete('service-{service_id}-template-folders') @cache.delete('user-{user_id}') - def set_user_permissions(self, user_id, service_id, permissions): + def set_user_permissions(self, user_id, service_id, permissions, folder_permissions=None): # permissions passed in are the combined admin roles, not db permissions data = { - 'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)] + 'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)], } + if folder_permissions is not None: + data['folder_permissions'] = folder_permissions + endpoint = '/user/{}/service/{}/permission'.format(user_id, service_id) self.post(endpoint, data=data) diff --git a/app/templates/views/manage-users/permissions.html b/app/templates/views/manage-users/permissions.html index 8b2e54f2c..50648da5e 100644 --- a/app/templates/views/manage-users/permissions.html +++ b/app/templates/views/manage-users/permissions.html @@ -14,7 +14,7 @@ All team members can see sent messages.

-{% if current_service.has_permission("edit_folder_permissions") %} +{% if current_service.has_permission("edit_folder_permissions") and form.folder_permissions.all_template_folders %} {{ checkboxes_nested(form.folder_permissions, form.folder_permissions.children()) }} {% endif %} diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index 122f58e9b..0ddf73223 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -447,6 +447,45 @@ def test_edit_user_permissions( fake_uuid, SERVICE_ONE_ID, permissions=permissions_sent_to_api, + folder_permissions=None + ) + + +def test_edit_user_folder_permissions( + client_request, + mocker, + service_one, + mock_get_users_by_service, + mock_get_invites_for_service, + mock_set_user_permissions, + mock_get_template_folders, + fake_uuid, +): + service_one['permissions'] = ['edit_folder_permissions'] + mock_get_template_folders.return_value = [ + {'id': 'folder-id-1', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []}, + {'id': 'folder-id-2', 'name': 'folder_one', 'parent_id': None, 'users_with_permission': []}, + {'id': 'folder-id-3', 'name': 'folder_one', 'parent_id': 'folder-id-1', 'users_with_permission': []}, + ] + client_request.post( + 'main.edit_user_permissions', + service_id=SERVICE_ONE_ID, + user_id=fake_uuid, + _data=dict( + folder_permissions=['folder-id-1', 'folder-id-3'] + ), + _expected_status=302, + _expected_redirect=url_for( + 'main.manage_users', + service_id=SERVICE_ONE_ID, + _external=True, + ), + ) + mock_set_user_permissions.assert_called_with( + fake_uuid, + SERVICE_ONE_ID, + permissions=set(), + folder_permissions=['folder-id-1', 'folder-id-3'] ) @@ -508,7 +547,8 @@ def test_edit_user_permissions_including_authentication_with_email_auth_service( 'manage_templates', 'manage_service', 'manage_api_keys', - } + }, + folder_permissions=None ) mock_update_user_attribute.assert_called_with( str(active_user_with_permissions.id), @@ -1021,6 +1061,7 @@ def test_edit_user_permissions_page_displays_redacted_mobile_number_and_change_l client_request, active_user_with_permissions, mock_get_users_by_service, + mock_get_template_folders, service_one, mocker ):