From af9ae77bbc35c2ac5b20a487c538d3f9a7fd6fb8 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 20 Aug 2024 17:25:17 +0500 Subject: [PATCH] feat: upgrading simple api to drf compatible. (#35260) --- lms/djangoapps/instructor/views/api.py | 125 +++++++++--------- lms/djangoapps/instructor/views/api_urls.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 31 +++++ 3 files changed, 95 insertions(+), 63 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index ceb699cd9054..2813daa8a318 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/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): diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 08b68b0760d4..599aead57303 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/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'), diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index b4f6f7626013..463f05ad45b8 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/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