Fix CourseMembership.clean() crash on unsaved course & enforce teacher requirement#284
Merged
smattymatty merged 1 commit intomainfrom Feb 13, 2026
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR Template CheckAll checks passed. |
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.
Fix CourseMembership.clean() crash on unsaved course & enforce teacher requirement
Description
Fixes two related issues with course creation and membership validation in the Django admin.
Closes #260
Mandatory Checklist
mainWhat Changed
1. Guard
CourseMembership.clean()against unsaved instances (courses/models.py)The duplicate-membership check ran
CourseMembership.objects.filter(user=self.user, course=self.course)unconditionally. When creating a course with an inline membership in the admin,self.coursehas no PK yet, causing Django to raiseValueError: Model instances passed to related filters must be saved.Fixed by wrapping the query in
if self.user_id and self.course_id:— skips the DB query when either FK is unsaved. TheUniqueConstrainton["user", "course"]still prevents duplicates at the DB level.2. Enforce teacher requirement in admin (
courses/admin.py)Added
CourseAdmin.save_related()override that checks after all inlines are saved that the course has at least oneCourseMembershipwithrole="teacher"andstatus="enrolled". RaisesValidationErrorif not, preventing:The API flow (auto-creating teacher membership on course creation) is unchanged.
3. Tests
courses/tests/test_CourseMembership.py— 3 new tests: unsaved course doesn't crash, unsaved user doesn't crash, duplicates still caught when both are savedcourses/tests/test_admin.py— new file, 5 tests: no teacher blocked, with teacher succeeds, only students blocked, invited teacher doesn't count, removing last teacher blockedHow to Test
All 1020 backend tests pass (157 in courses app, 8 new).
To manually verify: