fix(backend): refactor use of NumberFilter/BaseInFilter

backend/timed/employment/filters.py

@@ -4,23 +4,25 @@
 
 from django.db.models import Q
 from django_filters.constants import EMPTY_VALUES
-from django_filters.rest_framework import DateFilter, Filter, FilterSet, NumberFilter
+from django_filters.rest_framework import (
+    BooleanFilter,
+    DateFilter,
+    Filter,
+    FilterSet,
+)
 
 from timed.employment import models
 from timed.employment.models import User
+from timed.filters import IdFilter
 
 if TYPE_CHECKING:
-    from typing import TypeVar
-
     from django.db.models import QuerySet
 
-    T = TypeVar("T", QuerySet)
-
 
 class YearFilter(Filter):
     """Filter to filter a queryset by year."""
 
-    def filter(self, qs: T, value: int) -> T:
+    def filter[T: QuerySet](self, qs: T, value: int) -> T:
         if value in EMPTY_VALUES:
             return qs
 
@@ -48,7 +50,7 @@ class Meta:
 
 
 class AbsenceTypeFilterSet(FilterSet):
-    fill_worktime = NumberFilter(field_name="fill_worktime")
+    fill_worktime = BooleanFilter(field_name="fill_worktime")
 
     class Meta:
         """Meta information for the public holiday filter set."""
@@ -58,31 +60,34 @@ class Meta:
 
 
 class UserFilterSet(FilterSet):
-    active = NumberFilter(field_name="is_active")
-    supervisor = NumberFilter(field_name="supervisors")
-    is_reviewer = NumberFilter(method="filter_is_reviewer")
-    is_supervisor = NumberFilter(method="filter_is_supervisor")
-    is_accountant = NumberFilter(field_name="is_accountant")
-    is_external = NumberFilter(method="filter_is_external")
-
-    def filter_is_external(
-        self, queryset: QuerySet[models.User], _name: str, value: int
-    ) -> QuerySet[models.User]:
-        return queryset.filter(employments__is_external=value)
+    active = BooleanFilter(field_name="is_active")
+    supervisor = IdFilter(field_name="supervisors")
+    is_reviewer = BooleanFilter(method="filter_is_reviewer")
+    is_supervisor = BooleanFilter(method="filter_is_supervisor")
+    is_accountant = BooleanFilter(field_name="is_accountant")
+    is_external = BooleanFilter(field_name="employments__is_external")
 
     def filter_is_reviewer(
-        self, queryset: QuerySet[models.User], _name: str, value: int
-    ) -> QuerySet[models.User]:
+        self,
+        queryset: QuerySet[User],
+        _name: str,
+        value: bool,  # noqa: FBT001
+    ) -> QuerySet[User]:
+        is_reviewer = Q(pk__in=User.objects.all_reviewers())
         if value:
-            return queryset.filter(pk__in=User.objects.all_reviewers())
-        return queryset.exclude(pk__in=User.objects.all_reviewers())
+            return queryset.filter(is_reviewer)
+        return queryset.exclude(is_reviewer)
 
     def filter_is_supervisor(
-        self, queryset: QuerySet[models.User], _name: str, value: int
-    ) -> QuerySet[models.User]:
+        self,
+        queryset: QuerySet[User],
+        _name: str,
+        value: bool,  # noqa: FBT001
+    ) -> QuerySet[User]:
+        is_supervisor = Q(pk__in=User.objects.all_supervisors())
         if value:
-            return queryset.filter(pk__in=User.objects.all_supervisors())
-        return queryset.exclude(pk__in=User.objects.all_supervisors())
+            return queryset.filter(is_supervisor)
+        return queryset.exclude(is_supervisor)
 
     class Meta:
         model = models.User
@@ -148,16 +153,16 @@ class Meta:
 
 
 class WorktimeBalanceFilterSet(FilterSet):
-    user = NumberFilter(field_name="id")
-    supervisor = NumberFilter(field_name="supervisors")
+    user = IdFilter(field_name="id")
+    supervisor = IdFilter(field_name="supervisors")
 
     class Meta:
         model = models.User
         fields = ("user",)
 
 
 class AbsenceBalanceFilterSet(FilterSet):
-    absence_type = NumberFilter(field_name="id")
+    absence_type = IdFilter(field_name="id")
 
     class Meta:
         model = models.AbsenceType

backend/timed/filters.py

@@ -0,0 +1,30 @@
+"""In historic versions of timed, we used `NumberFilter`s or `BaseInFilter`s for filtering by one or more `id`s.
+
+`NumberFilter` defaults to `django.forms.DecimalField`s, therefore our `id`s were actually `Decimal`s, which allowed for bogus negative and/or non-whole numbers
+
+`BaseInFilter` is supposed to be combined with another filter, which it wasn't, not even with a `NumberFilter`, in newer versions we only accept positive integers in both.
+"""
+
+from django.forms import IntegerField
+from django_filters.rest_framework import BaseInFilter, NumberFilter
+
+
+class PositiveIntegerField(IntegerField):
+    """`PositiveIntegerField` for use with `NumberFilter`s.
+
+    `NumberFilter` defaults to `forms.DecimalField`, which isn't what we want in most cases.
+    """
+
+    def __init__(self, *args, **kwargs):
+        kwargs.setdefault("min_value", 1)
+        super().__init__(*args, **kwargs)
+
+
+class IdFilter(NumberFilter):
+    """A filter to use for primary keys (ids)."""
+
+    field_class = PositiveIntegerField
+
+
+class IdInFilter(BaseInFilter, IdFilter):
+    """In filter to use with primary keys (ids)."""

backend/timed/projects/filters.py

@@ -7,22 +7,19 @@
 
 from django.db.models import Count, Q
 from django_filters.constants import EMPTY_VALUES
-from django_filters.rest_framework import BaseInFilter, Filter, FilterSet, NumberFilter
+from django_filters.rest_framework import BooleanFilter, Filter, FilterSet
 
+from timed.filters import IdFilter, IdInFilter
 from timed.projects import models
 
 if TYPE_CHECKING:
     from django.db.models import QuerySet
 
 
-class NumberInFilter(BaseInFilter, NumberFilter):
-    pass
-
-
 class CustomerFilterSet(FilterSet):
     """Filter set for the customers endpoint."""
 
-    archived = NumberFilter(field_name="archived")
+    archived = BooleanFilter(field_name="archived")
 
     class Meta:
         """Meta information for the customer filter set."""
@@ -37,13 +34,16 @@ class Meta:
 class ProjectFilterSet(FilterSet):
     """Filter set for the projects endpoint."""
 
-    archived = NumberFilter(field_name="archived")
-    has_manager = NumberFilter(method="filter_has_manager")
-    has_reviewer = NumberFilter(method="filter_has_reviewer")
-    customer = NumberInFilter(field_name="customer")
+    archived = BooleanFilter(field_name="archived")
+    has_manager = IdFilter(method="filter_has_manager")
+    has_reviewer = IdFilter(method="filter_has_reviewer")
+    customer = IdInFilter(field_name="customer")
 
     def filter_has_manager(
-        self, queryset: QuerySet[models.Project], _name: str, value: int
+        self,
+        queryset: QuerySet[models.Project],
+        _name: str,
+        value: int,
     ) -> QuerySet[models.Project]:
         if not value:  # pragma: no cover
             return queryset
@@ -61,7 +61,10 @@ def filter_has_manager(
         )
 
     def filter_has_reviewer(
-        self, queryset: QuerySet[models.Project], _name: str, value: int
+        self,
+        queryset: QuerySet[models.Project],
+        _name: str,
+        value: int,
     ) -> QuerySet[models.Project]:
         if not value:  # pragma: no cover
             return queryset
@@ -128,8 +131,8 @@ class TaskFilterSet(FilterSet):
     """Filter set for the tasks endpoint."""
 
     my_most_frequent = MyMostFrequentTaskFilter()
-    archived = NumberFilter(field_name="archived")
-    project = NumberInFilter(field_name="project")
+    archived = BooleanFilter(field_name="archived")
+    project = IdInFilter(field_name="project")
 
     class Meta:
         """Meta information for the task filter set."""
@@ -147,9 +150,9 @@ class Meta:
 class TaskAssigneeFilterSet(FilterSet):
     """Filter set for the task assignees endpoint."""
 
-    task = NumberFilter(field_name="task")
-    tasks = NumberInFilter(field_name="task")
-    user = NumberFilter(field_name="user")
+    task = IdFilter(field_name="task")
+    tasks = IdInFilter(field_name="task")
+    user = IdFilter(field_name="user")
 
     class Meta:
         """Meta information for the task assignee filter set."""
@@ -167,9 +170,9 @@ class Meta:
 class ProjectAssigneeFilterSet(FilterSet):
     """Filter set for the project assignees endpoint."""
 
-    project = NumberFilter(field_name="project")
-    projects = NumberInFilter(field_name="project")
-    user = NumberFilter(field_name="user")
+    project = IdFilter(field_name="project")
+    projects = IdInFilter(field_name="project")
+    user = IdFilter(field_name="user")
 
     class Meta:
         """Meta information for the project assignee filter set."""
@@ -187,9 +190,9 @@ class Meta:
 class CustomerAssigneeFilterSet(FilterSet):
     """Filter set for the customer assignees endpoint."""
 
-    customer = NumberFilter(field_name="customer")
-    customers = NumberInFilter(field_name="customer")
-    user = NumberFilter(field_name="user")
+    customer = IdFilter(field_name="customer")
+    customers = IdInFilter(field_name="customer")
+    user = IdFilter(field_name="user")
 
     class Meta:
         """Meta information for the customer assignee filter set."""

backend/timed/reports/filters.py

@@ -5,12 +5,12 @@
 from django.db.models import DurationField, F, Q, Sum, Value
 from django.db.models.functions import Coalesce
 from django_filters.rest_framework import (
-    BaseInFilter,
+    BooleanFilter,
     DateFilter,
     FilterSet,
-    NumberFilter,
 )
 
+from timed.filters import IdFilter, IdInFilter
 from timed.projects.models import CustomerAssignee, ProjectAssignee, TaskAssignee
 
 if TYPE_CHECKING:
@@ -129,23 +129,23 @@ def statistic_filterset_builder(
             "to_date": DateFilter(
                 field_name=f"{reports_prefix}date", lookup_expr="lte"
             ),
-            "project": NumberFilter(field_name=f"{project_prefix}pk"),
-            "customer": NumberFilter(field_name=f"{customer_prefix}pk"),
-            "review": NumberFilter(field_name=f"{reports_prefix}review"),
-            "not_billable": NumberFilter(field_name=f"{reports_prefix}not_billable"),
-            "billed": NumberFilter(field_name=f"{reports_prefix}billed"),
-            "verified": NumberFilter(
+            "project": IdFilter(field_name=f"{project_prefix}pk"),
+            "customer": IdFilter(field_name=f"{customer_prefix}pk"),
+            "review": BooleanFilter(field_name=f"{reports_prefix}review"),
+            "not_billable": BooleanFilter(field_name=f"{reports_prefix}not_billable"),
+            "billed": BooleanFilter(field_name=f"{reports_prefix}billed"),
+            "verified": BooleanFilter(
                 field_name=f"{reports_prefix}verified_by_id",
                 lookup_expr="isnull",
                 exclude=True,
             ),
-            "verifier": NumberFilter(field_name=f"{reports_prefix}verified_by"),
-            "billing_type": NumberFilter(field_name=f"{project_prefix}billing_type"),
-            "user": NumberFilter(field_name=f"{reports_prefix}user_id"),
-            "rejected": NumberFilter(field_name=f"{reports_prefix}rejected"),
-            "id": BaseInFilter(),
-            "cost_center": NumberFilter(method="filter_cost_center"),
-            "reviewer": NumberFilter(method="filter_has_reviewer"),
+            "verifier": IdFilter(field_name=f"{reports_prefix}verified_by"),
+            "billing_type": IdFilter(field_name=f"{project_prefix}billing_type"),
+            "user": IdFilter(field_name=f"{reports_prefix}user_id"),
+            "rejected": BooleanFilter(field_name=f"{reports_prefix}rejected"),
+            "id": IdInFilter(),
+            "cost_center": IdFilter(method="filter_cost_center"),
+            "reviewer": IdFilter(method="filter_has_reviewer"),
         },
     )
 

backend/timed/subscription/filters.py

@@ -2,8 +2,9 @@
 
 from typing import TYPE_CHECKING
 
-from django_filters import FilterSet, NumberFilter
+from django_filters.rest_framework import BooleanFilter, FilterSet
 
+from timed.filters import IdFilter
 from timed.projects.models import Project
 
 from . import models
@@ -13,7 +14,7 @@
 
 
 class PackageFilter(FilterSet):
-    customer = NumberFilter(method="filter_customer")
+    customer = IdFilter(method="filter_customer")
 
     def filter_customer(
         self, queryset: QuerySet[models.Package], _name: str, value: int
@@ -30,8 +31,8 @@ class Meta:
 
 
 class OrderFilter(FilterSet):
-    customer = NumberFilter(field_name="project__customer")
-    acknowledged = NumberFilter(field_name="acknowledged")
+    customer = IdFilter(field_name="project__customer")
+    acknowledged = BooleanFilter(field_name="acknowledged")
 
     class Meta:
         model = models.Order