From 81d4239117576e4e387cabff15f31493e59c509a Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Mon, 2 Dec 2024 17:08:01 +0500 Subject: [PATCH] feat!: upgrade start_certificate_regeneration to drf ( 29 ) (#35599) * feat!: upgrading api to DRF. --- .../instructor/tests/test_certificates.py | 5 +- lms/djangoapps/instructor/views/api.py | 67 +++++++++---------- lms/djangoapps/instructor/views/api_urls.py | 3 +- lms/djangoapps/instructor/views/serializer.py | 24 ++++++- 4 files changed, 58 insertions(+), 41 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index 625b5b5a9c07..3b6a9a223586 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -367,7 +367,7 @@ def test_certificate_regeneration_error(self): # Assert Error Message assert res_json['message'] ==\ - 'Please select one or more certificate statuses that require certificate regeneration.' + 'Please select certificate statuses from the list only.' # Access the url passing 'certificate_statuses' that are not present in db url = reverse('start_certificate_regeneration', kwargs={'course_id': str(self.course.id)}) @@ -378,7 +378,8 @@ def test_certificate_regeneration_error(self): res_json = json.loads(response.content.decode('utf-8')) # Assert Error Message - assert res_json['message'] == 'Please select certificate statuses from the list only.' + assert (res_json['message'] == + 'Please select certificate statuses from the list only.') @override_settings(CERT_QUEUE='certificates') diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 6c1b1a014fdc..631500fe3246 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -77,9 +77,6 @@ from common.djangoapps.util.views import require_global_staff # pylint: disable=unused-import from lms.djangoapps.bulk_email.api import is_bulk_email_feature_enabled, create_course_email from lms.djangoapps.certificates import api as certs_api -from lms.djangoapps.certificates.models import ( - CertificateStatuses -) from lms.djangoapps.course_home_api.toggles import course_home_mfe_progress_tab_is_active from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.courses import get_course_with_access @@ -110,6 +107,7 @@ AccessSerializer, BlockDueDateSerializer, CertificateSerializer, + CertificateStatusesSerializer, ListInstructorTaskInputSerializer, RoleNameSerializer, SendEmailSerializer, @@ -3308,46 +3306,41 @@ def post(self, 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.START_CERTIFICATE_REGENERATION) -@require_POST -@common_exceptions_400 -def start_certificate_regeneration(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 StartCertificateRegeneration(DeveloperErrorViewMixin, APIView): """ Start regenerating certificates for students whose certificate statuses lie with in 'certificate_statuses' entry in POST data. """ - course_key = CourseKey.from_string(course_id) - certificates_statuses = request.POST.getlist('certificate_statuses', []) - if not certificates_statuses: - return JsonResponse( - {'message': _('Please select one or more certificate statuses that require certificate regeneration.')}, - status=400 - ) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.START_CERTIFICATE_REGENERATION + serializer_class = CertificateStatusesSerializer + http_method_names = ['post'] - # Check if the selected statuses are allowed - allowed_statuses = [ - CertificateStatuses.downloadable, - CertificateStatuses.error, - CertificateStatuses.notpassing, - CertificateStatuses.audit_passing, - CertificateStatuses.audit_notpassing, - ] - if not set(certificates_statuses).issubset(allowed_statuses): - return JsonResponse( - {'message': _('Please select certificate statuses from the list only.')}, - status=400 - ) + @method_decorator(transaction.non_atomic_requests, name='dispatch') + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + certificate_statuses 'certificate_statuses' in POST data. + """ + course_key = CourseKey.from_string(course_id) + serializer = self.serializer_class(data=request.data) - task_api.regenerate_certificates(request, course_key, certificates_statuses) - response_payload = { - 'message': _('Certificate regeneration task has been started. ' - 'You can view the status of the generation task in the "Pending Tasks" section.'), - 'success': True - } - return JsonResponse(response_payload) + if not serializer.is_valid(): + return JsonResponse( + {'message': _('Please select certificate statuses from the list only.')}, + status=400 + ) + + certificates_statuses = serializer.validated_data['certificate_statuses'] + task_api.regenerate_certificates(request, course_key, certificates_statuses) + response_payload = { + 'message': _('Certificate regeneration task has been started. ' + 'You can view the status of the generation task in the "Pending Tasks" section.'), + 'success': True + } + return JsonResponse(response_payload) @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index 72b93ba12766..ea27034d0942 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -83,7 +83,8 @@ # Certificates 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('start_certificate_regeneration', api.StartCertificateRegeneration.as_view(), + name='start_certificate_regeneration'), path('certificate_exception_view/', api.CertificateExceptionView.as_view(), name='certificate_exception_view'), re_path(r'^generate_certificate_exceptions/(?P[^/]*)', api.GenerateCertificateExceptions.as_view(), name='generate_certificate_exceptions'), diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index 48c5f7edec1c..f7fc685f658c 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -4,10 +4,12 @@ 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.certificates.models import CertificateStatuses from lms.djangoapps.instructor.access import ROLES +from .tools import get_student_from_identifier + class RoleNameSerializer(serializers.Serializer): # pylint: disable=abstract-method """ @@ -230,6 +232,26 @@ def __init__(self, *args, **kwargs): self.fields['due_datetime'].required = False +class CertificateStatusesSerializer(serializers.Serializer): + """ + Serializer for validating and serializing certificate status inputs. + + This serializer is used to ensure that the provided certificate statuses + conform to the predefined set of valid statuses defined in the + `CertificateStatuses` enumeration. + """ + certificate_statuses = serializers.ListField( + child=serializers.ChoiceField(choices=[ + CertificateStatuses.downloadable, + CertificateStatuses.error, + CertificateStatuses.notpassing, + CertificateStatuses.audit_passing, + CertificateStatuses.audit_notpassing, + ]), + allow_empty=False # Set to True if you want to allow empty lists + ) + + class CertificateSerializer(serializers.Serializer): """ Serializer for multiple operations related with certificates.