diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index ce3433f312d8..b24ef618c7ce 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -1085,9 +1085,7 @@ def test_missing_username_and_email_error(self): res_json = json.loads(response.content.decode('utf-8')) # Assert Error Message - assert res_json['message'] == \ - 'Student username/email field is required and can not be empty.' \ - ' Kindly fill in username/email and then press "Invalidate Certificate" button.' + assert res_json['message'] == {'user': ['This field may not be blank.']} def test_invalid_user_name_error(self): """ @@ -1106,7 +1104,6 @@ def test_invalid_user_name_error(self): # Assert 400 status code in response assert response.status_code == 400 res_json = json.loads(response.content.decode('utf-8')) - # Assert Error Message assert res_json['message'] == f'{invalid_user} does not exist in the LMS. Please check your spelling and retry.' @@ -1125,7 +1122,6 @@ def test_no_generated_certificate_error(self): # Assert 400 status code in response assert response.status_code == 400 res_json = json.loads(response.content.decode('utf-8')) - # Assert Error Message assert res_json['message'] == f'The student {self.enrolled_user_2.username} does not have certificate for the course {self.course.number}. Kindly verify student username/email and the selected course are correct and try again.' # pylint: disable=line-too-long diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 2b1f28e4ceac..f27ffc32dbdb 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -107,8 +107,16 @@ from lms.djangoapps.instructor_task.data import InstructorTaskTypes from lms.djangoapps.instructor_task.models import ReportStore from lms.djangoapps.instructor.views.serializer import ( - AccessSerializer, BlockDueDateSerializer, RoleNameSerializer, ShowStudentExtensionSerializer, UserSerializer, - SendEmailSerializer, StudentAttemptsSerializer, ListInstructorTaskInputSerializer, UniqueStudentIdentifierSerializer + AccessSerializer, + BlockDueDateSerializer, + CertificateSerializer, + ListInstructorTaskInputSerializer, + RoleNameSerializer, + SendEmailSerializer, + ShowStudentExtensionSerializer, + StudentAttemptsSerializer, + UserSerializer, + UniqueStudentIdentifierSerializer ) 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 @@ -3638,12 +3646,9 @@ def build_row_errors(key, _user, row_count): return JsonResponse(results) -@transaction.non_atomic_requests -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.CERTIFICATE_INVALIDATION_VIEW) -@require_http_methods(['POST', 'DELETE']) -def certificate_invalidation_view(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 CertificateInvalidationView(APIView): """ Invalidate/Re-Validate students to/from certificate. @@ -3651,17 +3656,39 @@ def certificate_invalidation_view(request, course_id): :param course_id: course identifier of the course for whom to add/remove certificates exception. :return: JsonResponse object with success/error message or certificate invalidation data. """ - course_key = CourseKey.from_string(course_id) - # Validate request data and return error response in case of invalid data - try: - certificate_invalidation_data = parse_request_data(request) - student = _get_student_from_request_data(certificate_invalidation_data) - certificate = _get_certificate_for_user(course_key, student) - except ValueError as error: - return JsonResponse({'message': str(error)}, status=400) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.CERTIFICATE_INVALIDATION_VIEW + serializer_class = CertificateSerializer + http_method_names = ['post', 'delete'] - # Invalidate certificate of the given student for the course course - if request.method == 'POST': + @method_decorator(ensure_csrf_cookie) + @method_decorator(transaction.non_atomic_requests) + def post(self, request, course_id): + """ + Invalidate/Re-Validate students to/from certificate. + """ + course_key = CourseKey.from_string(course_id) + # Validate request data and return error response in case of invalid data + serializer_data = self.serializer_class(data=request.data) + if not serializer_data.is_valid(): + # return HttpResponseBadRequest(reason=serializer_data.errors) + return JsonResponse({'message': serializer_data.errors}, status=400) + + student = serializer_data.validated_data.get('user') + notes = serializer_data.validated_data.get('notes') + + if not student: + invalid_user = request.data.get('user') + response_payload = f'{invalid_user} does not exist in the LMS. Please check your spelling and retry.' + + return JsonResponse({'message': response_payload}, status=400) + + try: + certificate = _get_certificate_for_user(course_key, student) + except Exception as ex: # pylint: disable=broad-except + return JsonResponse({'message': str(ex)}, status=400) + + # Invalidate certificate of the given student for the course course try: if certs_api.is_on_allowlist(student, course_key): log.warning(f"Invalidating certificate for student {student.id} in course {course_key} failed. " @@ -3674,15 +3701,39 @@ def certificate_invalidation_view(request, course_id): certificate_invalidation = invalidate_certificate( request, certificate, - certificate_invalidation_data, + notes, student ) + except ValueError as error: return JsonResponse({'message': str(error)}, status=400) return JsonResponse(certificate_invalidation) - # Re-Validate student certificate for the course course - elif request.method == 'DELETE': + @method_decorator(ensure_csrf_cookie) + @method_decorator(transaction.non_atomic_requests) + def delete(self, request, course_id): + """ + Invalidate/Re-Validate students to/from certificate. + """ + # Re-Validate student certificate for the course course + course_key = CourseKey.from_string(course_id) + try: + data = json.loads(self.request.body.decode('utf8') or '{}') + except Exception: # pylint: disable=broad-except + data = {} + + serializer_data = self.serializer_class(data=data) + + if not serializer_data.is_valid(): + return HttpResponseBadRequest(reason=serializer_data.errors) + + student = serializer_data.validated_data.get('user') + + try: + certificate = _get_certificate_for_user(course_key, student) + except Exception as ex: # pylint: disable=broad-except + return JsonResponse({'message': str(ex)}, status=400) + try: re_validate_certificate(request, course_key, certificate, student) except ValueError as error: @@ -3691,13 +3742,13 @@ def certificate_invalidation_view(request, course_id): return JsonResponse({}, status=204) -def invalidate_certificate(request, generated_certificate, certificate_invalidation_data, student): +def invalidate_certificate(request, generated_certificate, notes, student): """ Invalidate given GeneratedCertificate and add CertificateInvalidation record for future reference or re-validation. :param request: HttpRequest object :param generated_certificate: GeneratedCertificate object, the certificate we want to invalidate - :param certificate_invalidation_data: dict object containing data for CertificateInvalidation. + :param notes: notes values. :param student: User object, this user is tied to the generated_certificate we are going to invalidate :return: dict object containing updated certificate invalidation data. """ @@ -3720,7 +3771,7 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat certificate_invalidation = certs_api.create_certificate_invalidation_entry( generated_certificate, request.user, - certificate_invalidation_data.get("notes", ""), + notes, ) # Invalidate the certificate @@ -3731,7 +3782,7 @@ def invalidate_certificate(request, generated_certificate, certificate_invalidat 'user': student.username, 'invalidated_by': certificate_invalidation.invalidated_by.username, 'created': certificate_invalidation.created.strftime("%B %d, %Y"), - 'notes': certificate_invalidation.notes, + 'notes': notes, } diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index eef299d1d7cb..491de58ce3c5 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -89,5 +89,9 @@ name='generate_certificate_exceptions'), path('generate_bulk_certificate_exceptions', api.generate_bulk_certificate_exceptions, name='generate_bulk_certificate_exceptions'), - path('certificate_invalidation_view/', api.certificate_invalidation_view, name='certificate_invalidation_view'), + path( + 'certificate_invalidation_view/', + api.CertificateInvalidationView.as_view(), + name='certificate_invalidation_view' + ), ] diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 59ac66ab838b..2ac794bc2943 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -228,3 +228,25 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) if disable_due_datetime: self.fields['due_datetime'].required = False + + +class CertificateSerializer(serializers.Serializer): + """ + Serializer for resetting a students attempts counter or starts a task to reset all students + attempts counters. + """ + user = serializers.CharField( + help_text="Email or username of student.", required=True + ) + notes = serializers.CharField(required=False, allow_null=True, allow_blank=True) + + def validate_user(self, value): + """ + Validate that the user corresponds to an existing user. + """ + try: + user = get_student_from_identifier(value) + except User.DoesNotExist: + return None + + return user