From 14403ef2ac27f0d7dcb5d5cad6c74243b10cbefc Mon Sep 17 00:00:00 2001 From: SKairinos Date: Fri, 20 Sep 2024 17:30:52 +0000 Subject: [PATCH] fix: class queryset and filters --- codeforlife/user/filters/klass.py | 13 +++++-- codeforlife/user/models/teacher.py | 6 +--- codeforlife/user/views/klass.py | 15 ++------ codeforlife/user/views/klass_test.py | 54 ++++++++++++---------------- 4 files changed, 36 insertions(+), 52 deletions(-) diff --git a/codeforlife/user/filters/klass.py b/codeforlife/user/filters/klass.py index 2e85aa98..7e832290 100644 --- a/codeforlife/user/filters/klass.py +++ b/codeforlife/user/filters/klass.py @@ -3,12 +3,19 @@ Created on 24/07/2024 at 13:19:57(+01:00). """ -from ...filters import FilterSet -from ..models import Class +from django_filters import ( # type: ignore[import-untyped] # isort: skip + rest_framework as filters, +) + +from ...filters import FilterSet # isort: skip +from ..models import Class # isort: skip # pylint: disable-next=missing-class-docstring class ClassFilterSet(FilterSet): + _id = filters.CharFilter(method="_id__method") + _id__method = FilterSet.make_exclude_field_list_method("access_code") + class Meta: model = Class - fields = ["teacher"] + fields = ["_id", "teacher"] diff --git a/codeforlife/user/models/teacher.py b/codeforlife/user/models/teacher.py index 3ce2d699..660220da 100644 --- a/codeforlife/user/models/teacher.py +++ b/codeforlife/user/models/teacher.py @@ -66,11 +66,7 @@ def students(self): @property def classes(self): """All classes the teacher can query.""" - return ( - Class.objects.filter(teacher__school=self.school) - if self.is_admin - else self.class_teacher.all() - ) + return Class.objects.filter(teacher__school=self.school) @property def indy_users(self): diff --git a/codeforlife/user/views/klass.py b/codeforlife/user/views/klass.py index ddfdd71a..3da9c783 100644 --- a/codeforlife/user/views/klass.py +++ b/codeforlife/user/views/klass.py @@ -22,14 +22,9 @@ class ClassViewSet(ModelViewSet[RequestUser, Class]): def get_permissions(self): # Only school-teachers can list classes. if self.action == "list": - return [OR(IsTeacher(is_admin=True), IsTeacher(in_class=True))] + return [IsTeacher(in_school=True)] - return [ - OR( - IsStudent(), - OR(IsTeacher(is_admin=True), IsTeacher(in_class=True)), - ) - ] + return [OR(IsStudent(), IsTeacher(in_school=True))] # pylint: disable-next=missing-function-docstring def get_queryset(self): @@ -37,8 +32,4 @@ def get_queryset(self): if user.student: return Class.objects.filter(students=user.student) - user = self.request.school_teacher_user - if user.teacher.is_admin: - return Class.objects.filter(teacher__school=user.teacher.school) - - return Class.objects.filter(teacher=user.teacher) + return self.request.school_teacher_user.teacher.classes diff --git a/codeforlife/user/views/klass_test.py b/codeforlife/user/views/klass_test.py index c84bcd25..8fe707bd 100644 --- a/codeforlife/user/views/klass_test.py +++ b/codeforlife/user/views/klass_test.py @@ -5,13 +5,7 @@ from ...permissions import OR from ...tests import ModelViewSetTestCase -from ..models import ( - AdminSchoolTeacherUser, - Class, - NonAdminSchoolTeacherUser, - StudentUser, - User, -) +from ..models import AdminSchoolTeacherUser, Class, StudentUser, User from ..permissions import IsStudent, IsTeacher from ..views import ClassViewSet @@ -34,21 +28,14 @@ def setUp(self): def test_get_permissions__list(self): """Only school-teachers can list classes.""" self.assert_get_permissions( - permissions=[ - OR(IsTeacher(is_admin=True), IsTeacher(in_class=True)) - ], + permissions=[IsTeacher(in_school=True)], action="list", ) def test_get_permissions__retrieve(self): """Anyone in a school can retrieve a class.""" self.assert_get_permissions( - permissions=[ - OR( - IsStudent(), - OR(IsTeacher(is_admin=True), IsTeacher(in_class=True)), - ) - ], + permissions=[OR(IsStudent(), IsTeacher(in_school=True))], action="retrieve", ) @@ -64,10 +51,8 @@ def test_get_queryset__student(self): request=self.client.request_factory.get(user=user), ) - def test_get_queryset__teacher__admin(self): - """ - Admin-teacher-users can only target all the classes in their school. - """ + def test_get_queryset__teacher(self): + """Teacher-users can only target all the classes in their school.""" user = AdminSchoolTeacherUser.objects.first() assert user @@ -76,18 +61,6 @@ def test_get_queryset__teacher__admin(self): request=self.client.request_factory.get(user=user), ) - def test_get_queryset__teacher__non_admin(self): - """ - Non-admin-teacher-users can only target all the classes they teach. - """ - user = NonAdminSchoolTeacherUser.objects.first() - assert user - - self.assert_get_queryset( - values=user.teacher.classes, - request=self.client.request_factory.get(user=user), - ) - # test: actions def test_retrieve(self): @@ -107,6 +80,23 @@ def test_list(self): self.client.login_as(user) self.client.list(models=user.teacher.classes.all()) + def test_list___id(self): + """Can successfully list classes in a school, excluding some by ID.""" + user = self.admin_school_teacher_user + assert user + + classes = user.teacher.classes + assert classes.count() >= 2 + + first_class = classes[0] + classes = classes[1:] + + self.client.login_as(user) + self.client.list( + models=classes, + filters={"_id": first_class.access_code}, + ) + def test_list__teacher(self): """Can successfully list classes assigned to a teacher.""" user = self.admin_school_teacher_user