refactor: update endpoint responses

Author: BryanttV

openedx_authz/rest_api/v1/views.py

@@ -16,31 +16,27 @@
 from rest_framework.views import APIView
 
 from openedx_authz.api.data import ContentLibraryData
-from openedx_authz.api.roles import (
-    get_all_roles_and_subjects_in_scope,
-    get_role_definitions_in_scope,
-    get_all_users_by_role,
-)
+from openedx_authz.api.roles import get_all_users_by_role, get_role_definitions_in_scope
 from openedx_authz.api.users import (
     assign_role_to_user_in_scope,
-    get_user_role_assignments_for_role_in_scope,
+    get_all_user_role_assignments_in_scope_v2,
     unassign_role_from_user,
     user_has_permission,
-    get_all_user_role_assignments_in_scope,
 )
+from openedx_authz.rest_api.v1.paginators import RoleUserAPIViewPagination
 from openedx_authz.rest_api.v1.serializers import (
     AddUserToRoleWithScopeSerializer,
     ListRolesWithScopeResponseSerializer,
     ListRolesWithScopeSerializer,
-    ListUsersInRoleWithScopeResponseSerializer,
     ListUsersInRoleWithScopeSerializer,
     PermissionValidationResponseSerializer,
     PermissionValidationSerializer,
     RemoveUserFromRoleWithScopeSerializer,
+    RoleAssignmentSerializer,
 )
 
 try:
-    from common.djangoapps.student.models.user import get_user_by_username_or_email
+    from common.djangoapps.student.models.user import get_user_by_username_or_email  # type: ignore
 except ImportError:
     get_user_by_username_or_email = None
 
@@ -101,65 +97,33 @@ class RoleUserAPIView(APIView):
     """
 
     permission_classes = [IsAuthenticated]
+    pagination_class = RoleUserAPIViewPagination
 
     @apidocs.schema(
         parameters=[
             apidocs.query_parameter("role", str, description="The name of the role to query"),
             apidocs.query_parameter("scope", str, description="The authorization scope for the role"),
+            apidocs.query_parameter("page", int, description="Page number for pagination"),
+            apidocs.query_parameter("page_size", int, description="Number of items per page"),
         ],
         responses={
-            status.HTTP_200_OK: ListUsersInRoleWithScopeResponseSerializer(many=True),
+            status.HTTP_200_OK: "The users were retrieved successfully",
             status.HTTP_400_BAD_REQUEST: "The request parameters are invalid",
             status.HTTP_401_UNAUTHORIZED: "The user is not authenticated",
         },
     )
     def get(self, request: HttpRequest) -> Response:
-        """Retrieve all users assigned to a specific role within a scope."""
+        """Retrieve all users with role assignments within a specific scope."""
+        # TODO: Filter by role
         serializer = ListUsersInRoleWithScopeSerializer(data=request.query_params)
         serializer.is_valid(raise_exception=True)
+        user_role_assignments = get_all_user_role_assignments_in_scope_v2(serializer.validated_data["scope"])
 
-        role_name = serializer.validated_data.get("role")
-        scope = serializer.validated_data["scope"]
-
-        response_data = []
-
-        scope_data = ContentLibraryData(external_key=scope)
-
-        # TODO: Should this be another endpoint?
-        if not role_name:
-            roles = get_all_roles_and_subjects_in_scope(scope_data)
-            final_roles = []
-            for role in roles:
-                role_data = {
-                    "role": role["role"],
-                    "users": [],
-                }
-                for user_identifier in role["users"]:
-                    user = get_user_by_username_or_email(user_identifier)
-                    role_data["users"].append(
-                        {
-                            "username": user.username,
-                            "full_name": user.profile.name,
-                            "email": user.email,
-                        }
-                    )
-                final_roles.append(role_data)
-            return Response(final_roles, status=status.HTTP_200_OK)
-
-        role_assignments = get_user_role_assignments_for_role_in_scope(role_name, scope)
-        for assignment in role_assignments:
-            # TODO: Should we get all users at once instead of one by one?
-            user = get_user_by_username_or_email(assignment.subject.username)
-            response_data.append(
-                {
-                    "username": assignment.subject.username,
-                    "full_name": user.profile.name,
-                    "email": user.email,
-                }
-            )
+        paginator = self.pagination_class()
+        paginated_assignments = paginator.paginate_queryset(user_role_assignments, request)
 
-        serializer = ListUsersInRoleWithScopeResponseSerializer(response_data, many=True)
-        return Response(serializer.data, status=status.HTTP_200_OK)
+        serializer = RoleAssignmentSerializer(paginated_assignments, many=True)
+        return paginator.get_paginated_response(serializer.data)
 
     @apidocs.schema(
         body=AddUserToRoleWithScopeSerializer,
@@ -174,57 +138,62 @@ def put(self, request: HttpRequest) -> Response:
         serializer = AddUserToRoleWithScopeSerializer(data=request.data)
         serializer.is_valid(raise_exception=True)
 
-        completed, errors = [], []
+        # TODO: Should we validate that the role or scope exists?
         role_name = serializer.validated_data["role"]
         scope = serializer.validated_data["scope"]
 
+        completed, errors = [], []
         for user_identifier in serializer.validated_data["users"]:
             try:
                 user = get_user_by_username_or_email(user_identifier)
                 assign_role_to_user_in_scope(user.username, role_name, scope)
-                completed.append({"user": user_identifier, "status": "role_added"})
+                completed.append({"user_identifier": user_identifier, "status": "role_added"})
             except User.DoesNotExist:
-                errors.append({"user": user_identifier, "error": "user_not_found"})
+                errors.append({"user_identifier": user_identifier, "error": "user_not_found"})
             except Exception as e:  # pylint: disable=broad-exception-caught
                 logger.error(f"Error assigning role to user {user_identifier}: {e}")
-                errors.append({"user": user_identifier, "error": "assignment_failed"})
+                errors.append({"user_identifier": user_identifier, "error": "assignment_failed"})
 
         response_data = {"completed": completed, "errors": errors}
         return Response(response_data, status=status.HTTP_207_MULTI_STATUS)
 
     @apidocs.schema(
         parameters=[
-            apidocs.query_parameter("user", str, description="The user to remove from the role"),
-            apidocs.query_parameter("role", str, description="The role to remove the user from"),
-            apidocs.query_parameter("scope", str, description="The scope to remove the user from"),
+            apidocs.query_parameter(
+                "users", str, description="List of user identifiers (username or email) separated by a comma"
+            ),
+            apidocs.query_parameter("role", str, description="The role to remove the users from"),
+            apidocs.query_parameter("scope", str, description="The scope to remove the users from"),
         ],
         responses={
-            status.HTTP_200_OK: "The user was removed from the role",
-            status.HTTP_400_BAD_REQUEST: "The request data is invalid",
+            status.HTTP_207_MULTI_STATUS: "The users were removed from the role",
+            status.HTTP_400_BAD_REQUEST: "The request parameters are invalid",
             status.HTTP_401_UNAUTHORIZED: "The user is not authenticated",
-            status.HTTP_404_NOT_FOUND: "The user was not found",
-            status.HTTP_500_INTERNAL_SERVER_ERROR: "The user was not removed from the role",
         },
     )
     def delete(self, request: HttpRequest) -> Response:
-        """Remove a user from a specific role within a scope."""
+        """Remove multiple users from a specific role within a scope."""
         serializer = RemoveUserFromRoleWithScopeSerializer(data=request.query_params)
         serializer.is_valid(raise_exception=True)
 
-        # Should we allow a list of users separated by a comma?
-        user_identifier = serializer.validated_data["user"]
+        user_identifiers = serializer.validated_data["users"]
         role_name = serializer.validated_data["role"]
         scope = serializer.validated_data["scope"]
 
-        try:
-            user = get_user_by_username_or_email(user_identifier)
-            unassign_role_from_user(user.username, role_name, scope)
-            return Response({"message": "Role successfully removed from user"}, status=status.HTTP_200_OK)
-        except User.DoesNotExist:
-            return Response({"error": "User not found"}, status=status.HTTP_404_NOT_FOUND)
-        except Exception as e:  # pylint: disable=broad-exception-caught
-            logger.error(f"Error removing role from user {user_identifier}: {e}")
-            return Response({"error": "Internal server error"}, status=status.HTTP_500_INTERNAL_SERVER_ERROR)
+        completed, errors = [], []
+        for user_identifier in user_identifiers:
+            try:
+                user = get_user_by_username_or_email(user_identifier)
+                unassign_role_from_user(user.username, role_name, scope)
+                completed.append({"user_identifier": user_identifier, "status": "role_removed"})
+            except User.DoesNotExist:
+                errors.append({"user_identifier": user_identifier, "error": "user_not_found"})
+            except Exception as e:  # pylint: disable=broad-exception-caught
+                logger.error(f"Error removing role from user {user_identifier}: {e}")
+                errors.append({"user_identifier": user_identifier, "error": "removal_failed"})
+
+        response_data = {"completed": completed, "errors": errors}
+        return Response(response_data, status=status.HTTP_207_MULTI_STATUS)
 
 
 class RoleListView(APIView):