Skip to content

Commit

Permalink
fix: Portal frontend 52 (#136)
Browse files Browse the repository at this point in the history
* fix: class queryset and filters

* search classes by their id or name

* feedback
  • Loading branch information
SKairinos authored Sep 25, 2024
1 parent d1590f6 commit 5460bbf
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 52 deletions.
23 changes: 20 additions & 3 deletions codeforlife/user/filters/klass.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,29 @@
Created on 24/07/2024 at 13:19:57(+01:00).
"""

from ...filters import FilterSet
from ..models import Class
from django.db.models import Q # isort: skip
from django.db.models.query import QuerySet # isort: skip
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")

id_or_name = filters.CharFilter(method="id_or_name__method")

def id_or_name__method(self, queryset: QuerySet[Class], _: str, value: str):
"""Get classes where the id or the name contain a substring."""
return queryset.filter(
Q(access_code__icontains=value) | Q(name__icontains=value)
)

class Meta:
model = Class
fields = ["teacher"]
fields = ["_id", "teacher", "id_or_name"]
6 changes: 1 addition & 5 deletions codeforlife/user/models/teacher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
15 changes: 3 additions & 12 deletions codeforlife/user/views/klass.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,14 @@ 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):
user = self.request.auth_user
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
79 changes: 47 additions & 32 deletions codeforlife/user/views/klass_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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",
)

Expand All @@ -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

Expand All @@ -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):
Expand All @@ -107,6 +80,48 @@ 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__id_or_name(self):
"""
Can successfully list classes in a school, filtered by their ID or name.
"""
user = self.admin_school_teacher_user
assert user

klass = user.teacher.classes.first()
assert klass

partial_access_code = klass.access_code[:-1]
partial_name = klass.name[:-1]

self.client.login_as(user)
self.client.list(
models=user.teacher.classes.filter(
access_code__icontains=partial_access_code
),
filters={"id_or_name": partial_access_code},
)
self.client.list(
models=user.teacher.classes.filter(name__icontains=partial_name),
filters={"id_or_name": partial_name},
)

def test_list__teacher(self):
"""Can successfully list classes assigned to a teacher."""
user = self.admin_school_teacher_user
Expand Down

0 comments on commit 5460bbf

Please sign in to comment.