Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Portal frontend 52 #136

Merged
merged 3 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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