Claiming This Task
Before you start working, check the Assignees section. If no one is assigned, leave a comment claiming the issue and assign it to yourself. This prevents duplicate work.
See the Community Wiki for contributing guidelines and git workflow.
Problem
Two related issues with course creation and membership validation:
1. CourseMembership.clean() crashes with unsaved Course instances
When creating a course through the Django admin with an inline membership (e.g., adding yourself as teacher at the same time), the clean() method on CourseMembership raises:
ValueError: Model instances passed to related filters must be saved.
Root cause: CourseMembership.clean() (line 216 in courses/models.py) runs CourseMembership.objects.filter(user=self.user, course=self.course) — but when the course is being created alongside the membership via admin inlines, self.course hasn't been saved yet and has no PK, so Django can't use it in a queryset filter.
2. No enforcement that a course must have at least one teacher
- The API correctly auto-creates a teacher membership for the course creator (
views.py line 220-225), but this is only application-level logic — there's no model-level guarantee.
- The admin allows creating a course with zero members, leaving it in a broken state (no teacher).
- There's no validation preventing removal of the last teacher through the admin interface (the API has
is_last_teacher() checks, but the admin bypasses those).
Requirements
-
Fix the crash: Guard CourseMembership.clean() so it skips the duplicate-membership DB query when either self.user or self.course is unsaved (no PK). The UniqueConstraint on ["user", "course"] already prevents duplicates at the DB level, so the clean() check is a user-friendly validation layer — it's fine to skip it when the instances aren't persisted yet.
-
Enforce teacher requirement in admin: Override CourseAdmin.save_related() (or use a similar hook) to ensure that after all inlines are saved, the course has at least one teacher membership. If not, raise a ValidationError preventing the save.
-
All existing tests must continue to pass. The API flow (auto-creating teacher membership on course creation) should remain unchanged.
Architecture Context
courses/models.py — CourseMembership.clean() is the crash site. The Course.is_last_teacher() method already exists for last-teacher checks in the API views.
courses/admin.py — CourseAdmin uses CourseMembershipInline as a TabularInline. The save_related() hook runs after the course and all inlines have been saved, making it the right place to validate teacher existence.
courses/views.py — The API's post() method already handles auto-teacher-creation and should not be changed.
Files to be Altered
backend/EduLite/courses/models.py — Guard CourseMembership.clean() against unsaved instances
backend/EduLite/courses/admin.py — Add teacher-existence validation in CourseAdmin
backend/EduLite/courses/tests/ — Add tests for both fixes
Testing Requirements
- Test 1: Creating a
CourseMembership via admin inline on a new course does not crash
- Test 2: Creating a course via admin without a teacher membership shows a validation error
- Test 3: Removing the last teacher via admin is blocked
- Test 4: Existing API tests still pass (the auto-teacher flow is unchanged)
- Test 5:
CourseMembership.clean() still catches duplicate memberships when both instances are saved
Additional Context (Optional)
Claiming This Task
Before you start working, check the Assignees section. If no one is assigned, leave a comment claiming the issue and assign it to yourself. This prevents duplicate work.
See the Community Wiki for contributing guidelines and git workflow.
Problem
Two related issues with course creation and membership validation:
1.
CourseMembership.clean()crashes with unsaved Course instancesWhen creating a course through the Django admin with an inline membership (e.g., adding yourself as teacher at the same time), the
clean()method onCourseMembershipraises:Root cause:
CourseMembership.clean()(line 216 incourses/models.py) runsCourseMembership.objects.filter(user=self.user, course=self.course)— but when the course is being created alongside the membership via admin inlines,self.coursehasn't been saved yet and has no PK, so Django can't use it in a queryset filter.2. No enforcement that a course must have at least one teacher
views.pyline 220-225), but this is only application-level logic — there's no model-level guarantee.is_last_teacher()checks, but the admin bypasses those).Requirements
Fix the crash: Guard
CourseMembership.clean()so it skips the duplicate-membership DB query when eitherself.userorself.courseis unsaved (no PK). TheUniqueConstrainton["user", "course"]already prevents duplicates at the DB level, so theclean()check is a user-friendly validation layer — it's fine to skip it when the instances aren't persisted yet.Enforce teacher requirement in admin: Override
CourseAdmin.save_related()(or use a similar hook) to ensure that after all inlines are saved, the course has at least one teacher membership. If not, raise aValidationErrorpreventing the save.All existing tests must continue to pass. The API flow (auto-creating teacher membership on course creation) should remain unchanged.
Architecture Context
courses/models.py—CourseMembership.clean()is the crash site. TheCourse.is_last_teacher()method already exists for last-teacher checks in the API views.courses/admin.py—CourseAdminusesCourseMembershipInlineas aTabularInline. Thesave_related()hook runs after the course and all inlines have been saved, making it the right place to validate teacher existence.courses/views.py— The API'spost()method already handles auto-teacher-creation and should not be changed.Files to be Altered
backend/EduLite/courses/models.py— GuardCourseMembership.clean()against unsaved instancesbackend/EduLite/courses/admin.py— Add teacher-existence validation inCourseAdminbackend/EduLite/courses/tests/— Add tests for both fixesTesting Requirements
CourseMembershipvia admin inline on a new course does not crashCourseMembership.clean()still catches duplicate memberships when both instances are savedAdditional Context (Optional)
UniqueConstrainton["user", "course"]acts as a DB-level safety net for the duplicate check, so skipping theclean()query for unsaved instances is safe.