feat: upgrading simple api to drf compatible (6th) . modify_access

lms/djangoapps/instructor/views/api.py

@@ -105,7 +105,7 @@
 from lms.djangoapps.instructor_task.api_helper import AlreadyRunningError, QueueConnectionError
 from lms.djangoapps.instructor_task.data import InstructorTaskTypes
 from lms.djangoapps.instructor_task.models import ReportStore
-from lms.djangoapps.instructor.views.serializer import RoleNameSerializer, UserSerializer
+from lms.djangoapps.instructor.views.serializer import RoleNameSerializer, UserSerializer, AccessSerializer
 from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
 from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort, is_course_cohorted
 from openedx.core.djangoapps.course_groups.models import CourseUserGroup
@@ -987,17 +987,8 @@ def bulk_beta_modify_access(request, course_id):
     return JsonResponse(response_payload)
 
 
-@require_POST
-@ensure_csrf_cookie
-@cache_control(no_cache=True, no_store=True, must_revalidate=True)
-@require_course_permission(permissions.EDIT_COURSE_ACCESS)
-@require_post_params(
-    unique_student_identifier="email or username of user to change access",
-    rolename="'instructor', 'staff', 'beta', or 'ccx_coach'",
-    action="'allow' or 'revoke'"
-)
-@common_exceptions_400
-def modify_access(request, course_id):
+@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
+class ModifyAccess(APIView):
     """
     Modify staff/instructor access of other user.
     Requires instructor access.
@@ -1009,67 +1000,77 @@ def modify_access(request, course_id):
     rolename is one of ['instructor', 'staff', 'beta', 'ccx_coach']
     action is one of ['allow', 'revoke']
     """
-    course_id = CourseKey.from_string(course_id)
-    course = get_course_with_access(
-        request.user, 'instructor', course_id, depth=None
-    )
-    unique_student_identifier = request.POST.get('unique_student_identifier')
-    try:
-        user = get_student_from_identifier(unique_student_identifier)
-    except User.DoesNotExist:
-        response_payload = {
-            'unique_student_identifier': unique_student_identifier,
-            'userDoesNotExist': True,
-        }
-        return JsonResponse(response_payload)
+    permission_classes = (IsAuthenticated, permissions.InstructorPermission)
+    permission_name = permissions.EDIT_COURSE_ACCESS
+    serializer_class = AccessSerializer
 
-    # Check that user is active, because add_users
-    # in common/djangoapps/student/roles.py fails
-    # silently when we try to add an inactive user.
-    if not user.is_active:
-        response_payload = {
-            'unique_student_identifier': user.username,
-            'inactiveUser': True,
-        }
-        return JsonResponse(response_payload)
+    @method_decorator(ensure_csrf_cookie)
+    def post(self, request, course_id):
+        """
+        Modify staff/instructor access of other user.
+        Requires instructor access.
+        """
+        course_id = CourseKey.from_string(course_id)
+        course = get_course_with_access(
+            request.user, 'instructor', course_id, depth=None
+        )
 
-    rolename = request.POST.get('rolename')
-    action = request.POST.get('action')
+        serializer_data = AccessSerializer(data=request.data)
+        if not serializer_data.is_valid():
+            return HttpResponseBadRequest(reason=serializer_data.errors)
+
+        user = serializer_data.validated_data.get('unique_student_identifier')
+        if not user:
+            response_payload = {
+                'unique_student_identifier': request.data.get('unique_student_identifier'),
+                'userDoesNotExist': True,
+            }
+            return JsonResponse(response_payload)
 
-    if rolename not in ROLES:
-        error = strip_tags(f"unknown rolename '{rolename}'")
-        log.error(error)
-        return HttpResponseBadRequest(error)
+        if not user.is_active:
+            response_payload = {
+                'unique_student_identifier': user.username,
+                'inactiveUser': True,
+            }
+            return JsonResponse(response_payload)
+
+        rolename = serializer_data.data['rolename']
+        action = serializer_data.data['action']
+
+        if rolename not in ROLES:
+            error = strip_tags(f"unknown rolename '{rolename}'")
+            log.error(error)
+            return HttpResponseBadRequest(error)
+
+        # disallow instructors from removing their own instructor access.
+        if rolename == 'instructor' and user == request.user and action != 'allow':
+            response_payload = {
+                'unique_student_identifier': user.username,
+                'rolename': rolename,
+                'action': action,
+                'removingSelfAsInstructor': True,
+            }
+            return JsonResponse(response_payload)
+
+        if action == 'allow':
+            allow_access(course, user, rolename)
+            if not is_user_enrolled_in_course(user, course_id):
+                CourseEnrollment.enroll(user, course_id)
+        elif action == 'revoke':
+            revoke_access(course, user, rolename)
+        else:
+            return HttpResponseBadRequest(strip_tags(
+                f"unrecognized action u'{action}'"
+            ))
 
-    # disallow instructors from removing their own instructor access.
-    if rolename == 'instructor' and user == request.user and action != 'allow':
         response_payload = {
             'unique_student_identifier': user.username,
             'rolename': rolename,
             'action': action,
-            'removingSelfAsInstructor': True,
+            'success': 'yes',
         }
         return JsonResponse(response_payload)
 
-    if action == 'allow':
-        allow_access(course, user, rolename)
-        if not is_user_enrolled_in_course(user, course_id):
-            CourseEnrollment.enroll(user, course_id)
-    elif action == 'revoke':
-        revoke_access(course, user, rolename)
-    else:
-        return HttpResponseBadRequest(strip_tags(
-            f"unrecognized action u'{action}'"
-        ))
-
-    response_payload = {
-        'unique_student_identifier': user.username,
-        'rolename': rolename,
-        'action': action,
-        'success': 'yes',
-    }
-    return JsonResponse(response_payload)
-
 
 @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch')
 class ListCourseRoleMembersView(APIView):

lms/djangoapps/instructor/views/api_urls.py

@@ -24,7 +24,7 @@
     path('students_update_enrollment', api.students_update_enrollment, name='students_update_enrollment'),
     path('register_and_enroll_students', api.RegisterAndEnrollStudents.as_view(), name='register_and_enroll_students'),
     path('list_course_role_members', api.ListCourseRoleMembersView.as_view(), name='list_course_role_members'),
-    path('modify_access', api.modify_access, name='modify_access'),
+    path('modify_access', api.ModifyAccess.as_view(), name='modify_access'),
     path('bulk_beta_modify_access', api.bulk_beta_modify_access, name='bulk_beta_modify_access'),
     path('get_problem_responses', api.get_problem_responses, name='get_problem_responses'),
     path('get_grading_config', api.get_grading_config, name='get_grading_config'),

lms/djangoapps/instructor/views/serializer.py

@@ -4,6 +4,7 @@
 from django.core.exceptions import ValidationError
 from django.utils.translation import gettext as _
 from rest_framework import serializers
+from .tools import get_student_from_identifier
 
 from lms.djangoapps.instructor.access import ROLES
 
@@ -28,3 +29,33 @@ class UserSerializer(serializers.ModelSerializer):
     class Meta:
         model = User
         fields = ['username', 'email', 'first_name', 'last_name']
+
+
+class AccessSerializer(serializers.Serializer):
+    """
+    Serializer for managing user access changes.
+    This serializer validates and processes the data required to modify
+    user access within a system.
+    """
+    unique_student_identifier = serializers.CharField(
+        max_length=255,
+        help_text="Email or username of user to change access"
+    )
+    rolename = serializers.CharField(
+        help_text="Role name to assign to the user"
+    )
+    action = serializers.ChoiceField(
+        choices=['allow', 'revoke'],
+        help_text="Action to perform on the user's access"
+    )
+
+    def validate_unique_student_identifier(self, value):
+        """
+        Validate that the unique_student_identifier corresponds to an existing user.
+        """
+        try:
+            user = get_student_from_identifier(value)
+        except User.DoesNotExist:
+            return None
+
+        return user