From 1f6fb47549702ca97519c739066d08dcc9df5dcc Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Tue, 21 Apr 2026 17:14:18 -0400 Subject: [PATCH 01/20] feat: add certificate management v2 API endpoints - Add ToggleCertificateGenerationView endpoint to enable/disable certificate generation - Add CertificateExceptionsView endpoint to grant and remove certificate exceptions (allowlist) - Add CertificateInvalidationsView endpoint to invalidate and re-validate certificates - Update certificate status labels to be more user-friendly (e.g., "Received" instead of "already received") - Update certificate generation history labels (e.g., "All Learners" instead of "All learners") - Add invalidation notes to IssuedCertificateSerializer - Add certificatesEnabled flag to CourseInformationSerializerV2 --- lms/djangoapps/certificates/data.py | 11 +- lms/djangoapps/certificates/models.py | 6 +- lms/djangoapps/instructor/views/api_urls.py | 15 + lms/djangoapps/instructor/views/api_v2.py | 369 +++++++++++++++++- .../instructor/views/serializers_v2.py | 21 + 5 files changed, 413 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/certificates/data.py b/lms/djangoapps/certificates/data.py index f1131ad74886..51f8d4d0432f 100644 --- a/lms/djangoapps/certificates/data.py +++ b/lms/djangoapps/certificates/data.py @@ -57,11 +57,12 @@ class CertificateStatuses: requesting = 'requesting' readable_statuses = { - downloadable: "already received", - notpassing: "didn't receive", - error: "error states", - audit_passing: "audit passing states", - audit_notpassing: "audit not passing states", + downloadable: "Received", + notpassing: "Not Received", + unavailable: "Invalidated", + error: "Error State", + audit_passing: "Audit - Passing", + audit_notpassing: "Audit - Not Passing", } PASSED_STATUSES = (downloadable, generating) diff --git a/lms/djangoapps/certificates/models.py b/lms/djangoapps/certificates/models.py index ac5916879b4e..541ff41d7b15 100644 --- a/lms/djangoapps/certificates/models.py +++ b/lms/djangoapps/certificates/models.py @@ -588,7 +588,7 @@ def get_certificate_generation_candidates(self): if not task_input.strip(): # if task input is empty, it means certificates were generated for all learners # Translators: This string represents task was executed for all learners. - return _("All learners") + return _("All Learners") task_input_json = json.loads(task_input) @@ -607,9 +607,9 @@ def get_certificate_generation_candidates(self): # for backwards compatibility. if 'student_set' in task_input_json or 'students' in task_input_json: # Translators: This string represents task was executed for students having exceptions. - return _("For exceptions") + return _("Granted Exceptions") else: - return _("All learners") + return _("All Learners") class Meta: app_label = "certificates" diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index bc7548863368..8f735bbd917e 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -86,6 +86,21 @@ api_v2.CertificateConfigView.as_view(), name='certificate_config' ), + re_path( + rf'^courses/{COURSE_ID_PATTERN}/certificates/toggle_generation$', + api_v2.ToggleCertificateGenerationView.as_view(), + name='toggle_certificate_generation' + ), + re_path( + rf'^courses/{COURSE_ID_PATTERN}/certificates/exceptions$', + api_v2.CertificateExceptionsView.as_view(), + name='certificate_exceptions' + ), + re_path( + rf'^courses/{COURSE_ID_PATTERN}/certificates/invalidations$', + api_v2.CertificateInvalidationsView.as_view(), + name='certificate_invalidations' + ), re_path( rf'^courses/{COURSE_ID_PATTERN}/enrollments$', api_v2.CourseEnrollmentsView.as_view(), diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 1473aaa06df6..96a0986fc053 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1287,7 +1287,8 @@ def get_serializer_context(self): context['invalidation_dict'] = { inv.generated_certificate.user_id: { 'invalidated_by': inv.invalidated_by.email, - 'created': inv.created.isoformat() + 'created': inv.created.isoformat(), + 'notes': inv.notes or '' } for inv in invalidations } @@ -1578,6 +1579,372 @@ def get(self, request, course_id): return Response({'enabled': enabled}, status=status.HTTP_200_OK) +class ToggleCertificateGenerationView(DeveloperErrorViewMixin, APIView): + """ + View to toggle certificate generation for a course. + + **Example Requests** + + POST /api/instructor/v2/courses/{course_id}/certificates/toggle_generation + + **Request Body** + + { + "enabled": true + } + + **Response Values** + + { + "enabled": true + } + + **Returns** + + * 200: OK - Certificate generation toggled successfully + * 400: Bad Request - Invalid request body + * 401: Unauthorized - User is not authenticated + * 403: Forbidden - User lacks instructor permissions + """ + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.ENABLE_CERTIFICATE_GENERATION + + def post(self, request, course_id): + """Toggle certificate generation for a course.""" + course_key = CourseKey.from_string(course_id) + enabled = request.data.get('enabled', False) + + try: + certs_api.set_cert_generation_enabled(course_key, enabled) + return Response({'enabled': enabled}, status=status.HTTP_200_OK) + except Exception as exc: + log.error("Error toggling certificate generation: %s", exc) + return Response( + {'message': str(exc)}, + status=status.HTTP_400_BAD_REQUEST + ) + + +class CertificateExceptionsView(DeveloperErrorViewMixin, APIView): + """ + View to grant or remove certificate exceptions (allowlist). + + **Example Requests** + + POST /api/instructor/v2/courses/{course_id}/certificates/exceptions + DELETE /api/instructor/v2/courses/{course_id}/certificates/exceptions + + **POST Request Body** + + { + "learners": ["username1", "username2"], + "notes": "Reason for granting exceptions" + } + + **DELETE Request Body** + + { + "username": "username1" + } + + **Returns** + + * 200: OK - Exception granted/removed successfully + * 400: Bad Request - Invalid request or user not found + * 401: Unauthorized - User is not authenticated + * 403: Forbidden - User lacks instructor permissions + """ + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.CERTIFICATE_EXCEPTION_VIEW + + def post(self, request, course_id): + """Grant certificate exceptions (add to allowlist).""" + course_key = CourseKey.from_string(course_id) + learners_input = request.data.get('learners', '') + notes = request.data.get('notes', '') + + # Handle both string (comma-separated) and list inputs + if isinstance(learners_input, str): + learners = [l.strip() for l in learners_input.split(',') if l.strip()] + else: + learners = learners_input + + if not learners: + return Response( + {'message': _('No learners provided')}, + status=status.HTTP_400_BAD_REQUEST + ) + + results = { + 'success': [], + 'errors': [] + } + + for learner in learners: + try: + user = get_user_by_username_or_email(learner) + if not user: + results['errors'].append({ + 'learner': learner, + 'message': _('User not found') + }) + continue + + # Check if user is enrolled + if not is_user_enrolled_in_course(user, course_key): + results['errors'].append({ + 'learner': learner, + 'message': _('User is not enrolled in this course') + }) + continue + + # Check if user already has an exception + if CertificateAllowlist.objects.filter( + course_id=course_key, + user=user + ).exists(): + results['errors'].append({ + 'learner': learner, + 'message': _('User already has a certificate exception') + }) + continue + + # Check if user has an active invalidation + if CertificateInvalidation.objects.filter( + generated_certificate__course_id=course_key, + generated_certificate__user=user, + active=True + ).exists(): + results['errors'].append({ + 'learner': learner, + 'message': _('User has an active certificate invalidation') + }) + continue + + # Grant exception + CertificateAllowlist.objects.create( + user=user, + course_id=course_key, + allowlist=True, + notes=notes + ) + results['success'].append(learner) + + except Exception as exc: + results['errors'].append({ + 'learner': learner, + 'message': str(exc) + }) + + return Response(results, status=status.HTTP_200_OK) + + def delete(self, request, course_id): + """Remove certificate exception (remove from allowlist).""" + course_key = CourseKey.from_string(course_id) + username = request.data.get('username') + + if not username: + return Response( + {'message': _('No username provided')}, + status=status.HTTP_400_BAD_REQUEST + ) + + try: + user = get_user_by_username_or_email(username) + if not user: + return Response( + {'message': _('User not found')}, + status=status.HTTP_400_BAD_REQUEST + ) + + # Remove exception + deleted_count, _ = CertificateAllowlist.objects.filter( + course_id=course_key, + user=user + ).delete() + + if deleted_count == 0: + return Response( + {'message': _('No certificate exception found for this user')}, + status=status.HTTP_404_NOT_FOUND + ) + + return Response( + {'message': _('Certificate exception removed successfully')}, + status=status.HTTP_200_OK + ) + + except Exception as exc: + log.error("Error removing certificate exception: %s", exc) + return Response( + {'message': str(exc)}, + status=status.HTTP_400_BAD_REQUEST + ) + + +class CertificateInvalidationsView(DeveloperErrorViewMixin, APIView): + """ + View to invalidate or re-validate certificates. + + **Example Requests** + + POST /api/instructor/v2/courses/{course_id}/certificates/invalidations + DELETE /api/instructor/v2/courses/{course_id}/certificates/invalidations + + **POST Request Body** + + { + "learners": ["username1", "username2"], + "notes": "Reason for invalidation" + } + + **DELETE Request Body** + + { + "username": "username1" + } + + **Returns** + + * 200: OK - Certificate invalidated/re-validated successfully + * 400: Bad Request - Invalid request or certificate not found + * 401: Unauthorized - User is not authenticated + * 403: Forbidden - User lacks instructor permissions + """ + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.CERTIFICATE_INVALIDATION_VIEW + + def post(self, request, course_id): + """Invalidate certificates.""" + course_key = CourseKey.from_string(course_id) + learners_input = request.data.get('learners', '') + notes = request.data.get('notes', '') + + # Handle both string (comma-separated) and list inputs + if isinstance(learners_input, str): + learners = [l.strip() for l in learners_input.split(',') if l.strip()] + else: + learners = learners_input + + if not learners: + return Response( + {'message': _('No learners provided')}, + status=status.HTTP_400_BAD_REQUEST + ) + + results = { + 'success': [], + 'errors': [] + } + + for learner in learners: + try: + user = get_user_by_username_or_email(learner) + if not user: + results['errors'].append({ + 'learner': learner, + 'message': _('User not found') + }) + continue + + # Get the certificate + try: + certificate = GeneratedCertificate.objects.get( + course_id=course_key, + user=user + ) + except GeneratedCertificate.DoesNotExist: + results['errors'].append({ + 'learner': learner, + 'message': _('Certificate not found for this user') + }) + continue + + # Check if already invalidated + if CertificateInvalidation.objects.filter( + generated_certificate=certificate, + active=True + ).exists(): + results['errors'].append({ + 'learner': learner, + 'message': _('Certificate is already invalidated') + }) + continue + + # Invalidate certificate + CertificateInvalidation.objects.create( + generated_certificate=certificate, + invalidated_by=request.user, + notes=notes, + active=True + ) + certificate.invalidate() + results['success'].append(learner) + + except Exception as exc: + results['errors'].append({ + 'learner': learner, + 'message': str(exc) + }) + + return Response(results, status=status.HTTP_200_OK) + + def delete(self, request, course_id): + """Re-validate certificate (remove invalidation).""" + course_key = CourseKey.from_string(course_id) + username = request.data.get('username') + + if not username: + return Response( + {'message': _('No username provided')}, + status=status.HTTP_400_BAD_REQUEST + ) + + try: + user = get_user_by_username_or_email(username) + if not user: + return Response( + {'message': _('User not found')}, + status=status.HTTP_400_BAD_REQUEST + ) + + # Get the certificate + try: + certificate = GeneratedCertificate.objects.get( + course_id=course_key, + user=user + ) + except GeneratedCertificate.DoesNotExist: + return Response( + {'message': _('Certificate not found for this user')}, + status=status.HTTP_404_NOT_FOUND + ) + + # Remove invalidation + updated_count = CertificateInvalidation.objects.filter( + generated_certificate=certificate, + active=True + ).update(active=False) + + if updated_count == 0: + return Response( + {'message': _('No active invalidation found for this certificate')}, + status=status.HTTP_404_NOT_FOUND + ) + + return Response( + {'message': _('Certificate invalidation removed successfully')}, + status=status.HTTP_200_OK + ) + + except Exception as exc: + log.error("Error removing certificate invalidation: %s", exc) + return Response( + {'message': str(exc)}, + status=status.HTTP_400_BAD_REQUEST + ) + + class CourseEnrollmentsView(DeveloperErrorViewMixin, ListAPIView): """ List all active enrollments for a course with optional search, filtering, and pagination. diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index b163869b4fc9..7ced0839b2d4 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -82,6 +82,9 @@ class CourseInformationSerializerV2(serializers.Serializer): analytics_dashboard_message = serializers.SerializerMethodField( help_text="Message about analytics dashboard availability" ) + certificates_enabled = serializers.SerializerMethodField( + help_text="Whether certificate management features are enabled for this course" + ) @staticmethod def _build_tab_url(setting_name, *path_parts, strip_url=True): @@ -467,6 +470,14 @@ def get_analytics_dashboard_message(self, data): """Get analytics dashboard availability message.""" return get_analytics_dashboard_message(data['course'].id) + def get_certificates_enabled(self, data): + """Check if certificate management features are enabled.""" + from lms.djangoapps.certificates import api as certs_api + + course_key = data['course'].id + # Check if certificate generation is enabled (not available for CCX courses) + return certs_api.is_certificate_generation_enabled() and not hasattr(course_key, 'ccx') + class InstructorTaskSerializer(serializers.Serializer): """Serializer for instructor task details.""" @@ -624,6 +635,10 @@ class IssuedCertificateSerializer(serializers.Serializer): allow_null=True, help_text="Date when certificate was invalidated in ISO 8601 format" ) + invalidation_note = serializers.SerializerMethodField( + allow_null=True, + help_text="Notes about the invalidation" + ) def get_enrollment_track(self, obj): """Get enrollment track from context.""" @@ -665,6 +680,12 @@ def get_invalidation_date(self, obj): invalidation_info = invalidation_dict.get(obj.user_id) return invalidation_info['created'] if invalidation_info else None + def get_invalidation_note(self, obj): + """Get invalidation notes from invalidation data in context.""" + invalidation_dict = self.context.get('invalidation_dict', {}) + invalidation_info = invalidation_dict.get(obj.user_id) + return invalidation_info['notes'] if invalidation_info else None + class CertificateGenerationHistorySerializer(serializers.Serializer): """ From d2de81c769c69c70f2f2783366999ee0138dae1c Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Tue, 21 Apr 2026 17:37:28 -0400 Subject: [PATCH 02/20] fix: linting --- lms/djangoapps/instructor/views/api_v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 96a0986fc053..3cdbf4ba8312 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1758,7 +1758,7 @@ def delete(self, request, course_id): ) # Remove exception - deleted_count, _ = CertificateAllowlist.objects.filter( + deleted_count, __ = CertificateAllowlist.objects.filter( course_id=course_key, user=user ).delete() From 4a355f5de79c3fe60677839b3577395ab0334935 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Tue, 21 Apr 2026 18:04:34 -0400 Subject: [PATCH 03/20] fix: linting --- lms/djangoapps/instructor/views/api_v2.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 3cdbf4ba8312..6581f959a68e 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1617,7 +1617,7 @@ def post(self, request, course_id): try: certs_api.set_cert_generation_enabled(course_key, enabled) return Response({'enabled': enabled}, status=status.HTTP_200_OK) - except Exception as exc: + except Exception as exc: # pylint: disable=broad-except log.error("Error toggling certificate generation: %s", exc) return Response( {'message': str(exc)}, @@ -1730,7 +1730,7 @@ def post(self, request, course_id): ) results['success'].append(learner) - except Exception as exc: + except Exception as exc: # pylint: disable=broad-except results['errors'].append({ 'learner': learner, 'message': str(exc) @@ -1774,7 +1774,7 @@ def delete(self, request, course_id): status=status.HTTP_200_OK ) - except Exception as exc: + except Exception as exc: # pylint: disable=broad-except log.error("Error removing certificate exception: %s", exc) return Response( {'message': str(exc)}, @@ -1881,7 +1881,7 @@ def post(self, request, course_id): certificate.invalidate() results['success'].append(learner) - except Exception as exc: + except Exception as exc: # pylint: disable=broad-except results['errors'].append({ 'learner': learner, 'message': str(exc) @@ -1937,7 +1937,7 @@ def delete(self, request, course_id): status=status.HTTP_200_OK ) - except Exception as exc: + except Exception as exc: # pylint: disable=broad-except log.error("Error removing certificate invalidation: %s", exc) return Response( {'message': str(exc)}, From ea687fee0d5078f4d38dfa509b80a5638e448bbf Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Tue, 21 Apr 2026 18:33:53 -0400 Subject: [PATCH 04/20] fix: linting --- lms/djangoapps/instructor/tests/test_api_v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/tests/test_api_v2.py b/lms/djangoapps/instructor/tests/test_api_v2.py index 3669a284dd75..1de288e5236c 100644 --- a/lms/djangoapps/instructor/tests/test_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_api_v2.py @@ -2102,7 +2102,7 @@ def test_history_entry_structure(self): # Verify all required fields are present (snake_case from serializer) assert entry['task_name'] == 'Regenerated' assert 'date' in entry - assert entry['details'] == 'All learners' + assert entry['details'] == 'All Learners' # Verify data types assert isinstance(entry['task_name'], str) From 5f7d3df60256fea8244a57b7cd4e62dd02a78e3f Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Tue, 21 Apr 2026 18:56:05 -0400 Subject: [PATCH 05/20] fix: linting --- .../certificates/tests/test_models.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/certificates/tests/test_models.py b/lms/djangoapps/certificates/tests/test_models.py index 526b9a8f9392..5d7ad08fe615 100644 --- a/lms/djangoapps/certificates/tests/test_models.py +++ b/lms/djangoapps/certificates/tests/test_models.py @@ -267,22 +267,22 @@ class TestCertificateGenerationHistory(OpenEdxEventsTestMixin, TestCase): ENABLED_OPENEDX_EVENTS = [] @ddt.data( - ({"student_set": "allowlisted_not_generated"}, "For exceptions", True), - ({"student_set": "allowlisted_not_generated"}, "For exceptions", False), + ({"student_set": "allowlisted_not_generated"}, "Granted Exceptions", True), + ({"student_set": "allowlisted_not_generated"}, "Granted Exceptions", False), # check "students" key for backwards compatibility - ({"students": [1, 2, 3]}, "For exceptions", True), - ({"students": [1, 2, 3]}, "For exceptions", False), - ({}, "All learners", True), - ({}, "All learners", False), + ({"students": [1, 2, 3]}, "Granted Exceptions", True), + ({"students": [1, 2, 3]}, "Granted Exceptions", False), + ({}, "All Learners", True), + ({}, "All Learners", False), # test single status to regenerate returns correctly - ({"statuses_to_regenerate": ['downloadable']}, 'already received', True), - ({"statuses_to_regenerate": ['downloadable']}, 'already received', False), + ({"statuses_to_regenerate": ['downloadable']}, 'Received', True), + ({"statuses_to_regenerate": ['downloadable']}, 'Received', False), # test that list of > 1 statuses render correctly - ({"statuses_to_regenerate": ['downloadable', 'error']}, 'already received, error states', True), - ({"statuses_to_regenerate": ['downloadable', 'error']}, 'already received, error states', False), + ({"statuses_to_regenerate": ['downloadable', 'error']}, 'Received, Error State', True), + ({"statuses_to_regenerate": ['downloadable', 'error']}, 'Received, Error State', False), # test that only "readable" statuses are returned - ({"statuses_to_regenerate": ['downloadable', 'not_readable']}, 'already received', True), - ({"statuses_to_regenerate": ['downloadable', 'not_readable']}, 'already received', False), + ({"statuses_to_regenerate": ['downloadable', 'not_readable']}, 'Received', True), + ({"statuses_to_regenerate": ['downloadable', 'not_readable']}, 'Received', False), ) @ddt.unpack def test_get_certificate_generation_candidates(self, task_input, expected, is_regeneration): From 5d08560852f2d81167db09c00b53875d161d8a25 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 11:06:07 -0400 Subject: [PATCH 06/20] feat: PR feedback --- lms/djangoapps/instructor/views/api_v2.py | 247 +++++++++++++----- .../instructor/views/serializers_v2.py | 13 +- 2 files changed, 190 insertions(+), 70 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 6581f959a68e..589a359bb997 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1611,16 +1611,26 @@ class ToggleCertificateGenerationView(DeveloperErrorViewMixin, APIView): def post(self, request, course_id): """Toggle certificate generation for a course.""" + from .serializers_v2 import ToggleCertificateGenerationSerializer + course_key = CourseKey.from_string(course_id) - enabled = request.data.get('enabled', False) + # Validate that the course exists before updating certificate settings + get_course_by_id(course_key) + + # Validate request body + serializer = ToggleCertificateGenerationSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + enabled = serializer.validated_data['enabled'] try: certs_api.set_cert_generation_enabled(course_key, enabled) return Response({'enabled': enabled}, status=status.HTTP_200_OK) - except Exception as exc: # pylint: disable=broad-except - log.error("Error toggling certificate generation: %s", exc) + except Exception: # pylint: disable=broad-except + log.exception("Error toggling certificate generation for course %s", course_id) return Response( - {'message': str(exc)}, + {'message': _('Unable to update certificate generation settings')}, status=status.HTTP_400_BAD_REQUEST ) @@ -1660,6 +1670,8 @@ class CertificateExceptionsView(DeveloperErrorViewMixin, APIView): def post(self, request, course_id): """Grant certificate exceptions (add to allowlist).""" course_key = CourseKey.from_string(course_id) + # Validate that the course exists + get_course_by_id(course_key) learners_input = request.data.get('learners', '') notes = request.data.get('notes', '') @@ -1680,18 +1692,58 @@ def post(self, request, course_id): 'errors': [] } + # Resolve all usernames/emails to users upfront + learner_to_user = {} for learner in learners: try: user = get_user_by_username_or_email(learner) - if not user: + if user: + learner_to_user[learner] = user + else: results['errors'].append({ 'learner': learner, 'message': _('User not found') }) - continue + except Exception as exc: # pylint: disable=broad-except + results['errors'].append({ + 'learner': learner, + 'message': str(exc) + }) + + if not learner_to_user: + return Response(results, status=status.HTTP_200_OK) + + users = list(learner_to_user.values()) + user_ids = [u.id for u in users] + + # Bulk fetch enrollments + enrollments = CourseEnrollment.objects.filter( + course_id=course_key, + user_id__in=user_ids + ).values_list('user_id', flat=True) + enrolled_user_ids = set(enrollments) + + # Bulk fetch existing allowlist entries + existing_allowlist = CertificateAllowlist.objects.filter( + course_id=course_key, + user_id__in=user_ids + ).values_list('user_id', flat=True) + allowlisted_user_ids = set(existing_allowlist) + # Bulk fetch active invalidations + active_invalidations = CertificateInvalidation.objects.filter( + generated_certificate__course_id=course_key, + generated_certificate__user_id__in=user_ids, + active=True + ).values_list('generated_certificate__user_id', flat=True) + invalidated_user_ids = set(active_invalidations) + + # Process each learner + exceptions_to_create = [] + for learner, user in learner_to_user.items(): + try: # Check if user is enrolled - if not is_user_enrolled_in_course(user, course_key): + if user.id not in enrolled_user_ids: results['errors'].append({ 'learner': learner, 'message': _('User is not enrolled in this course') @@ -1699,10 +1751,7 @@ def post(self, request, course_id): continue # Check if user already has an exception - if CertificateAllowlist.objects.filter( - course_id=course_key, - user=user - ).exists(): + if user.id in allowlisted_user_ids: results['errors'].append({ 'learner': learner, 'message': _('User already has a certificate exception') @@ -1710,27 +1759,33 @@ def post(self, request, course_id): continue # Check if user has an active invalidation - if CertificateInvalidation.objects.filter( - generated_certificate__course_id=course_key, - generated_certificate__user=user, - active=True - ).exists(): + if user.id in invalidated_user_ids: results['errors'].append({ 'learner': learner, 'message': _('User has an active certificate invalidation') }) continue - # Grant exception - CertificateAllowlist.objects.create( - user=user, - course_id=course_key, - allowlist=True, - notes=notes - ) - results['success'].append(learner) + # Add to list for processing + exceptions_to_create.append((learner, user)) + + except Exception as exc: # pylint: disable=broad-except + results['errors'].append({ + 'learner': learner, + 'message': str(exc) + }) + # Create all exceptions using the certificates API to ensure idempotency + # and avoid race conditions with the unique_together constraint + for learner, user in exceptions_to_create: + try: + certs_api.create_or_update_certificate_allowlist_entry(user, course_key, notes) + results['success'].append(learner) except Exception as exc: # pylint: disable=broad-except + log.exception( + "Error creating certificate exception for user %s in course %s", + user.id, course_key + ) results['errors'].append({ 'learner': learner, 'message': str(exc) @@ -1741,6 +1796,8 @@ def post(self, request, course_id): def delete(self, request, course_id): """Remove certificate exception (remove from allowlist).""" course_key = CourseKey.from_string(course_id) + # Validate that the course exists + get_course_by_id(course_key) username = request.data.get('username') if not username: @@ -1757,27 +1814,25 @@ def delete(self, request, course_id): status=status.HTTP_400_BAD_REQUEST ) - # Remove exception - deleted_count, __ = CertificateAllowlist.objects.filter( - course_id=course_key, - user=user - ).delete() - - if deleted_count == 0: + # Remove exception via certificates API so any existing certificate + # is invalidated before the allowlist entry is removed + if not certs_api.get_allowlist_entry(user, course_key): return Response( {'message': _('No certificate exception found for this user')}, status=status.HTTP_404_NOT_FOUND ) + certs_api.remove_allowlist_entry(user, course_key) + return Response( {'message': _('Certificate exception removed successfully')}, status=status.HTTP_200_OK ) - except Exception as exc: # pylint: disable=broad-except - log.error("Error removing certificate exception: %s", exc) + except Exception: # pylint: disable=broad-except + log.exception("Error removing certificate exception for course %s", course_id) return Response( - {'message': str(exc)}, + {'message': _('Unable to remove certificate exception')}, status=status.HTTP_400_BAD_REQUEST ) @@ -1817,6 +1872,8 @@ class CertificateInvalidationsView(DeveloperErrorViewMixin, APIView): def post(self, request, course_id): """Invalidate certificates.""" course_key = CourseKey.from_string(course_id) + # Validate that the course exists + get_course_by_id(course_key) learners_input = request.data.get('learners', '') notes = request.data.get('notes', '') @@ -1837,51 +1894,93 @@ def post(self, request, course_id): 'errors': [] } + # Resolve all usernames/emails to users upfront + learner_to_user = {} for learner in learners: try: user = get_user_by_username_or_email(learner) - if not user: + if user: + learner_to_user[learner] = user + else: results['errors'].append({ 'learner': learner, 'message': _('User not found') }) - continue + except Exception as exc: # pylint: disable=broad-except + results['errors'].append({ + 'learner': learner, + 'message': str(exc) + }) + + if not learner_to_user: + return Response(results, status=status.HTTP_200_OK) + users = list(learner_to_user.values()) + user_ids = [u.id for u in users] + + # Bulk fetch certificates + certificates = GeneratedCertificate.objects.filter( + course_id=course_key, + user_id__in=user_ids + ).select_related('user') + user_id_to_certificate = {cert.user_id: cert for cert in certificates} + + # Bulk fetch existing active invalidations + active_invalidations = CertificateInvalidation.objects.filter( + generated_certificate__course_id=course_key, + generated_certificate__user_id__in=user_ids, + active=True + ).values_list('generated_certificate__user_id', flat=True) + invalidated_user_ids = set(active_invalidations) + + # Process each learner + certificates_to_invalidate = [] + for learner, user in learner_to_user.items(): + try: # Get the certificate - try: - certificate = GeneratedCertificate.objects.get( - course_id=course_key, - user=user - ) - except GeneratedCertificate.DoesNotExist: + certificate = user_id_to_certificate.get(user.id) + if not certificate: results['errors'].append({ 'learner': learner, 'message': _('Certificate not found for this user') }) continue - # Check if already invalidated - if CertificateInvalidation.objects.filter( - generated_certificate=certificate, - active=True - ).exists(): + # Verify that the certificate is valid before invalidating (matches v1 behavior) + if not certificate.is_valid(): results['errors'].append({ 'learner': learner, - 'message': _('Certificate is already invalidated') + 'message': _('Certificate is already invalid') }) continue - # Invalidate certificate - CertificateInvalidation.objects.create( - generated_certificate=certificate, - invalidated_by=request.user, - notes=notes, - active=True + # Add to list for processing + certificates_to_invalidate.append((learner, certificate)) + + except Exception as exc: # pylint: disable=broad-except + results['errors'].append({ + 'learner': learner, + 'message': str(exc) + }) + + # Invalidate certificates using the certificates API to ensure idempotency + # and consistency with v1 behavior + for learner, certificate in certificates_to_invalidate: + try: + # Create invalidation entry (uses update_or_create for idempotency) + certs_api.create_certificate_invalidation_entry( + certificate, + request.user, + notes, ) - certificate.invalidate() + # Invalidate the certificate with explicit source for auditability + certificate.invalidate(source='instructor_api_v2') results['success'].append(learner) - except Exception as exc: # pylint: disable=broad-except + log.exception( + "Error invalidating certificate for user %s in course %s", + certificate.user_id, course_key + ) results['errors'].append({ 'learner': learner, 'message': str(exc) @@ -1892,6 +1991,8 @@ def post(self, request, course_id): def delete(self, request, course_id): """Re-validate certificate (remove invalidation).""" course_key = CourseKey.from_string(course_id) + # Validate that the course exists + get_course_by_id(course_key) username = request.data.get('username') if not username: @@ -1920,16 +2021,26 @@ def delete(self, request, course_id): status=status.HTTP_404_NOT_FOUND ) - # Remove invalidation - updated_count = CertificateInvalidation.objects.filter( - generated_certificate=certificate, - active=True - ).update(active=False) + # Remove invalidation and restore certificate generation + with transaction.atomic(): + updated_count = CertificateInvalidation.objects.filter( + generated_certificate=certificate, + active=True + ).update(active=False) - if updated_count == 0: - return Response( - {'message': _('No active invalidation found for this certificate')}, - status=status.HTTP_404_NOT_FOUND + if updated_count == 0: + return Response( + {'message': _('No active invalidation found for this certificate')}, + status=status.HTTP_404_NOT_FOUND + ) + + # Trigger certificate regeneration for this student + log.info( + "Re-validating certificate for student %s in course %s - triggering regeneration", + user.id, course_key + ) + task_api.generate_certificates_for_students( + request, course_key, student_set="specific_student", specific_student_id=user.id ) return Response( @@ -1937,10 +2048,10 @@ def delete(self, request, course_id): status=status.HTTP_200_OK ) - except Exception as exc: # pylint: disable=broad-except - log.error("Error removing certificate invalidation: %s", exc) + except Exception: # pylint: disable=broad-except + log.exception("Error removing certificate invalidation for course %s", course_id) return Response( - {'message': str(exc)}, + {'message': _('Unable to remove certificate invalidation')}, status=status.HTTP_400_BAD_REQUEST ) diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index 7ced0839b2d4..edc7d74b7cc4 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -636,7 +636,6 @@ class IssuedCertificateSerializer(serializers.Serializer): help_text="Date when certificate was invalidated in ISO 8601 format" ) invalidation_note = serializers.SerializerMethodField( - allow_null=True, help_text="Notes about the invalidation" ) @@ -684,7 +683,7 @@ def get_invalidation_note(self, obj): """Get invalidation notes from invalidation data in context.""" invalidation_dict = self.context.get('invalidation_dict', {}) invalidation_info = invalidation_dict.get(obj.user_id) - return invalidation_info['notes'] if invalidation_info else None + return invalidation_info.get('notes', '') if invalidation_info else '' class CertificateGenerationHistorySerializer(serializers.Serializer): @@ -712,6 +711,16 @@ def get_details(self, obj): return str(obj.get_certificate_generation_candidates()) +class ToggleCertificateGenerationSerializer(serializers.Serializer): + """ + Serializer for toggling certificate generation request. + """ + enabled = serializers.BooleanField( + required=True, + help_text="Whether to enable or disable certificate generation" + ) + + class RegenerateCertificatesSerializer(serializers.Serializer): """ Serializer for regenerating certificates request. From a3940088f59289b73c170d16a6705877f96b2509 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 11:43:43 -0400 Subject: [PATCH 07/20] fix: Removed the unused invalidated_user_ids --- lms/djangoapps/instructor/views/api_v2.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 589a359bb997..dc6deed7a6bf 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1925,14 +1925,6 @@ def post(self, request, course_id): ).select_related('user') user_id_to_certificate = {cert.user_id: cert for cert in certificates} - # Bulk fetch existing active invalidations - active_invalidations = CertificateInvalidation.objects.filter( - generated_certificate__course_id=course_key, - generated_certificate__user_id__in=user_ids, - active=True - ).values_list('generated_certificate__user_id', flat=True) - invalidated_user_ids = set(active_invalidations) - # Process each learner certificates_to_invalidate = [] for learner, user in learner_to_user.items(): From 0d8809ebf33096a7e8bb5d6e72cdf3013271419a Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 13:16:18 -0400 Subject: [PATCH 08/20] feat: update tests --- .../tests/test_certificates_api_v2.py | 429 ++++++++++++++++++ 1 file changed, 429 insertions(+) create mode 100644 lms/djangoapps/instructor/tests/test_certificates_api_v2.py diff --git a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py new file mode 100644 index 000000000000..7e67c58b0881 --- /dev/null +++ b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py @@ -0,0 +1,429 @@ +""" +Unit tests for instructor API v2 certificate management endpoints. +""" +from unittest.mock import patch + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APIClient + +from common.djangoapps.student.tests.factories import ( + CourseEnrollmentFactory, + InstructorFactory, + StaffFactory, + UserFactory, +) +from lms.djangoapps.certificates.models import CertificateAllowlist, CertificateInvalidation +from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory +from lms.djangoapps.certificates.data import CertificateStatuses +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +class ToggleCertificateGenerationViewTest(SharedModuleStoreTestCase): + """Tests for ToggleCertificateGenerationView.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.client = APIClient() + self.instructor = InstructorFactory.create(course_key=self.course.id) + self.student = UserFactory.create() + self.url = reverse( + 'instructor_api_v2:toggle_certificate_generation', + kwargs={'course_id': str(self.course.id)} + ) + + def test_permission_required(self): + """Test that only instructors can toggle certificate generation.""" + self.client.force_authenticate(user=self.student) + response = self.client.post(self.url, {'enabled': True}, format='json') + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_course_not_found(self): + """Test 404 when course doesn't exist.""" + self.client.force_authenticate(user=self.instructor) + url = reverse( + 'instructor_api_v2:toggle_certificate_generation', + kwargs={'course_id': 'course-v1:edX+Invalid+2024'} + ) + response = self.client.post(url, {'enabled': True}, format='json') + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_missing_enabled_field(self): + """Test validation error when enabled field is missing.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post(self.url, {}, format='json') + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert 'enabled' in response.data + + def test_invalid_enabled_field_type(self): + """Test validation error when enabled is not boolean.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post(self.url, {'enabled': 'true'}, format='json') + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @patch('lms.djangoapps.instructor.views.api_v2.certs_api.set_cert_generation_enabled') + def test_successful_toggle(self, mock_set_enabled): + """Test successful certificate generation toggle.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post(self.url, {'enabled': False}, format='json') + assert response.status_code == status.HTTP_200_OK + assert response.data == {'enabled': False} + mock_set_enabled.assert_called_once_with(self.course.id, False) + + +class CertificateExceptionsViewTest(SharedModuleStoreTestCase): + """Tests for CertificateExceptionsView.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.client = APIClient() + self.instructor = InstructorFactory.create(course_key=self.course.id) + self.student = UserFactory.create() + self.enrolled_student = UserFactory.create() + CourseEnrollmentFactory.create(user=self.enrolled_student, course_id=self.course.id) + self.url = reverse( + 'instructor_api_v2:certificate_exceptions', + kwargs={'course_id': str(self.course.id)} + ) + + def test_post_permission_required(self): + """Test that only instructors can grant exceptions.""" + self.client.force_authenticate(user=self.student) + response = self.client.post( + self.url, + {'learners': [self.enrolled_student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_post_course_not_found(self): + """Test 404 when course doesn't exist.""" + self.client.force_authenticate(user=self.instructor) + url = reverse( + 'instructor_api_v2:certificate_exceptions', + kwargs={'course_id': 'course-v1:edX+Invalid+2024'} + ) + response = self.client.post( + url, + {'learners': [self.enrolled_student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_post_no_learners(self): + """Test error when no learners provided.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post(self.url, {'learners': [], 'notes': 'Test'}, format='json') + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_post_user_not_found(self): + """Test error for non-existent user.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + {'learners': ['nonexistent'], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['errors']) == 1 + assert 'User not found' in response.data['errors'][0]['message'] + + def test_post_user_not_enrolled(self): + """Test error when user is not enrolled.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + {'learners': [self.student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['errors']) == 1 + assert 'not enrolled' in response.data['errors'][0]['message'] + + def test_post_user_has_invalidation(self): + """Test error when user has active invalidation.""" + cert = GeneratedCertificateFactory.create( + user=self.enrolled_student, + course_id=self.course.id, + status=CertificateStatuses.unavailable + ) + CertificateInvalidation.objects.create( + generated_certificate=cert, + invalidated_by=self.instructor, + notes='Test invalidation', + active=True + ) + + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + {'learners': [self.enrolled_student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['errors']) == 1 + assert 'invalidation' in response.data['errors'][0]['message'] + + @patch('lms.djangoapps.instructor.views.api_v2.certs_api.create_or_update_certificate_allowlist_entry') + def test_post_successful_single(self, mock_create): + """Test successful exception grant for single learner.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + {'learners': [self.enrolled_student.username], 'notes': 'Test exception'}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['success']) == 1 + assert self.enrolled_student.username in response.data['success'] + mock_create.assert_called_once() + + @patch('lms.djangoapps.instructor.views.api_v2.certs_api.create_or_update_certificate_allowlist_entry') + def test_post_successful_bulk(self, mock_create): + """Test successful bulk exception grant.""" + student2 = UserFactory.create() + CourseEnrollmentFactory.create(user=student2, course_id=self.course.id) + + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + { + 'learners': [self.enrolled_student.username, student2.username], + 'notes': 'Bulk test' + }, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['success']) == 2 + assert mock_create.call_count == 2 + + def test_delete_permission_required(self): + """Test that only instructors can remove exceptions.""" + self.client.force_authenticate(user=self.student) + response = self.client.delete( + self.url, + {'username': self.enrolled_student.username}, + format='json' + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_delete_no_username(self): + """Test error when username not provided.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.delete(self.url, {}, format='json') + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_delete_user_not_found(self): + """Test error when user doesn't exist.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.delete( + self.url, + {'username': 'nonexistent'}, + format='json' + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + @patch('lms.djangoapps.instructor.views.api_v2.certs_api.get_allowlist_entry') + def test_delete_no_exception(self, mock_get_entry): + """Test 404 when no exception exists.""" + mock_get_entry.return_value = None + self.client.force_authenticate(user=self.instructor) + response = self.client.delete( + self.url, + {'username': self.enrolled_student.username}, + format='json' + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @patch('lms.djangoapps.instructor.views.api_v2.certs_api.remove_allowlist_entry') + @patch('lms.djangoapps.instructor.views.api_v2.certs_api.get_allowlist_entry') + def test_delete_successful(self, mock_get_entry, mock_remove): + """Test successful exception removal.""" + mock_entry = CertificateAllowlist( + user=self.enrolled_student, + course_id=self.course.id, + allowlist=True, + notes='Test' + ) + mock_get_entry.return_value = mock_entry + + self.client.force_authenticate(user=self.instructor) + response = self.client.delete( + self.url, + {'username': self.enrolled_student.username}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + mock_remove.assert_called_once_with(self.enrolled_student, self.course.id) + + +class CertificateInvalidationsViewTest(SharedModuleStoreTestCase): + """Tests for CertificateInvalidationsView.""" + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.course = CourseFactory.create() + + def setUp(self): + super().setUp() + self.client = APIClient() + self.instructor = InstructorFactory.create(course_key=self.course.id) + self.student = UserFactory.create() + self.enrolled_student = UserFactory.create() + CourseEnrollmentFactory.create(user=self.enrolled_student, course_id=self.course.id) + self.url = reverse( + 'instructor_api_v2:certificate_invalidations', + kwargs={'course_id': str(self.course.id)} + ) + + def test_post_permission_required(self): + """Test that only instructors can invalidate certificates.""" + self.client.force_authenticate(user=self.student) + response = self.client.post( + self.url, + {'learners': [self.enrolled_student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_post_course_not_found(self): + """Test 404 when course doesn't exist.""" + self.client.force_authenticate(user=self.instructor) + url = reverse( + 'instructor_api_v2:certificate_invalidations', + kwargs={'course_id': 'course-v1:edX+Invalid+2024'} + ) + response = self.client.post( + url, + {'learners': [self.enrolled_student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + def test_post_no_certificate(self): + """Test error when certificate doesn't exist.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + {'learners': [self.enrolled_student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['errors']) == 1 + assert 'not found' in response.data['errors'][0]['message'] + + def test_post_certificate_already_invalid(self): + """Test error when certificate is already invalid.""" + GeneratedCertificateFactory.create( + user=self.enrolled_student, + course_id=self.course.id, + status=CertificateStatuses.unavailable + ) + + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + {'learners': [self.enrolled_student.username], 'notes': 'Test'}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['errors']) == 1 + assert 'invalid' in response.data['errors'][0]['message'] + + @patch('lms.djangoapps.instructor.views.api_v2.certs_api.create_certificate_invalidation_entry') + def test_post_successful(self, mock_create): + """Test successful certificate invalidation.""" + cert = GeneratedCertificateFactory.create( + user=self.enrolled_student, + course_id=self.course.id, + status=CertificateStatuses.downloadable + ) + + self.client.force_authenticate(user=self.instructor) + response = self.client.post( + self.url, + {'learners': [self.enrolled_student.username], 'notes': 'Test invalidation'}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + assert len(response.data['success']) == 1 + mock_create.assert_called_once() + + def test_delete_permission_required(self): + """Test that only instructors can re-validate certificates.""" + self.client.force_authenticate(user=self.student) + response = self.client.delete( + self.url, + {'username': self.enrolled_student.username}, + format='json' + ) + assert response.status_code == status.HTTP_403_FORBIDDEN + + def test_delete_no_certificate(self): + """Test 404 when certificate doesn't exist.""" + self.client.force_authenticate(user=self.instructor) + response = self.client.delete( + self.url, + {'username': self.enrolled_student.username}, + format='json' + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @patch('lms.djangoapps.instructor_task.api.generate_certificates_for_students') + def test_delete_no_invalidation(self, mock_generate): + """Test 404 when no active invalidation exists.""" + GeneratedCertificateFactory.create( + user=self.enrolled_student, + course_id=self.course.id, + status=CertificateStatuses.downloadable + ) + + self.client.force_authenticate(user=self.instructor) + response = self.client.delete( + self.url, + {'username': self.enrolled_student.username}, + format='json' + ) + assert response.status_code == status.HTTP_404_NOT_FOUND + mock_generate.assert_not_called() + + @patch('lms.djangoapps.instructor_task.api.generate_certificates_for_students') + def test_delete_successful_regeneration(self, mock_generate): + """Test successful re-validation triggers certificate regeneration.""" + cert = GeneratedCertificateFactory.create( + user=self.enrolled_student, + course_id=self.course.id, + status=CertificateStatuses.unavailable + ) + CertificateInvalidation.objects.create( + generated_certificate=cert, + invalidated_by=self.instructor, + notes='Test invalidation', + active=True + ) + + self.client.force_authenticate(user=self.instructor) + response = self.client.delete( + self.url, + {'username': self.enrolled_student.username}, + format='json' + ) + assert response.status_code == status.HTTP_200_OK + + # Verify certificate regeneration was triggered + mock_generate.assert_called_once() + call_args = mock_generate.call_args + assert call_args.kwargs['student_set'] == 'specific_student' + assert call_args.kwargs['specific_student_id'] == self.enrolled_student.id From 33364c059e268ac1eeb7a5d52c3cc0b7002e3c8c Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 13:28:56 -0400 Subject: [PATCH 09/20] feat: update tests --- .../instructor/tests/test_certificates_api_v2.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py index 7e67c58b0881..d053d7e88d21 100644 --- a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py @@ -6,18 +6,17 @@ from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import ( CourseEnrollmentFactory, InstructorFactory, - StaffFactory, UserFactory, ) +from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import CertificateAllowlist, CertificateInvalidation from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory -from lms.djangoapps.certificates.data import CertificateStatuses -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory class ToggleCertificateGenerationViewTest(SharedModuleStoreTestCase): @@ -345,7 +344,7 @@ def test_post_certificate_already_invalid(self): @patch('lms.djangoapps.instructor.views.api_v2.certs_api.create_certificate_invalidation_entry') def test_post_successful(self, mock_create): """Test successful certificate invalidation.""" - cert = GeneratedCertificateFactory.create( + GeneratedCertificateFactory.create( user=self.enrolled_student, course_id=self.course.id, status=CertificateStatuses.downloadable From 2d2c62ac4bcec1dad1b524bc476910e3ef8c283d Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 13:39:21 -0400 Subject: [PATCH 10/20] feat: update tests --- lms/djangoapps/instructor/tests/test_certificates_api_v2.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py index d053d7e88d21..ceaa07d4003e 100644 --- a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py @@ -6,8 +6,6 @@ from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import ( CourseEnrollmentFactory, @@ -17,6 +15,8 @@ from lms.djangoapps.certificates.data import CertificateStatuses from lms.djangoapps.certificates.models import CertificateAllowlist, CertificateInvalidation from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory class ToggleCertificateGenerationViewTest(SharedModuleStoreTestCase): From 6b9a2e91f127fc5f61660221bdd66d93587df2f3 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 14:21:55 -0400 Subject: [PATCH 11/20] feat: update tests --- lms/djangoapps/instructor/tests/test_certificates_api_v2.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py index ceaa07d4003e..25fc6b6f80ff 100644 --- a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py @@ -63,7 +63,7 @@ def test_missing_enabled_field(self): def test_invalid_enabled_field_type(self): """Test validation error when enabled is not boolean.""" self.client.force_authenticate(user=self.instructor) - response = self.client.post(self.url, {'enabled': 'true'}, format='json') + response = self.client.post(self.url, {'enabled': 'invalid'}, format='json') assert response.status_code == status.HTTP_400_BAD_REQUEST @patch('lms.djangoapps.instructor.views.api_v2.certs_api.set_cert_generation_enabled') @@ -136,7 +136,8 @@ def test_post_user_not_found(self): ) assert response.status_code == status.HTTP_200_OK assert len(response.data['errors']) == 1 - assert 'User not found' in response.data['errors'][0]['message'] + # The actual error message from get_user_by_username_or_email + assert 'does not exist' in response.data['errors'][0]['message'] def test_post_user_not_enrolled(self): """Test error when user is not enrolled.""" From 66e1243545278149c2847fae5a294d51cc0e171f Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 17:04:58 -0400 Subject: [PATCH 12/20] feat: PR feedback --- lms/djangoapps/instructor/views/api_v2.py | 408 ++++++++++-------- .../instructor/views/serializers_v2.py | 58 +++ 2 files changed, 288 insertions(+), 178 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index dc6deed7a6bf..47c4d9cbeeee 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -108,7 +108,9 @@ BetaTesterModifyRequestSerializerV2, BetaTesterModifyResponseSerializerV2, BlockDueDateSerializerV2, + CertificateExceptionSerializer, CertificateGenerationHistorySerializer, + CertificateInvalidationSerializer, CourseEnrollmentSerializerV2, CourseInformationSerializerV2, CourseTeamModifySerializer, @@ -122,6 +124,9 @@ ORASerializer, ORASummarySerializer, ProblemSerializer, + RemoveCertificateExceptionSerializer, + RemoveCertificateInvalidationSerializer, + ToggleCertificateGenerationSerializer, RegenerateCertificatesSerializer, TaskStatusSerializer, UnitExtensionSerializer, @@ -1611,8 +1616,6 @@ class ToggleCertificateGenerationView(DeveloperErrorViewMixin, APIView): def post(self, request, course_id): """Toggle certificate generation for a course.""" - from .serializers_v2 import ToggleCertificateGenerationSerializer - course_key = CourseKey.from_string(course_id) # Validate that the course exists before updating certificate settings get_course_by_id(course_key) @@ -1672,20 +1675,14 @@ def post(self, request, course_id): course_key = CourseKey.from_string(course_id) # Validate that the course exists get_course_by_id(course_key) - learners_input = request.data.get('learners', '') - notes = request.data.get('notes', '') - # Handle both string (comma-separated) and list inputs - if isinstance(learners_input, str): - learners = [l.strip() for l in learners_input.split(',') if l.strip()] - else: - learners = learners_input + # Validate request data + serializer = CertificateExceptionSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - if not learners: - return Response( - {'message': _('No learners provided')}, - status=status.HTTP_400_BAD_REQUEST - ) + learners = serializer.validated_data['learners'] + notes = serializer.validated_data['notes'] results = { 'success': [], @@ -1693,87 +1690,14 @@ def post(self, request, course_id): } # Resolve all usernames/emails to users upfront - learner_to_user = {} - for learner in learners: - try: - user = get_user_by_username_or_email(learner) - if user: - learner_to_user[learner] = user - else: - results['errors'].append({ - 'learner': learner, - 'message': _('User not found') - }) - except Exception as exc: # pylint: disable=broad-except - results['errors'].append({ - 'learner': learner, - 'message': str(exc) - }) - - if not learner_to_user: - return Response(results, status=status.HTTP_200_OK) - - users = list(learner_to_user.values()) - user_ids = [u.id for u in users] + learner_to_user, user_errors = _resolve_learners_to_users(learners) + results['errors'].extend(user_errors) - # Bulk fetch enrollments - enrollments = CourseEnrollment.objects.filter( - course_id=course_key, - user_id__in=user_ids - ).values_list('user_id', flat=True) - enrolled_user_ids = set(enrollments) - - # Bulk fetch existing allowlist entries - existing_allowlist = CertificateAllowlist.objects.filter( - course_id=course_key, - user_id__in=user_ids - ).values_list('user_id', flat=True) - allowlisted_user_ids = set(existing_allowlist) - - # Bulk fetch active invalidations - active_invalidations = CertificateInvalidation.objects.filter( - generated_certificate__course_id=course_key, - generated_certificate__user_id__in=user_ids, - active=True - ).values_list('generated_certificate__user_id', flat=True) - invalidated_user_ids = set(active_invalidations) - - # Process each learner - exceptions_to_create = [] - for learner, user in learner_to_user.items(): - try: - # Check if user is enrolled - if user.id not in enrolled_user_ids: - results['errors'].append({ - 'learner': learner, - 'message': _('User is not enrolled in this course') - }) - continue - - # Check if user already has an exception - if user.id in allowlisted_user_ids: - results['errors'].append({ - 'learner': learner, - 'message': _('User already has a certificate exception') - }) - continue - - # Check if user has an active invalidation - if user.id in invalidated_user_ids: - results['errors'].append({ - 'learner': learner, - 'message': _('User has an active certificate invalidation') - }) - continue - - # Add to list for processing - exceptions_to_create.append((learner, user)) - - except Exception as exc: # pylint: disable=broad-except - results['errors'].append({ - 'learner': learner, - 'message': str(exc) - }) + # Validate learners for certificate exceptions + exceptions_to_create, validation_errors = _validate_learners_for_certificate_exceptions( + learner_to_user, course_key + ) + results['errors'].extend(validation_errors) # Create all exceptions using the certificates API to ensure idempotency # and avoid race conditions with the unique_together constraint @@ -1798,13 +1722,13 @@ def delete(self, request, course_id): course_key = CourseKey.from_string(course_id) # Validate that the course exists get_course_by_id(course_key) - username = request.data.get('username') - if not username: - return Response( - {'message': _('No username provided')}, - status=status.HTTP_400_BAD_REQUEST - ) + # Validate request data + serializer = RemoveCertificateExceptionSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + username = serializer.validated_data['username'] try: user = get_user_by_username_or_email(username) @@ -1837,6 +1761,189 @@ def delete(self, request, course_id): ) +def _resolve_learners_to_users(learners): + """ + Resolve a list of learner identifiers (usernames or emails) to User objects. + + Args: + learners: List of learner identifiers (usernames or email addresses) + + Returns: + tuple: (learner_to_user, errors) where: + - learner_to_user: Dictionary mapping learner identifiers to User objects + - errors: List of error dictionaries with 'learner' and 'message' keys + """ + User = get_user_model() + learner_to_user = {} + errors = [] + + for learner in learners: + try: + user = get_user_by_username_or_email(learner) + if user: + learner_to_user[learner] = user + else: + errors.append({ + 'learner': learner, + 'message': _('User not found') + }) + except User.DoesNotExist: + errors.append({ + 'learner': learner, + 'message': _('User not found') + }) + + return learner_to_user, errors + + +def _validate_learners_for_certificate_exceptions(learner_to_user, course_key): + """ + Validate learners to ensure they can receive certificate exceptions. + + Args: + learner_to_user: Dictionary mapping learner identifiers to User objects + course_key: Course key for the course + + Returns: + tuple: (exceptions_to_create, errors) where: + - exceptions_to_create: List of (learner, user) tuples ready for exception creation + - errors: List of error dictionaries with 'learner' and 'message' keys + """ + errors = [] + exceptions_to_create = [] + + if not learner_to_user: + return exceptions_to_create, errors + + users = list(learner_to_user.values()) + user_ids = [u.id for u in users] + + # Bulk fetch active enrollments + enrollments = CourseEnrollment.objects.filter( + course_id=course_key, + user_id__in=user_ids, + is_active=True + ).values_list('user_id', flat=True) + enrolled_user_ids = set(enrollments) + + # Bulk fetch existing active allowlist entries + existing_allowlist = CertificateAllowlist.objects.filter( + course_id=course_key, + user_id__in=user_ids, + allowlist=True + ).values_list('user_id', flat=True) + allowlisted_user_ids = set(existing_allowlist) + + # Bulk fetch active invalidations + active_invalidations = CertificateInvalidation.objects.filter( + generated_certificate__course_id=course_key, + generated_certificate__user_id__in=user_ids, + active=True + ).values_list('generated_certificate__user_id', flat=True) + invalidated_user_ids = set(active_invalidations) + + # Validate each learner + for learner, user in learner_to_user.items(): + try: + # Check if user is enrolled + if user.id not in enrolled_user_ids: + errors.append({ + 'learner': learner, + 'message': _('User is not enrolled in this course') + }) + continue + + # Check if user already has an exception + if user.id in allowlisted_user_ids: + errors.append({ + 'learner': learner, + 'message': _('User already has a certificate exception') + }) + continue + + # Check if user has an active invalidation + if user.id in invalidated_user_ids: + errors.append({ + 'learner': learner, + 'message': _('User has an active certificate invalidation') + }) + continue + + # Learner is ready for exception creation + exceptions_to_create.append((learner, user)) + + except Exception as exc: # pylint: disable=broad-except + errors.append({ + 'learner': learner, + 'message': str(exc) + }) + + return exceptions_to_create, errors + + +def _validate_certificates_for_invalidation(learner_to_user, course_key): + """ + Validate certificates for a set of users to ensure they can be invalidated. + + Args: + learner_to_user: Dictionary mapping learner identifiers to User objects + course_key: Course key for the course + + Returns: + tuple: (certificates_to_invalidate, errors) where: + - certificates_to_invalidate: List of (learner, certificate) tuples ready for invalidation + - errors: List of error dictionaries with 'learner' and 'message' keys + """ + errors = [] + certificates_to_invalidate = [] + + if not learner_to_user: + return certificates_to_invalidate, errors + + users = list(learner_to_user.values()) + user_ids = [u.id for u in users] + + # Bulk fetch certificates (exclude deleted/deleting status) + certificates = GeneratedCertificate.objects.filter( + course_id=course_key, + user_id__in=user_ids + ).exclude( + status__in=[CertificateStatuses.deleted, CertificateStatuses.deleting] + ).select_related('user') + user_id_to_certificate = {cert.user_id: cert for cert in certificates} + + # Validate each learner's certificate + for learner, user in learner_to_user.items(): + try: + # Check if certificate exists + certificate = user_id_to_certificate.get(user.id) + if not certificate: + errors.append({ + 'learner': learner, + 'message': _('Certificate not found for this user') + }) + continue + + # Verify that the certificate is valid before invalidating + if not certificate.is_valid(): + errors.append({ + 'learner': learner, + 'message': _('Certificate is already invalid') + }) + continue + + # Certificate is ready for invalidation + certificates_to_invalidate.append((learner, certificate)) + + except Exception as exc: # pylint: disable=broad-except + errors.append({ + 'learner': learner, + 'message': str(exc) + }) + + return certificates_to_invalidate, errors + + class CertificateInvalidationsView(DeveloperErrorViewMixin, APIView): """ View to invalidate or re-validate certificates. @@ -1874,20 +1981,14 @@ def post(self, request, course_id): course_key = CourseKey.from_string(course_id) # Validate that the course exists get_course_by_id(course_key) - learners_input = request.data.get('learners', '') - notes = request.data.get('notes', '') - # Handle both string (comma-separated) and list inputs - if isinstance(learners_input, str): - learners = [l.strip() for l in learners_input.split(',') if l.strip()] - else: - learners = learners_input + # Validate request data + serializer = CertificateInvalidationSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - if not learners: - return Response( - {'message': _('No learners provided')}, - status=status.HTTP_400_BAD_REQUEST - ) + learners = serializer.validated_data['learners'] + notes = serializer.validated_data['notes'] results = { 'success': [], @@ -1895,65 +1996,14 @@ def post(self, request, course_id): } # Resolve all usernames/emails to users upfront - learner_to_user = {} - for learner in learners: - try: - user = get_user_by_username_or_email(learner) - if user: - learner_to_user[learner] = user - else: - results['errors'].append({ - 'learner': learner, - 'message': _('User not found') - }) - except Exception as exc: # pylint: disable=broad-except - results['errors'].append({ - 'learner': learner, - 'message': str(exc) - }) - - if not learner_to_user: - return Response(results, status=status.HTTP_200_OK) - - users = list(learner_to_user.values()) - user_ids = [u.id for u in users] - - # Bulk fetch certificates - certificates = GeneratedCertificate.objects.filter( - course_id=course_key, - user_id__in=user_ids - ).select_related('user') - user_id_to_certificate = {cert.user_id: cert for cert in certificates} + learner_to_user, user_errors = _resolve_learners_to_users(learners) + results['errors'].extend(user_errors) - # Process each learner - certificates_to_invalidate = [] - for learner, user in learner_to_user.items(): - try: - # Get the certificate - certificate = user_id_to_certificate.get(user.id) - if not certificate: - results['errors'].append({ - 'learner': learner, - 'message': _('Certificate not found for this user') - }) - continue - - # Verify that the certificate is valid before invalidating (matches v1 behavior) - if not certificate.is_valid(): - results['errors'].append({ - 'learner': learner, - 'message': _('Certificate is already invalid') - }) - continue - - # Add to list for processing - certificates_to_invalidate.append((learner, certificate)) - - except Exception as exc: # pylint: disable=broad-except - results['errors'].append({ - 'learner': learner, - 'message': str(exc) - }) + # Validate certificates for invalidation + certificates_to_invalidate, validation_errors = _validate_certificates_for_invalidation( + learner_to_user, course_key + ) + results['errors'].extend(validation_errors) # Invalidate certificates using the certificates API to ensure idempotency # and consistency with v1 behavior @@ -1985,13 +2035,13 @@ def delete(self, request, course_id): course_key = CourseKey.from_string(course_id) # Validate that the course exists get_course_by_id(course_key) - username = request.data.get('username') - if not username: - return Response( - {'message': _('No username provided')}, - status=status.HTTP_400_BAD_REQUEST - ) + # Validate request data + serializer = RemoveCertificateInvalidationSerializer(data=request.data) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + username = serializer.validated_data['username'] try: user = get_user_by_username_or_email(username) @@ -2001,9 +2051,11 @@ def delete(self, request, course_id): status=status.HTTP_400_BAD_REQUEST ) - # Get the certificate + # Get the certificate (exclude deleted/deleting status) try: - certificate = GeneratedCertificate.objects.get( + certificate = GeneratedCertificate.objects.exclude( + status__in=[CertificateStatuses.deleted, CertificateStatuses.deleting] + ).get( course_id=course_key, user=user ) diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index edc7d74b7cc4..97992c0f6ee3 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -721,6 +721,64 @@ class ToggleCertificateGenerationSerializer(serializers.Serializer): ) +class CertificateExceptionSerializer(serializers.Serializer): + """ + Serializer for granting certificate exceptions (bulk). + """ + learners = serializers.ListField( + child=serializers.CharField(max_length=255, allow_blank=False), + allow_empty=False, + help_text="List of usernames or email addresses of learners to grant exceptions" + ) + notes = serializers.CharField( + required=False, + allow_blank=True, + default='', + help_text="Notes about why the exception is being granted" + ) + + +class CertificateInvalidationSerializer(serializers.Serializer): + """ + Serializer for invalidating certificates (bulk). + """ + learners = serializers.ListField( + child=serializers.CharField(max_length=255, allow_blank=False), + allow_empty=False, + help_text="List of usernames or email addresses of learners to invalidate certificates" + ) + notes = serializers.CharField( + required=False, + allow_blank=True, + default='', + help_text="Notes about why the certificate is being invalidated" + ) + + +class RemoveCertificateExceptionSerializer(serializers.Serializer): + """ + Serializer for removing a certificate exception. + """ + username = serializers.CharField( + required=True, + max_length=255, + allow_blank=False, + help_text="Username or email address of the learner" + ) + + +class RemoveCertificateInvalidationSerializer(serializers.Serializer): + """ + Serializer for re-validating a certificate (removing invalidation). + """ + username = serializers.CharField( + required=True, + max_length=255, + allow_blank=False, + help_text="Username or email address of the learner" + ) + + class RegenerateCertificatesSerializer(serializers.Serializer): """ Serializer for regenerating certificates request. From 1b43fc58d23e42b34e5f21724ca00d9c97381996 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Wed, 22 Apr 2026 17:47:50 -0400 Subject: [PATCH 13/20] fix: tests --- lms/djangoapps/instructor/views/api_v2.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 47c4d9cbeeee..3e78293405a6 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -124,11 +124,11 @@ ORASerializer, ORASummarySerializer, ProblemSerializer, + RegenerateCertificatesSerializer, RemoveCertificateExceptionSerializer, RemoveCertificateInvalidationSerializer, - ToggleCertificateGenerationSerializer, - RegenerateCertificatesSerializer, TaskStatusSerializer, + ToggleCertificateGenerationSerializer, UnitExtensionSerializer, ) from .tools import find_unit, get_units_with_due_date, keep_field_private, set_due_date_extension, title_or_url @@ -1773,24 +1773,17 @@ def _resolve_learners_to_users(learners): - learner_to_user: Dictionary mapping learner identifiers to User objects - errors: List of error dictionaries with 'learner' and 'message' keys """ - User = get_user_model() learner_to_user = {} errors = [] for learner in learners: try: user = get_user_by_username_or_email(learner) - if user: - learner_to_user[learner] = user - else: - errors.append({ - 'learner': learner, - 'message': _('User not found') - }) - except User.DoesNotExist: + learner_to_user[learner] = user + except User.DoesNotExist as exc: errors.append({ 'learner': learner, - 'message': _('User not found') + 'message': str(exc) }) return learner_to_user, errors From d8ff883a4f3adb65a571f79faa48cad4d58f3c9c Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Thu, 23 Apr 2026 14:53:33 -0400 Subject: [PATCH 14/20] feat: PR feedback --- .../tests/test_certificates_api_v2.py | 4 +- lms/djangoapps/instructor/views/api_v2.py | 81 +++++++------------ .../instructor/views/serializers_v2.py | 43 ++++++++++ 3 files changed, 74 insertions(+), 54 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py index 25fc6b6f80ff..db915f5452a3 100644 --- a/lms/djangoapps/instructor/tests/test_certificates_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_certificates_api_v2.py @@ -66,14 +66,12 @@ def test_invalid_enabled_field_type(self): response = self.client.post(self.url, {'enabled': 'invalid'}, format='json') assert response.status_code == status.HTTP_400_BAD_REQUEST - @patch('lms.djangoapps.instructor.views.api_v2.certs_api.set_cert_generation_enabled') - def test_successful_toggle(self, mock_set_enabled): + def test_successful_toggle(self): """Test successful certificate generation toggle.""" self.client.force_authenticate(user=self.instructor) response = self.client.post(self.url, {'enabled': False}, format='json') assert response.status_code == status.HTTP_200_OK assert response.data == {'enabled': False} - mock_set_enabled.assert_called_once_with(self.course.id, False) class CertificateExceptionsViewTest(SharedModuleStoreTestCase): diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 3e78293405a6..b258ee90f1b3 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1634,7 +1634,7 @@ def post(self, request, course_id): log.exception("Error toggling certificate generation for course %s", course_id) return Response( {'message': _('Unable to update certificate generation settings')}, - status=status.HTTP_400_BAD_REQUEST + status=status.HTTP_500_INTERNAL_SERVER_ERROR ) @@ -1728,16 +1728,9 @@ def delete(self, request, course_id): if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - username = serializer.validated_data['username'] + user = serializer.validated_data['username'] try: - user = get_user_by_username_or_email(username) - if not user: - return Response( - {'message': _('User not found')}, - status=status.HTTP_400_BAD_REQUEST - ) - # Remove exception via certificates API so any existing certificate # is invalidated before the allowlist entry is removed if not certs_api.get_allowlist_entry(user, course_key): @@ -1757,7 +1750,7 @@ def delete(self, request, course_id): log.exception("Error removing certificate exception for course %s", course_id) return Response( {'message': _('Unable to remove certificate exception')}, - status=status.HTTP_400_BAD_REQUEST + status=status.HTTP_500_INTERNAL_SERVER_ERROR ) @@ -2034,16 +2027,9 @@ def delete(self, request, course_id): if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - username = serializer.validated_data['username'] + user = serializer.validated_data['username'] try: - user = get_user_by_username_or_email(username) - if not user: - return Response( - {'message': _('User not found')}, - status=status.HTTP_400_BAD_REQUEST - ) - # Get the certificate (exclude deleted/deleting status) try: certificate = GeneratedCertificate.objects.exclude( @@ -2076,20 +2062,27 @@ def delete(self, request, course_id): "Re-validating certificate for student %s in course %s - triggering regeneration", user.id, course_key ) - task_api.generate_certificates_for_students( - request, course_key, student_set="specific_student", specific_student_id=user.id - ) + try: + task_api.generate_certificates_for_students( + request, course_key, student_set="specific_student", specific_student_id=user.id + ) + except Exception as cert_gen_error: # pylint: disable=broad-except + # Log but don't fail - the invalidation was already removed + log.warning( + "Certificate regeneration failed for student %s in course %s: %s", + user.id, course_key, str(cert_gen_error) + ) return Response( {'message': _('Certificate invalidation removed successfully')}, status=status.HTTP_200_OK ) - except Exception: # pylint: disable=broad-except - log.exception("Error removing certificate invalidation for course %s", course_id) + except Exception as exc: # pylint: disable=broad-except + log.exception("Error removing certificate invalidation for course %s: %s", course_id, str(exc)) return Response( - {'message': _('Unable to remove certificate invalidation')}, - status=status.HTTP_400_BAD_REQUEST + {'message': _('Unable to remove certificate invalidation: {}').format(str(exc))}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR ) @@ -2218,19 +2211,12 @@ def get(self, request, course_id, email_or_username): status=status.HTTP_400_BAD_REQUEST ) - UserModel = get_user_model() - try: - student = get_user_by_username_or_email(email_or_username) - except UserModel.DoesNotExist: - return Response( - {'error': 'Learner not found'}, - status=status.HTTP_404_NOT_FOUND - ) - except UserModel.MultipleObjectsReturned: - return Response( - {'error': 'Multiple learners found for the given identifier'}, - status=status.HTTP_400_BAD_REQUEST - ) + # Validate learner identifier + serializer = LearnerSerializer(data={'email_or_username': email_or_username}) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + student = serializer.validated_data['email_or_username'] # Build progress URL (MFE or legacy depending on feature flag) if course_home_mfe_progress_tab_is_active(course_key): @@ -2366,19 +2352,12 @@ def get(self, request, course_id, location): learner_identifier = request.query_params.get('email_or_username') if learner_identifier: - UserModel = get_user_model() - try: - student = get_user_by_username_or_email(learner_identifier) - except UserModel.DoesNotExist: - return Response( - {'error': 'Learner not found'}, - status=status.HTTP_404_NOT_FOUND - ) - except UserModel.MultipleObjectsReturned: - return Response( - {'error': 'Multiple learners found for the given identifier'}, - status=status.HTTP_400_BAD_REQUEST - ) + # Validate learner identifier + serializer = LearnerSerializer(data={'email_or_username': learner_identifier}) + if not serializer.is_valid(): + return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) + + student = serializer.validated_data['email_or_username'] try: student_module = StudentModule.objects.get( diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index 97992c0f6ee3..bce3996f3f0c 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -9,6 +9,7 @@ from urllib.parse import urlparse from django.conf import settings +from django.contrib.auth import get_user_model from django.utils.html import escape from django.utils.translation import gettext as _ from edx_when.api import is_enabled_for_course @@ -16,6 +17,7 @@ from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.student.models import CourseEnrollment +from common.djangoapps.student.models.user import get_user_by_username_or_email from common.djangoapps.student.roles import ( CourseFinanceAdminRole, CourseInstructorRole, @@ -36,6 +38,7 @@ from .tools import DashboardError, get_student_from_identifier, parse_datetime +User = get_user_model() log = logging.getLogger(__name__) @@ -731,6 +734,7 @@ class CertificateExceptionSerializer(serializers.Serializer): help_text="List of usernames or email addresses of learners to grant exceptions" ) notes = serializers.CharField( + max_length=1000, required=False, allow_blank=True, default='', @@ -748,6 +752,7 @@ class CertificateInvalidationSerializer(serializers.Serializer): help_text="List of usernames or email addresses of learners to invalidate certificates" ) notes = serializers.CharField( + max_length=1000, required=False, allow_blank=True, default='', @@ -766,6 +771,14 @@ class RemoveCertificateExceptionSerializer(serializers.Serializer): help_text="Username or email address of the learner" ) + def validate_username(self, value): + """Validate and resolve username/email to user object.""" + try: + user = get_user_by_username_or_email(value) + return user + except User.DoesNotExist as exc: + raise serializers.ValidationError(str(exc)) + class RemoveCertificateInvalidationSerializer(serializers.Serializer): """ @@ -778,6 +791,14 @@ class RemoveCertificateInvalidationSerializer(serializers.Serializer): help_text="Username or email address of the learner" ) + def validate_username(self, value): + """Validate and resolve username/email to user object.""" + try: + user = get_user_by_username_or_email(value) + return user + except User.DoesNotExist as exc: + raise serializers.ValidationError(str(exc)) + class RegenerateCertificatesSerializer(serializers.Serializer): """ @@ -803,6 +824,28 @@ class RegenerateCertificatesSerializer(serializers.Serializer): ) +class LearnerSerializer(serializers.Serializer): + """ + Serializer for validating learner identifier (username or email). + """ + email_or_username = serializers.CharField( + required=True, + max_length=255, + allow_blank=False, + help_text="Username or email address of the learner" + ) + + def validate_email_or_username(self, value): + """Validate and resolve username/email to user object.""" + try: + user = get_user_by_username_or_email(value) + return user + except User.DoesNotExist as exc: + raise serializers.ValidationError(str(exc)) + except User.MultipleObjectsReturned: + raise serializers.ValidationError('Multiple learners found for the given identifier') + + class CourseEnrollmentSerializerV2(serializers.Serializer): """ Serializer for course enrollment data. From 684a245e49a5cfd6a69b58f385ff6bfd04997f09 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Thu, 23 Apr 2026 16:14:08 -0400 Subject: [PATCH 15/20] fix: build --- lms/djangoapps/instructor/views/api_v2.py | 5 +++-- lms/djangoapps/instructor/views/serializers_v2.py | 12 ++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index b258ee90f1b3..3559c0634fe4 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -120,6 +120,7 @@ GradingConfigSerializer, InstructorTaskListSerializer, IssuedCertificateSerializer, + LearnerInputSerializer, LearnerSerializer, ORASerializer, ORASummarySerializer, @@ -2212,7 +2213,7 @@ def get(self, request, course_id, email_or_username): ) # Validate learner identifier - serializer = LearnerSerializer(data={'email_or_username': email_or_username}) + serializer = LearnerInputSerializer(data={'email_or_username': email_or_username}) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) @@ -2353,7 +2354,7 @@ def get(self, request, course_id, location): learner_identifier = request.query_params.get('email_or_username') if learner_identifier: # Validate learner identifier - serializer = LearnerSerializer(data={'email_or_username': learner_identifier}) + serializer = LearnerInputSerializer(data={'email_or_username': learner_identifier}) if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index bce3996f3f0c..5841fffd1d6d 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -777,7 +777,7 @@ def validate_username(self, value): user = get_user_by_username_or_email(value) return user except User.DoesNotExist as exc: - raise serializers.ValidationError(str(exc)) + raise serializers.ValidationError(str(exc)) from exc class RemoveCertificateInvalidationSerializer(serializers.Serializer): @@ -797,7 +797,7 @@ def validate_username(self, value): user = get_user_by_username_or_email(value) return user except User.DoesNotExist as exc: - raise serializers.ValidationError(str(exc)) + raise serializers.ValidationError(str(exc)) from exc class RegenerateCertificatesSerializer(serializers.Serializer): @@ -824,7 +824,7 @@ class RegenerateCertificatesSerializer(serializers.Serializer): ) -class LearnerSerializer(serializers.Serializer): +class LearnerInputSerializer(serializers.Serializer): """ Serializer for validating learner identifier (username or email). """ @@ -841,9 +841,9 @@ def validate_email_or_username(self, value): user = get_user_by_username_or_email(value) return user except User.DoesNotExist as exc: - raise serializers.ValidationError(str(exc)) - except User.MultipleObjectsReturned: - raise serializers.ValidationError('Multiple learners found for the given identifier') + raise serializers.ValidationError(str(exc)) from exc + except User.MultipleObjectsReturned as exc: + raise serializers.ValidationError('Multiple learners found for the given identifier') from exc class CourseEnrollmentSerializerV2(serializers.Serializer): From 2902be94695f10701a730ff5dd1c47bd31013449 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Thu, 23 Apr 2026 16:56:56 -0400 Subject: [PATCH 16/20] fix: tests --- lms/djangoapps/instructor/tests/views/test_api_v2.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/tests/views/test_api_v2.py b/lms/djangoapps/instructor/tests/views/test_api_v2.py index 8b6fc465d7ea..7c32324cec35 100644 --- a/lms/djangoapps/instructor/tests/views/test_api_v2.py +++ b/lms/djangoapps/instructor/tests/views/test_api_v2.py @@ -211,14 +211,14 @@ def test_get_problem_with_learner_no_submission_returns_nulls(self): self.assertIsNone(data['attempts']) # noqa: PT009 def test_get_problem_with_unknown_learner_returns_404(self): - """Test that a 404 is returned when learner does not exist""" + """Test that a 400 is returned when learner does not exist""" url = reverse('instructor_api_v2:problem_detail', kwargs={ 'course_id': str(self.course.id), 'location': str(self.problem.location) }) response = self.client.get(url, {'email_or_username': 'nonexistent_user'}) - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) # noqa: PT009 + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) # noqa: PT009 def test_get_problem_requires_authentication(self): """Test that endpoint requires authentication""" From daf8ce0cae420afa121299f9c647b6cb025b2327 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Thu, 23 Apr 2026 19:20:29 -0400 Subject: [PATCH 17/20] feat: wrap create_certificate_invalidation_entry in atomic --- lms/djangoapps/instructor/views/api_v2.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 3559c0634fe4..6eefd12e5025 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1996,15 +1996,16 @@ def post(self, request, course_id): # and consistency with v1 behavior for learner, certificate in certificates_to_invalidate: try: - # Create invalidation entry (uses update_or_create for idempotency) - certs_api.create_certificate_invalidation_entry( - certificate, - request.user, - notes, - ) - # Invalidate the certificate with explicit source for auditability - certificate.invalidate(source='instructor_api_v2') - results['success'].append(learner) + with transaction.atomic(): + # Create invalidation entry (uses update_or_create for idempotency) + certs_api.create_certificate_invalidation_entry( + certificate, + request.user, + notes, + ) + # Invalidate the certificate with explicit source for auditability + certificate.invalidate(source='instructor_api_v2') + results['success'].append(learner) except Exception as exc: # pylint: disable=broad-except log.exception( "Error invalidating certificate for user %s in course %s", From 9e74963348ac225739547026b201c58adb27addf Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Fri, 24 Apr 2026 08:03:09 -0400 Subject: [PATCH 18/20] feat: add logging and max_length --- lms/djangoapps/instructor/views/api_v2.py | 18 ++++++++++++++++++ .../instructor/views/serializers_v2.py | 2 ++ 2 files changed, 20 insertions(+) diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index 6eefd12e5025..54a86e9a7e20 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1705,6 +1705,10 @@ def post(self, request, course_id): for learner, user in exceptions_to_create: try: certs_api.create_or_update_certificate_allowlist_entry(user, course_key, notes) + log.info( + "Certificate exception granted for user %s (%s) in course %s by %s", + user.id, learner, course_key, request.user.username + ) results['success'].append(learner) except Exception as exc: # pylint: disable=broad-except log.exception( @@ -2005,7 +2009,21 @@ def post(self, request, course_id): ) # Invalidate the certificate with explicit source for auditability certificate.invalidate(source='instructor_api_v2') + log.info( + "Certificate invalidated for user %s (%s) in course %s by %s", + certificate.user_id, learner, course_key, request.user.username + ) results['success'].append(learner) + except AlreadyRunningError: + log.warning( + "Certificate generation already running for user %s in course %s", + certificate.user_id, course_key + ) + results['errors'].append({ + 'learner': learner, + 'message': _('Cannot invalidate certificate while certificate generation is in progress. ' + 'Please wait for it to complete.') + }) except Exception as exc: # pylint: disable=broad-except log.exception( "Error invalidating certificate for user %s in course %s", diff --git a/lms/djangoapps/instructor/views/serializers_v2.py b/lms/djangoapps/instructor/views/serializers_v2.py index 5841fffd1d6d..b4b70e003d0c 100644 --- a/lms/djangoapps/instructor/views/serializers_v2.py +++ b/lms/djangoapps/instructor/views/serializers_v2.py @@ -731,6 +731,7 @@ class CertificateExceptionSerializer(serializers.Serializer): learners = serializers.ListField( child=serializers.CharField(max_length=255, allow_blank=False), allow_empty=False, + max_length=1000, help_text="List of usernames or email addresses of learners to grant exceptions" ) notes = serializers.CharField( @@ -749,6 +750,7 @@ class CertificateInvalidationSerializer(serializers.Serializer): learners = serializers.ListField( child=serializers.CharField(max_length=255, allow_blank=False), allow_empty=False, + max_length=1000, help_text="List of usernames or email addresses of learners to invalidate certificates" ) notes = serializers.CharField( From a317b19d50367fb7efb0636f1b1c49f26f18084f Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Fri, 24 Apr 2026 08:56:53 -0400 Subject: [PATCH 19/20] feat: show all exceptions granted records --- .../instructor/tests/test_api_v2.py | 51 +++++ lms/djangoapps/instructor/views/api_v2.py | 185 +++++++++++++++++- 2 files changed, 235 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/tests/test_api_v2.py b/lms/djangoapps/instructor/tests/test_api_v2.py index 3165e2e6efce..08fba17fdaaf 100644 --- a/lms/djangoapps/instructor/tests/test_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_api_v2.py @@ -2091,6 +2091,57 @@ def test_pagination(self): assert 'previous' in response.data assert 'results' in response.data + def test_granted_exceptions_without_certificates(self): + """ + Test that granted_exceptions filter shows allowlisted users + even if they don't have GeneratedCertificate records yet. + """ + # Add student1 to allowlist (has verified enrollment) + CertificateAllowlist.objects.create( + user=self.student1, + course_id=self.course_key, + allowlist=True, + notes='Medical emergency' + ) + + # Add student2 to allowlist (has audit enrollment, no certificate) + CertificateAllowlist.objects.create( + user=self.student2, + course_id=self.course_key, + allowlist=True, + notes='Special case' + ) + + # Create certificate only for student1 + GeneratedCertificateFactory.create( + user=self.student1, + course_id=self.course_key, + status=CertificateStatuses.downloadable + ) + + self.client.force_authenticate(user=self.instructor) + params = {'filter': 'granted_exceptions'} + response = self.client.get(self._get_url(), params) + + assert response.status_code == status.HTTP_200_OK + assert response.data['count'] == 2 # Both students should appear + + results = {r['username']: r for r in response.data['results']} + + # Verify student1 (has certificate) + assert 'student1' in results + assert results['student1']['enrollment_track'] == 'verified' + assert results['student1']['certificate_status'] == 'downloadable' + assert results['student1']['special_case'] == 'Exception' + assert results['student1']['exception_notes'] == 'Medical emergency' + + # Verify student2 (no certificate, but should appear with enrollment data) + assert 'student2' in results + assert results['student2']['enrollment_track'] == 'audit' + assert results['student2']['certificate_status'] == 'audit_notpassing' + assert results['student2']['special_case'] == 'Exception' + assert results['student2']['exception_notes'] == 'Special case' + @ddt.ddt class CertificateGenerationHistoryViewTest(SharedModuleStoreTestCase): diff --git a/lms/djangoapps/instructor/views/api_v2.py b/lms/djangoapps/instructor/views/api_v2.py index bd6761bcff5e..3f2255d996c7 100644 --- a/lms/djangoapps/instructor/views/api_v2.py +++ b/lms/djangoapps/instructor/views/api_v2.py @@ -1258,6 +1258,169 @@ class IssuedCertificatesView(ListAPIView): permission_name = permissions.VIEW_ISSUED_CERTIFICATES serializer_class = IssuedCertificateSerializer + def _create_certificate_dict_for_allowlisted_user(self, allowlist_entry, enrollment_dict): + """ + Create a dictionary representing certificate data for an allowlisted user + who may not have a GeneratedCertificate record yet. + """ + user = allowlist_entry.user + enrollment_mode = enrollment_dict.get(user.id, '') + + # Determine certificate status based on enrollment + if enrollment_mode == 'audit': + cert_status = 'audit_notpassing' + elif enrollment_mode == 'verified': + cert_status = 'downloadable' + else: + cert_status = 'notpassing' + + return { + 'username': user.username, + 'email': user.email, + 'enrollment_track': enrollment_mode, + 'certificate_status': cert_status, + 'special_case': 'Exception', + 'exception_granted': allowlist_entry.created.isoformat(), + 'exception_notes': allowlist_entry.notes or '', + 'invalidated_by': None, + 'invalidation_date': None, + 'invalidation_note': '', + } + + def _create_certificate_dict_for_invalidated_user(self, invalidation, enrollment_dict): + """ + Create a dictionary representing certificate data for an invalidated certificate + that may not have a GeneratedCertificate record. + """ + user = invalidation.generated_certificate.user if invalidation.generated_certificate else None + enrollment_mode = enrollment_dict.get(user.id, '') if user else '' + + return { + 'username': user.username if user else '', + 'email': user.email if user else '', + 'enrollment_track': enrollment_mode, + 'certificate_status': 'unavailable', + 'special_case': 'Invalidation', + 'exception_granted': None, + 'exception_notes': '', + 'invalidated_by': invalidation.invalidated_by.email if invalidation.invalidated_by else '', + 'invalidation_date': invalidation.created.isoformat(), + 'invalidation_note': invalidation.notes or '', + } + + def list(self, request, *args, **kwargs): + """ + Override list to handle granted_exceptions and invalidated filters specially. + + For these filters, we need to show ALL relevant users, + even those without GeneratedCertificate records yet. + """ + filter_type = request.query_params.get("filter", "all") + + if filter_type == "granted_exceptions": + course_id = self.kwargs["course_id"] + course_key = CourseKey.from_string(course_id) + search = request.query_params.get("search", "").strip() + + # Get enrollment data for context + enrollments = CourseEnrollment.objects.filter( + course_id=course_key + ).select_related('user') + enrollment_dict = {e.user_id: e.mode for e in enrollments} + + # Get all allowlist entries + allowlist_qs = CertificateAllowlist.objects.filter( + course_id=course_key, + allowlist=True + ).select_related('user') + + # Apply search filter + if search: + allowlist_qs = allowlist_qs.filter( + Q(user__username__icontains=search) | Q(user__email__icontains=search) + ) + + # Get existing certificates for allowlisted users + allowlist_user_ids = list(allowlist_qs.values_list('user_id', flat=True)) + existing_certs = GeneratedCertificate.objects.filter( + course_id=course_key, + user_id__in=allowlist_user_ids + ).select_related('user') + existing_cert_user_ids = set(existing_certs.values_list('user_id', flat=True)) + + # Build list of certificate data + certificate_data = [] + + # Add existing certificates + context = self.get_serializer_context() + for cert in existing_certs: + serializer = self.get_serializer(cert, context=context) + certificate_data.append(serializer.data) + + # Add synthetic certificates for allowlisted users without GeneratedCertificate + for entry in allowlist_qs: + if entry.user_id not in existing_cert_user_ids: + cert_dict = self._create_certificate_dict_for_allowlisted_user(entry, enrollment_dict) + certificate_data.append(cert_dict) + + # Sort by username + certificate_data.sort(key=lambda x: x['username']) + + # Paginate manually + paginator = self.pagination_class() + page = paginator.paginate_queryset(certificate_data, request) + + return paginator.get_paginated_response(page if page is not None else certificate_data) + + elif filter_type == "invalidated": + course_id = self.kwargs["course_id"] + course_key = CourseKey.from_string(course_id) + search = request.query_params.get("search", "").strip() + + # Get enrollment data for context + enrollments = CourseEnrollment.objects.filter( + course_id=course_key + ).select_related('user') + enrollment_dict = {e.user_id: e.mode for e in enrollments} + + # Get all invalidations + invalidations_qs = CertificateInvalidation.objects.filter( + generated_certificate__course_id=course_key, + active=True + ).select_related('generated_certificate__user', 'invalidated_by') + + # Apply search filter + if search: + invalidations_qs = invalidations_qs.filter( + Q(generated_certificate__user__username__icontains=search) | + Q(generated_certificate__user__email__icontains=search) + ) + + # Get existing certificates for invalidated users + invalidated_cert_ids = list(invalidations_qs.values_list('generated_certificate_id', flat=True)) + existing_certs = GeneratedCertificate.objects.filter( + id__in=invalidated_cert_ids + ).select_related('user') + + # Build list of certificate data using existing certificates + certificate_data = [] + context = self.get_serializer_context() + for cert in existing_certs: + serializer = self.get_serializer(cert, context=context) + certificate_data.append(serializer.data) + + # Sort by username + certificate_data.sort(key=lambda x: x['username']) + + # Paginate manually + paginator = self.pagination_class() + page = paginator.paginate_queryset(certificate_data, request) + + return paginator.get_paginated_response(page if page is not None else certificate_data) + + # For other filters, use default behavior + return super().list(request, *args, **kwargs) + def _apply_certificate_status_filter(self, certificates, filter_type, cert_statuses, course_key): """Apply status-based filters to certificate queryset.""" if filter_type == "received": @@ -1346,6 +1509,26 @@ def get_queryset(self): filter_type = self.request.query_params.get("filter", "all") search = self.request.query_params.get("search", "").strip() + # Special handling for granted_exceptions: show all allowlisted users + if filter_type == "granted_exceptions": + allowlist_user_ids = CertificateAllowlist.objects.filter( + course_id=course_key, allowlist=True + ).values_list('user_id', flat=True) + + # Get all certificates for allowlisted users + certificates = GeneratedCertificate.objects.filter( + course_id=course_key, + user_id__in=allowlist_user_ids + ).select_related('user', 'user__profile') + + # Apply search filter if provided + if search: + certificates = certificates.filter( + Q(user__username__icontains=search) | Q(user__email__icontains=search) + ) + + return certificates.order_by('user__username') + # Get certificates for the course if filter_type in ['audit_passing', 'audit_not_passing', 'all']: certificates = GeneratedCertificate.objects.filter( @@ -1368,7 +1551,7 @@ def get_queryset(self): course_key, filter_type ) - # Apply filter based on filter type (includes granted_exceptions and invalidated) + # Apply filter based on filter type (includes invalidated) certificates = self._apply_certificate_status_filter( certificates, filter_type, CertificateStatuses, course_key ) From 57360772c0c1123545413b41d091399d944affc3 Mon Sep 17 00:00:00 2001 From: Jesse Stewart Date: Fri, 24 Apr 2026 09:40:09 -0400 Subject: [PATCH 20/20] fix: tests --- lms/djangoapps/instructor/tests/test_api_v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/instructor/tests/test_api_v2.py b/lms/djangoapps/instructor/tests/test_api_v2.py index 08fba17fdaaf..66dd6710a720 100644 --- a/lms/djangoapps/instructor/tests/test_api_v2.py +++ b/lms/djangoapps/instructor/tests/test_api_v2.py @@ -36,7 +36,7 @@ UserFactory, ) from lms.djangoapps.certificates.data import CertificateStatuses -from lms.djangoapps.certificates.models import CertificateGenerationHistory +from lms.djangoapps.certificates.models import CertificateAllowlist, CertificateGenerationHistory from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.courseware.models import StudentModule from lms.djangoapps.instructor.access import ROLE_DISPLAY_NAMES