feat: add generate exception certificates modal#38547
feat: add generate exception certificates modal#38547wgu-jesse-stewart wants to merge 4 commits intoopenedx:masterfrom
Conversation
|
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. |
Description
Adds comprehensive test coverage for the certificate regeneration API's student set filtering contract between the API layer and task layer.
This PR introduces
RegenerateCertificatesViewTest, a new test class with three tests that verify the exact calling contract betweenRegenerateCertificatesView(API v2) andtask_api.generate_certificates_for_students. These tests prevent silent failures if either layer is renamed independently.User Impact: None. This is a test-only change with no functional modifications.
Context: During code review, a naming asymmetry was flagged between how different
student_setvalues are handled:'allowlisted'→ translated to'all_allowlisted'(legacy compatibility)'allowlisted_not_generated'→ passed through unchanged (modern native value)'all'→ omitted entirely (default behavior)While the existing code is correct, there were no tests pinning this contract. These tests serve as both documentation and regression prevention.
Changes
Added
RegenerateCertificatesViewTesttolms/djangoapps/instructor/tests/test_api_v2.pywith three test methods:test_allowlisted_not_generated_passes_correct_student_setVerifies that
student_set='allowlisted_not_generated'is passed to the task layer without translation.test_allowlisted_translates_to_all_allowlistedVerifies that
student_set='allowlisted'is translated to'all_allowlisted'for backward compatibility with the pre-allowlist "whitelist" naming era.test_all_students_omits_student_set_kwargVerifies that
student_set='all'results in calling the task layer with nostudent_setkwarg, preserving the default "generate for all enrolled students" behavior.Supporting information
Testing instructions
Run the new test class:
All three tests should pass and verify:
task_api.generate_certificates_for_studentsDeadline
None
Other information
Naming asymmetry explanation
The intentional asymmetry in student_set handling exists because:
'all_allowlisted'is a legacy value from the pre-allowlist "whitelist" naming era. The API layer accepts the modern term'allowlisted'and translates it for backward compatibility with the task layer.'allowlisted_not_generated'is a native value that the task layer was updated to accept directly without translation.This asymmetry is now documented and locked in by tests, preventing future drift between the API and task layers.
Dependencies
Code archaeology notes
For future maintainers: If you're renaming student_set values, these tests will fail loudly rather than allowing silent misbehavior. Update both the view logic AND the tests together, ensuring the contract remains explicit.