diff --git a/app/models/user.py b/app/models/user.py index cd62db52a..95278abb6 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -18,7 +18,7 @@ from app.notify_client.user_api_client import user_api_client from app.utils.user import is_gov_user from app.utils.user_permissions import ( all_ui_permissions, - translate_permissions_from_db_to_admin_roles, + translate_permissions_from_db_to_ui, ) @@ -103,7 +103,7 @@ class User(JSONModel, UserMixin): them out later, we'll need to rework this function. """ self._permissions = { - service: translate_permissions_from_db_to_admin_roles(permissions) + service: translate_permissions_from_db_to_ui(permissions) for service, permissions in permissions_by_service.items() } @@ -505,7 +505,7 @@ class InvitedUser(JSONModel): self._permissions = permissions else: self._permissions = permissions.split(',') - self._permissions = translate_permissions_from_db_to_admin_roles(self.permissions) + self._permissions = translate_permissions_from_db_to_ui(self.permissions) @property def from_user(self): diff --git a/app/notify_client/invite_api_client.py b/app/notify_client/invite_api_client.py index b76d7d279..464c3ba9c 100644 --- a/app/notify_client/invite_api_client.py +++ b/app/notify_client/invite_api_client.py @@ -1,7 +1,7 @@ from app.notify_client import NotifyAdminAPIClient, _attach_current_user, cache from app.utils.user_permissions import ( all_ui_permissions, - translate_permissions_from_admin_roles_to_db, + translate_permissions_from_ui_to_db, ) @@ -23,7 +23,7 @@ class InviteApiClient(NotifyAdminAPIClient): 'service': service_id, 'email_address': email_address, 'from_user': invite_from_id, - 'permissions': ','.join(sorted(translate_permissions_from_admin_roles_to_db(permissions))), + 'permissions': ','.join(sorted(translate_permissions_from_ui_to_db(permissions))), 'auth_type': auth_type, 'invite_link_host': self.admin_url, 'folder_permissions': folder_permissions, diff --git a/app/notify_client/user_api_client.py b/app/notify_client/user_api_client.py index f89de2df9..379d4b9b7 100644 --- a/app/notify_client/user_api_client.py +++ b/app/notify_client/user_api_client.py @@ -1,9 +1,7 @@ from notifications_python_client.errors import HTTPError from app.notify_client import NotifyAdminAPIClient, cache -from app.utils.user_permissions import ( - translate_permissions_from_admin_roles_to_db, -) +from app.utils.user_permissions import translate_permissions_from_ui_to_db ALLOWED_ATTRIBUTES = { 'name', @@ -152,7 +150,7 @@ class UserApiClient(NotifyAdminAPIClient): # permissions passed in are the combined admin roles, not db permissions endpoint = '/service/{}/users/{}'.format(service_id, user_id) data = { - 'permissions': [{'permission': x} for x in translate_permissions_from_admin_roles_to_db(permissions)], + 'permissions': [{'permission': x} for x in translate_permissions_from_ui_to_db(permissions)], 'folder_permissions': folder_permissions, } @@ -168,7 +166,7 @@ class UserApiClient(NotifyAdminAPIClient): 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_ui_to_db(permissions)], } if folder_permissions is not None: diff --git a/app/utils/user_permissions.py b/app/utils/user_permissions.py index b9944d532..cddd87ca4 100644 --- a/app/utils/user_permissions.py +++ b/app/utils/user_permissions.py @@ -28,25 +28,27 @@ broadcast_permission_options = ( ) -def translate_permissions_from_db_to_admin_roles(permissions): +def translate_permissions_from_db_to_ui(db_permissions): """ - Given a list of database permissions, return a set of roles + Given a list of database permissions, return a set of UI permissions - A role is returned if all of its database permissions are in the permission list that is passed in. - Any permissions in the list that are not database permissions are also returned. + A UI permission is returned if all of its DB permissions are in the permission list that is passed in. + Any DB permissions in the list that are not known permissions are also returned. """ - unknown_database_permissions = {p for p in permissions if p not in all_db_permissions} + unknown_database_permissions = set(db_permissions) - all_db_permissions return { - admin_role for admin_role, db_role_list in permission_mappings.items() - if set(db_role_list) <= set(permissions) + ui_permission for ui_permission, db_permissions_for_ui_permission in permission_mappings.items() + if set(db_permissions_for_ui_permission) <= set(db_permissions) } | unknown_database_permissions -def translate_permissions_from_admin_roles_to_db(permissions): +def translate_permissions_from_ui_to_db(ui_permissions): """ - Given a list of admin roles (ie: checkboxes on a permissions edit page for example), return a set of db permissions + Given a list of UI permissions (ie: checkboxes on a permissions edit page), return a set of DB permissions - Looks them up in the roles dict, falling back to just passing through if they're not recognised. + Looks them up in the mapping, falling back to just passing through if they're not recognised. """ - return set(chain.from_iterable(permission_mappings.get(permission, [permission]) for permission in permissions)) + return set(chain.from_iterable( + permission_mappings.get(ui_permission, [ui_permission]) for ui_permission in ui_permissions + )) diff --git a/tests/app/utils/test_user_permissions.py b/tests/app/utils/test_user_permissions.py index 9ad639022..b40994597 100644 --- a/tests/app/utils/test_user_permissions.py +++ b/tests/app/utils/test_user_permissions.py @@ -1,12 +1,12 @@ import pytest from app.utils.user_permissions import ( - translate_permissions_from_admin_roles_to_db, - translate_permissions_from_db_to_admin_roles, + translate_permissions_from_db_to_ui, + translate_permissions_from_ui_to_db, ) -@pytest.mark.parametrize('db_roles,admin_roles', [ +@pytest.mark.parametrize('db_permissions,expected_ui_permissions', [ ( ['approve_broadcasts', 'reject_broadcasts', 'cancel_broadcasts'], {'approve_broadcasts'}, @@ -28,15 +28,18 @@ from app.utils.user_permissions import ( {'send_messages', 'manage_templates', 'some_unknown_permission'}, ), ]) -def test_translate_permissions_from_db_to_admin_roles( - db_roles, - admin_roles, +def test_translate_permissions_from_db_to_ui( + db_permissions, + expected_ui_permissions, ): - roles = translate_permissions_from_db_to_admin_roles(db_roles) - assert roles == admin_roles + ui_permissions = translate_permissions_from_db_to_ui(db_permissions) + assert ui_permissions == expected_ui_permissions -def test_translate_permissions_from_admin_roles_to_db(): - roles = ['send_messages', 'manage_templates', 'some_unknown_permission'] - db_perms = translate_permissions_from_admin_roles_to_db(roles) - assert db_perms == {'send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission'} +def test_translate_permissions_from_ui_to_db(): + ui_permissions = ['send_messages', 'manage_templates', 'some_unknown_permission'] + db_permissions = translate_permissions_from_ui_to_db(ui_permissions) + + assert db_permissions == { + 'send_texts', 'send_emails', 'send_letters', 'manage_templates', 'some_unknown_permission' + }