diff --git a/django_email_learning/migrations/0021_courseinstructor.py b/django_email_learning/migrations/0021_courseinstructor.py new file mode 100644 index 0000000..e736b14 --- /dev/null +++ b/django_email_learning/migrations/0021_courseinstructor.py @@ -0,0 +1,45 @@ +# Generated by Django 6.0.4 on 2026-04-30 09:10 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("django_email_learning", "0020_assignment_reminder_interval_days"), + ] + + operations = [ + migrations.CreateModel( + name="CourseInstructor", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "course", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="instructors", + to="django_email_learning.course", + ), + ), + ( + "org_user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to="django_email_learning.organizationuser", + ), + ), + ], + options={ + "unique_together": {("course", "org_user")}, + }, + ), + ] diff --git a/django_email_learning/models.py b/django_email_learning/models.py index 1718278..3d8633d 100644 --- a/django_email_learning/models.py +++ b/django_email_learning/models.py @@ -133,6 +133,13 @@ class OrganizationUser(models.Model): def __str__(self) -> str: return f"{self.user.username} - {self.organization.name}" + def can_act_as_instructor(self) -> bool: + # TODO: When we add display name for org user we can also accept admin role as instructor + # if they have display name set, for now only users with instructor role can be course instructors + if self.role == "instructor": + return True + return False + class Meta: unique_together = [["user", "organization"]] @@ -305,6 +312,28 @@ def replace_image(self, file_path: str) -> str: raise ValueError("Image file does not exist.") +class CourseInstructor(models.Model): + course = models.ForeignKey( + Course, on_delete=models.CASCADE, related_name="instructors" + ) + org_user = models.ForeignKey(OrganizationUser, on_delete=models.CASCADE) + + def __str__(self) -> str: + return f"{self.course.title} - {self.org_user.user.email}" + + def save(self, *args, **kwargs) -> None: # type: ignore[no-untyped-def] + if self.org_user.organization != self.course.organization: + raise ValidationError( + "Instructor must belong to the same organization as the course." + ) + if not self.org_user.can_act_as_instructor(): + raise ValidationError("Organization user doesn't have instructor role.") + super().save(*args, **kwargs) + + class Meta: + unique_together = [["course", "org_user"]] + + class ExternalReference(models.Model): course = models.ForeignKey( Course, on_delete=models.CASCADE, related_name="external_references" diff --git a/django_email_learning/platform/api/serializers.py b/django_email_learning/platform/api/serializers.py index 0a74dcd..d29480c 100644 --- a/django_email_learning/platform/api/serializers.py +++ b/django_email_learning/platform/api/serializers.py @@ -13,6 +13,7 @@ from django_email_learning.models import ( ApiKey, ContentDelivery, + CourseInstructor, DeliveryStatus, Organization, ImapConnection, @@ -118,6 +119,11 @@ class CreateCourseRequest(BaseModel): ], ) is_public: bool = Field(default=True, examples=[True]) + instructors: Optional[list[int]] = Field( + None, + examples=[[1, 2, 3]], + description="List of organization user IDs to be assigned as instructors for this course.", + ) def to_django_model(self, organization_id: int) -> Course: organization = Organization.objects.get(id=organization_id) @@ -146,6 +152,22 @@ def to_django_model(self, organization_id: int) -> Course: ) if imap_connection: course.imap_connection = imap_connection + if self.instructors: + course.save() # Save course before adding instructors + for instructor_id in self.instructors: + try: + org_user = OrganizationUser.objects.get( + id=instructor_id, organization=organization + ) + except OrganizationUser.DoesNotExist: + raise ValueError( + f"OrganizationUser with id {instructor_id} does not exist in organization {organization.name}." + ) + if not org_user.can_act_as_instructor(): + raise ValueError( + f"OrganizationUser with id {instructor_id} does not have instructor role." + ) + CourseInstructor.objects.create(course=course, org_user=org_user) if self.image: course.replace_image(self.image) if self.target_audience: @@ -189,6 +211,7 @@ class UpdateCourseRequest(BaseModel): ], ) is_public: Optional[bool] = Field(None, examples=[True]) + instructors: Optional[list[int]] = Field(None, examples=[1, 2, 3]) def to_django_model(self, course_id: int) -> Course: try: @@ -227,9 +250,39 @@ def to_django_model(self, course_id: int) -> Course: course.external_references.create(name=ref["name"], url=ref["url"]) if self.is_public is not None: course.is_public = self.is_public + if self.instructors is not None: + instructors_to_remove = course.instructors.exclude( + org_user_id__in=self.instructors + ) + for instructor in instructors_to_remove: + instructor.delete() + instructors_to_add = set(self.instructors) - set( + course.instructors.values_list("org_user_id", flat=True) + ) + for instructor_id in instructors_to_add: + try: + org_user = OrganizationUser.objects.get( + id=instructor_id, organization=course.organization + ) + except OrganizationUser.DoesNotExist: + raise ValueError( + f"OrganizationUser with id {instructor_id} does not exist in organization {course.organization.name}." + ) + if not org_user.can_act_as_instructor(): + raise ValueError( + f"OrganizationUser with id {instructor_id} does not have instructor role." + ) + CourseInstructor.objects.create(course=course, org_user=org_user) return course +class InstructorResponse(BaseModel): + id: int + email: str + + model_config = ConfigDict(from_attributes=True) + + class CourseResponse(BaseModel): id: int title: str @@ -246,6 +299,7 @@ class CourseResponse(BaseModel): target_audience: Optional[str] = None external_references: Optional[list[dict[str, str]]] = None is_public: bool + instructors: Optional[list[InstructorResponse]] = None model_config = ConfigDict(from_attributes=True) @@ -278,6 +332,12 @@ def from_django_model( if course.external_references.exists() else None, "is_public": course.is_public, + "instructors": [ + InstructorResponse( + id=instructor.org_user.id, email=instructor.org_user.user.email + ) + for instructor in course.instructors.all() + ], } ) @@ -450,6 +510,7 @@ class OrganizationUserResponse(BaseModel): organization_id: int email: str role: UserRole + can_act_as_instructor: bool @staticmethod def from_django_model(org_user: OrganizationUser) -> "OrganizationUserResponse": @@ -459,6 +520,7 @@ def from_django_model(org_user: OrganizationUser) -> "OrganizationUserResponse": organization_id=org_user.organization.id, email=org_user.user.email, role=UserRole(org_user.role), + can_act_as_instructor=org_user.can_act_as_instructor(), ) diff --git a/django_email_learning/platform/views.py b/django_email_learning/platform/views.py index 79a2d80..fc92101 100644 --- a/django_email_learning/platform/views.py +++ b/django_email_learning/platform/views.py @@ -241,6 +241,15 @@ def get_locale_messages(self) -> Dict[str, str]: "add_folder_helper_text": _( "Add folders to fetch emails from. The 'inbox' folder is required and will always be included." ), # noqa: E501 + "add_instructors": _("Add Instructors"), + "instructors_tooltip": _( + "Assign instructors from your organization to this course. Instructors can review and approve learner assignment submissions." + ), + "select_instructors": _("Select Instructors"), + "new_instructor": _("New Instructor"), + "instructor_email": _("Instructor Email"), + "add_instructor": _("Add Instructor"), + "instructor_add_failed": _("Failed to add instructor. Please try again."), } diff --git a/frontend/platform/courses/Courses.jsx b/frontend/platform/courses/Courses.jsx index ca4c5fc..f13b122 100644 --- a/frontend/platform/courses/Courses.jsx +++ b/frontend/platform/courses/Courses.jsx @@ -191,7 +191,7 @@ function Courses() { - setDialogOpen(false)} fullWidth maxWidth="sm"> + setDialogOpen(false)} fullWidth maxWidth="md"> {dialogContent} diff --git a/frontend/platform/courses/components/AddInstructorsSection.jsx b/frontend/platform/courses/components/AddInstructorsSection.jsx new file mode 100644 index 0000000..f54c3f8 --- /dev/null +++ b/frontend/platform/courses/components/AddInstructorsSection.jsx @@ -0,0 +1,138 @@ +import { useState, useEffect, useMemo } from 'react'; +import { + Accordion, + AccordionDetails, + AccordionSummary, + Box, + Chip, + FormControl, + InputLabel, + MenuItem, + OutlinedInput, + Select, + Typography, +} from '@mui/material'; +import { useAppContext } from '../../../src/render.jsx'; +import ExpandMoreIcon from '@mui/icons-material/ExpandMore'; +import PlusIcon from '@mui/icons-material/Add'; +import CreateInstructorForm from './CreateInstructorForm'; + + +function AddInstructorsSection({ onChangeCallback, activeOrganizationId, initialInstructorIds = [] }) { + const [orgInstructors, setOrgInstructors] = useState([]); + const [selectedIds, setSelectedIds] = useState(initialInstructorIds); + const [expanded, setExpanded] = useState(false); + const { localeMessages, apiBaseUrl } = useAppContext(); + + const hasInstructors = useMemo(() => orgInstructors.length > 0, [orgInstructors]); + + const switchExpanded = () => { + if (hasInstructors) { + setExpanded(!expanded); + } + }; + + useEffect(() => { + fetch(`${apiBaseUrl}/organizations/${activeOrganizationId}/users/`, { + method: 'GET', + credentials: 'include', + headers: { 'Content-Type': 'application/json' }, + }) + .then((response) => response.json()) + .then((data) => { + const instructors = (data.organization_users || []).filter( + (u) => u.can_act_as_instructor + ); + setOrgInstructors(instructors); + if (instructors.length === 0) { + setExpanded(true); + } + }) + .catch((error) => { + console.error('Error fetching organization users:', error); + }); + }, []); + + const handleSelectionChange = (event) => { + const value = event.target.value; + setSelectedIds(value); + if (onChangeCallback) { + onChangeCallback(value); + } + }; + + return ( +
+ {hasInstructors && ( + + + {localeMessages['select_instructors']} + + + + )} + + : null} + aria-controls="new-instructor-content" + id="new-instructor-header" + > + + + {localeMessages['new_instructor']} + + + + { + const updatedInstructors = [...orgInstructors, newOrgUser]; + setOrgInstructors(updatedInstructors); + const updatedIds = [...selectedIds, newOrgUser.id]; + setSelectedIds(updatedIds); + if (onChangeCallback) { + onChangeCallback(updatedIds); + } + setExpanded(false); + }} + /> + + +
+ ); +} + +export default AddInstructorsSection; diff --git a/frontend/platform/courses/components/CourseForm.jsx b/frontend/platform/courses/components/CourseForm.jsx index 62caa49..b9811d4 100644 --- a/frontend/platform/courses/components/CourseForm.jsx +++ b/frontend/platform/courses/components/CourseForm.jsx @@ -2,6 +2,7 @@ import { Alert, Box, Button, Divider, IconButton, MenuItem, Stack, TextField, To import InfoOutlinedIcon from '@mui/icons-material/InfoOutlined'; import RequiredTextField from '../../../src/components/RequiredTextField.jsx'; import AddImapConnectionForm from '../components/AddImapConnectionForm.jsx'; +import AddInstructorsSection from '../components/AddInstructorsSection.jsx'; import { useAppContext } from '../../../src/render.jsx'; import ImageUpload from '../../../src/components/ImageUpload.jsx'; import { useEffect, useState } from 'react'; @@ -39,6 +40,8 @@ function CourseForm({successCallback, failureCallback, cancelCallback, activeOrg const [isPublic, setIsPublic] = useState(createMode) const [addImapConnection, setAddImapConnection] = useState(false) const [imapConnectionId, setImapConnectionId] = useState(null) + const [addInstructors, setAddInstructors] = useState(false) + const [selectedInstructorIds, setSelectedInstructorIds] = useState([]) const [titleHelperText, setTitleHelperText] = useState("") const [slugHelperText, setSlugHelperText] = useState("") const [descriptionHelperText, setDescriptionHelperText] = useState("") @@ -57,6 +60,7 @@ function CourseForm({successCallback, failureCallback, cancelCallback, activeOrg isPublic: true, imapConnectionId: null, imageServerPath: null, + instructors: [], }) const switchImapConnection = () => { @@ -106,6 +110,7 @@ function CourseForm({successCallback, failureCallback, cancelCallback, activeOrg isPublic: data.is_public ?? true, imapConnectionId: data.imap_connection_id ?? null, imageServerPath: data.image_path ?? null, + instructors: (data.instructors || []).map((i) => i.id), }); const initialExternalReferences = (data.external_references || []).map((reference) => ({ name: reference.name || '', @@ -117,6 +122,11 @@ function CourseForm({successCallback, failureCallback, cancelCallback, activeOrg setImapConnectionId(data.imap_connection_id); setAddImapConnection(true); } + const initialInstructors = (data.instructors || []).map((i) => i.id); + setSelectedInstructorIds(initialInstructors); + if (initialInstructors.length > 0) { + setAddInstructors(true); + } }) .catch((error) => { console.error('Error:', error); @@ -262,6 +272,13 @@ function CourseForm({successCallback, failureCallback, cancelCallback, activeOrg updatePayload.external_references = normalizedExternalReferences; } + const currentInstructors = addInstructors ? selectedInstructorIds : []; + const sortedCurrent = [...currentInstructors].sort((a, b) => a - b); + const sortedInitial = [...(initialValues.instructors || [])].sort((a, b) => a - b); + if (JSON.stringify(sortedCurrent) !== JSON.stringify(sortedInitial)) { + updatePayload.instructors = currentInstructors; + } + fetch(apiBaseUrl + '/organizations/' + activeOrganizationId + '/courses/' + courseId + '/', { method: 'POST', headers: { @@ -327,7 +344,8 @@ function CourseForm({successCallback, failureCallback, cancelCallback, activeOrg is_public: isPublic, imap_connection_id: imapConnectionId ? parseInt(imapConnectionId) : null, external_references: normalizedExternalReferences.length > 0 ? normalizedExternalReferences : null, - image: imageServerPath ? imageServerPath : null + image: imageServerPath ? imageServerPath : null, + instructors: addInstructors && selectedInstructorIds.length > 0 ? selectedInstructorIds : null, }), }) .then(response => { @@ -494,6 +512,26 @@ function CourseForm({successCallback, failureCallback, cancelCallback, activeOrg initialImapConnectionId={imapConnectionId} /> } + + + setAddInstructors(!addInstructors)} checked={addInstructors} dir={direction} />} + label={localeMessages["add_instructors"]} + sx={{ m: 0 }} + /> + + + + + + + {addInstructors && + setSelectedInstructorIds(ids)} + activeOrganizationId={activeOrganizationId} + initialInstructorIds={selectedInstructorIds} + /> + } { setImageUrl(data.file_url); diff --git a/frontend/platform/courses/components/CreateInstructorForm.jsx b/frontend/platform/courses/components/CreateInstructorForm.jsx new file mode 100644 index 0000000..b2805fb --- /dev/null +++ b/frontend/platform/courses/components/CreateInstructorForm.jsx @@ -0,0 +1,86 @@ +import { useState } from 'react'; +import { Alert, Box, Button } from '@mui/material'; +import RequiredTextField from '../../../src/components/RequiredTextField'; +import { useAppContext } from '../../../src/render.jsx'; +import { getCookie } from '../../../src/utils'; + + +const CreateInstructorForm = ({ onSuccess, activeOrganizationId }) => { + const [email, setEmail] = useState(''); + const [emailHelperText, setEmailHelperText] = useState(''); + const [errorMessage, setErrorMessage] = useState(''); + const { localeMessages, apiBaseUrl } = useAppContext(); + + const isValidEmail = (value) => /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value); + + const handleSubmit = () => { + const trimmedEmail = email.trim(); + if (!trimmedEmail) { + setEmailHelperText(localeMessages['email_required_helper_text']); + return; + } + if (!isValidEmail(trimmedEmail)) { + setEmailHelperText(localeMessages['invalid_email_helper_text']); + return; + } + setEmailHelperText(''); + setErrorMessage(''); + + fetch(`${apiBaseUrl}/users/get-or-create-by-email/`, { + method: 'POST', + credentials: 'include', + headers: { + 'Content-Type': 'application/json', + 'X-CSRFToken': getCookie('csrftoken'), + }, + body: JSON.stringify({ email: trimmedEmail, organization_id: activeOrganizationId }), + }) + .then((response) => { + if (!response.ok) throw new Error('Failed to get or create user'); + return response.json(); + }) + .then((userData) => + fetch(`${apiBaseUrl}/organizations/${activeOrganizationId}/users/`, { + method: 'POST', + credentials: 'include', + headers: { + 'Content-Type': 'application/json', + 'X-CSRFToken': getCookie('csrftoken'), + }, + body: JSON.stringify({ user_id: userData.id, role: 'instructor' }), + }) + ) + .then((response) => { + if (!response.ok) throw new Error('Failed to add instructor to organization'); + return response.json(); + }) + .then((orgUserData) => { + if (onSuccess) onSuccess(orgUserData); + setEmail(''); + }) + .catch((error) => { + console.error('Error adding instructor:', error); + setErrorMessage(localeMessages['instructor_add_failed']); + }); + }; + + return ( + + {errorMessage && {errorMessage}} + setEmail(e.target.value)} + type="email" + /> + + + ); +}; + +export default CreateInstructorForm; diff --git a/tests/platform/api/test_views/test_course_view.py b/tests/platform/api/test_views/test_course_view.py index 8920f0d..f5bf081 100644 --- a/tests/platform/api/test_views/test_course_view.py +++ b/tests/platform/api/test_views/test_course_view.py @@ -1,5 +1,9 @@ from django.urls import reverse -from django_email_learning.models import Organization +from django_email_learning.models import ( + Organization, + OrganizationUser, + CourseInstructor, +) from django_email_learning.models import Course import json import uuid @@ -469,3 +473,308 @@ def valid_update_course_payload( "enabled": enabled, "reset_imap_connection": reset_imap_connection, } + + +# --------------------------------------------------------------------------- +# Helper: get the OrganizationUser id for a given role (relies on users fixture) +# --------------------------------------------------------------------------- + + +def _org_user_id(role: str) -> int: + return OrganizationUser.objects.filter(organization_id=1, role=role).first().id + + +# --------------------------------------------------------------------------- +# Instructor — create +# --------------------------------------------------------------------------- + + +def test_create_course_with_instructor_succeeds(users, superadmin_client): + instructor_org_user_id = _org_user_id("instructor") + payload = valid_create_course_payload() + payload["instructors"] = [instructor_org_user_id] + + response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + + assert response.status_code == 201 + data = response.json() + assert "instructors" in data + assert len(data["instructors"]) == 1 + assert data["instructors"][0]["id"] == instructor_org_user_id + assert data["instructors"][0]["email"] == "instructor@example.com" + + +def test_create_course_response_has_empty_instructors_list_when_none_assigned( + users, superadmin_client +): + payload = valid_create_course_payload() + # No instructors key in payload + + response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + + assert response.status_code == 201 + data = response.json() + assert "instructors" in data + # Serializer returns [] when no instructors exist (from_django_model always populates the list) + assert data["instructors"] == [] or data["instructors"] is None + + +def test_create_course_with_non_instructor_org_user_fails(users, superadmin_client): + editor_org_user_id = _org_user_id("editor") + payload = valid_create_course_payload(slug=uuid.uuid4().hex) + payload["instructors"] = [editor_org_user_id] + + response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + + assert response.status_code == 409 + assert "error" in response.json() + assert "instructor role" in response.json()["error"] + + +def test_create_course_with_nonexistent_org_user_fails(users, superadmin_client): + payload = valid_create_course_payload(slug=uuid.uuid4().hex) + payload["instructors"] = [99999] # does not exist + + response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + + assert response.status_code == 409 + assert "error" in response.json() + assert "does not exist" in response.json()["error"] + + +def test_create_course_with_multiple_instructors(users, superadmin_client): + """Only org users with instructor role are valid; add a second instructor org user first.""" + from django.contrib.auth.models import User as DjangoUser + + second_instructor_user = DjangoUser.objects.create_user( + username="instructor2", email="instructor2@example.com", password="pass" + ) + second_org_user = OrganizationUser.objects.create( + user=second_instructor_user, organization_id=1, role="instructor" + ) + instructor_org_user_id = _org_user_id("instructor") + + payload = valid_create_course_payload(slug=uuid.uuid4().hex) + payload["instructors"] = [instructor_org_user_id, second_org_user.id] + + response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + + assert response.status_code == 201 + data = response.json() + assert len(data["instructors"]) == 2 + returned_ids = {i["id"] for i in data["instructors"]} + assert instructor_org_user_id in returned_ids + assert second_org_user.id in returned_ids + + +# --------------------------------------------------------------------------- +# Instructor — response check on GET single course +# --------------------------------------------------------------------------- + + +def test_get_single_course_response_includes_instructors(users, superadmin_client): + instructor_org_user_id = _org_user_id("instructor") + payload = valid_create_course_payload(slug=uuid.uuid4().hex) + payload["instructors"] = [instructor_org_user_id] + + create_response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + assert create_response.status_code == 201 + course_id = create_response.json()["id"] + + get_url_single = reverse( + "django_email_learning:api_platform:single_course_view", + kwargs={"organization_id": 1, "course_id": course_id}, + ) + get_response = superadmin_client.get(get_url_single) + + assert get_response.status_code == 200 + data = get_response.json() + assert "instructors" in data + assert len(data["instructors"]) == 1 + assert data["instructors"][0]["id"] == instructor_org_user_id + assert data["instructors"][0]["email"] == "instructor@example.com" + + +# --------------------------------------------------------------------------- +# Instructor — update (add, replace, remove) +# --------------------------------------------------------------------------- + + +def _single_course_url(course_id: int) -> str: + return reverse( + "django_email_learning:api_platform:single_course_view", + kwargs={"organization_id": 1, "course_id": course_id}, + ) + + +def test_update_course_adds_instructor(users, superadmin_client): + instructor_org_user_id = _org_user_id("instructor") + + # Create course without instructors + create_response = superadmin_client.post( + get_url(1), + json.dumps(valid_create_course_payload(slug=uuid.uuid4().hex)), + content_type="application/json", + ) + assert create_response.status_code == 201 + course_id = create_response.json()["id"] + + # Update to assign instructor + update_response = superadmin_client.post( + _single_course_url(course_id), + json.dumps({"instructors": [instructor_org_user_id]}), + content_type="application/json", + ) + + assert update_response.status_code == 200 + data = update_response.json() + assert len(data["instructors"]) == 1 + assert data["instructors"][0]["id"] == instructor_org_user_id + + +def test_update_course_removes_instructor_when_not_in_list(users, superadmin_client): + from django.contrib.auth.models import User as DjangoUser + + # Create a second instructor org user + second_user = DjangoUser.objects.create_user( + username="instructor_remove", email="instr_remove@example.com", password="pass" + ) + second_org_user = OrganizationUser.objects.create( + user=second_user, organization_id=1, role="instructor" + ) + instructor_org_user_id = _org_user_id("instructor") + + # Create course with both instructors + payload = valid_create_course_payload(slug=uuid.uuid4().hex) + payload["instructors"] = [instructor_org_user_id, second_org_user.id] + create_response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + assert create_response.status_code == 201 + course_id = create_response.json()["id"] + assert len(create_response.json()["instructors"]) == 2 + + # Update: only keep the first instructor (remove second) + update_response = superadmin_client.post( + _single_course_url(course_id), + json.dumps({"instructors": [instructor_org_user_id]}), + content_type="application/json", + ) + + assert update_response.status_code == 200 + data = update_response.json() + + assert len(data["instructors"]) == 1 + assert data["instructors"][0]["id"] == instructor_org_user_id + # Confirm DB record is gone + assert not CourseInstructor.objects.filter( + course_id=course_id, org_user=second_org_user + ).exists() + + +def test_update_course_clears_all_instructors_with_empty_list(users, superadmin_client): + instructor_org_user_id = _org_user_id("instructor") + + payload = valid_create_course_payload(slug=uuid.uuid4().hex) + payload["instructors"] = [instructor_org_user_id] + create_response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + assert create_response.status_code == 201 + course_id = create_response.json()["id"] + + # Pass empty list to clear all instructors + update_response = superadmin_client.post( + _single_course_url(course_id), + json.dumps({"instructors": []}), + content_type="application/json", + ) + + assert update_response.status_code == 200 + data = update_response.json() + assert data["instructors"] == [] + assert not CourseInstructor.objects.filter(course_id=course_id).exists() + + +def test_update_course_omitting_instructors_does_not_change_them( + users, superadmin_client +): + """Passing no 'instructors' key should leave existing instructors untouched.""" + instructor_org_user_id = _org_user_id("instructor") + + payload = valid_create_course_payload(slug=uuid.uuid4().hex) + payload["instructors"] = [instructor_org_user_id] + create_response = superadmin_client.post( + get_url(1), json.dumps(payload), content_type="application/json" + ) + assert create_response.status_code == 201 + course_id = create_response.json()["id"] + + # Update title only — no 'instructors' key + update_response = superadmin_client.post( + _single_course_url(course_id), + json.dumps({"title": "New Title"}), + content_type="application/json", + ) + + assert update_response.status_code == 200 + data = update_response.json() + assert data["title"] == "New Title" + assert len(data["instructors"]) == 1 + assert data["instructors"][0]["id"] == instructor_org_user_id + + +def test_update_course_with_non_instructor_role_fails(users, superadmin_client): + editor_org_user_id = _org_user_id("editor") + + create_response = superadmin_client.post( + get_url(1), + json.dumps(valid_create_course_payload(slug=uuid.uuid4().hex)), + content_type="application/json", + ) + assert create_response.status_code == 201 + course_id = create_response.json()["id"] + + update_response = superadmin_client.post( + _single_course_url(course_id), + json.dumps({"instructors": [editor_org_user_id]}), + content_type="application/json", + ) + + assert update_response.status_code == 409 + assert "error" in update_response.json() + assert "instructor role" in update_response.json()["error"] + + +def test_update_course_with_nonexistent_instructor_org_user_fails( + users, superadmin_client +): + create_response = superadmin_client.post( + get_url(1), + json.dumps(valid_create_course_payload(slug=uuid.uuid4().hex)), + content_type="application/json", + ) + assert create_response.status_code == 201 + course_id = create_response.json()["id"] + + update_response = superadmin_client.post( + _single_course_url(course_id), + json.dumps({"instructors": [99999]}), + content_type="application/json", + ) + + assert update_response.status_code == 409 + assert "error" in update_response.json() + assert "does not exist" in update_response.json()["error"] diff --git a/tests/test_models/test_course_instructor.py b/tests/test_models/test_course_instructor.py new file mode 100644 index 0000000..7bb281f --- /dev/null +++ b/tests/test_models/test_course_instructor.py @@ -0,0 +1,265 @@ +from django_email_learning.models import ( + CourseInstructor, + Course, + OrganizationUser, + Organization, +) +import pytest +from django.core.exceptions import ValidationError +from django.db import IntegrityError +from django.contrib.auth.models import User + + +@pytest.fixture +def org_instructor(users) -> OrganizationUser: + instructor_user = users["instructor_user"] + instructor = OrganizationUser.objects.get(user=instructor_user) + + return instructor + + +@pytest.fixture +def org_editor(users) -> OrganizationUser: + editor_user = users["editor_user"] + editor = OrganizationUser.objects.get(user=editor_user) + + return editor + + +@pytest.fixture +def organization() -> Organization: + return Organization.objects.first() + + +@pytest.fixture +def org_instructor_different_org() -> OrganizationUser: + instructor_user = User.objects.create_user( + username="instructor_different_org", + email="instructor_different_org@example.com", + password="password123", + ) + different_org = Organization.objects.create(name="Different Org") + instructor = OrganizationUser.objects.create( + user=instructor_user, organization=different_org, role="instructor" + ) + return instructor + + +class TestCourseInstructorCreation: + """Test CourseInstructor creation and basic functionality.""" + + def test_create_valid_course_instructor(self, db, org_instructor, course): + """Test creating a valid CourseInstructor.""" + course_instructor = CourseInstructor.objects.create( + course=course, + org_user=org_instructor, + ) + + assert course_instructor.id is not None + assert course_instructor.course == course + assert course_instructor.org_user == org_instructor + + def test_course_instructor_str_representation(self, db, course, org_instructor): + """Test the __str__ method returns expected format.""" + course_instructor = CourseInstructor.objects.create( + course=course, + org_user=org_instructor, + ) + + expected = f"{course.title} - {org_instructor.user.email}" + assert str(course_instructor) == expected + + def test_course_instructor_str_with_different_email( + self, db, course, org_instructor + ): + """Test __str__ method reflects org_user email correctly.""" + course_instructor = CourseInstructor.objects.create( + course=course, + org_user=org_instructor, + ) + + assert org_instructor.user.email in str(course_instructor) + assert course.title in str(course_instructor) + + +class TestCourseInstructorValidation: + """Test CourseInstructor validation rules.""" + + def test_cannot_add_instructor_from_different_organization( + self, db, course, org_instructor_different_org + ): + """Test that instructor from different organization cannot be added.""" + with pytest.raises(ValidationError) as exc_info: + CourseInstructor( + course=course, + org_user=org_instructor_different_org, + ).save() + + assert "Instructor must belong to the same organization as the course." in str( + exc_info.value + ) + + def test_cannot_add_non_instructor_role(self, db, course, org_editor): + """Test that org_user without instructor role cannot be added.""" + with pytest.raises(ValidationError) as exc_info: + CourseInstructor( + course=course, + org_user=org_editor, + ).save() + + assert "Organization user doesn't have instructor role." in str(exc_info.value) + + def test_validation_happens_on_save(self, db, course, org_instructor_different_org): + """Test that validation occurs during save, not instantiation.""" + # Instantiation should work + course_instructor = CourseInstructor( + course=course, + org_user=org_instructor_different_org, + ) + assert course_instructor is not None + + # Save should fail + with pytest.raises(ValidationError): + course_instructor.save() + + +class TestCourseInstructorUniqueness: + """Test CourseInstructor uniqueness constraints.""" + + def test_unique_constraint_course_org_user(self, db, course, org_instructor): + """Test that same course-org_user combination is unique.""" + CourseInstructor.objects.create( + course=course, + org_user=org_instructor, + ) + + with pytest.raises(IntegrityError): + CourseInstructor.objects.create( + course=course, + org_user=org_instructor, + ) + + def test_same_instructor_can_teach_multiple_courses( + self, db, organization, org_instructor + ): + """Test that same instructor can be added to multiple courses.""" + course1 = Course.objects.create( + title="Course 1", + slug="course-1", + organization=organization, + ) + course2 = Course.objects.create( + title="Course 2", + slug="course-2", + organization=organization, + ) + + instructor1 = CourseInstructor.objects.create( + course=course1, + org_user=org_instructor, + ) + instructor2 = CourseInstructor.objects.create( + course=course2, + org_user=org_instructor, + ) + + assert instructor1.id != instructor2.id + assert instructor1.org_user == instructor2.org_user + assert instructor1.course != instructor2.course + + def test_different_instructors_can_teach_same_course( + self, db, course, organization + ): + """Test that multiple instructors can be added to same course.""" + instructor_user2 = User.objects.create_user( + username="instructor2", + email="instructor2@example.com", + password="testpass123", + ) + org_instructor2 = OrganizationUser.objects.create( + user=instructor_user2, + organization=organization, + role="instructor", + ) + + instructor_user3 = User.objects.create_user( + username="instructor3", + email="instructor3@example.com", + password="testpass123", + ) + org_instructor3 = OrganizationUser.objects.create( + user=instructor_user3, + organization=organization, + role="instructor", + ) + + course_instr1 = CourseInstructor.objects.create( + course=course, + org_user=org_instructor2, + ) + course_instr2 = CourseInstructor.objects.create( + course=course, + org_user=org_instructor3, + ) + + assert course_instr1.course == course_instr2.course + assert course_instr1.org_user != course_instr2.org_user + + +class TestCourseInstructorRelationships: + """Test CourseInstructor relationships and related_name.""" + + def test_related_name_instructors_on_course(self, db, course, org_instructor): + """Test accessing instructors through course.instructors related_name.""" + CourseInstructor.objects.create( + course=course, + org_user=org_instructor, + ) + + instructors = course.instructors.all() + assert instructors.count() == 1 + assert instructors.first().org_user == org_instructor + + def test_multiple_instructors_via_related_name(self, db, course, organization): + """Test accessing multiple instructors through related_name.""" + instructor_user1 = User.objects.create_user( + username="inst1", + email="inst1@example.com", + password="pass123", + ) + instructor_user2 = User.objects.create_user( + username="inst2", + email="inst2@example.com", + password="pass123", + ) + + org_instructor1 = OrganizationUser.objects.create( + user=instructor_user1, + organization=organization, + role="instructor", + ) + org_instructor2 = OrganizationUser.objects.create( + user=instructor_user2, + organization=organization, + role="instructor", + ) + + CourseInstructor.objects.create(course=course, org_user=org_instructor1) + CourseInstructor.objects.create(course=course, org_user=org_instructor2) + + instructors = course.instructors.all() + assert instructors.count() == 2 + assert org_instructor1 in [ci.org_user for ci in instructors] + assert org_instructor2 in [ci.org_user for ci in instructors] + + def test_delete_course_instructor_cascade(self, db, course, org_instructor): + """Test that deleting a CourseInstructor works correctly.""" + course_instructor = CourseInstructor.objects.create( + course=course, + org_user=org_instructor, + ) + course_instructor_id = course_instructor.id + + course_instructor.delete() + + assert not CourseInstructor.objects.filter(id=course_instructor_id).exists()