From 07e5488ead768572195e87b55924b4967dd79654 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Sat, 21 Aug 2021 13:13:05 -0400 Subject: [PATCH 1/2] Move build_user_list to outside of WorkspaceViewSet --- multinet/api/views/workspace.py | 35 +++++++++++++++++---------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index c42c967..c195c12 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -25,6 +25,21 @@ from .common import MultinetPagination +def build_user_list(validated_data: OrderedDict) -> list: + """ + Build a list of user objects from an ordered dictionary of user data. + + Accepts an ordered dictionary containing a list of validated user data, e.g. + as a result of validating a PermissionsSerializer with request data. + Returns a list of user objects. + """ + user_list = [] + for valid_user in validated_data: + user_object = get_object_or_404(User, username=valid_user['username']) + user_list.append(user_object) + return user_list + + class WorkspaceViewSet(ReadOnlyModelViewSet): queryset = Workspace.objects.select_related('owner').all() lookup_field = 'name' @@ -124,20 +139,6 @@ def get_current_user_workspace_permissions(self, request, name: str): serializer.is_valid(raise_exception=True) return Response(serializer.data, status=status.HTTP_200_OK) - def build_user_list(self, validated_data: OrderedDict) -> list: - """ - Build a list of user objects from an ordered dictionary of user data. - - Accepts an ordered dictionary containing a list of validated user data, e.g. - as a result of validating a PermissionsSerializer with request data. - Returns a list of user objects. - """ - user_list = [] - for valid_user in validated_data: - user_object = get_object_or_404(User, username=valid_user['username']) - user_list.append(user_object) - return user_list - @swagger_auto_schema( request_body=PermissionsCreateSerializer(), responses={200: PermissionsReturnSerializer()} ) @@ -154,9 +155,9 @@ def put_workspace_permissions(self, request, name: str): workspace.public = validated_data['public'] workspace.save() - new_readers = self.build_user_list(validated_data['readers']) - new_writers = self.build_user_list(validated_data['writers']) - new_maintainers = self.build_user_list(validated_data['maintainers']) + new_readers = build_user_list(validated_data['readers']) + new_writers = build_user_list(validated_data['writers']) + new_maintainers = build_user_list(validated_data['maintainers']) workspace.set_user_permissions_bulk( readers=new_readers, writers=new_writers, maintainers=new_maintainers ) From 79307b44f735552ce9d7fe0a2934cbbf0edf04e1 Mon Sep 17 00:00:00 2001 From: Jacob Nesbitt Date: Sat, 21 Aug 2021 13:44:48 -0400 Subject: [PATCH 2/2] Return a permission tuple from the workspace model Change the function of `get_user_permission_label`, to return a tuple of the permission level and label. This also renames the function to `get_user_permission_tuple`. --- multinet/api/models/workspace.py | 36 ++++++++++++++-------------- multinet/api/tests/test_workspace.py | 7 ++++-- multinet/api/views/serializers.py | 3 ++- multinet/api/views/workspace.py | 9 +++++-- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/multinet/api/models/workspace.py b/multinet/api/models/workspace.py index 67a6e72..b1cf279 100644 --- a/multinet/api/models/workspace.py +++ b/multinet/api/models/workspace.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import List, Optional, Type +from typing import List, Optional, Tuple, Type, Union from uuid import uuid4 from arango.database import StandardDatabase @@ -81,31 +81,31 @@ def get_user_permission(self, user: User) -> Optional[WorkspaceRole]: """Get the WorkspaceRole for a given user on this workspace.""" return WorkspaceRole.objects.filter(workspace=self.pk, user=user.pk).first() - def get_user_permission_label(self, user: User) -> Optional[str]: + def get_user_permission_tuple(self, user: User) -> Union[Tuple[int, str], Tuple[None, None]]: """ - Get a the string label of a user's workspace role, or None. - - This function differs from Workspace::get_user_permisssion. Whereas that - function returns a WorkspaceRole object describing a user's permission for - a particular workspace, this function returns a human-readable representation - of that permission level. Since ownership of a workspace is modeled by a property - on that workspace, this function takes that into account, and returns 'owner' if - the given user is the owner of this workspace. Moreover, even if a user does not - have explicit permission via the ownersip relationship or a WorkspaceRole object, - they are still considered a reader if this workspace is public. Therefore, if this - workspace is public and the user passed in has no explicit permission, 'reader' will - be returned. + Return a tuple of the user's permission level and string label, or (None, None). + + This function differs from Workspace().get_user_permission, as that function returns a + WorkspaceRole object from the database, and this function returns a tuple of the level and + label corresponding to the 'role' field of that object. Since ownership of a workspace is + modeled by a field on that workspace, get_user_permission doesn't address ownership at all, + whereas this function returns the artificail tuple (4, 'owner') if the given user is the + owner of this workspace. Moreover, if a user has no permission on this workspace, or is the + anonymous user (not logged in), but the workspace is public, the reader tuple is returned. """ if self.owner == user: - return 'owner' + return 4, 'owner' workspace_role = self.get_user_permission(user) if workspace_role is None: if self.public: - return WorkspaceRoleChoice.READER.label - return None + return WorkspaceRoleChoice.READER.value, WorkspaceRoleChoice.READER.label + return None, None else: - return WorkspaceRoleChoice(workspace_role.role).label + return ( + WorkspaceRoleChoice(workspace_role.role).value, + WorkspaceRoleChoice(workspace_role.role).label, + ) def set_user_permission(self, user: User, permission: WorkspaceRoleChoice) -> bool: """ diff --git a/multinet/api/tests/test_workspace.py b/multinet/api/tests/test_workspace.py index 9cbd980..910a535 100644 --- a/multinet/api/tests/test_workspace.py +++ b/multinet/api/tests/test_workspace.py @@ -393,10 +393,12 @@ def test_workspace_rest_get_user_permission( assert r.status_code == status_code if success: + permission, permission_label = workspace.get_user_permission_tuple(user) assert r.data == { 'username': user.username, 'workspace': workspace.name, - 'permission': workspace.get_user_permission_label(user), + 'permission': permission, + 'permission_label': permission_label, } @@ -410,5 +412,6 @@ def test_workspace_rest_get_user_permission_public( assert r.data == { 'username': '', # anonymous user 'workspace': workspace.name, - 'permission': WorkspaceRoleChoice.READER.label, + 'permission': WorkspaceRoleChoice.READER.value, + 'permission_label': WorkspaceRoleChoice.READER.label, } diff --git a/multinet/api/views/serializers.py b/multinet/api/views/serializers.py index c1bd317..937fd62 100644 --- a/multinet/api/views/serializers.py +++ b/multinet/api/views/serializers.py @@ -84,7 +84,8 @@ class SingleUserWorkspacePermissionSerializer(serializers.Serializer): # Allow empty username since anonymous user is a reader for public workspaces username = serializers.CharField(validators=[UnicodeUsernameValidator()], allow_blank=True) workspace = serializers.CharField() - permission = serializers.CharField(allow_blank=True, allow_null=True) + permission = serializers.IntegerField(allow_null=True) + permission_label = serializers.CharField(allow_null=True) # The required fields for table creation diff --git a/multinet/api/views/workspace.py b/multinet/api/views/workspace.py index c195c12..628150b 100644 --- a/multinet/api/views/workspace.py +++ b/multinet/api/views/workspace.py @@ -132,9 +132,14 @@ def get_current_user_workspace_permissions(self, request, name: str): """Get the workspace permission for the user of the request.""" workspace: Workspace = get_object_or_404(Workspace, name=name) user = request.user - permission = workspace.get_user_permission_label(user) + permission, permission_label = workspace.get_user_permission_tuple(user) + data = { + 'username': user.username, + 'workspace': name, + 'permission': permission, + 'permission_label': permission_label, + } - data = {'username': user.username, 'workspace': name, 'permission': permission} serializer = SingleUserWorkspacePermissionSerializer(data=data) serializer.is_valid(raise_exception=True) return Response(serializer.data, status=status.HTTP_200_OK)