From 6cc0e2892930d36d1a3959549f2579a44d5eb8ac Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Wed, 10 Jul 2024 16:48:44 +0500 Subject: [PATCH 1/3] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 66 +++++++++++-------- lms/djangoapps/instructor/views/api_urls.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 28 ++++++++ 3 files changed, 67 insertions(+), 29 deletions(-) create mode 100644 lms/djangoapps/instructor/views/serializer.py diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index e7a82496456e..85dbecba6da1 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -105,6 +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 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 @@ -1064,15 +1065,11 @@ def 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(rolename="'instructor', 'staff', or 'beta'") -def list_course_role_members(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +class ListCourseRoleMembersView(APIView): """ - List instructors and staff. - Requires instructor access. + View to list instructors and staff for a specific course. + Requires the user to have instructor access. rolename is one of ['instructor', 'staff', 'beta', 'ccx_coach'] @@ -1088,33 +1085,46 @@ def list_course_role_members(request, course_id): ] } """ - course_id = CourseKey.from_string(course_id) - course = get_course_with_access( - request.user, 'instructor', course_id, depth=None + authentication_classes = ( + JwtAuthentication, + BearerAuthenticationAllowInactiveUser, + SessionAuthenticationAllowInactiveUser, ) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.EDIT_COURSE_ACCESS - rolename = request.POST.get('rolename') + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + Handles POST request to list instructors and staff. - if rolename not in ROLES: - return HttpResponseBadRequest() + Args: + request (HttpRequest): The request object containing user data. + course_id (str): The ID of the course to list instructors and staff for. - def extract_user_info(user): - """ convert user into dicts for json view """ + Returns: + Response: A Response object containing the list of instructors and staff or an error message. - return { - 'username': user.username, - 'email': user.email, - 'first_name': user.first_name, - 'last_name': user.last_name, + Raises: + Http404: If the course does not exist. + """ + course_id = CourseKey.from_string(course_id) + course = get_course_with_access( + request.user, 'instructor', course_id, depth=None + ) + role_serializer = RoleNameSerializer(data=request.data) + role_serializer.is_valid(raise_exception=True) + rolename = role_serializer.data['rolename'] + + users = list_with_level(course.id, rolename) + serializer = UserSerializer(users, many=True) + + response_payload = { + 'course_id': str(course_id), + rolename: serializer.data, } - response_payload = { - 'course_id': str(course_id), - rolename: list(map(extract_user_info, list_with_level( - course.id, rolename - ))), - } - return JsonResponse(response_payload) + return Response(response_payload, status=status.HTTP_200_OK) class ProblemResponseReportPostParamsSerializer(serializers.Serializer): # pylint: disable=abstract-method diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 6b072ef0c12e..f35265f0817b 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -23,7 +23,7 @@ urlpatterns = [ path('students_update_enrollment', api.students_update_enrollment, name='students_update_enrollment'), path('register_and_enroll_students', api.register_and_enroll_students, name='register_and_enroll_students'), - path('list_course_role_members', api.list_course_role_members, name='list_course_role_members'), + path('list_course_role_members', api.ListCourseRoleMembersView.as_view(), name='list_course_role_members'), path('modify_access', api.modify_access, 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'), diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py new file mode 100644 index 000000000000..261bc314665d --- /dev/null +++ b/lms/djangoapps/instructor/views/serializer.py @@ -0,0 +1,28 @@ +from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user +from django.core.exceptions import ValidationError +from django.utils.translation import gettext as _ +from rest_framework import serializers, status + +from lms.djangoapps.instructor.access import ROLES + + +class RoleNameSerializer(serializers.Serializer): # pylint: disable=abstract-method + """ + Serializer that describes the response of the problem response report generation API. + """ + + rolename = serializers.CharField(help_text=_("Role name")) + + def validate_rolename(self, value): + """ + Check that the rolename is valid. + """ + if value not in ROLES: + raise ValidationError(_("Invalid role name.")) + return value + + +class UserSerializer(serializers.ModelSerializer): + class Meta: + model = User + fields = ['username', 'email', 'first_name', 'last_name'] From dd4973d1a5a8c61ada7ef911789236c493c3928e Mon Sep 17 00:00:00 2001 From: awais qureshi Date: Tue, 30 Jul 2024 20:51:18 +0500 Subject: [PATCH 2/3] feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/serializer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 261bc314665d..b4f6f7626013 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -1,7 +1,9 @@ +""" Instructor apis serializers. """ + from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import ValidationError from django.utils.translation import gettext as _ -from rest_framework import serializers, status +from rest_framework import serializers from lms.djangoapps.instructor.access import ROLES From 20f66cc08ea32a2ba18b9610b150fe11b1fb549a Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Wed, 7 Aug 2024 12:10:33 +0500 Subject: [PATCH 3/3] chore: removed authentication classes. --- lms/djangoapps/instructor/views/api.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 85dbecba6da1..265b8d3e6f60 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -1085,11 +1085,6 @@ class ListCourseRoleMembersView(APIView): ] } """ - authentication_classes = ( - JwtAuthentication, - BearerAuthenticationAllowInactiveUser, - SessionAuthenticationAllowInactiveUser, - ) permission_classes = (IsAuthenticated, permissions.InstructorPermission) permission_name = permissions.EDIT_COURSE_ACCESS