Release 1.144.5#3454
Merged
Merged
Conversation
…d program enrollment API (#3451)
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
OpenAPI ChangesShow/hide ## Changes for v0.yaml:Unexpected changes? Ensure your branch is up-to-date with |
Comment on lines
+1101
to
+1111
| ).exists() | ||
| ): | ||
| log.warning( | ||
| "Skipping program enrollment generation for %s in %s: no verified enrollment", | ||
| user, | ||
| program, | ||
| ) | ||
| return ( | ||
| None, | ||
| False, | ||
| ) |
There was a problem hiding this comment.
Bug: The new check for a verified ProgramEnrollment prevents returning existing certificates for users whose enrollments were auto-created with the default 'audit' mode by the old system.
Severity: HIGH
Suggested Fix
The logic should be reordered. First, check if a valid ProgramCertificate already exists for the user. If it does, return it. Only if no certificate exists should the code proceed to check for a verified ProgramEnrollment as a condition for creating a new certificate. This ensures backward compatibility for users with existing certificates.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: courses/api.py#L1097-L1111
Potential issue: The function `generate_program_certificate` now checks for a
`ProgramEnrollment` with `enrollment_mode=EDX_ENROLLMENT_VERIFIED_MODE` before checking
for an existing `ProgramCertificate`. Previously, `ProgramEnrollment` records were
created without specifying a mode, defaulting to 'audit'. Consequently, users with
existing certificates and these default audit enrollments will now fail the new check,
causing the function to incorrectly return `(None, False)` instead of their valid,
existing certificate. This is a regression that affects users who earned certificates
under the old logic.
Did we get this right? 👍 / 👎 to inform future reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
James Kachel
Tobias Macey
pre-commit-ci[bot]
annagav
Muhammad Anas