transfer class

backend/api/serializers/klass.py

@@ -13,11 +13,6 @@
 
 # pylint: disable-next=missing-class-docstring,too-many-ancestors
 class ClassSerializer(_ClassSerializer):
-    teacher = serializers.IntegerField(
-        source="teacher.id",
-        required=False,
-    )
-
     read_classmates_data = serializers.BooleanField(
         source="classmates_data_viewable",
     )
@@ -31,26 +26,20 @@ class Meta(_ClassSerializer.Meta):
         extra_kwargs = {
             **_ClassSerializer.Meta.extra_kwargs,
             "name": {"read_only": False},
+            "teacher": {"required": False},
         }
 
     # pylint: disable-next=missing-function-docstring
-    def validate_teacher(self, value: int):
-        queryset = Teacher.objects.filter(id=value)
-        if not queryset.exists():
-            raise serializers.ValidationError(
-                "This teacher does not exist.",
-                code="does_not_exist",
-            )
-
+    def validate_teacher(self, value: Teacher):
         user = self.request.school_teacher_user
-        if not queryset.filter(school=user.teacher.school_id).exists():
+        if value.school_id != user.teacher.school_id:
             raise serializers.ValidationError(
                 "This teacher is not in your school.",
                 code="not_in_school",
             )
-        if value != user.teacher.id and not user.teacher.is_admin:
+        if value != user.teacher and not user.teacher.is_admin:
             raise serializers.ValidationError(
-                "Cannot assign another teacher if you're not admin.",
+                "Cannot assign another teacher if you're not an admin.",
                 code="not_admin",
             )
 
@@ -87,10 +76,10 @@ def create(self, validated_data):
             {
                 "access_code": access_code,
                 "name": validated_data["name"],
-                "teacher_id": (
-                    validated_data["teacher"]["id"]
+                "teacher": (
+                    validated_data["teacher"]
                     if "teacher" in validated_data
-                    else self.request.school_teacher_user.teacher.id
+                    else self.request.school_teacher_user.teacher
                 ),
                 "classmates_data_viewable": validated_data[
                     "classmates_data_viewable"

backend/api/tests/serializers/test_klass.py

@@ -4,128 +4,96 @@
 """
 
 from codeforlife.tests import ModelSerializerTestCase
-from codeforlife.user.models import Class, SchoolTeacherUser, Teacher
+from codeforlife.user.models import Class, NonAdminSchoolTeacherUser, Teacher
 
 from ...serializers import ClassSerializer
 
 
 # pylint: disable-next=missing-class-docstring
 class TestClassSerializer(ModelSerializerTestCase[Class]):
     model_serializer_class = ClassSerializer
-    fixtures = ["school_1"]
+    fixtures = ["school_1", "school_2"]
 
     def setUp(self):
-        self.school_teacher_user = SchoolTeacherUser.objects.get(
-            email="[email protected]"
+        self.non_admin_school_1_teacher_user = (
+            NonAdminSchoolTeacherUser.objects.get(email="[email protected]")
         )
-        self.class_1 = Class.objects.get(name="Class 1 @ School 1")
-
-    def test_validate_teacher__does_not_exist(self):
-        """
-        Teacher must exist.
-        """
-
-        self.assert_validate_field(
-            name="teacher",
-            value=-1,
-            error_code="does_not_exist",
+        self.non_admin_school_2_teacher_user = (
+            NonAdminSchoolTeacherUser.objects.get(email="[email protected]")
         )
+        self.class_1_at_school_1 = Class.objects.get(name="Class 1 @ School 1")
 
     def test_validate_teacher__not_in_school(self):
-        """
-        Teacher must be in school.
-        """
-
-        teacher = Teacher.objects.exclude(
-            school=self.school_teacher_user.teacher.school
-        ).first()
-        assert teacher
-
+        """Teacher must be in school."""
         self.assert_validate_field(
             name="teacher",
-            value=teacher.id,
+            value=self.non_admin_school_2_teacher_user.teacher,
             error_code="not_in_school",
             context={
                 "request": self.request_factory.post(
-                    user=self.school_teacher_user
+                    user=self.non_admin_school_1_teacher_user
                 )
             },
         )
 
     def test_validate_teacher__not_admin(self):
-        """
-        Teacher cannot assign another teacher if they're not an admin.
-        """
-
-        assert not self.school_teacher_user.teacher.is_admin
-
+        """Teacher cannot assign another teacher if they're not an admin."""
         teacher = (
             Teacher.objects.filter(
-                school=self.school_teacher_user.teacher.school
+                school=self.non_admin_school_1_teacher_user.teacher.school
             )
-            .exclude(pk=self.school_teacher_user.teacher.pk)
+            .exclude(pk=self.non_admin_school_1_teacher_user.teacher.pk)
             .first()
         )
         assert teacher
 
         self.assert_validate_field(
             name="teacher",
-            value=teacher.id,
+            value=teacher,
             error_code="not_admin",
             context={
                 "request": self.request_factory.post(
-                    user=self.school_teacher_user
+                    user=self.non_admin_school_1_teacher_user
                 )
             },
         )
 
     def test_validate_name__name_not_unique(self):
-        """
-        Class names must be unique per school.
-        """
-
+        """Class names must be unique per school."""
         self.assert_validate_field(
             name="name",
-            value=self.class_1.name,
+            value=self.class_1_at_school_1.name,
             error_code="name_not_unique",
             context={
                 "request": self.request_factory.post(
-                    user=self.school_teacher_user
+                    user=self.non_admin_school_1_teacher_user
                 )
             },
         )
 
     def test_create__teacher(self):
-        """
-        Can successfully create with setting the teacher field.
-        """
-
+        """Can successfully create with setting the teacher field."""
         self.assert_create(
             {
                 "name": "ExampleClass",
-                "teacher": {
-                    "id": self.school_teacher_user.teacher.id,
-                },
+                "teacher": self.non_admin_school_1_teacher_user.teacher,
                 "classmates_data_viewable": False,
             }
         )
 
     def test_create__no_teacher(self):
-        """
-        Can successfully create without setting the teacher field.
-        """
-
+        """Can successfully create without setting the teacher field."""
         self.assert_create(
             {
                 "name": "ExampleClass",
                 "classmates_data_viewable": False,
             },
             new_data={
-                "teacher": self.school_teacher_user.teacher.id,
+                "teacher": self.non_admin_school_1_teacher_user.teacher.id,
             },
             context={
                 "request": self.request_factory.post(
-                    user=self.school_teacher_user
+                    user=self.non_admin_school_1_teacher_user
                 ),
             },
         )

backend/api/tests/views/test_klass.py

@@ -3,23 +3,29 @@
 Created on 05/02/2024 at 16:13:46(+00:00).
 """
 
+import typing as t
 from datetime import timedelta
 
 from codeforlife.permissions import OR, AllowNone
 from codeforlife.tests import ModelViewSetTestCase
-from codeforlife.user.models import Class, Teacher
+from codeforlife.user.models import AdminSchoolTeacherUser, Class, Teacher
 from codeforlife.user.permissions import IsStudent, IsTeacher
 from django.utils import timezone
 
 from ...views import ClassViewSet
 
 
-# pylint: disable-next=missing-class-docstring
+# pylint: disable-next=missing-class-docstring,too-many-ancestors
 class TestClassViewSet(ModelViewSetTestCase[Class]):
     basename = "class"
     model_view_set_class = ClassViewSet
     fixtures = ["school_1"]
 
+    def setUp(self):
+        self.admin_school_teacher_user = AdminSchoolTeacherUser.objects.get(
+            email="[email protected]"
+        )
+
     # test: get permissions
 
     def test_get_permissions__bulk(self):
@@ -36,13 +42,13 @@ def test_get_permissions__create(self):
             action="create",
         )
 
-    def test_get_permissions__update(self):
+    def test_get_permissions__partial_update(self):
         """Only admin-teachers or class-teachers can update a class."""
         self.assert_get_permissions(
             permissions=[
                 OR(IsTeacher(is_admin=True), IsTeacher(in_class=True))
             ],
-            action="update",
+            action="partial_update",
         )
 
     def test_get_permissions__destroy(self):
@@ -77,15 +83,13 @@ def test_get_permissions__retrieve(self):
             action="retrieve",
         )
 
+    # test: default actions
+
     def test_create__self(self):
-        """
-        Teacher can create a class with themself as the class owner.
-        """
+        """Teacher can create a class with themself as the class owner."""
+        user = self.admin_school_teacher_user
 
-        user = self.client.login_school_teacher(
-            email="[email protected]",
-            password="password",
-        )
+        self.client.login_as(user)
 
         self.client.create(
             {
@@ -98,13 +102,11 @@ def test_create__self(self):
 
     def test_create__other(self):
         """
-        Teacher can create a class with another teacher as the class owner.
+        Admin-teacher can create a class with another teacher as the class owner.
         """
+        user = self.admin_school_teacher_user
 
-        user = self.client.login_admin_school_teacher(
-            email="[email protected]",
-            password="password",
-        )
+        self.client.login_as(user)
 
         teacher = (
             Teacher.objects.filter(school=user.teacher.school)
@@ -122,3 +124,22 @@ def test_create__other(self):
                 "teacher": teacher.id,
             },
         )
+
+    def test_partial_update__teacher(self):
+        """Teacher can transfer a class to another teacher."""
+        user = self.admin_school_teacher_user
+
+        self.client.login_as(user)
+
+        klass = t.cast(t.Optional[Class], user.teacher.class_teacher.first())
+        assert klass
+
+        teacher = t.cast(
+            Teacher,
+            user.teacher.school.teacher_school.exclude(
+                pk=user.teacher.pk
+            ).first(),
+        )
+        assert teacher
+
+        self.client.partial_update(model=klass, data={"teacher": teacher.pk})

backend/api/views/klass.py

@@ -21,7 +21,7 @@ def get_permissions(self):
             return [AllowNone()]
         if self.action == "create":
             return [IsTeacher(in_school=True)]
-        if self.action in ["update", "destroy"]:
+        if self.action in ["partial_update", "destroy"]:
             return [OR(IsTeacher(is_admin=True), IsTeacher(in_class=True))]
 
         return super().get_permissions()