From 74a13715c6eebeb7fc68c945b5e38c55c5c9d1d3 Mon Sep 17 00:00:00 2001 From: Jose Tomas Robles Hahn Date: Wed, 10 Sep 2025 10:09:48 -0300 Subject: [PATCH 1/2] feat(extras): Improve Django filter for `Rut` Refactor how substring lookups are handled for Chilean RUT fields in Django filters, centralizing logic in the `RutFilter` class and improving test coverage. The main change is that the selection of the form field class for substring lookups (like `contains` and `icontains`) is now handled within `RutFilter.__init__()`, instead of being managed by a custom `filter_for_lookup()` override in `SiiFilterSet`. This results in cleaner code and more reliable behavior. Key changes include: - Refactoring and Logic Centralization: - Moved the logic for selecting the appropriate `field_class` for substring lookups (`contains`, `icontains`) from `SiiFilterSet.filter_for_lookup()` into the `RutFilter` constructor, ensuring that substring lookups use a plain `CharField` instead of a strict `RutField` that would fail on invalid substrings. - Test Enhancements: - Added comprehensive tests for `RutFilter` to verify that the correct `field_class` is chosen for each lookup expression, and for `SiiFilterSet` to ensure `filter_for_lookup()` returns filters with the correct field classes for different lookup types. --- src/cl_sii/extras/dj_filters.py | 44 +++++++++++++-------- src/tests/test_extras_dj_filters.py | 60 +++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 17 deletions(-) diff --git a/src/cl_sii/extras/dj_filters.py b/src/cl_sii/extras/dj_filters.py index 20ec8f36..44badc0f 100644 --- a/src/cl_sii/extras/dj_filters.py +++ b/src/cl_sii/extras/dj_filters.py @@ -12,8 +12,9 @@ except ImportError as exc: # pragma: no cover raise ImportError("Package 'django-filter' is required to use this module.") from exc +from collections.abc import Sequence from copy import deepcopy -from typing import ClassVar, Mapping, Tuple, Type +from typing import Any, ClassVar, Mapping, Type import django.db.models import django.forms @@ -37,9 +38,33 @@ class RutFilter(django_filters.filters.CharFilter): - https://github.com/carltongibson/django-filter/blob/24.2/docs/ref/filters.txt """ - field_class: ClassVar[Type[django.forms.Field]] + field_class: Type[django.forms.Field] field_class = cl_sii.extras.dj_form_fields.RutField + field_class_for_substrings: Type[django.forms.Field] + field_class_for_substrings = django_filters.filters.CharFilter.field_class + + lookup_expressions_for_substrings: Sequence[str] = [ + 'contains', + 'icontains', + ] + + def __init__( + self, + field_name: Any = None, + lookup_expr: Any = None, + *args: Any, + **kwargs: Any, + ) -> None: + if lookup_expr in self.lookup_expressions_for_substrings: + # Lookups that can be used to search for substrings will not always + # work with the default field class because some substrings cannot + # be converted to instances of class `Rut`. For example, + # `…__contains="803"` fails because `Rut("803")` raises a `ValueError`. + self.field_class = self.field_class_for_substrings + + super().__init__(field_name, lookup_expr, *args, **kwargs) + FILTER_FOR_DBFIELD_DEFAULTS = { **FILTER_FOR_DBFIELD_DEFAULTS, @@ -62,18 +87,3 @@ class SiiFilterSet(django_filters.filterset.FilterSet): FILTER_DEFAULTS: ClassVar[Mapping[Type[django.db.models.Field], Mapping[str, object]]] FILTER_DEFAULTS = FILTER_FOR_DBFIELD_DEFAULTS - - @classmethod - def filter_for_lookup( - cls, field: django.db.models.Field, lookup_type: str - ) -> Tuple[Type[django_filters.filters.Filter], Mapping[str, object]]: - filter_class, params = super().filter_for_lookup(field, lookup_type) - - # Override RUT containment lookups. - if isinstance(field, cl_sii.extras.dj_model_fields.RutField) and lookup_type in ( - 'contains', - 'icontains', - ): - filter_class, params = django_filters.filters.CharFilter, {} - - return filter_class, params diff --git a/src/tests/test_extras_dj_filters.py b/src/tests/test_extras_dj_filters.py index 1f83e209..0dee6671 100644 --- a/src/tests/test_extras_dj_filters.py +++ b/src/tests/test_extras_dj_filters.py @@ -4,6 +4,7 @@ import django_filters +from cl_sii.extras import dj_form_fields, dj_model_fields from cl_sii.extras.dj_filters import RutFilter, SiiFilterSet @@ -22,6 +23,30 @@ def test_new_instance(self) -> None: self.assertIsInstance(filter, RutFilter) self.assertIsInstance(filter, django_filters.filters.Filter) + def test_filter_class_lookup_expressions(self) -> None: + expected_field_class = dj_form_fields.RutField + for lookup_expr in [ + 'exact', + 'iexact', + 'in', + 'gt', + 'gte', + 'lt', + 'lte', + ]: + with self.subTest(field_class=expected_field_class, lookup_expr=lookup_expr): + filter_instance = RutFilter(lookup_expr=lookup_expr) + self.assertIs(filter_instance.field_class, expected_field_class) + + expected_field_class = django_filters.CharFilter.field_class + for lookup_expr in [ + 'contains', + 'icontains', + ]: + with self.subTest(field_class=expected_field_class, lookup_expr=lookup_expr): + filter_instance = RutFilter(lookup_expr=lookup_expr) + self.assertIs(filter_instance.field_class, expected_field_class) + # TODO: Add tests. @@ -34,4 +59,39 @@ class SiiFilterSetTest(unittest.TestCase): def test_filter_for_lookup(self) -> None: assert SiiFilterSet.filter_for_lookup() + def test_filter_for_lookup_types(self) -> None: + field = dj_model_fields.RutField() + + expected_field_class = dj_form_fields.RutField + for lookup_type in [ + 'exact', + 'iexact', + 'gt', + 'gte', + 'lt', + 'lte', + ]: + with self.subTest(field_class=expected_field_class, lookup_type=lookup_type): + filter_class, params = SiiFilterSet.filter_for_lookup(field, lookup_type) + filter_instance = filter_class(**{'lookup_expr': lookup_type, **params}) + self.assertIs(filter_instance.field_class, expected_field_class) + + for lookup_type in [ + 'in', + ]: + with self.subTest(field_class=expected_field_class, lookup_type=lookup_type): + filter_class, params = SiiFilterSet.filter_for_lookup(field, lookup_type) + filter_instance = filter_class(**{'lookup_expr': lookup_type, **params}) + self.assertTrue(issubclass(filter_instance.field_class, expected_field_class)) + + expected_field_class = django_filters.CharFilter.field_class + for lookup_type in [ + 'contains', + 'icontains', + ]: + with self.subTest(field_class=expected_field_class, lookup_type=lookup_type): + filter_class, params = SiiFilterSet.filter_for_lookup(field, lookup_type) + filter_instance = filter_class(**{'lookup_expr': lookup_type, **params}) + self.assertIs(filter_instance.field_class, expected_field_class) + # TODO: Add tests. From c4df64a7a7fbea58c8fe09340ff45be4407c58dc Mon Sep 17 00:00:00 2001 From: Jose Tomas Robles Hahn Date: Wed, 10 Sep 2025 10:26:10 -0300 Subject: [PATCH 2/2] feat(extras): Add additional lookup expressions to Django `RutFilter` --- src/cl_sii/extras/dj_filters.py | 4 ++++ src/tests/test_extras_dj_filters.py | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/cl_sii/extras/dj_filters.py b/src/cl_sii/extras/dj_filters.py index 44badc0f..9cde5986 100644 --- a/src/cl_sii/extras/dj_filters.py +++ b/src/cl_sii/extras/dj_filters.py @@ -47,6 +47,10 @@ class RutFilter(django_filters.filters.CharFilter): lookup_expressions_for_substrings: Sequence[str] = [ 'contains', 'icontains', + 'startswith', + 'istartswith', + 'endswith', + 'iendswith', ] def __init__( diff --git a/src/tests/test_extras_dj_filters.py b/src/tests/test_extras_dj_filters.py index 0dee6671..1531ca90 100644 --- a/src/tests/test_extras_dj_filters.py +++ b/src/tests/test_extras_dj_filters.py @@ -42,6 +42,10 @@ def test_filter_class_lookup_expressions(self) -> None: for lookup_expr in [ 'contains', 'icontains', + 'startswith', + 'istartswith', + 'endswith', + 'iendswith', ]: with self.subTest(field_class=expected_field_class, lookup_expr=lookup_expr): filter_instance = RutFilter(lookup_expr=lookup_expr) @@ -88,6 +92,10 @@ def test_filter_for_lookup_types(self) -> None: for lookup_type in [ 'contains', 'icontains', + 'startswith', + 'istartswith', + 'endswith', + 'iendswith', ]: with self.subTest(field_class=expected_field_class, lookup_type=lookup_type): filter_class, params = SiiFilterSet.filter_for_lookup(field, lookup_type)