From 7bf97aaef0bf2ce98b9966b5a981c7e0912ec53c Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Thu, 22 Aug 2024 14:34:33 +0500 Subject: [PATCH 1/4] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 109 +++++++++++--------- lms/djangoapps/instructor/views/api_urls.py | 2 +- 2 files changed, 61 insertions(+), 50 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 7ca1e70467b0..887f5ef94785 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2814,17 +2814,8 @@ def send_email(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_FORUM_ROLES) -@require_post_params( - unique_student_identifier="email or username of user to change access", - rolename="the forum role", - action="'allow' or 'revoke'", -) -@common_exceptions_400 -def update_forum_role_membership(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +class UpdateForumRoleMembership(APIView): """ Modify user's forum role. @@ -2833,52 +2824,72 @@ def update_forum_role_membership(request, course_id): which is limited to instructors. No one can revoke an instructors FORUM_ROLE_ADMINISTRATOR status. - Query parameters: - - `email` is the target users email - - `rolename` is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_GROUP_MODERATOR, - FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] - - `action` is one of ['allow', 'revoke'] """ - course_id = CourseKey.from_string(course_id) - course = get_course_by_id(course_id) - has_instructor_access = has_access(request.user, 'instructor', course) - has_forum_admin = has_forum_access( - request.user, course_id, FORUM_ROLE_ADMINISTRATOR - ) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.EDIT_FORUM_ROLES + serializer_class = AccessSerializer - unique_student_identifier = request.POST.get('unique_student_identifier') - rolename = request.POST.get('rolename') - action = request.POST.get('action') + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + Handles role modification requests for a forum user. - # default roles require either (staff & forum admin) or (instructor) - if not (has_forum_admin or has_instructor_access): - return HttpResponseBadRequest( - "Operation requires staff & forum admin or instructor access" + Query parameters: + - `email` is the target users email + - `rolename` is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] + - `action` is one of ['allow', 'revoke'] + """ + course_id = CourseKey.from_string(course_id) + course = get_course_by_id(course_id) + has_instructor_access = has_access(request.user, 'instructor', course) + has_forum_admin = has_forum_access( + request.user, course_id, FORUM_ROLE_ADMINISTRATOR ) - # EXCEPT FORUM_ROLE_ADMINISTRATOR requires (instructor) - if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access: - return HttpResponseBadRequest("Operation requires instructor access.") + serializer_data = AccessSerializer(data=request.data) + if not serializer_data.is_valid(): + return HttpResponseBadRequest(reason=serializer_data.errors) - if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, - FORUM_ROLE_COMMUNITY_TA]: - return HttpResponseBadRequest(strip_tags( - f"Unrecognized rolename '{rolename}'." - )) + 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) - user = get_student_from_identifier(unique_student_identifier) - if action == 'allow' and not is_user_enrolled_in_course(user, course_id): - CourseEnrollment.enroll(user, course_id) - try: - update_forum_role(course_id, user, rolename, action) - except Role.DoesNotExist: - return HttpResponseBadRequest("Role does not exist.") + rolename = serializer_data.data['rolename'] + action = serializer_data.data['action'] - response_payload = { - 'course_id': str(course_id), - 'action': action, - } - return JsonResponse(response_payload) + # default roles require either (staff & forum admin) or (instructor) + if not (has_forum_admin or has_instructor_access): + return HttpResponseBadRequest( + "Operation requires staff & forum admin or instructor access" + ) + + # EXCEPT FORUM_ROLE_ADMINISTRATOR requires (instructor) + if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access: + return HttpResponseBadRequest("Operation requires instructor access.") + + if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_COMMUNITY_TA]: + return HttpResponseBadRequest(strip_tags( + f"Unrecognized rolename '{rolename}'." + )) + + if action == 'allow' and not is_user_enrolled_in_course(user, course_id): + CourseEnrollment.enroll(user, course_id) + try: + update_forum_role(course_id, user, rolename, action) + except Role.DoesNotExist: + return HttpResponseBadRequest("Role does not exist.") + + response_payload = { + 'course_id': str(course_id), + 'action': action, + } + return JsonResponse(response_payload) @require_POST diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index c0e12e46756d..b3abeabfc72f 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -48,7 +48,7 @@ path('list_background_email_tasks', api.list_background_email_tasks, name='list_background_email_tasks'), path('list_email_content', api.ListEmailContent.as_view(), name='list_email_content'), path('list_forum_members', api.list_forum_members, name='list_forum_members'), - path('update_forum_role_membership', api.update_forum_role_membership, name='update_forum_role_membership'), + path('update_forum_role_membership', api.UpdateForumRoleMembership.as_view(), name='update_forum_role_membership'), path('send_email', api.send_email, name='send_email'), path('change_due_date', api.change_due_date, name='change_due_date'), path('reset_due_date', api.reset_due_date, name='reset_due_date'), From b6b211adfd0905b0c97a973f2c0c0bc0e1062995 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Thu, 22 Aug 2024 14:47:52 +0500 Subject: [PATCH 2/4] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 887f5ef94785..d82a0f51c940 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2853,11 +2853,7 @@ def post(self, request, course_id): 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) + return JsonResponse({'error': 'User does not exist.'}, status=400) rolename = serializer_data.data['rolename'] action = serializer_data.data['action'] From 31cd26938bce295cd0fb0a1c65d80b5acc3f6dde Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Thu, 22 Aug 2024 18:21:34 +0500 Subject: [PATCH 3/4] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 8 +++-- lms/djangoapps/instructor/views/serializer.py | 31 ++++++++++++++++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index d82a0f51c940..eb93eea29b3b 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -105,7 +105,9 @@ 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, AccessSerializer +from lms.djangoapps.instructor.views.serializer import ( + RoleNameSerializer, UserSerializer, AccessSerializer, UpdateForumRoleMembershipSerializer +) 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 @@ -2827,7 +2829,7 @@ class UpdateForumRoleMembership(APIView): """ permission_classes = (IsAuthenticated, permissions.InstructorPermission) permission_name = permissions.EDIT_FORUM_ROLES - serializer_class = AccessSerializer + serializer_class = UpdateForumRoleMembershipSerializer @method_decorator(ensure_csrf_cookie) def post(self, request, course_id): @@ -2847,7 +2849,7 @@ def post(self, request, course_id): request.user, course_id, FORUM_ROLE_ADMINISTRATOR ) - serializer_data = AccessSerializer(data=request.data) + serializer_data = UpdateForumRoleMembershipSerializer(data=request.data) if not serializer_data.is_valid(): return HttpResponseBadRequest(reason=serializer_data.errors) diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 463f05ad45b8..3611f98f735c 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -4,9 +4,16 @@ 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 +from openedx.core.djangoapps.django_comment_common.models import ( + FORUM_ROLE_ADMINISTRATOR, + FORUM_ROLE_COMMUNITY_TA, + FORUM_ROLE_GROUP_MODERATOR, + FORUM_ROLE_MODERATOR +) + +from .tools import get_student_from_identifier class RoleNameSerializer(serializers.Serializer): # pylint: disable=abstract-method @@ -59,3 +66,25 @@ def validate_unique_student_identifier(self, value): return None return user + + +class UpdateForumRoleMembershipSerializer(AccessSerializer): + """ + Serializer for managing user's forum role. + + This serializer extends the AccessSerializer to allow for different action + choices specific to this API. It validates and processes the data required + to modify user access within a system. + + Attributes: + unique_student_identifier (str): The email or username of the user whose access is being modified. + rolename (str): The role name to assign to the user. + action (str): The specific action to perform on the user's access, with options 'activate' or 'deactivate'. + """ + rolename = serializers.ChoiceField( + choices=[ + FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, + FORUM_ROLE_GROUP_MODERATOR, FORUM_ROLE_COMMUNITY_TA + ], + help_text="Rolename assign to given user." + ) From 8cb1bb7936f027d807726993c93c750fc5b70c03 Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Thu, 22 Aug 2024 18:44:02 +0500 Subject: [PATCH 4/4] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index eb93eea29b3b..af6412211c7c 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -2870,12 +2870,6 @@ def post(self, request, course_id): if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access: return HttpResponseBadRequest("Operation requires instructor access.") - if rolename not in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_GROUP_MODERATOR, - FORUM_ROLE_COMMUNITY_TA]: - return HttpResponseBadRequest(strip_tags( - f"Unrecognized rolename '{rolename}'." - )) - if action == 'allow' and not is_user_enrolled_in_course(user, course_id): CourseEnrollment.enroll(user, course_id) try: