Skip to content

Commit

Permalink
Update teacher (#263)
Browse files Browse the repository at this point in the history
* create school

* remove is_unique_email action

* merge from development

* fix unit tests

* Merge branch 'development' into create_class

* everything but the serializer

* fix: create class

* Merge branch 'development' into create_class

* fix feedback

* use py package v0.12.8

* update teacher

* merge from dev

* fix child serializers
  • Loading branch information
SKairinos authored Feb 9, 2024
1 parent 233dc83 commit 80968af
Show file tree
Hide file tree
Showing 11 changed files with 249 additions and 82 deletions.
12 changes: 6 additions & 6 deletions backend/Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ google-cloud-logging = "==1.*"
google-auth = "==2.*"
google-cloud-container = "==2.3.0"
# "django-anymail[amazon_ses]" = "==7.0.*"
codeforlife = {ref = "v0.12.8", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
django = "==3.2.23"
codeforlife = {ref = "v0.13.0", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"}
django = "==3.2.24"
djangorestframework = "==3.13.1"
django-cors-headers = "==4.1.0"
# https://pypi.org/user/codeforlife/
cfl-common = "==6.40.0" # TODO: remove
codeforlife-portal = "==6.40.0" # TODO: remove
aimmo = "==2.11.0" # TODO: remove
rapid-router = "==5.16.12" # TODO: remove
cfl-common = "==6.40.1" # TODO: remove
codeforlife-portal = "==6.40.1" # TODO: remove
aimmo = "==2.11.1" # TODO: remove
rapid-router = "==5.16.13" # TODO: remove
phonenumbers = "==8.12.12" # TODO: remove

[dev-packages]
Expand Down
40 changes: 20 additions & 20 deletions backend/Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 12 additions & 8 deletions backend/api/serializers/student.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import typing as t

from codeforlife.user.models import Class, Teacher
from codeforlife.user.models import Class, User
from codeforlife.user.serializers import StudentSerializer as _StudentSerializer
from rest_framework import serializers

Expand All @@ -20,13 +20,7 @@ class Meta(_StudentSerializer.Meta):
# pylint: disable-next=missing-function-docstring
def validate_klass(self, value: str):
# Only teachers can manage students.
teacher = t.cast(Teacher, self.request_user.teacher)

if teacher.school is None:
raise serializers.ValidationError(
"The requesting teacher must be in a school.",
code="teacher_not_in_school",
)
teacher = self.request_school_teacher_user.teacher

try:
klass = Class.objects.get(access_code=value)
Expand All @@ -48,3 +42,13 @@ def validate_klass(self, value: str):
)

return value

def validate(self, attrs):
instance = t.cast(t.Optional[User], self.parent.instance)
if instance and not instance.student:
raise serializers.ValidationError(
"Target user is not a student.",
code="not_student",
)

return attrs
34 changes: 32 additions & 2 deletions backend/api/serializers/teacher.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,40 @@
Created on 29/01/2024 at 10:13:58(+00:00).
"""

import typing as t

from codeforlife.user.models import User
from codeforlife.user.serializers import TeacherSerializer as _TeacherSerializer
from rest_framework import serializers


# pylint: disable-next=missing-class-docstring
# pylint: disable-next=missing-class-docstring,too-many-ancestors
class TeacherSerializer(_TeacherSerializer):
class Meta(_TeacherSerializer.Meta):
pass
extra_kwargs = {
**_TeacherSerializer.Meta.extra_kwargs,
"is_admin": {"read_only": False},
}

# pylint: disable-next=missing-function-docstring
def validate_is_admin(self, value: bool):
instance = t.cast(t.Optional[User], self.parent.instance)
if instance:
user = self.request_school_teacher_user
if user.pk == instance.pk:
raise serializers.ValidationError(
"Cannot update own permissions.",
code="is_self",
)

return value

def validate(self, attrs):
instance = t.cast(t.Optional[User], self.parent.instance)
if instance and not instance.teacher:
raise serializers.ValidationError(
"Target user is not a teacher.",
code="not_teacher",
)

return attrs
18 changes: 13 additions & 5 deletions backend/api/tests/serializers/test_klass.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@


# pylint: disable-next=missing-class-docstring
class ClassSerializerTestCase(ModelSerializerTestCase[Class]):
class TestClassSerializer(ModelSerializerTestCase[Class]):
model_serializer_class = ClassSerializer
fixtures = ["school_1"]

Expand Down Expand Up @@ -46,7 +46,9 @@ def test_validate_teacher__not_in_school(self):
value=teacher.id,
error_code="not_in_school",
context={
"request": self.init_request("POST", self.school_teacher_user)
"request": self.request_factory.post(
user=self.school_teacher_user
)
},
)

Expand All @@ -71,7 +73,9 @@ def test_validate_teacher__not_admin(self):
value=teacher.id,
error_code="not_admin",
context={
"request": self.init_request("POST", self.school_teacher_user)
"request": self.request_factory.post(
user=self.school_teacher_user
)
},
)

Expand All @@ -85,7 +89,9 @@ def test_validate_name__name_not_unique(self):
value=self.class_1.name,
error_code="name_not_unique",
context={
"request": self.init_request("POST", self.school_teacher_user)
"request": self.request_factory.post(
user=self.school_teacher_user
)
},
)

Expand Down Expand Up @@ -118,6 +124,8 @@ def test_create__no_teacher(self):
"teacher": self.school_teacher_user.teacher.id,
},
context={
"request": self.init_request("POST", self.school_teacher_user),
"request": self.request_factory.post(
user=self.school_teacher_user
),
},
)
2 changes: 1 addition & 1 deletion backend/api/tests/serializers/test_school.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@


# pylint: disable-next=missing-class-docstring
class SchoolSerializerTestCase(ModelSerializerTestCase[School]):
class TestSchoolSerializer(ModelSerializerTestCase[School]):
model_serializer_class = SchoolSerializer
fixtures = ["school_1"]

Expand Down
66 changes: 36 additions & 30 deletions backend/api/tests/serializers/test_student.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,41 @@
"""

from codeforlife.tests import ModelSerializerTestCase
from codeforlife.user.models import Class, Student, User
from codeforlife.user.models import (
Class,
NonSchoolTeacherUser,
SchoolTeacherUser,
Student,
TeacherUser,
)

from ...serializers import StudentSerializer
from ...serializers import StudentSerializer, UserSerializer


# pylint: disable-next=missing-class-docstring
class StudentSerializerTestCase(ModelSerializerTestCase[Student]):
class TestStudentSerializer(ModelSerializerTestCase[Student]):
model_serializer_class = StudentSerializer
fixtures = [
"non_school_teacher",
"school_1",
"school_2",
]

def test_validate_klass__teacher_not_in_school(self):
"""
Requesting teacher cannot assign a student to a class if they're not in
a school.
"""

user = User.objects.get(email="[email protected]")
assert user.teacher and not user.teacher.school

self.assert_validate_field(
name="klass",
value="",
error_code="teacher_not_in_school",
context={"request": self.init_request("POST", user)},
)

def test_validate_klass__does_not_exist(self):
"""
Requesting teacher cannot assign a student to a class that doesn't
exist.
"""

user = User.objects.get(email="[email protected]")
assert user.teacher and user.teacher.school
user = SchoolTeacherUser.objects.get(email="[email protected]")

self.assert_validate_field(
name="klass",
value="",
error_code="does_not_exist",
context={"request": self.init_request("POST", user)},
parent=UserSerializer(
context={"request": self.request_factory.post(user=user)}
),
)

def test_validate_klass__teacher_not_in_same_school(self):
Expand All @@ -56,8 +47,7 @@ def test_validate_klass__teacher_not_in_same_school(self):
the same school.
"""

user = User.objects.get(email="[email protected]")
assert user.teacher and user.teacher.school
user = SchoolTeacherUser.objects.get(email="[email protected]")

klass = Class.objects.exclude(
teacher__school=user.teacher.school
Expand All @@ -68,7 +58,9 @@ def test_validate_klass__teacher_not_in_same_school(self):
name="klass",
value=klass.access_code,
error_code="teacher_not_in_same_school",
context={"request": self.init_request("POST", user)},
parent=UserSerializer(
context={"request": self.request_factory.post(user=user)}
),
)

def test_validate_klass__teacher_not_admin_or_class_owner(self):
Expand All @@ -77,10 +69,8 @@ def test_validate_klass__teacher_not_admin_or_class_owner(self):
admin or they don't own the class.
"""

user = User.objects.get(email="[email protected]")
assert (
user.teacher and user.teacher.school and not user.teacher.is_admin
)
user = SchoolTeacherUser.objects.get(email="[email protected]")
assert not user.teacher.is_admin

klass = (
Class.objects.filter(teacher__school=user.teacher.school)
Expand All @@ -93,5 +83,21 @@ def test_validate_klass__teacher_not_admin_or_class_owner(self):
name="klass",
value=klass.access_code,
error_code="teacher_not_admin_or_class_owner",
context={"request": self.init_request("POST", user)},
parent=UserSerializer(
context={"request": self.request_factory.post(user=user)}
),
)

def test_validate__not_student(self):
"""
Target user must be a student.
"""

teacher_user = TeacherUser.objects.first()
assert teacher_user

self.assert_validate(
attrs={},
error_code="not_student",
parent=UserSerializer(instance=teacher_user),
)
Loading

0 comments on commit 80968af

Please sign in to comment.