diff --git a/backend/Pipfile b/backend/Pipfile index c6005685..eb003004 100644 --- a/backend/Pipfile +++ b/backend/Pipfile @@ -28,7 +28,7 @@ google-cloud-logging = "==1.*" google-auth = "==2.*" google-cloud-container = "==2.3.0" # "django-anymail[amazon_ses]" = "==7.0.*" -codeforlife = {ref = "v0.12.4", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} +codeforlife = {ref = "v0.12.8", git = "https://github.com/ocadotechnology/codeforlife-package-python.git"} django = "==3.2.23" djangorestframework = "==3.13.1" django-cors-headers = "==4.1.0" diff --git a/backend/Pipfile.lock b/backend/Pipfile.lock index c7954a72..b0811cad 100644 --- a/backend/Pipfile.lock +++ b/backend/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "6ef3be9e49bc02002dd44d1f3b17d6822f638ed1afe1408dcda45de95fa4b043" + "sha256": "eaaeae63a1ea6566c6008ffc0f3ecd38cbb4a3c8e36db2ce148bec03125b0442" }, "pipfile-spec": 6, "requires": { @@ -170,7 +170,7 @@ }, "codeforlife": { "git": "https://github.com/ocadotechnology/codeforlife-package-python.git", - "ref": "8afb1bb8d20aac1d091886dadaed906b52a1907a" + "ref": "8e078930e3f1e6796d939cde3bf847711140916d" }, "codeforlife-portal": { "hashes": [ @@ -816,7 +816,6 @@ "sha256:a6f5977418eff3b2d5500d54d9db50c8277a368436f4e4f8ddb1be3422870184", "sha256:f91456ead12ab3c6c2e9491cf33ba6d08357d802192379bb482f1033ade496f5" ], - "markers": "python_version >= '3.6'", "version": "==3.1.2" }, "pandas": { @@ -1317,7 +1316,6 @@ "sha256:6a33ee89877bd9abc1158129f6e94be74e2679636b8a205b43b85206c3f0bbdd", "sha256:f72f148f54442c6b056bf931dbc34f986fd0c3b0b6b5a58d013c9aef274d0c88" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'", "version": "==2.0.1" }, "xlwt": { @@ -1768,7 +1766,6 @@ "sha256:a6f5977418eff3b2d5500d54d9db50c8277a368436f4e4f8ddb1be3422870184", "sha256:f91456ead12ab3c6c2e9491cf33ba6d08357d802192379bb482f1033ade496f5" ], - "markers": "python_version >= '3.6'", "version": "==3.1.2" }, "packaging": { @@ -2123,7 +2120,6 @@ "sha256:6a33ee89877bd9abc1158129f6e94be74e2679636b8a205b43b85206c3f0bbdd", "sha256:f72f148f54442c6b056bf931dbc34f986fd0c3b0b6b5a58d013c9aef274d0c88" ], - "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'", "version": "==2.0.1" }, "xlwt": { @@ -2134,4 +2130,4 @@ "version": "==1.3.0" } } -} \ No newline at end of file +} diff --git a/backend/api/serializers/klass.py b/backend/api/serializers/klass.py index 26caabb3..8b758535 100644 --- a/backend/api/serializers/klass.py +++ b/backend/api/serializers/klass.py @@ -3,10 +3,100 @@ Created on 24/01/2024 at 12:14:21(+00:00). """ +import string + +from codeforlife.user.models import Class, Teacher from codeforlife.user.serializers import ClassSerializer as _ClassSerializer +from django.utils.crypto import get_random_string +from rest_framework import serializers -# pylint: disable-next=missing-class-docstring +# 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", + ) + + receive_requests_until = serializers.DateTimeField( + source="accept_requests_until", + required=False, + ) + class Meta(_ClassSerializer.Meta): - pass + extra_kwargs = { + **_ClassSerializer.Meta.extra_kwargs, + "name": {"read_only": 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", + ) + + user = self.request_school_teacher_user + if not queryset.filter(school=user.teacher.school_id).exists(): + 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: + raise serializers.ValidationError( + "Cannot assign another teacher if you're not admin.", + code="not_admin", + ) + + return value + + # TODO: set unique_together=("name", "school") for in new Class model. + # pylint: disable-next=missing-function-docstring + def validate_name(self, value: str): + if Class.objects.filter( + teacher__school=self.request_school_teacher_user.teacher.school, + name=value, + ).exists(): + raise serializers.ValidationError( + "Name already taken.", + code="name_not_unique", + ) + + return value + + def create(self, validated_data): + # TODO: move generation logic to new Class model. + access_code = None + while ( + access_code is None + or Class.objects.filter(access_code=access_code).exists() + ): + access_code = get_random_string( + length=5, + allowed_chars=string.ascii_uppercase, + ) + + # TODO: set school to teacher's school on new Class model. + return super().create( + { + "access_code": access_code, + "name": validated_data["name"], + "teacher_id": ( + validated_data["teacher"]["id"] + if "teacher" in validated_data + else self.request_school_teacher_user.teacher.id + ), + "classmates_data_viewable": validated_data[ + "classmates_data_viewable" + ], + "accept_requests_until": validated_data.get( + "accept_requests_until" + ), + } + ) diff --git a/backend/api/serializers/school.py b/backend/api/serializers/school.py index 132c48d5..23909a67 100644 --- a/backend/api/serializers/school.py +++ b/backend/api/serializers/school.py @@ -266,7 +266,6 @@ class SchoolSerializer(_SchoolSerializer): uk_county = serializers.ChoiceField( # type: ignore[assignment] source="county", - default="", choices=[ "Aberdeen City", "Aberdeenshire", diff --git a/backend/api/tests/serializers/test_klass.py b/backend/api/tests/serializers/test_klass.py new file mode 100644 index 00000000..65e6e973 --- /dev/null +++ b/backend/api/tests/serializers/test_klass.py @@ -0,0 +1,123 @@ +""" +© Ocado Group +Created on 05/02/2024 at 15:31:59(+00:00). +""" + +from codeforlife.tests import ModelSerializerTestCase +from codeforlife.user.models import Class, SchoolTeacherUser, Teacher + +from ...serializers import ClassSerializer + + +# pylint: disable-next=missing-class-docstring +class ClassSerializerTestCase(ModelSerializerTestCase[Class]): + model_serializer_class = ClassSerializer + fixtures = ["school_1"] + + def setUp(self): + self.school_teacher_user = SchoolTeacherUser.objects.get( + email="teacher@school1.com" + ) + 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", + ) + + 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 + + self.assert_validate_field( + name="teacher", + value=teacher.id, + error_code="not_in_school", + context={ + "request": self.init_request("POST", self.school_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 = ( + Teacher.objects.filter( + school=self.school_teacher_user.teacher.school + ) + .exclude(pk=self.school_teacher_user.teacher.pk) + .first() + ) + assert teacher + + self.assert_validate_field( + name="teacher", + value=teacher.id, + error_code="not_admin", + context={ + "request": self.init_request("POST", self.school_teacher_user) + }, + ) + + def test_validate_name__name_not_unique(self): + """ + Class names must be unique per school. + """ + + self.assert_validate_field( + name="name", + value=self.class_1.name, + error_code="name_not_unique", + context={ + "request": self.init_request("POST", self.school_teacher_user) + }, + ) + + def test_create__teacher(self): + """ + Can successfully create with setting the teacher field. + """ + + self.assert_create( + { + "name": "ExampleClass", + "teacher": { + "id": self.school_teacher_user.teacher.id, + }, + "classmates_data_viewable": False, + } + ) + + def test_create__no_teacher(self): + """ + 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, + }, + context={ + "request": self.init_request("POST", self.school_teacher_user), + }, + ) diff --git a/backend/api/tests/serializers/test_student.py b/backend/api/tests/serializers/test_student.py index c6ba9053..57d6c27a 100644 --- a/backend/api/tests/serializers/test_student.py +++ b/backend/api/tests/serializers/test_student.py @@ -31,7 +31,7 @@ def test_validate_klass__teacher_not_in_school(self): name="klass", value="", error_code="teacher_not_in_school", - user=user, + context={"request": self.init_request("POST", user)}, ) def test_validate_klass__does_not_exist(self): @@ -47,7 +47,7 @@ def test_validate_klass__does_not_exist(self): name="klass", value="", error_code="does_not_exist", - user=user, + context={"request": self.init_request("POST", user)}, ) def test_validate_klass__teacher_not_in_same_school(self): @@ -68,7 +68,7 @@ def test_validate_klass__teacher_not_in_same_school(self): name="klass", value=klass.access_code, error_code="teacher_not_in_same_school", - user=user, + context={"request": self.init_request("POST", user)}, ) def test_validate_klass__teacher_not_admin_or_class_owner(self): @@ -93,5 +93,5 @@ 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", - user=user, + context={"request": self.init_request("POST", user)}, ) diff --git a/backend/api/tests/views/test_klass.py b/backend/api/tests/views/test_klass.py new file mode 100644 index 00000000..550a8c84 --- /dev/null +++ b/backend/api/tests/views/test_klass.py @@ -0,0 +1,128 @@ +""" +© Ocado Group +Created on 05/02/2024 at 16:13:46(+00:00). +""" + +from datetime import timedelta + +from codeforlife.permissions import AllowNone +from codeforlife.tests import ModelViewSetTestCase +from codeforlife.user.models import Class, Teacher +from codeforlife.user.permissions import InSchool, IsTeacher +from django.utils import timezone + +from ...views import ClassViewSet + + +# pylint: disable-next=missing-class-docstring +class TestClassViewSet(ModelViewSetTestCase[Class]): + basename = "class" + model_view_set_class = ClassViewSet + fixtures = ["school_1"] + + def test_get_permissions__bulk(self): + """ + No one is allowed to perform bulk actions. + """ + + self.assert_get_permissions( + permissions=[AllowNone()], + action="bulk", + ) + + def test_get_permissions__create(self): + """ + Only a school-teacher can create a class. + """ + + self.assert_get_permissions( + permissions=[IsTeacher(), InSchool()], + action="create", + ) + + def test_get_permissions__update(self): + """ + Only a school-teacher can update a class. + """ + + self.assert_get_permissions( + permissions=[IsTeacher(), InSchool()], + action="update", + ) + + def test_get_permissions__destroy(self): + """ + Only a school-teacher can destroy a class. + """ + + self.assert_get_permissions( + permissions=[IsTeacher(), InSchool()], + action="destroy", + ) + + def test_get_permissions__list(self): + """ + Only a school-teacher can list classes. + """ + + self.assert_get_permissions( + permissions=[IsTeacher(), InSchool()], + action="list", + ) + + def test_get_permissions__retrieve(self): + """ + Any school-user can retrieve a class. + """ + + self.assert_get_permissions( + permissions=[InSchool()], + action="retrieve", + ) + + def test_create__self(self): + """ + Teacher can create a class with themself as the class owner. + """ + + user = self.client.login_school_teacher( + email="teacher@school1.com", + password="password", + ) + + self.client.create( + { + "name": "ExampleClass", + "school": user.teacher.school.id, + "read_classmates_data": False, + "receive_requests_until": timezone.now() + timedelta(days=1), + }, + ) + + def test_create__other(self): + """ + Teacher can create a class with another teacher as the class owner. + """ + + user = self.client.login_school_teacher( + email="admin.teacher@school1.com", + password="password", + is_admin=True, + ) + + teacher = ( + Teacher.objects.filter(school=user.teacher.school) + .exclude(pk=user.teacher.pk) + .first() + ) + assert teacher + + self.client.create( + { + "name": "ExampleClass", + "school": user.teacher.school.id, + "read_classmates_data": False, + "receive_requests_until": timezone.now() + timedelta(days=1), + "teacher": teacher.id, + }, + ) diff --git a/backend/api/tests/views/test_school.py b/backend/api/tests/views/test_school.py index fc095573..f5c84f5c 100644 --- a/backend/api/tests/views/test_school.py +++ b/backend/api/tests/views/test_school.py @@ -63,7 +63,6 @@ def test_create(self): self.client.login_non_school_teacher( email="teacher@noschool.com", password="password", - in_school=False, ) self.client.create( diff --git a/backend/api/tests/views/test_user.py b/backend/api/tests/views/test_user.py index 1fb4dfcc..f5e068b4 100644 --- a/backend/api/tests/views/test_user.py +++ b/backend/api/tests/views/test_user.py @@ -49,7 +49,7 @@ def test_bulk_create__students(self): klass: t.Optional[Class] = user.teacher.class_teacher.first() assert klass is not None - self.client.bulk_create( + response = self.client.bulk_create( [ { "first_name": "Peter", @@ -59,9 +59,12 @@ def test_bulk_create__students(self): "first_name": "Mary", "student": {"klass": klass.access_code}, }, - ] + ], + make_assertions=False, ) + response.json() # TODO: make custom assertions and check for password + def test_request_password_reset__invalid_email(self): """ Request password reset doesn't generate reset password URL if email diff --git a/backend/api/views/klass.py b/backend/api/views/klass.py index 052109fd..17e5d1ed 100644 --- a/backend/api/views/klass.py +++ b/backend/api/views/klass.py @@ -3,6 +3,8 @@ Created on 23/01/2024 at 17:53:37(+00:00). """ +from codeforlife.permissions import AllowNone +from codeforlife.user.permissions import InSchool, IsTeacher from codeforlife.user.views import ClassViewSet as _ClassViewSet from ..serializers import ClassSerializer @@ -12,3 +14,12 @@ class ClassViewSet(_ClassViewSet): http_method_names = ["get", "post", "patch", "delete"] serializer_class = ClassSerializer + + def get_permissions(self): + # Bulk actions not allowed for classes. + if self.action == "bulk": + return [AllowNone()] + if self.action in ["create", "update", "destroy"]: + return [IsTeacher(), InSchool()] + + return super().get_permissions() diff --git a/backend/api/views/school.py b/backend/api/views/school.py index 35b13a51..30df17f5 100644 --- a/backend/api/views/school.py +++ b/backend/api/views/school.py @@ -17,7 +17,7 @@ class SchoolViewSet(_SchoolViewSet): def get_permissions(self): # Bulk actions not allowed for schools. - if self.action in ["bulk", "list"]: + if self.action == "bulk": return [AllowNone()] # Only teachers not in a school can create a school. if self.action == "create": @@ -26,4 +26,4 @@ def get_permissions(self): if self.action == "update": return [IsTeacher(is_admin=True), InSchool()] - return [InSchool()] + return super().get_permissions()