fix: handle missing CourseOverview during manual enrollment retirement #72
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical issue in the user retirement process where 133+ users were failing with 500 errors when ManualEnrollmentAudit records attempted to update historical data for courses that no longer exist in the CourseOverview table.
Key Changes:
- Added try-except block around historical record updates in
ManualEnrollmentAudit.retire_manual_enrollments() - Gracefully handles missing CourseOverview by logging a warning and continuing retirement
- Ensures main audit record updates still complete successfully
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chintanjoshi-apphelix-2u
left a comment
chintanjoshi-apphelix-2u
left a comment
There was a problem hiding this comment.
Can we also have another broad except apart from this specific except ?
Something like
try :
..........
except CourseOverview.DoesNotExist:
........
except Exception as e:
........
This makes sure that any other issues are also bypassed.
Unless we explicitly want to raise them as issues.
In that case please dis-regard this comment.
| f"CourseOverview missing for enrollment during retirement, skipping historical update " | ||
| f"for audit {manual_enrollment_audit.id}" | ||
| ) | ||
| manual_enrollment_audits.update(reason="", enrolled_email=retired_email) |
There was a problem hiding this comment.
Will this also require similar bullet proofing.
There was a problem hiding this comment.
No, this will not require bullet proofing. This is a bulk SQL update that directly updates student_manualenrollmentaudit, modifying only reason and enrolled_email.
It runs as a single UPDATE statement, does not trigger django-simple-history, and does not access related models like CourseOverview.
We want to handle only this specific case. Catching a broad exception could hide other real issues, so we’re avoiding that on purpose. |
Akanshu-2u
requested review from
chintanjoshi-apphelix-2u and
vgulati-apphelix
Description:
User retirement process is failing with 500 errors for 133+ users due to ManualEnrollmentAudit records attempting to access CourseOverview data during historical record updates. This occurs when courses are deleted from the overview table but their associated enrollment records remain, causing django-simple-history to trigger CourseOverview lookups that fail with DoesNotExist exceptions.
Solution:
Added exception handling around historical updates in
ManualEnrollmentAudit.retire_manual_enrollments()method. When CourseOverview is missing, the system now:Private JIRA Link:
BOMS-370