feat!: upgrade certificate_exception_view to DRF ( 28 )

lms/djangoapps/instructor/tests/test_certificates.py

@@ -488,9 +488,7 @@ def test_certificate_exception_missing_username_and_email_error(self):
         assert not res_json['success']
 
         # 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 "Add to Exception List" button.'
+        assert res_json['message'] == {'user': ['This field may not be blank.']}
 
     def test_certificate_exception_duplicate_user_error(self):
         """
@@ -604,6 +602,34 @@ def test_certificate_exception_removed_successfully(self):
         # Verify that certificate exception does not exist
         assert not certs_api.is_on_allowlist(self.user2, self.course.id)
 
+    def test_certificate_exception_removed_successfully_form_url(self):
+        """
+        In case of deletion front-end is sending content-type x-www-form-urlencoded.
+        Just to handle that some logic added in api and this test is for that part.
+        Test certificates exception removal api endpoint returns success status
+        when called with valid course key and certificate exception id
+        """
+        GeneratedCertificateFactory.create(
+            user=self.user2,
+            course_id=self.course.id,
+            status=CertificateStatuses.downloadable,
+            grade='1.0'
+        )
+        # Verify that certificate exception exists
+        assert certs_api.is_on_allowlist(self.user2, self.course.id)
+
+        response = self.client.post(
+            self.url,
+            data=json.dumps(self.certificate_exception_in_db),
+            content_type='application/x-www-form-urlencoded',
+            REQUEST_METHOD='DELETE'
+        )
+        # Assert successful request processing
+        assert response.status_code == 204
+
+        # Verify that certificate exception does not exist
+        assert not certs_api.is_on_allowlist(self.user2, self.course.id)
+
     def test_remove_certificate_exception_invalid_request_error(self):
         """
         Test certificates exception removal api endpoint returns error

lms/djangoapps/instructor/views/api.py

@@ -22,15 +22,15 @@
 from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist, PermissionDenied, ValidationError
 from django.core.validators import validate_email
 from django.db import IntegrityError, transaction
-from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound
+from django.http import QueryDict, HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound
 from django.shortcuts import redirect
 from django.urls import reverse
 from django.utils.decorators import method_decorator
 from django.utils.html import strip_tags
 from django.utils.translation import gettext as _
 from django.views.decorators.cache import cache_control
 from django.views.decorators.csrf import ensure_csrf_cookie
-from django.views.decorators.http import require_POST, require_http_methods
+from django.views.decorators.http import require_POST
 from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
 from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
 from edx_when.api import get_date_for_block
@@ -3350,42 +3350,93 @@ def start_certificate_regeneration(request, course_id):
     return JsonResponse(response_payload)
 
 
-@transaction.non_atomic_requests
-@ensure_csrf_cookie
-@cache_control(no_cache=True, no_store=True, must_revalidate=True)
-@require_course_permission(permissions.CERTIFICATE_EXCEPTION_VIEW)
-@require_http_methods(['POST', 'DELETE'])
-def certificate_exception_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 CertificateExceptionView(DeveloperErrorViewMixin, APIView):
     """
     Add/Remove students to/from the certificate allowlist.
-
-    :param request: HttpRequest object
-    :param course_id: course identifier of the course for whom to add/remove certificates exception.
-    :return: JsonResponse object with success/error message or certificate exception data.
     """
-    course_key = CourseKey.from_string(course_id)
-    # Validate request data and return error response in case of invalid data
-    try:
-        certificate_exception, student = parse_request_data_and_get_user(request)
-    except ValueError as error:
-        return JsonResponse({'success': False, 'message': str(error)}, status=400)
+    permission_classes = (IsAuthenticated, permissions.InstructorPermission)
+    permission_name = permissions.CERTIFICATE_EXCEPTION_VIEW
+    serializer_class = CertificateSerializer
+    http_method_names = ['post', 'delete']
+
+    @method_decorator(transaction.non_atomic_requests, name='dispatch')
+    @method_decorator(ensure_csrf_cookie)
+    def post(self, request, course_id):
+        """
+        Add certificate exception for a student.
+        """
+        return self._handle_certificate_exception(request, course_id, action="post")
+
+    @method_decorator(ensure_csrf_cookie)
+    @method_decorator(transaction.non_atomic_requests)
+    def delete(self, request, course_id):
+        """
+        Remove certificate exception for a student.
+        """
+        return self._handle_certificate_exception(request, course_id, action="delete")
+
+    def _handle_certificate_exception(self, request, course_id, action):
+        """
+        Handles adding or removing certificate exceptions.
+        """
+        course_key = CourseKey.from_string(course_id)
+        try:
+            data = request.data
+        except Exception:  # pylint: disable=broad-except
+            return JsonResponse(
+                {
+                    'success': False,
+                    'message':
+                        _('The record is not in the correct format. Please add a valid username or email address.')},
+                status=400
+            )
+
+        # Extract and validate the student information
+        student, error_response = self._get_and_validate_user(data)
+
+        if error_response:
+            return error_response
 
-    # Add new Certificate Exception for the student passed in request data
-    if request.method == 'POST':
         try:
-            exception = add_certificate_exception(course_key, student, certificate_exception)
+            if action == "post":
+                exception = add_certificate_exception(course_key, student, data)
+                return JsonResponse(exception)
+            elif action == "delete":
+                remove_certificate_exception(course_key, student)
+                return JsonResponse({}, status=204)
         except ValueError as error:
             return JsonResponse({'success': False, 'message': str(error)}, status=400)
-        return JsonResponse(exception)
 
-    # Remove Certificate Exception for the student passed in request data
-    elif request.method == 'DELETE':
+    def _get_and_validate_user(self, raw_data):
+        """
+        Extracts the user data from the request and validates the student.
+        """
+        # This is only happening in case of delete.
+        # because content-type is coming as x-www-form-urlencoded from front-end.
+        if isinstance(raw_data, QueryDict):
+            raw_data = list(raw_data.keys())[0]
+            try:
+                raw_data = json.loads(raw_data)
+            except Exception as error:  # pylint: disable=broad-except
+                return None, JsonResponse({'success': False, 'message': str(error)}, status=400)
+
         try:
-            remove_certificate_exception(course_key, student)
+            user_data = raw_data.get('user_name', '') or raw_data.get('user_email', '')
         except ValueError as error:
-            return JsonResponse({'success': False, 'message': str(error)}, status=400)
+            return None, JsonResponse({'success': False, 'message': str(error)}, status=400)
 
-        return JsonResponse({}, status=204)
+        serializer_data = self.serializer_class(data={'user': user_data})
+        if not serializer_data.is_valid():
+            return None, JsonResponse({'success': False, 'message': serializer_data.errors}, status=400)
+
+        student = serializer_data.validated_data.get('user')
+        if not student:
+            response_payload = f'{user_data} does not exist in the LMS. Please check your spelling and retry.'
+            return None, JsonResponse({'success': False, 'message': response_payload}, status=400)
+
+        return student, None
 
 
 def add_certificate_exception(course_key, student, certificate_exception):

lms/djangoapps/instructor/views/api_urls.py

@@ -84,7 +84,7 @@
     path('enable_certificate_generation', api.enable_certificate_generation, name='enable_certificate_generation'),
     path('start_certificate_generation', api.StartCertificateGeneration.as_view(), name='start_certificate_generation'),
     path('start_certificate_regeneration', api.start_certificate_regeneration, name='start_certificate_regeneration'),
-    path('certificate_exception_view/', api.certificate_exception_view, name='certificate_exception_view'),
+    path('certificate_exception_view/', api.CertificateExceptionView.as_view(), name='certificate_exception_view'),
     re_path(r'^generate_certificate_exceptions/(?P<generate_for>[^/]*)', api.GenerateCertificateExceptions.as_view(),
             name='generate_certificate_exceptions'),
     path('generate_bulk_certificate_exceptions', api.generate_bulk_certificate_exceptions,

lms/djangoapps/instructor/views/serializer.py

@@ -232,7 +232,11 @@ def __init__(self, *args, **kwargs):
 
 class CertificateSerializer(serializers.Serializer):
     """
-    Serializer for resetting a students attempts counter or starts a task to reset all students
+    Serializer for multiple operations related with certificates.
+    resetting a students attempts counter or starts a task to reset all students
+    attempts counters
+    Also Add/Remove students to/from the certificate allowlist.
+    Also For resetting a students attempts counter or starts a task to reset all students
     attempts counters.
     """
     user = serializers.CharField(