From 08bd8b312e11a4d4916981fd6f1af93faf48ee61 Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 10 Sep 2024 16:26:41 +0500 Subject: [PATCH] feat: upgrade reset_student_attempts api to drf ( 19th ) (#35404) * feat: upgrading simple api to drf compatible. --- lms/djangoapps/instructor/views/api.py | 140 ++++++++++-------- lms/djangoapps/instructor/views/api_urls.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 51 +++++++ 3 files changed, 128 insertions(+), 65 deletions(-) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index c6db96b3ca..09a2b9ffd1 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -108,7 +108,7 @@ from lms.djangoapps.instructor_task.data import InstructorTaskTypes from lms.djangoapps.instructor_task.models import ReportStore from lms.djangoapps.instructor.views.serializer import ( AccessSerializer, RoleNameSerializer, ShowStudentExtensionSerializer, - UserSerializer, SendEmailSerializer + UserSerializer, SendEmailSerializer, StudentAttemptsSerializer ) 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 @@ -1817,23 +1817,24 @@ class StudentProgressUrl(APIView): return Response(serializer.data) -@transaction.non_atomic_requests -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.GIVE_STUDENT_EXTENSION) -@require_post_params( - problem_to_reset="problem urlname to reset" -) -@common_exceptions_400 -def reset_student_attempts(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +@method_decorator(transaction.non_atomic_requests, name='dispatch') +class ResetStudentAttempts(DeveloperErrorViewMixin, APIView): """ - Resets a students attempts counter or starts a task to reset all students attempts counters. Optionally deletes student state for a problem. Limited to staff access. Some sub-methods limited to instructor access. + """ + http_method_names = ['post'] + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.GIVE_STUDENT_EXTENSION + serializer_class = StudentAttemptsSerializer - Takes some of the following query parameters + @method_decorator(ensure_csrf_cookie) + @transaction.non_atomic_requests + def post(self, request, course_id): + """ + Takes some of the following query parameters - problem_to_reset is a urlname of a problem - unique_student_identifier is an email or username - all_students is a boolean @@ -1843,65 +1844,74 @@ def reset_student_attempts(request, course_id): - delete_module is a boolean requires instructor access mutually exclusive with all_students - """ - course_id = CourseKey.from_string(course_id) - course = get_course_with_access( - request.user, 'staff', course_id, depth=None - ) - all_students = _get_boolean_param(request, 'all_students') + """ + course_id = CourseKey.from_string(course_id) + serializer_data = self.serializer_class(data=request.data) - if all_students and not has_access(request.user, 'instructor', course): - return HttpResponseForbidden("Requires instructor access.") + if not serializer_data.is_valid(): + return HttpResponseBadRequest(reason=serializer_data.errors) - problem_to_reset = strip_if_string(request.POST.get('problem_to_reset')) - student_identifier = request.POST.get('unique_student_identifier', None) - student = None - if student_identifier is not None: - student = get_student_from_identifier(student_identifier) - delete_module = _get_boolean_param(request, 'delete_module') - - # parameter combinations - if all_students and student: - return HttpResponseBadRequest( - "all_students and unique_student_identifier are mutually exclusive." - ) - if all_students and delete_module: - return HttpResponseBadRequest( - "all_students and delete_module are mutually exclusive." + course = get_course_with_access( + request.user, 'staff', course_id, depth=None ) - try: - module_state_key = UsageKey.from_string(problem_to_reset).map_into_course(course_id) - except InvalidKeyError: - return HttpResponseBadRequest() + all_students = serializer_data.validated_data.get('all_students') - response_payload = {} - response_payload['problem_to_reset'] = problem_to_reset + if all_students and not has_access(request.user, 'instructor', course): + return HttpResponseForbidden("Requires instructor access.") - if student: - try: - enrollment.reset_student_attempts( - course_id, - student, - module_state_key, - requesting_user=request.user, - delete_module=delete_module + problem_to_reset = strip_if_string(serializer_data.validated_data.get('problem_to_reset')) + student_identifier = request.POST.get('unique_student_identifier', None) + student = serializer_data.validated_data.get('unique_student_identifier') + delete_module = serializer_data.validated_data.get('delete_module') + + # parameter combinations + if all_students and student: + return HttpResponseBadRequest( + "all_students and unique_student_identifier are mutually exclusive." + ) + if all_students and delete_module: + return HttpResponseBadRequest( + "all_students and delete_module are mutually exclusive." ) - except StudentModule.DoesNotExist: - return HttpResponseBadRequest(_("Module does not exist.")) - except sub_api.SubmissionError: - # Trust the submissions API to log the error - error_msg = _("An error occurred while deleting the score.") - return HttpResponse(error_msg, status=500) - response_payload['student'] = student_identifier - elif all_students: - task_api.submit_reset_problem_attempts_for_all_students(request, module_state_key) - response_payload['task'] = TASK_SUBMISSION_OK - response_payload['student'] = 'All Students' - else: - return HttpResponseBadRequest() - return JsonResponse(response_payload) + try: + module_state_key = UsageKey.from_string(problem_to_reset).map_into_course(course_id) + except InvalidKeyError: + return HttpResponseBadRequest() + + response_payload = {} + response_payload['problem_to_reset'] = problem_to_reset + + if student: + try: + enrollment.reset_student_attempts( + course_id, + student, + module_state_key, + requesting_user=request.user, + delete_module=delete_module + ) + except StudentModule.DoesNotExist: + return HttpResponseBadRequest(_("Module does not exist.")) + except sub_api.SubmissionError: + # Trust the submissions API to log the error + error_msg = _("An error occurred while deleting the score.") + return HttpResponse(error_msg, status=500) + response_payload['student'] = student_identifier + + elif all_students: + try: + task_api.submit_reset_problem_attempts_for_all_students(request, module_state_key) + response_payload['task'] = TASK_SUBMISSION_OK + response_payload['student'] = 'All Students' + except Exception: # pylint: disable=broad-except + error_msg = _("An error occurred while attempting to reset for all students.") + return HttpResponse(error_msg, status=500) + else: + return HttpResponseBadRequest() + + return JsonResponse(response_payload) @transaction.non_atomic_requests @@ -1938,8 +1948,10 @@ def reset_student_attempts_for_entrance_exam(request, course_id): student_identifier = request.POST.get('unique_student_identifier', None) student = None + if student_identifier is not None: student = get_student_from_identifier(student_identifier) + all_students = _get_boolean_param(request, 'all_students') delete_module = _get_boolean_param(request, 'delete_module') diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index d4719b90b4..b54d38d804 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -34,7 +34,7 @@ urlpatterns = [ path('get_anon_ids', api.GetAnonIds.as_view(), name='get_anon_ids'), path('get_student_enrollment_status', api.get_student_enrollment_status, name="get_student_enrollment_status"), path('get_student_progress_url', api.StudentProgressUrl.as_view(), name='get_student_progress_url'), - path('reset_student_attempts', api.reset_student_attempts, name='reset_student_attempts'), + path('reset_student_attempts', api.ResetStudentAttempts.as_view(), name='reset_student_attempts'), path('rescore_problem', api.rescore_problem, name='rescore_problem'), path('override_problem_score', api.override_problem_score, name='override_problem_score'), path('reset_student_attempts_for_entrance_exam', api.reset_student_attempts_for_entrance_exam, diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index ff79124d02..d4a3d4e832 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -79,6 +79,57 @@ class ShowStudentExtensionSerializer(serializers.Serializer): return user +class StudentAttemptsSerializer(serializers.Serializer): + """ + Serializer for resetting a students attempts counter or starts a task to reset all students + attempts counters. + """ + problem_to_reset = serializers.CharField( + help_text="The identifier or description of the problem that needs to be reset." + ) + + # following are optional params. + unique_student_identifier = serializers.CharField( + help_text="Email or username of student.", required=False + ) + all_students = serializers.CharField(required=False) + delete_module = serializers.CharField(required=False) + + def validate_all_students(self, value): + """ + converts the all_student params value to bool. + """ + return self.verify_bool(value) + + def validate_delete_module(self, value): + """ + converts the all_student params value. + """ + return self.verify_bool(value) + + def validate_unique_student_identifier(self, value): + """ + Validate that the student corresponds to an existing user. + """ + try: + user = get_student_from_identifier(value) + except User.DoesNotExist: + return None + + return user + + def verify_bool(self, value): + """ + Returns the value of the boolean parameter with the given + name in the POST request. Handles translation from string + values to boolean values. + """ + if value is not None: + return value in ['true', 'True', True] + + return False + + class SendEmailSerializer(serializers.Serializer): """ Serializer for sending an email with optional scheduling.