diff --git a/backend/api/locale/en/LC_MESSAGES/django.po b/backend/api/locale/en/LC_MESSAGES/django.po index ac864a0a..045ca9b8 100644 --- a/backend/api/locale/en/LC_MESSAGES/django.po +++ b/backend/api/locale/en/LC_MESSAGES/django.po @@ -52,13 +52,21 @@ msgid "courses.error.students.already_present" msgstr "The student is already present in the course." #: serializers/course_serializer.py:68 serializers/course_serializer.py:87 -msgid "courses.error.students.past_course" +msgid "courses.error.past_course" msgstr "The course is from a past year, thus cannot be manipulated." #: serializers/course_serializer.py:83 msgid "courses.error.students.not_present" msgstr "The student is not present in the course." +#: serializers/course_serializer.py:97 +msgid "courses.error.teachers.already_present" +msgstr "The teacher is already present in the course." + +#: serializers/course_serializer.py:116 +msgid "courses.error.teachers.not_present" +msgstr "The teacher is not present in the course." + #: serializers/group_serializer.py:47 msgid "group.errors.score_exceeds_max" msgstr "The score exceeds the group's max score." @@ -127,6 +135,14 @@ msgstr "The student was successfully added to the course." msgid "courses.success.students.remove" msgstr "The student was successfully removed from the course." +#: views/course_view.py:172 +msgid "course.success.teachers.add" +msgstr "The teacher was successfully added to the course." + +#: views/course_view.py:195 +msgid "course.success.teachers.remove" +msgstr "The teacher was successfully removed from the course." + #: views/course_view.py:186 msgid "course.success.project.add" msgstr "The project was successfully added to the course." diff --git a/backend/api/locale/nl/LC_MESSAGES/django.po b/backend/api/locale/nl/LC_MESSAGES/django.po index 320b5cb0..0a993617 100644 --- a/backend/api/locale/nl/LC_MESSAGES/django.po +++ b/backend/api/locale/nl/LC_MESSAGES/django.po @@ -52,13 +52,21 @@ msgid "courses.error.students.already_present" msgstr "De student bevindt zich al in de opleiding." #: serializers/course_serializer.py:68 serializers/course_serializer.py:87 -msgid "courses.error.students.past_course" +msgid "courses.error.past_course" msgstr "De opleiding die men probeert te manipuleren is van een vorig jaar." #: serializers/course_serializer.py:83 msgid "courses.error.students.not_present" msgstr "De student bevindt zich niet in de opleiding." +#: serializers/course_serializer.py:97 +msgid "courses.error.teachers.already_present" +msgstr "De lesgever bevindt zich al in de opleiding." + +#: serializers/course_serializer.py:116 +msgid "courses.error.teachers.not_present" +msgstr "De lesgever bevindt zich niet in de opleiding." + #: serializers/group_serializer.py:47 msgid "group.errors.score_exceeds_max" msgstr "De score van de groep is groter dan de maximum score." @@ -128,6 +136,14 @@ msgstr "De student is succesvol toegevoegd aan de opleiding." msgid "courses.success.students.remove" msgstr "De student is succesvol verwijderd uit de opleiding." +#: views/course_view.py:172 +msgid "course.success.teachers.add" +msgstr "De lesgever is succesvol toegevoegd aan de opleiding." + +#: views/course_view.py:195 +msgid "course.success.teachers.remove" +msgstr "De lesgever is succesvol verwijderd uit de opleiding." + #: views/course_view.py:186 msgid "course.success.project.add" msgstr "Het project is succesvol toegevoegd aan de opleiding." diff --git a/backend/api/models/course.py b/backend/api/models/course.py index 403cae8a..b4fdb972 100644 --- a/backend/api/models/course.py +++ b/backend/api/models/course.py @@ -36,7 +36,7 @@ def is_past(self) -> bool: """Returns whether the course is from a past academic year""" return datetime(self.academic_startyear + 1, 10, 1) < datetime.now() - def clone(self, clone_assistants=True) -> Self: + def clone(self, clone_teachers=True, clone_assistants=True) -> Self: """Clone the course to the next academic start year""" course = Course.objects.create( name=self.name, @@ -50,6 +50,11 @@ def clone(self, clone_assistants=True) -> Self: for assistant in self.assistants.all(): course.assistants.add(assistant) + if clone_teachers: + # Add all the teachers of the current course to the follow up course + for teacher in self.teachers.all(): + course.teachers.add(teacher) + return course @property diff --git a/backend/api/permissions/course_permissions.py b/backend/api/permissions/course_permissions.py index 4a87fca6..ccb1314e 100644 --- a/backend/api/permissions/course_permissions.py +++ b/backend/api/permissions/course_permissions.py @@ -65,3 +65,16 @@ def has_object_permission(self, request: Request, view: ViewSet, course: Course) # Teachers and assistants can add and remove any student. return super().has_object_permission(request, view, course) + + +class CourseTeacherPermission(CoursePermission): + """Permission class for teacher related endpoints.""" + def has_object_permission(self, request: Request, view: ViewSet, course: Course): + user: User = request.user + + # Logged-in users can fetch course teachers. + if request.method in SAFE_METHODS: + return user.is_authenticated + + # Only teachers can add or remove themselves from a course. + return is_teacher(user) and request.data.get("teacher_id") == user.id diff --git a/backend/api/serializers/course_serializer.py b/backend/api/serializers/course_serializer.py index c0f8ea72..4d7cf067 100644 --- a/backend/api/serializers/course_serializer.py +++ b/backend/api/serializers/course_serializer.py @@ -2,6 +2,7 @@ from rest_framework import serializers from rest_framework.exceptions import ValidationError from api.serializers.student_serializer import StudentIDSerializer +from api.serializers.teacher_serializer import TeacherIDSerializer from api.models.course import Course @@ -42,6 +43,7 @@ class CourseIDSerializer(serializers.Serializer): class CourseCloneSerializer(serializers.Serializer): + clone_teachers = serializers.BooleanField() clone_assistants = serializers.BooleanField() @@ -59,7 +61,7 @@ def validate(self, data): # Check if the course is not from a past academic year. if course.is_past(): - raise ValidationError(gettext("courses.error.students.past_course")) + raise ValidationError(gettext("courses.error.past_course")) return data @@ -72,12 +74,50 @@ def validate(self, data): course: Course = self.context["course"] - # Check if the student isn't already enrolled. + # Make sure the student is enrolled. if not course.students.contains(data["student_id"]): raise ValidationError(gettext("courses.error.students.not_present")) # Check if the course is not from a past academic year. if course.is_past(): - raise ValidationError(gettext("courses.error.students.past_course")) + raise ValidationError(gettext("courses.error.past_course")) + + return data + + +class TeacherJoinSerializer(TeacherIDSerializer): + def validate(self, data): + # The validator needs the course context. + if "course" not in self.context: + raise ValidationError(gettext("courses.error.context")) + + course: Course = self.context["course"] + + # Check if the teacher isn't already enrolled. + if course.teachers.contains(data["teacher_id"]): + raise ValidationError(gettext("courses.error.teachers.already_present")) + + # Check if the course is not from a past academic year. + if course.is_past(): + raise ValidationError(gettext("courses.error.past_course")) + + return data + + +class TeacherLeaveSerializer(TeacherIDSerializer): + def validate(self, data): + # The validator needs the course context. + if "course" not in self.context: + raise ValidationError(gettext("courses.error.context")) + + course: Course = self.context["course"] + + # Make sure the teacher is enrolled. + if not course.teachers.contains(data["teacher_id"]): + raise ValidationError(gettext("courses.error.teachers.not_present")) + + # Check if the course is not from a past academic year. + if course.is_past(): + raise ValidationError(gettext("courses.error.past_course")) return data diff --git a/backend/api/serializers/teacher_serializer.py b/backend/api/serializers/teacher_serializer.py index 984f75a6..75550d65 100644 --- a/backend/api/serializers/teacher_serializer.py +++ b/backend/api/serializers/teacher_serializer.py @@ -15,3 +15,9 @@ class TeacherSerializer(serializers.ModelSerializer): class Meta: model = Teacher fields = "__all__" + + +class TeacherIDSerializer(serializers.Serializer): + teacher_id = serializers.PrimaryKeyRelatedField( + queryset=Teacher.objects.all() + ) diff --git a/backend/api/tests/test_course.py b/backend/api/tests/test_course.py index e8d01c2d..036081cb 100644 --- a/backend/api/tests/test_course.py +++ b/backend/api/tests/test_course.py @@ -98,6 +98,13 @@ def get_assistant(): return create_assistant(id=5, first_name="Simon", last_name="Mignolet", email="Simon.Mignolet@gmail.com") +def get_teacher(): + """ + Return a random teacher to use in tests. + """ + return create_teacher(id=5, first_name="Sinan", last_name="Bolat", email="Sinan.Bolat@gmail.com") + + def get_student(): """ Return a random student to use in tests. @@ -532,6 +539,22 @@ def test_add_self_to_course(self): self.assertEqual(response.status_code, 200) self.assertTrue(course.students.filter(id=self.user.id).exists()) + def test_try_add_self_as_teacher_to_course(self): + """ + Students should not be able to add themselves as teachers to a course. + """ + course = get_course() + + response = self.client.post( + reverse("course-teachers", args=[str(course.id)]), + data={"teacher_id": self.user.id}, + follow=True, + ) + + # Make sure that the student has not been added as a teacher + self.assertEqual(response.status_code, 403) + self.assertFalse(course.teachers.filter(id=self.user.id).exists()) + def test_remove_self_from_course(self): """ Able to remove self from a course. @@ -702,6 +725,54 @@ def setUp(self) -> None: self.user ) + def test_add_self(self): + """ + Teacher should be able to add him/herself to a course. + """ + course = get_course() + + response = self.client.post( + reverse("course-teachers", args=[str(course.id)]), + data={"teacher_id": self.user.id}, + follow=True, + ) + + # Make sure the current logged in teacher was added correctly + self.assertEqual(response.status_code, 200) + self.assertTrue(course.teachers.filter(id=self.user.id).exists()) + + def test_remove_self(self): + """ + Teacher should be able to remove him/herself from a course. + """ + course = get_course() + course.teachers.add(self.user) + + response = self.client.delete( + reverse("course-teachers", args=[str(course.id)]), + data={"teacher_id": self.user.id}, + follow=True, + ) + + # Make sure the current logged in teacher was removed correctly + self.assertEqual(response.status_code, 200) + self.assertFalse(course.teachers.filter(id=self.user.id).exists()) + + def test_try_remove_self_when_not_part_of(self): + """ + Teacher should not be able to remove him/herself from a course he/she is not part of. + """ + course = get_course() + + response = self.client.delete( + reverse("course-teachers", args=[str(course.id)]), + data={"teacher_id": self.user.id}, + follow=True, + ) + + # Make sure the check was successful + self.assertEqual(response.status_code, 400) + def test_add_assistant(self): """ Able to add an assistant to a course. @@ -908,7 +979,7 @@ def test_clone_course(self): response = self.client.post( reverse("course-clone", args=[str(course.id)]), - data={"clone_assistants": False}, + data={"clone_assistants": False, "clone_teachers": False}, follow=True, ) @@ -916,17 +987,32 @@ def test_clone_course(self): self.assertTrue(Course.objects.filter(name=course.name, academic_startyear=course.academic_startyear + 1).exists()) - # Make sure there are no assistants in the cloned course + # Make sure the returned course is the cloned course + retrieved_course = json.loads(response.content.decode("utf-8")) + + self.assertEqual(retrieved_course["name"], course.name) + self.assertEqual(retrieved_course["academic_startyear"], course.academic_startyear + 1) + + # Get the cloned course cloned_course = Course.objects.get(name=course.name, academic_startyear=course.academic_startyear + 1) + + # Make sure there are no assistants in the cloned course self.assertFalse(cloned_course.assistants.exists()) - def test_clone_with_assistants(self): + # Make sure there are no teachers in the cloned course + self.assertFalse(cloned_course.teachers.exists()) + + def test_clone_with_assistants_and_teachers(self): """ - Able to clone a course with assistants. + Able to clone a course with assistants and teachers. """ course = get_course() course.teachers.add(self.user) + # Add another teacher to the course + teacher = get_teacher() + course.teachers.add(teacher) + # Create an assistant and add it to the course assistant = get_assistant() course.assistants.add(assistant) @@ -934,7 +1020,7 @@ def test_clone_with_assistants(self): # Clone the course with the assistants response = self.client.post( reverse("course-clone", args=[str(course.id)]), - data={"clone_assistants": True}, + data={"clone_assistants": True, "clone_teachers": True}, follow=True, ) @@ -942,6 +1028,40 @@ def test_clone_with_assistants(self): self.assertTrue(Course.objects.filter(name=course.name, academic_startyear=course.academic_startyear + 1).exists()) - # Make sure the assistant is also cloned cloned_course = Course.objects.get(name=course.name, academic_startyear=course.academic_startyear + 1) + + # Make sure the assistant is also cloned self.assertTrue(cloned_course.assistants.filter(id=assistant.id).exists()) + self.assertEqual(cloned_course.assistants.count(), 1) + + # Make sure the two teachers are also cloned + self.assertTrue(cloned_course.teachers.filter(id=self.user.id).exists()) + self.assertTrue(cloned_course.teachers.filter(id=teacher.id).exists()) + self.assertEqual(cloned_course.teachers.count(), 2) + + def test_clone_course_that_already_has_child_course(self): + """ + Course that has already a child course should not be cloned, but the child course should be returned. + """ + course = get_course() + course.teachers.add(self.user) + + # Create a child course + child_course = create_course(name="Chemistry 101", academic_startyear=2024, + description="An introductory chemistry course.", parent_course=course) + + # Clone the course + response = self.client.post( + reverse("course-clone", args=[str(course.id)]), + data={"clone_assistants": False, "clone_teachers": False}, + follow=True, + ) + + self.assertEqual(response.status_code, 200) + + # Make sure the returned course is just the child course + retrieved_course = json.loads(response.content.decode("utf-8")) + + self.assertEqual(retrieved_course["id"], child_course.id) + self.assertEqual(retrieved_course["name"], child_course.name) + self.assertEqual(retrieved_course["academic_startyear"], child_course.academic_startyear) diff --git a/backend/api/views/course_view.py b/backend/api/views/course_view.py index 137e4fd1..5f91561d 100644 --- a/backend/api/views/course_view.py +++ b/backend/api/views/course_view.py @@ -6,15 +6,16 @@ from rest_framework.request import Request from drf_yasg.utils import swagger_auto_schema from api.models.course import Course -from api.models.group import Group from api.permissions.course_permissions import ( CoursePermission, CourseAssistantPermission, - CourseStudentPermission + CourseStudentPermission, + CourseTeacherPermission ) from api.permissions.role_permissions import IsTeacher from api.serializers.course_serializer import ( - CourseSerializer, StudentJoinSerializer, StudentLeaveSerializer, CourseCloneSerializer + CourseSerializer, StudentJoinSerializer, StudentLeaveSerializer, CourseCloneSerializer, + TeacherJoinSerializer, TeacherLeaveSerializer ) from api.serializers.teacher_serializer import TeacherSerializer from api.serializers.assistant_serializer import AssistantSerializer, AssistantIDSerializer @@ -124,7 +125,7 @@ def _remove_student(self, request: Request, **_): # Get the course course = self.get_object() - # Add student to course + # Remove the student from the course serializer = StudentLeaveSerializer(data=request.data, context={ "course": course }) @@ -138,7 +139,7 @@ def _remove_student(self, request: Request, **_): "message": gettext("courses.success.students.remove") }) - @action(detail=True, methods=["get"]) + @action(detail=True, methods=["get"], permission_classes=[IsAdminUser | CourseTeacherPermission]) def teachers(self, request, **_): """Returns a list of teachers for the given course""" course = self.get_object() @@ -151,6 +152,49 @@ def teachers(self, request, **_): return Response(serializer.data) + @teachers.mapping.post + @teachers.mapping.put + @swagger_auto_schema(request_body=TeacherJoinSerializer) + def _add_teacher(self, request, **_): + """Add a teacher to the course""" + # Get the course + course = self.get_object() + + # Add teacher to course + serializer = TeacherJoinSerializer(data=request.data, context={ + "course": course + }) + + if serializer.is_valid(raise_exception=True): + course.teachers.add( + serializer.validated_data["teacher_id"] + ) + + return Response({ + "message": gettext("courses.success.teachers.add") + }) + + @teachers.mapping.delete + @swagger_auto_schema(request_body=TeacherLeaveSerializer) + def _remove_teacher(self, request, **_): + """Remove a teacher from the course""" + # Get the course + course = self.get_object() + + # Remove the teacher from the course + serializer = TeacherLeaveSerializer(data=request.data, context={ + "course": course + }) + + if serializer.is_valid(raise_exception=True): + course.teachers.remove( + serializer.validated_data["teacher_id"] + ) + + return Response({ + "message": gettext("courses.success.teachers.remove") + }) + @action(detail=True, methods=["get"]) def projects(self, request, **_): """Returns a list of projects for the given course""" @@ -204,7 +248,8 @@ def clone(self, request: Request, **__): except Course.DoesNotExist: # Else, we clone the course - course.clone( + course = course.clone( + clone_teachers=serializer.validated_data["clone_teachers"], clone_assistants=serializer.validated_data["clone_assistants"] )