From 6b27542210267d0d40a685c2feabc7c3e45be601 Mon Sep 17 00:00:00 2001 From: Torrent Date: Thu, 16 Oct 2025 15:58:05 +0200 Subject: [PATCH 1/7] add club search api filters --- club/api.py | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/club/api.py b/club/api.py index 2e59d3e5..70dd4b40 100644 --- a/club/api.py +++ b/club/api.py @@ -1,4 +1,4 @@ -from typing import Annotated +from typing import Annotated, Optional from annotated_types import MinLen from django.db.models import Prefetch @@ -23,8 +23,35 @@ class ClubController(ControllerBase): url_name="search_club", ) @paginate(PageNumberPaginationExtra, page_size=50) - def search_club(self, search: Annotated[str, MinLen(1)]): - return Club.objects.filter(name__icontains=search).values() + def search_club( + self, + search: Annotated[Optional[str], MinLen(1), "filter by name"] = None, + club_id: Annotated[Optional[int], "filter by club id"] = None, + excluded_ids: Annotated[ + Optional[list[int]], "filter by excluded club ids" + ] = None, + is_active: Annotated[Optional[bool], "filter by club activity"] = None, + parent_id: Annotated[Optional[int], "filter by parent id"] = None, + parent_name: Annotated[ + Optional[str], MinLen(1), "filter by parent name" + ] = None, + ): + queryset = Club.objects.all() + + if search: + queryset = queryset.filter(name__icontains=search) + if club_id: + queryset = queryset.filter(id=club_id) + if is_active: + queryset = queryset.filter(is_active=is_active) + if parent_name: + queryset = queryset.filter(parent__name__icontains=parent_name) + if parent_id: + queryset = queryset.filter(parent_id=parent_id) + if excluded_ids: + queryset = queryset.exclude(id__in=excluded_ids) + + return queryset.values() @route.get( "/{int:club_id}", From 3ea2d2aaf2ddd36a746e569f1384c70195e6f5ca Mon Sep 17 00:00:00 2001 From: Torrent Date: Thu, 16 Oct 2025 19:47:12 +0200 Subject: [PATCH 2/7] filter using schema --- club/api.py | 35 +++++------------------------------ club/schemas.py | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/club/api.py b/club/api.py index 70dd4b40..c952ed2f 100644 --- a/club/api.py +++ b/club/api.py @@ -1,7 +1,5 @@ -from typing import Annotated, Optional - -from annotated_types import MinLen from django.db.models import Prefetch +from ninja import Query from ninja.security import SessionAuth from ninja_extra import ControllerBase, api_controller, paginate, route from ninja_extra.pagination import PageNumberPaginationExtra @@ -10,7 +8,7 @@ from ninja_extra.schemas import PaginatedResponseSchema from api.auth import ApiKeyAuth from api.permissions import CanAccessLookup, HasPerm from club.models import Club, Membership -from club.schemas import ClubSchema, SimpleClubSchema +from club.schemas import ClubSchema, ClubSearchFilterSchema, SimpleClubSchema @api_controller("/club") @@ -25,33 +23,10 @@ class ClubController(ControllerBase): @paginate(PageNumberPaginationExtra, page_size=50) def search_club( self, - search: Annotated[Optional[str], MinLen(1), "filter by name"] = None, - club_id: Annotated[Optional[int], "filter by club id"] = None, - excluded_ids: Annotated[ - Optional[list[int]], "filter by excluded club ids" - ] = None, - is_active: Annotated[Optional[bool], "filter by club activity"] = None, - parent_id: Annotated[Optional[int], "filter by parent id"] = None, - parent_name: Annotated[ - Optional[str], MinLen(1), "filter by parent name" - ] = None, + filters: Query[ClubSearchFilterSchema], ): - queryset = Club.objects.all() - - if search: - queryset = queryset.filter(name__icontains=search) - if club_id: - queryset = queryset.filter(id=club_id) - if is_active: - queryset = queryset.filter(is_active=is_active) - if parent_name: - queryset = queryset.filter(parent__name__icontains=parent_name) - if parent_id: - queryset = queryset.filter(parent_id=parent_id) - if excluded_ids: - queryset = queryset.exclude(id__in=excluded_ids) - - return queryset.values() + clubs = Club.objects.all() + return filters.filter(clubs) @route.get( "/{int:club_id}", diff --git a/club/schemas.py b/club/schemas.py index b0601af8..9ba4d235 100644 --- a/club/schemas.py +++ b/club/schemas.py @@ -1,9 +1,24 @@ -from ninja import ModelSchema +from typing import Optional + +from django.db.models import Q +from ninja import Field, FilterSchema, ModelSchema from club.models import Club, Membership from core.schemas import SimpleUserSchema +class ClubSearchFilterSchema(FilterSchema): + search: Optional[str] = Field(None, q="name__icontains") + club_id: Optional[int] = Field(None, q="id") + is_active: Optional[bool] = None + parent_id: Optional[int] = None + parent_name: Optional[str] = Field(None, q="parent__name__icontains") + exclude_ids: Optional[list[int]] = None + + def filter_exclude_ids(self, value: list[int]): + return ~Q(id__in=value) + + class SimpleClubSchema(ModelSchema): class Meta: model = Club From edd31d5d56813d1a91ed81bdf761a8c11c3df011 Mon Sep 17 00:00:00 2001 From: Kenneth SOARES Date: Mon, 20 Oct 2025 20:08:58 +0200 Subject: [PATCH 3/7] used 3.10 types --- club/schemas.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/club/schemas.py b/club/schemas.py index 9ba4d235..918e2e21 100644 --- a/club/schemas.py +++ b/club/schemas.py @@ -1,5 +1,6 @@ -from typing import Optional +from typing import Annotated, Optional +from annotated_types import MinLen from django.db.models import Q from ninja import Field, FilterSchema, ModelSchema @@ -8,14 +9,15 @@ from core.schemas import SimpleUserSchema class ClubSearchFilterSchema(FilterSchema): - search: Optional[str] = Field(None, q="name__icontains") - club_id: Optional[int] = Field(None, q="id") - is_active: Optional[bool] = None - parent_id: Optional[int] = None - parent_name: Optional[str] = Field(None, q="parent__name__icontains") - exclude_ids: Optional[list[int]] = None + search: Annotated[str, MinLen(1)] | None = Field(None, q="name__icontains") + is_active: bool | None = None + parent_id: int | None = None + parent_name: str | None = Field(None, q="parent__name__icontains") + exclude_ids: set[int] | None = None - def filter_exclude_ids(self, value: list[int]): + def filter_exclude_ids(self, value: set[int] | None): + if value is None: + return Q() return ~Q(id__in=value) From 49b0a13dbdb492739269684d4055dd2da3edb6b8 Mon Sep 17 00:00:00 2001 From: Kenneth SOARES Date: Mon, 20 Oct 2025 20:11:25 +0200 Subject: [PATCH 4/7] fixed imports --- club/schemas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/club/schemas.py b/club/schemas.py index 918e2e21..5a7ccccb 100644 --- a/club/schemas.py +++ b/club/schemas.py @@ -1,4 +1,4 @@ -from typing import Annotated, Optional +from typing import Annotated from annotated_types import MinLen from django.db.models import Q From 7f504d9ee214d1398502ff05f2095afb9cb9098e Mon Sep 17 00:00:00 2001 From: Kenneth SOARES Date: Mon, 20 Oct 2025 21:58:44 +0200 Subject: [PATCH 5/7] add test cases --- club/tests/test_api.py | 44 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 club/tests/test_api.py diff --git a/club/tests/test_api.py b/club/tests/test_api.py new file mode 100644 index 00000000..12e00aa0 --- /dev/null +++ b/club/tests/test_api.py @@ -0,0 +1,44 @@ +from django.test import Client, TestCase +from django.urls import reverse + +from core.models import User + + +class TestClubSearch(TestCase): + @classmethod + def setUpTestData(cls): + cls.url = reverse("api:search_club") + cls.client = Client() + cls.user = User.objects.get(username="root") + + def test_inactive_club(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"is_active": False}) + assert response.status_code == 200 + + data = response.json() + names = [item["name"] for item in data["results"]] + assert "AE" not in names + assert "Troll Penché" not in names + + def test_excluded_id(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"exclude_ids": [1]}) + assert response.status_code == 200 + + data = response.json() + names = [item["name"] for item in data["results"]] + assert "AE" not in names + + def test_club_search(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"search": "AE"}) + assert response.status_code == 200 + + data = response.json() + names = [item["name"] for item in data["results"]] + assert len(names) > 1 + + def test_anonymous_user_unauthorized(self): + response = self.client.get(self.url) + assert response.status_code == 401 From 5697b4e9c8189acc9d902fe32d225daf0ae708df Mon Sep 17 00:00:00 2001 From: Kenneth SOARES Date: Mon, 3 Nov 2025 20:50:04 +0100 Subject: [PATCH 6/7] move club api test to test_controller_club.py --- club/tests/test_api.py | 44 ----------------------------- club/tests/test_club_controller.py | 45 +++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 45 deletions(-) delete mode 100644 club/tests/test_api.py diff --git a/club/tests/test_api.py b/club/tests/test_api.py deleted file mode 100644 index 12e00aa0..00000000 --- a/club/tests/test_api.py +++ /dev/null @@ -1,44 +0,0 @@ -from django.test import Client, TestCase -from django.urls import reverse - -from core.models import User - - -class TestClubSearch(TestCase): - @classmethod - def setUpTestData(cls): - cls.url = reverse("api:search_club") - cls.client = Client() - cls.user = User.objects.get(username="root") - - def test_inactive_club(self): - self.client.force_login(self.user) - response = self.client.get(self.url, {"is_active": False}) - assert response.status_code == 200 - - data = response.json() - names = [item["name"] for item in data["results"]] - assert "AE" not in names - assert "Troll Penché" not in names - - def test_excluded_id(self): - self.client.force_login(self.user) - response = self.client.get(self.url, {"exclude_ids": [1]}) - assert response.status_code == 200 - - data = response.json() - names = [item["name"] for item in data["results"]] - assert "AE" not in names - - def test_club_search(self): - self.client.force_login(self.user) - response = self.client.get(self.url, {"search": "AE"}) - assert response.status_code == 200 - - data = response.json() - names = [item["name"] for item in data["results"]] - assert len(names) > 1 - - def test_anonymous_user_unauthorized(self): - response = self.client.get(self.url) - assert response.status_code == 401 diff --git a/club/tests/test_club_controller.py b/club/tests/test_club_controller.py index 18a3aef1..57a53346 100644 --- a/club/tests/test_club_controller.py +++ b/club/tests/test_club_controller.py @@ -1,7 +1,7 @@ from datetime import date, timedelta import pytest -from django.test import Client +from django.test import Client, TestCase from django.urls import reverse from model_bakery import baker from model_bakery.recipe import Recipe @@ -9,6 +9,49 @@ from pytest_django.asserts import assertNumQueries from club.models import Club, Membership from core.baker_recipes import subscriber_user +from core.models import User + + +class TestClubSearch(TestCase): + @classmethod + def setUpTestData(cls): + cls.url = reverse("api:search_club") + cls.user = User.objects.get(username="root") + + def test_inactive_club(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"is_active": False}) + assert response.status_code == 200 + + data = response.json() + names = [item["name"] for item in data["results"]] + assert "AE" not in names + assert "Troll Penché" not in names + + def test_excluded_id(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"exclude_ids": [1]}) + assert response.status_code == 200 + + data = response.json() + names = [item["name"] for item in data["results"]] + assert "AE" not in names + + def test_club_search(self): + self.client.force_login(self.user) + response = self.client.get(self.url, {"search": "AE"}) + assert response.status_code == 200 + + data = response.json() + names = [item["name"] for item in data["results"]] + assert len(names) > 1 + + for name in names: + assert "AE" in name.upper() + + def test_anonymous_user_unauthorized(self): + response = self.client.get(self.url) + assert response.status_code == 401 @pytest.mark.django_db From f0b1e8af4a399cfc0b0e1a56934c9cd818b4e8be Mon Sep 17 00:00:00 2001 From: imperosol Date: Fri, 7 Nov 2025 15:57:53 +0100 Subject: [PATCH 7/7] improve tests --- club/api.py | 8 ++--- club/tests/test_club_controller.py | 52 +++++++++++++++++------------- 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/club/api.py b/club/api.py index c952ed2f..9bf7bcad 100644 --- a/club/api.py +++ b/club/api.py @@ -21,12 +21,8 @@ class ClubController(ControllerBase): url_name="search_club", ) @paginate(PageNumberPaginationExtra, page_size=50) - def search_club( - self, - filters: Query[ClubSearchFilterSchema], - ): - clubs = Club.objects.all() - return filters.filter(clubs) + def search_club(self, filters: Query[ClubSearchFilterSchema]): + return filters.filter(Club.objects.all()) @route.get( "/{int:club_id}", diff --git a/club/tests/test_club_controller.py b/club/tests/test_club_controller.py index 57a53346..500dbe6a 100644 --- a/club/tests/test_club_controller.py +++ b/club/tests/test_club_controller.py @@ -1,6 +1,7 @@ from datetime import date, timedelta import pytest +from django.contrib.auth.models import Permission from django.test import Client, TestCase from django.urls import reverse from model_bakery import baker @@ -9,49 +10,54 @@ from pytest_django.asserts import assertNumQueries from club.models import Club, Membership from core.baker_recipes import subscriber_user -from core.models import User +from core.models import Group, Page, User class TestClubSearch(TestCase): @classmethod def setUpTestData(cls): cls.url = reverse("api:search_club") - cls.user = User.objects.get(username="root") + cls.user = baker.make( + User, user_permissions=[Permission.objects.get(codename="access_lookup")] + ) + # delete existing clubs to avoid side effect + groups = list( + Group.objects.exclude(club=None, club_board=None).values_list( + "id", flat=True + ) + ) + Page.objects.exclude(club=None).delete() + Club.objects.all().delete() + Group.objects.filter(id__in=groups).delete() + + cls.clubs = baker.make( + Club, + _quantity=5, + name=iter(["AE", "ae 1", "Troll", "Dev AE", "pdf"]), + is_active=True, + ) def test_inactive_club(self): self.client.force_login(self.user) + inactive_ids = {self.clubs[0].id, self.clubs[2].id} + Club.objects.filter(id__in=inactive_ids).update(is_active=False) response = self.client.get(self.url, {"is_active": False}) assert response.status_code == 200 - - data = response.json() - names = [item["name"] for item in data["results"]] - assert "AE" not in names - assert "Troll Penché" not in names + assert {d["id"] for d in response.json()["results"]} == inactive_ids def test_excluded_id(self): self.client.force_login(self.user) - response = self.client.get(self.url, {"exclude_ids": [1]}) + response = self.client.get(self.url, {"exclude_ids": [self.clubs[1].id]}) assert response.status_code == 200 - - data = response.json() - names = [item["name"] for item in data["results"]] - assert "AE" not in names + ids = {d["id"] for d in response.json()["results"]} + assert ids == {c.id for c in [self.clubs[0], *self.clubs[2:]]} def test_club_search(self): self.client.force_login(self.user) response = self.client.get(self.url, {"search": "AE"}) assert response.status_code == 200 - - data = response.json() - names = [item["name"] for item in data["results"]] - assert len(names) > 1 - - for name in names: - assert "AE" in name.upper() - - def test_anonymous_user_unauthorized(self): - response = self.client.get(self.url) - assert response.status_code == 401 + ids = {d["id"] for d in response.json()["results"]} + assert ids == {c.id for c in [self.clubs[0], self.clubs[1], self.clubs[3]]} @pytest.mark.django_db