feat: add certificate management v2 API endpoints#38404
feat: add certificate management v2 API endpoints#38404wgu-jesse-stewart wants to merge 5 commits intoopenedx:masterfrom
Conversation
- 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
openedx-webhooks
added
open-source-contribution
|
Thanks for the pull request, @wgu-jesse-stewart! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. 🔘 Update the status of your PRYour PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
There was a problem hiding this comment.
Pull request overview
Adds new Instructor Dashboard v2 endpoints to manage course certificate generation, exceptions (allowlist), and invalidations, plus updates to certificate-related labels/serialization to support the React-based instructor dashboard UI.
Changes:
- Introduces v2 API endpoints to toggle certificate generation, manage certificate exceptions, and manage certificate invalidations.
- Extends v2 serializers to expose certificate-management availability and invalidation notes.
- Updates certificate status/history “human-readable” labels and adjusts related tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| lms/djangoapps/instructor/views/serializers_v2.py | Adds certificates_enabled and invalidation_note to v2 serializers. |
| lms/djangoapps/instructor/views/api_v2.py | Adds new v2 certificate management APIViews and extends issued-certs context with invalidation notes. |
| lms/djangoapps/instructor/views/api_urls.py | Wires new v2 certificate management routes. |
| lms/djangoapps/instructor/tests/test_api_v2.py | Updates expected certificate generation history label casing. |
| lms/djangoapps/certificates/tests/test_models.py | Updates expected label strings for generation history candidate rendering. |
| lms/djangoapps/certificates/models.py | Updates generation history candidate strings (“All Learners”, “Granted Exceptions”). |
| lms/djangoapps/certificates/data.py | Updates human-readable certificate status labels and adds mapping for unavailable. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| help_text="Date when certificate was invalidated in ISO 8601 format" | ||
| ) | ||
| invalidation_note = serializers.SerializerMethodField( | ||
| allow_null=True, |
There was a problem hiding this comment.
invalidation_note is declared with allow_null=True, but with the current context-building (inv.notes or '') this will almost always be an empty string rather than null. Either return None when notes are missing or drop allow_null=True if the field is always a string.
| allow_null=True, |
| # Check if already invalidated | ||
| if CertificateInvalidation.objects.filter( | ||
| generated_certificate=certificate, | ||
| active=True | ||
| ).exists(): |
There was a problem hiding this comment.
Bulk invalidation does multiple queries per learner (GeneratedCertificate.objects.get(...) + active invalidation .exists() + .create()), which can be costly for large batches. Consider fetching all target certificates and existing active invalidations in bulk and then processing in-memory.
|
|
||
| def post(self, request, course_id): | ||
| """Toggle certificate generation for a course.""" | ||
| course_key = CourseKey.from_string(course_id) |
There was a problem hiding this comment.
This endpoint doesn’t validate that the course actually exists (no get_course_by_id(course_key)), so a valid-looking course key could still create/update certificate-generation settings for a non-existent course. Recommend calling get_course_by_id(course_key) up front (consistent with other v2 views).
| course_key = CourseKey.from_string(course_id) | |
| course_key = CourseKey.from_string(course_id) | |
| # Validate that the course exists before updating certificate settings. | |
| get_course_by_id(course_key) |
|
|
||
| def post(self, request, course_id): | ||
| """Grant certificate exceptions (add to allowlist).""" | ||
| course_key = CourseKey.from_string(course_id) |
There was a problem hiding this comment.
This view does not validate that course_id corresponds to an existing course (no get_course_by_id(course_key) call). Because CertificateAllowlist.course_id is not a foreign key, this can create allowlist entries for nonexistent courses if an arbitrary course key is provided.
| course_key = CourseKey.from_string(course_id) | |
| course_key = CourseKey.from_string(course_id) | |
| try: | |
| course = get_course_by_id(course_key) | |
| except (ItemNotFoundError, NotFound): | |
| raise NotFound(_('Course not found')) | |
| if course is None: | |
| raise NotFound(_('Course not found')) |
| certificates_enabled = serializers.SerializerMethodField( | ||
| help_text="Whether certificate management features are enabled for this course" | ||
| ) |
There was a problem hiding this comment.
PR description mentions adding a certificatesEnabled flag, but the serializer exposes certificates_enabled (snake_case). If API consumers expect camelCase here, this will be a breaking mismatch—either rename the field to match the contract or update the PR description/frontend to use certificates_enabled.
| # 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 | ||
| ) | ||
|
|
There was a problem hiding this comment.
This delete path bypasses certs_api.remove_allowlist_entry(...), which (per certificates API) invalidates any existing certificate before removing the allowlist entry. Deleting the row directly may leave an already-generated certificate valid/downloadable even after the exception is removed. Recommend using certs_api.remove_allowlist_entry(user, course_key) (or duplicating its invalidation behavior) instead of raw .delete().
| # 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 | |
| ) | |
| # Remove exception via certificates API so any existing certificate | |
| # is invalidated before the allowlist entry is removed. | |
| allowlist_entry_exists = CertificateAllowlist.objects.filter( | |
| course_id=course_key, | |
| user=user | |
| ).exists() | |
| if not allowlist_entry_exists: | |
| return Response( | |
| {'message': _('No certificate exception found for this user')}, | |
| status=status.HTTP_404_NOT_FOUND | |
| ) | |
| certs_api.remove_allowlist_entry(user, course_key) |
| # Grant exception | ||
| CertificateAllowlist.objects.create( | ||
| user=user, | ||
| course_id=course_key, | ||
| allowlist=True, | ||
| notes=notes |
There was a problem hiding this comment.
Creating CertificateAllowlist entries directly after a separate .exists() check is race-prone given the model’s unique_together (course_id, user) constraint (concurrent requests can still raise IntegrityError). Prefer using certs_api.create_or_update_certificate_allowlist_entry(...) / update_or_create to make this idempotent and consistent with v1 behavior.
| 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() |
There was a problem hiding this comment.
This invalidation flow bypasses the certificates API helpers (e.g., certs_api.create_certificate_invalidation_entry and v1 checks like generated_certificate.is_valid()) and calls certificate.invalidate() without a source. Reusing the existing helper logic and passing an explicit source would keep behavior consistent and improve auditability.
| 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() | |
| if not certificate.is_valid(): | |
| results['errors'].append({ | |
| 'learner': learner, | |
| 'message': _('Certificate is already invalidated') | |
| }) | |
| continue | |
| # Invalidate certificate using the shared certificates API flow | |
| certs_api.create_certificate_invalidation_entry( | |
| generated_certificate=certificate, | |
| invalidated_by=request.user, | |
| notes=notes, | |
| source='instructor_api_v2', | |
| ) |
| class ToggleCertificateGenerationView(DeveloperErrorViewMixin, APIView): | ||
| """ | ||
| View to toggle certificate generation for a course. | ||
|
|
||
| **Example Requests** |
There was a problem hiding this comment.
New v2 certificate-management endpoints are introduced here, but the PR only updates an existing history-label assertion in tests. Please add API v2 tests covering permission enforcement, course-level 404s, request validation, and key side-effects (e.g., allowlist removal invalidates certs; re-validation regenerates/restores certs).
| 'invalidated_by': inv.invalidated_by.email, | ||
| 'created': inv.created.isoformat() | ||
| 'created': inv.created.isoformat(), | ||
| 'notes': inv.notes or '' | ||
| } |
There was a problem hiding this comment.
invalidation_dict coerces missing notes to an empty string (inv.notes or ''). If the API intends to represent “no notes” as null (e.g., serializer fields use allow_null=True), consider preserving None here instead of "", or align serializer definitions accordingly.
Description
This PR adds comprehensive v2 API endpoints for certificate management in the instructor dashboard, supporting the new React-based instructor dashboard UI.
Changes include:
New API Endpoints:
POST /api/instructor/v2/courses/{course_id}/certificates/toggle_generation- Enable/disable certificate generation for a coursePOST /api/instructor/v2/courses/{course_id}/certificates/exceptions- Grant certificate exceptions (allowlist) to learnersDELETE /api/instructor/v2/courses/{course_id}/certificates/exceptions- Remove certificate exceptionsPOST /api/instructor/v2/courses/{course_id}/certificates/invalidations- Invalidate certificates for learnersDELETE /api/instructor/v2/courses/{course_id}/certificates/invalidations- Re-validate (remove invalidation) certificatesData Model Updates:
Serializer Enhancements:
invalidation_notefield toIssuedCertificateSerializerto include notes about certificate invalidationscertificatesEnabledflag toCourseInformationSerializerV2to indicate if certificate management is availableKey Features:
User Roles Impacted:
Supporting information
Related to the instructor dashboard v2 modernization effort.
Testing instructions
Toggle Certificate Generation:
curl -X POST /api/instructor/v2/courses/{course_id}/certificates/toggle_generation \ -H "Authorization: Bearer {token}" \ -d '{"enabled": false}'Expected: Returns
{"enabled": false}Grant Certificate Exceptions (bulk):
curl -X POST /api/instructor/v2/courses/{course_id}/certificates/exceptions \ -H "Authorization: Bearer {token}" \ -d '{"learners": ["user1", "user2@example.com"], "notes": "Test exception"}'Expected: Returns success/error lists for each learner
Remove Certificate Exception:
curl -X DELETE /api/instructor/v2/courses/{course_id}/certificates/exceptions \ -H "Authorization: Bearer {token}" \ -d '{"username": "user1"}'Expected: Returns success message
Invalidate Certificates (bulk):
curl -X POST /api/instructor/v2/courses/{course_id}/certificates/invalidations \ -H "Authorization: Bearer {token}" \ -d '{"learners": ["user1", "user2@example.com"], "notes": "Reason for invalidation"}'Expected: Returns success/error lists for each learner
Remove Certificate Invalidation:
curl -X DELETE /api/instructor/v2/courses/{course_id}/certificates/invalidations \ -H "Authorization: Bearer {token}" \ -d '{"username": "user1"}'Expected: Returns success message
Deadline
None
Other information