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 frontend19 #127

Merged
merged 7 commits into from
Aug 7, 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
33 changes: 33 additions & 0 deletions codeforlife/filters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""
© Ocado Group
Created on 26/07/2024 at 11:26:14(+01:00).
"""

from django.db.models.query import QuerySet

# pylint: disable-next=line-too-long
from django_filters.rest_framework import ( # type: ignore[import-untyped] # isort: skip
FilterSet as _FilterSet,
)


class FilterSet(_FilterSet):
"""Base filter set all other filter sets must inherit."""

@staticmethod
def make_exclude_field_list_method(field: str):
"""Make a class-method that excludes a list of values for a field.

Args:
field: The field to exclude a list of values for.

Returns:
A class-method.
"""

def method(self: FilterSet, queryset: QuerySet, name: str, *args):
return queryset.exclude(
**{f"{field}__in": self.request.GET.getlist(name)}
)

return method
15 changes: 12 additions & 3 deletions codeforlife/tests/model_view_set.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,12 @@ def retrieve(

def list(
self,
models: t.Iterable[AnyModel],
models: t.Collection[AnyModel],
status_code_assertion: APIClient.StatusCodeAssertion = (
status.HTTP_200_OK
),
make_assertions: bool = True,
filters: t.Optional[t.Dict[str, str]] = None,
filters: t.Optional[t.Dict[str, t.Union[str, t.Iterable[str]]]] = None,
reverse_kwargs: t.Optional[KwArgs] = None,
**kwargs,
):
Expand All @@ -221,10 +221,18 @@ def list(
"""
# pylint: enable=line-too-long

query: t.List[t.Tuple[str, str]] = []
for key, values in (filters or {}).items():
if isinstance(values, str):
query.append((key, values))
else:
for value in values:
query.append((key, value))

response: Response = self.get(
(
self._test_case.reverse_action("list", kwargs=reverse_kwargs)
+ f"?{urlencode(filters or {})}"
+ f"?{urlencode(query)}"
),
status_code_assertion=status_code_assertion,
**kwargs,
Expand All @@ -234,6 +242,7 @@ def list(

def _make_assertions(response_json: JsonDict):
json_models = t.cast(t.List[JsonDict], response_json["data"])
assert len(models) == len(json_models)
for model, json_model in zip(models, json_models):
self._test_case.assert_serialized_model_equals_json_model(
model, json_model, action="list", request_method="get"
Expand Down
1 change: 1 addition & 0 deletions codeforlife/user/filters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@
Created on 12/04/2024 at 14:52:22(+01:00).
"""

from .klass import ClassFilterSet
from .user import UserFilterSet
14 changes: 14 additions & 0 deletions codeforlife/user/filters/klass.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
"""
© Ocado Group
Created on 24/07/2024 at 13:19:57(+01:00).
"""

from ...filters import FilterSet
from ..models import Class


# pylint: disable-next=missing-class-docstring
class ClassFilterSet(FilterSet):
class Meta:
model = Class
fields = ["teacher"]
44 changes: 41 additions & 3 deletions codeforlife/user/filters/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,58 @@
Created on 03/04/2024 at 16:37:44(+01:00).
"""

import typing as t

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 ..models import User
from ...filters import FilterSet # isort: skip
from ..models import User # isort: skip


# pylint: disable-next=missing-class-docstring
class UserFilterSet(filters.FilterSet):
class UserFilterSet(FilterSet):
students_in_class = filters.CharFilter(
"new_student__class_field__access_code",
"exact",
)

_id = filters.NumberFilter(method="_id_method")
_id_method = FilterSet.make_exclude_field_list_method("id")

name = filters.CharFilter(method="name_method")

only_teachers = filters.BooleanFilter(method="only_teachers__method")

def name_method(
self: FilterSet, queryset: QuerySet[User], name: str, *args
):
"""Get all first names and last names that contain a substring."""
names = t.cast(str, self.request.GET[name]).split(" ", maxsplit=1)
first_name, last_name = (
names if len(names) == 2 else (names[0], names[0])
)

# TODO: use PostgreSQL specific lookup
# https://docs.djangoproject.com/en/5.0/ref/contrib/postgres/lookups/#std-fieldlookup-trigram_similar
return queryset.filter(
Q(first_name__icontains=first_name)
| Q(last_name__icontains=last_name)
)

def only_teachers__method(
self: FilterSet, queryset: QuerySet[User], _: str, value: bool
):
"""Get only teacher-users."""
return (
queryset.filter(new_teacher__isnull=False, new_student__isnull=True)
if value
else queryset
)

class Meta:
model = User
fields = ["students_in_class"]
fields = ["students_in_class", "only_teachers", "_id", "name"]
48 changes: 35 additions & 13 deletions codeforlife/user/models/teacher.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from common.models import Teacher, TeacherModelManager
from django.db import models
from django.db.models import Q

from .klass import Class
from .school import School
Expand Down Expand Up @@ -81,6 +82,40 @@
new_student__pending_class_request__in=self.classes
)

@property
def school_teacher_users(self):
"""All school-teacher-users the teacher can query."""
# pylint: disable-next=import-outside-toplevel
from .user import SchoolTeacherUser

return SchoolTeacherUser.objects.filter(new_teacher__school=self.school)

@property
def school_teachers(self):
"""All school-teachers the teacher can query."""
return SchoolTeacher.objects.filter(school=self.school)

Check warning on line 96 in codeforlife/user/models/teacher.py

View check run for this annotation

Codecov / codecov/patch

codeforlife/user/models/teacher.py#L96

Added line #L96 was not covered by tests

@property
def school_users(self):
"""All users in the school the teacher can query."""
# pylint: disable-next=import-outside-toplevel
from .user import User

return User.objects.filter(
Q( # student-users
new_teacher__isnull=True,
**(
{"new_student__class_field__teacher__school": self.school}
if self.is_admin
else {"new_student__class_field__teacher": self}
)
)
| Q( # school-teacher-users
new_student__isnull=True,
new_teacher__school=self.school,
)
)


class AdminSchoolTeacher(SchoolTeacher):
"""An admin-teacher that is in a school."""
Expand All @@ -106,19 +141,6 @@
.exists()
)

@property
def school_teacher_users(self):
"""All school-teacher-users the teacher can query."""
# pylint: disable-next=import-outside-toplevel
from .user import SchoolTeacherUser

return SchoolTeacherUser.objects.filter(new_teacher__school=self.school)

@property
def school_teachers(self):
"""All school-teachers the teacher can query."""
return SchoolTeacher.objects.filter(school=self.school)


class NonAdminSchoolTeacher(SchoolTeacher):
"""A non-admin-teacher that is in a school."""
Expand Down
2 changes: 2 additions & 0 deletions codeforlife/user/views/klass.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from ...permissions import OR
from ...views import ModelViewSet
from ..filters import ClassFilterSet
from ..models import Class
from ..models import User as RequestUser
from ..permissions import IsStudent, IsTeacher
Expand All @@ -16,6 +17,7 @@ class ClassViewSet(ModelViewSet[RequestUser, Class]):
http_method_names = ["get"]
lookup_field = "access_code"
serializer_class = ClassSerializer
filterset_class = ClassFilterSet

def get_permissions(self):
# Only school-teachers can list classes.
Expand Down
12 changes: 12 additions & 0 deletions codeforlife/user/views/klass_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,15 @@ def test_list(self):

self.client.login_as(user)
self.client.list(models=user.teacher.classes.all())

def test_list__teacher(self):
"""Can successfully list classes assigned to a teacher."""
user = self.admin_school_teacher_user
classes = Class.objects.filter(teacher=user.teacher)
assert classes.exists()

self.client.login_as(user)
self.client.list(
models=classes,
filters={"teacher": str(user.teacher.id)},
)
54 changes: 54 additions & 0 deletions codeforlife/user/views/user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,60 @@ def test_list__students_in_class(self):
filters={"students_in_class": klass.access_code},
)

def test_list__only_teachers(self):
"""Can successfully list only teacher-users."""
user = self.admin_school_teacher_user
school_teacher_users = user.teacher.school_teacher_users.all()
assert school_teacher_users.exists()

self.client.login_as(user)
self.client.list(
models=school_teacher_users,
filters={"only_teachers": str(True)},
)

def test_list___id(self):
"""Can successfully list all users in a school and exclude some IDs."""
user = AdminSchoolTeacherUser.objects.first()
assert user

users = [
*list(user.teacher.school_teacher_users),
*list(user.teacher.student_users),
]
users.sort(key=lambda user: user.pk)

exclude_user_1: User = users.pop()
exclude_user_2: User = users.pop()

self.client.login_as(user, password="abc123")
self.client.list(
models=users,
filters={
"_id": [
str(exclude_user_1.id),
str(exclude_user_2.id),
]
},
)

def test_list__name(self):
"""Can successfully list all users by name."""
user = AdminSchoolTeacherUser.objects.first()
assert user

school_users = user.teacher.school_users
first_name, last_name = user.first_name, user.last_name[:1]

self.client.login_as(user, password="abc123")
self.client.list(
models=(
school_users.filter(first_name__icontains=first_name)
| school_users.filter(last_name__icontains=last_name)
),
filters={"name": f"{first_name} {last_name}"},
)

def test_retrieve(self):
"""Can successfully retrieve users."""
user = AdminSchoolTeacherUser.objects.first()
Expand Down