Skip to content

Commit 2dc2c72

Browse files
authored
fix: Portal frontend19 (#127)
* fix: add class filterset * teachers in school * exclude user ids * name filter * only teachers method * fix isort * feedback
1 parent f988e39 commit 2dc2c72

File tree

9 files changed

+204
-19
lines changed

9 files changed

+204
-19
lines changed

codeforlife/filters.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
"""
2+
© Ocado Group
3+
Created on 26/07/2024 at 11:26:14(+01:00).
4+
"""
5+
6+
from django.db.models.query import QuerySet
7+
8+
# pylint: disable-next=line-too-long
9+
from django_filters.rest_framework import ( # type: ignore[import-untyped] # isort: skip
10+
FilterSet as _FilterSet,
11+
)
12+
13+
14+
class FilterSet(_FilterSet):
15+
"""Base filter set all other filter sets must inherit."""
16+
17+
@staticmethod
18+
def make_exclude_field_list_method(field: str):
19+
"""Make a class-method that excludes a list of values for a field.
20+
21+
Args:
22+
field: The field to exclude a list of values for.
23+
24+
Returns:
25+
A class-method.
26+
"""
27+
28+
def method(self: FilterSet, queryset: QuerySet, name: str, *args):
29+
return queryset.exclude(
30+
**{f"{field}__in": self.request.GET.getlist(name)}
31+
)
32+
33+
return method

codeforlife/tests/model_view_set.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,12 +197,12 @@ def retrieve(
197197

198198
def list(
199199
self,
200-
models: t.Iterable[AnyModel],
200+
models: t.Collection[AnyModel],
201201
status_code_assertion: APIClient.StatusCodeAssertion = (
202202
status.HTTP_200_OK
203203
),
204204
make_assertions: bool = True,
205-
filters: t.Optional[t.Dict[str, str]] = None,
205+
filters: t.Optional[t.Dict[str, t.Union[str, t.Iterable[str]]]] = None,
206206
reverse_kwargs: t.Optional[KwArgs] = None,
207207
**kwargs,
208208
):
@@ -221,10 +221,18 @@ def list(
221221
"""
222222
# pylint: enable=line-too-long
223223

224+
query: t.List[t.Tuple[str, str]] = []
225+
for key, values in (filters or {}).items():
226+
if isinstance(values, str):
227+
query.append((key, values))
228+
else:
229+
for value in values:
230+
query.append((key, value))
231+
224232
response: Response = self.get(
225233
(
226234
self._test_case.reverse_action("list", kwargs=reverse_kwargs)
227-
+ f"?{urlencode(filters or {})}"
235+
+ f"?{urlencode(query)}"
228236
),
229237
status_code_assertion=status_code_assertion,
230238
**kwargs,
@@ -234,6 +242,7 @@ def list(
234242

235243
def _make_assertions(response_json: JsonDict):
236244
json_models = t.cast(t.List[JsonDict], response_json["data"])
245+
assert len(models) == len(json_models)
237246
for model, json_model in zip(models, json_models):
238247
self._test_case.assert_serialized_model_equals_json_model(
239248
model, json_model, action="list", request_method="get"

codeforlife/user/filters/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@
33
Created on 12/04/2024 at 14:52:22(+01:00).
44
"""
55

6+
from .klass import ClassFilterSet
67
from .user import UserFilterSet

codeforlife/user/filters/klass.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
"""
2+
© Ocado Group
3+
Created on 24/07/2024 at 13:19:57(+01:00).
4+
"""
5+
6+
from ...filters import FilterSet
7+
from ..models import Class
8+
9+
10+
# pylint: disable-next=missing-class-docstring
11+
class ClassFilterSet(FilterSet):
12+
class Meta:
13+
model = Class
14+
fields = ["teacher"]

codeforlife/user/filters/user.py

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,58 @@
33
Created on 03/04/2024 at 16:37:44(+01:00).
44
"""
55

6+
import typing as t
7+
8+
from django.db.models import Q # isort: skip
9+
from django.db.models.query import QuerySet # isort: skip
610
from django_filters import ( # type: ignore[import-untyped] # isort: skip
711
rest_framework as filters,
812
)
913

10-
from ..models import User
14+
from ...filters import FilterSet # isort: skip
15+
from ..models import User # isort: skip
1116

1217

1318
# pylint: disable-next=missing-class-docstring
14-
class UserFilterSet(filters.FilterSet):
19+
class UserFilterSet(FilterSet):
1520
students_in_class = filters.CharFilter(
1621
"new_student__class_field__access_code",
1722
"exact",
1823
)
1924

25+
_id = filters.NumberFilter(method="_id_method")
26+
_id_method = FilterSet.make_exclude_field_list_method("id")
27+
28+
name = filters.CharFilter(method="name_method")
29+
30+
only_teachers = filters.BooleanFilter(method="only_teachers__method")
31+
32+
def name_method(
33+
self: FilterSet, queryset: QuerySet[User], name: str, *args
34+
):
35+
"""Get all first names and last names that contain a substring."""
36+
names = t.cast(str, self.request.GET[name]).split(" ", maxsplit=1)
37+
first_name, last_name = (
38+
names if len(names) == 2 else (names[0], names[0])
39+
)
40+
41+
# TODO: use PostgreSQL specific lookup
42+
# https://docs.djangoproject.com/en/5.0/ref/contrib/postgres/lookups/#std-fieldlookup-trigram_similar
43+
return queryset.filter(
44+
Q(first_name__icontains=first_name)
45+
| Q(last_name__icontains=last_name)
46+
)
47+
48+
def only_teachers__method(
49+
self: FilterSet, queryset: QuerySet[User], _: str, value: bool
50+
):
51+
"""Get only teacher-users."""
52+
return (
53+
queryset.filter(new_teacher__isnull=False, new_student__isnull=True)
54+
if value
55+
else queryset
56+
)
57+
2058
class Meta:
2159
model = User
22-
fields = ["students_in_class"]
60+
fields = ["students_in_class", "only_teachers", "_id", "name"]

codeforlife/user/models/teacher.py

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
from common.models import Teacher, TeacherModelManager
1111
from django.db import models
12+
from django.db.models import Q
1213

1314
from .klass import Class
1415
from .school import School
@@ -81,6 +82,40 @@ def indy_users(self):
8182
new_student__pending_class_request__in=self.classes
8283
)
8384

85+
@property
86+
def school_teacher_users(self):
87+
"""All school-teacher-users the teacher can query."""
88+
# pylint: disable-next=import-outside-toplevel
89+
from .user import SchoolTeacherUser
90+
91+
return SchoolTeacherUser.objects.filter(new_teacher__school=self.school)
92+
93+
@property
94+
def school_teachers(self):
95+
"""All school-teachers the teacher can query."""
96+
return SchoolTeacher.objects.filter(school=self.school)
97+
98+
@property
99+
def school_users(self):
100+
"""All users in the school the teacher can query."""
101+
# pylint: disable-next=import-outside-toplevel
102+
from .user import User
103+
104+
return User.objects.filter(
105+
Q( # student-users
106+
new_teacher__isnull=True,
107+
**(
108+
{"new_student__class_field__teacher__school": self.school}
109+
if self.is_admin
110+
else {"new_student__class_field__teacher": self}
111+
)
112+
)
113+
| Q( # school-teacher-users
114+
new_student__isnull=True,
115+
new_teacher__school=self.school,
116+
)
117+
)
118+
84119

85120
class AdminSchoolTeacher(SchoolTeacher):
86121
"""An admin-teacher that is in a school."""
@@ -106,19 +141,6 @@ def is_last_admin(self):
106141
.exists()
107142
)
108143

109-
@property
110-
def school_teacher_users(self):
111-
"""All school-teacher-users the teacher can query."""
112-
# pylint: disable-next=import-outside-toplevel
113-
from .user import SchoolTeacherUser
114-
115-
return SchoolTeacherUser.objects.filter(new_teacher__school=self.school)
116-
117-
@property
118-
def school_teachers(self):
119-
"""All school-teachers the teacher can query."""
120-
return SchoolTeacher.objects.filter(school=self.school)
121-
122144

123145
class NonAdminSchoolTeacher(SchoolTeacher):
124146
"""A non-admin-teacher that is in a school."""

codeforlife/user/views/klass.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from ...permissions import OR
77
from ...views import ModelViewSet
8+
from ..filters import ClassFilterSet
89
from ..models import Class
910
from ..models import User as RequestUser
1011
from ..permissions import IsStudent, IsTeacher
@@ -16,6 +17,7 @@ class ClassViewSet(ModelViewSet[RequestUser, Class]):
1617
http_method_names = ["get"]
1718
lookup_field = "access_code"
1819
serializer_class = ClassSerializer
20+
filterset_class = ClassFilterSet
1921

2022
def get_permissions(self):
2123
# Only school-teachers can list classes.

codeforlife/user/views/klass_test.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,3 +106,15 @@ def test_list(self):
106106

107107
self.client.login_as(user)
108108
self.client.list(models=user.teacher.classes.all())
109+
110+
def test_list__teacher(self):
111+
"""Can successfully list classes assigned to a teacher."""
112+
user = self.admin_school_teacher_user
113+
classes = Class.objects.filter(teacher=user.teacher)
114+
assert classes.exists()
115+
116+
self.client.login_as(user)
117+
self.client.list(
118+
models=classes,
119+
filters={"teacher": str(user.teacher.id)},
120+
)

codeforlife/user/views/user_test.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,60 @@ def test_list__students_in_class(self):
160160
filters={"students_in_class": klass.access_code},
161161
)
162162

163+
def test_list__only_teachers(self):
164+
"""Can successfully list only teacher-users."""
165+
user = self.admin_school_teacher_user
166+
school_teacher_users = user.teacher.school_teacher_users.all()
167+
assert school_teacher_users.exists()
168+
169+
self.client.login_as(user)
170+
self.client.list(
171+
models=school_teacher_users,
172+
filters={"only_teachers": str(True)},
173+
)
174+
175+
def test_list___id(self):
176+
"""Can successfully list all users in a school and exclude some IDs."""
177+
user = AdminSchoolTeacherUser.objects.first()
178+
assert user
179+
180+
users = [
181+
*list(user.teacher.school_teacher_users),
182+
*list(user.teacher.student_users),
183+
]
184+
users.sort(key=lambda user: user.pk)
185+
186+
exclude_user_1: User = users.pop()
187+
exclude_user_2: User = users.pop()
188+
189+
self.client.login_as(user, password="abc123")
190+
self.client.list(
191+
models=users,
192+
filters={
193+
"_id": [
194+
str(exclude_user_1.id),
195+
str(exclude_user_2.id),
196+
]
197+
},
198+
)
199+
200+
def test_list__name(self):
201+
"""Can successfully list all users by name."""
202+
user = AdminSchoolTeacherUser.objects.first()
203+
assert user
204+
205+
school_users = user.teacher.school_users
206+
first_name, last_name = user.first_name, user.last_name[:1]
207+
208+
self.client.login_as(user, password="abc123")
209+
self.client.list(
210+
models=(
211+
school_users.filter(first_name__icontains=first_name)
212+
| school_users.filter(last_name__icontains=last_name)
213+
),
214+
filters={"name": f"{first_name} {last_name}"},
215+
)
216+
163217
def test_retrieve(self):
164218
"""Can successfully retrieve users."""
165219
user = AdminSchoolTeacherUser.objects.first()

0 commit comments

Comments
 (0)