From 47bd81fdd5db734c673e95b31c3c0ac62a4ab178 Mon Sep 17 00:00:00 2001 From: Anna Shamray Date: Wed, 11 Dec 2024 17:07:14 +0100 Subject: [PATCH] :sparkles: [#472] add 'data_attr' query param --- src/objects/api/v2/filters.py | 116 ++++++++++++++++-------- src/objects/tests/v2/test_filters.py | 130 +++++++++++++++------------ src/objects/utils/filters.py | 22 +++++ 3 files changed, 175 insertions(+), 93 deletions(-) diff --git a/src/objects/api/v2/filters.py b/src/objects/api/v2/filters.py index 442dfa18..8af36773 100644 --- a/src/objects/api/v2/filters.py +++ b/src/objects/api/v2/filters.py @@ -1,6 +1,7 @@ from datetime import date as date_ from django import forms +from django.db.models import QuerySet from django.utils.translation import gettext_lazy as _ from django_filters import filters @@ -8,12 +9,51 @@ from vng_api_common.filtersets import FilterSet from objects.core.models import ObjectRecord, ObjectType -from objects.utils.filters import ObjectTypeFilter +from objects.utils.filters import ManyCharFilter, ObjectTypeFilter from ..constants import Operators from ..utils import display_choice_values_for_help_text, string_to_value from ..validators import validate_data_attrs +DATA_ATTR_VALUE_HELP_TEXT = f"""A valid parameter value has the form `key__operator__value`. +`key` is the attribute name, `operator` is the comparison operator to be used and `value` is the attribute value. +Note: Values can be string, numeric, or dates (ISO format; YYYY-MM-DD). + +Valid operator values are: +{display_choice_values_for_help_text(Operators)} + +`value` may not contain double underscore or comma characters. +`key` may not contain comma characters and includes double underscore only if it indicates nested attributes. + +""" + + +def filter_data_attr_value_part(value_part: str, queryset: QuerySet) -> QuerySet: + """ + filter one value part for data_attr and data_attrs filters + """ + variable, operator, str_value = value_part.rsplit("__", 2) + real_value = string_to_value(str_value) + + if operator == "exact": + # for exact operator try to filter on string and numeric values + in_vals = [str_value] + if real_value != str_value: + in_vals.append(real_value) + queryset = queryset.filter(**{f"data__{variable}__in": in_vals}) + elif operator == "icontains": + # icontains treats everything like strings + queryset = queryset.filter(**{f"data__{variable}__icontains": str_value}) + elif operator == "in": + # in must be a list + values = str_value.split("|") + queryset = queryset.filter(**{f"data__{variable}__in": values}) + + else: + # gt, gte, lt, lte operators + queryset = queryset.filter(**{f"data__{variable}__{operator}": real_value}) + return queryset + class ObjectRecordFilterForm(forms.Form): def clean(self): @@ -62,25 +102,46 @@ class ObjectRecordFilterSet(FilterSet): method="filter_data_attrs", validators=[validate_data_attrs], help_text=_( - """Only include objects that have attributes with certain values. + """**DEPRECATED**: Only include objects that have attributes with certain values. Data filtering expressions are comma-separated and are structured as follows: -A valid parameter value has the form `key__operator__value`. -`key` is the attribute name, `operator` is the comparison operator to be used and `value` is the attribute value. -Note: Values can be string, numeric, or dates (ISO format; YYYY-MM-DD). - -Valid operator values are: -%(operator_choices)s -`value` may not contain double underscore or comma characters. -`key` may not contain comma characters and includes double underscore only if it indicates nested attributes. +%(value_part_help_text)s Example: in order to display only objects with `height` equal to 100, query `data_attrs=height__exact__100` should be used. If `height` is nested inside `dimensions` attribute, query should look like `data_attrs=dimensions__height__exact__100` + +`value` may not contain comma, since commas are used as separator between filtering expressions. +If you want to use commas in `value` you can use `data_attr` query parameter. +""" + ) + % {"value_part_help_text": DATA_ATTR_VALUE_HELP_TEXT}, + ) + + data_attr = ManyCharFilter( + method="filter_data_attr", + # validators=[validate_data_attrs], + help_text=_( + """Only include objects that have attributes with certain values. + +%(value_part_help_text)s + +Example: in order to display only objects with `height` equal to 100, query `data_attr=height__exact__100` +should be used. If `height` is nested inside `dimensions` attribute, query should look like +`data_attr=dimensions__height__exact__100` + +This filter is very similar to the old `data_attrs` filter, but it has two differences: + +* `value` may contain commas +* only one filtering expression is allowed + +If you want to use several filtering expressions, just use this `data_attr` several times in the query string. +Example: `data_attr=height__exact__100&data_attr=naam__icontains__boom` """ ) - % {"operator_choices": display_choice_values_for_help_text(Operators)}, + % {"value_part_help_text": DATA_ATTR_VALUE_HELP_TEXT}, ) + data_icontains = filters.CharFilter( method="filter_data_icontains", help_text=_("Search in all `data` values of string properties."), @@ -88,37 +149,20 @@ class ObjectRecordFilterSet(FilterSet): class Meta: model = ObjectRecord - fields = ("type", "data_attrs", "date", "registrationDate") + fields = ("type", "data_attrs", "data_attr", "date", "registrationDate") form = ObjectRecordFilterForm def filter_data_attrs(self, queryset, name, value: str): parts = value.split(",") for value_part in parts: - variable, operator, str_value = value_part.rsplit("__", 2) - real_value = string_to_value(str_value) - - if operator == "exact": - # for exact operator try to filter on string and numeric values - in_vals = [str_value] - if real_value != value: - in_vals.append(real_value) - queryset = queryset.filter(**{f"data__{variable}__in": in_vals}) - elif operator == "icontains": - # icontains treats everything like strings - queryset = queryset.filter( - **{f"data__{variable}__icontains": str_value} - ) - elif operator == "in": - # in must be a list - values = str_value.split("|") - queryset = queryset.filter(**{f"data__{variable}__in": values}) - - else: - # gt, gte, lt, lte operators - queryset = queryset.filter( - **{f"data__{variable}__{operator}": real_value} - ) + queryset = filter_data_attr_value_part(value_part, queryset) + + return queryset + + def filter_data_attr(self, queryset, name, value: list): + for value_part in value: + queryset = filter_data_attr_value_part(value_part, queryset) return queryset diff --git a/src/objects/tests/v2/test_filters.py b/src/objects/tests/v2/test_filters.py index 339adbae..684b3da8 100644 --- a/src/objects/tests/v2/test_filters.py +++ b/src/objects/tests/v2/test_filters.py @@ -3,6 +3,7 @@ from django.db.utils import ProgrammingError +from furl import furl from rest_framework import status from rest_framework.test import APITestCase @@ -359,63 +360,6 @@ def test_filter_icontains_numeric(self): f"http://testserver{reverse('object-detail', args=[record.object.uuid])}", ) - def test_filter_icontains_string_with_comma(self): - """ - regression test for https://github.com/maykinmedia/objects-api/issues/472 - """ - record = ObjectRecordFactory.create( - data={"name": "Something important"}, object__object_type=self.object_type - ) - ObjectRecordFactory.create( - data={"name": "Advies, support en kennis om te weten"}, - object__object_type=self.object_type, - ) - ObjectRecordFactory.create(data={}, object__object_type=self.object_type) - - response = self.client.get( - self.url, {"data_attrs": "name__icontains__Advies, support en kennis"} - ) - - self.assertEqual(response.status_code, status.HTTP_200_OK) - - data = response.json()["results"] - - self.assertEqual(len(data), 1) - self.assertEqual( - data[0]["url"], - f"http://testserver{reverse('object-detail', args=[record.object.uuid])}", - ) - - def test_filter_two_icontains_with_comma(self): - """ - regression test for https://github.com/maykinmedia/objects-api/issues/472 - """ - record = ObjectRecordFactory.create( - data={"name": "Something important"}, object__object_type=self.object_type - ) - ObjectRecordFactory.create( - data={"name": "Advies, support en kennis om te weten"}, - object__object_type=self.object_type, - ) - ObjectRecordFactory.create(data={}, object__object_type=self.object_type) - - response = self.client.get( - self.url, - { - "data_attrs": "name__icontains__Advies, support en kennis,name__icontains__om" - }, - ) - - self.assertEqual(response.status_code, status.HTTP_200_OK) - - data = response.json()["results"] - - self.assertEqual(len(data), 1) - self.assertEqual( - data[0]["url"], - f"http://testserver{reverse('object-detail', args=[record.object.uuid])}", - ) - def test_filter_exclude_old_records(self): record_old = ObjectRecordFactory.create( data={"diameter": 45}, @@ -485,6 +429,78 @@ def test_filter_in_string(self): ) +class FilterDataAttrTests(TokenAuthMixin, APITestCase): + url = reverse_lazy("object-list") + + @classmethod + def setUpTestData(cls): + super().setUpTestData() + + cls.object_type = ObjectTypeFactory(service__api_root=OBJECT_TYPES_API) + PermissionFactory.create( + object_type=cls.object_type, + mode=PermissionModes.read_only, + token_auth=cls.token_auth, + ) + + def test_filter_icontains_string_with_comma(self): + """ + regression test for https://github.com/maykinmedia/objects-api/issues/472 + """ + ObjectRecordFactory.create( + data={"name": "Something important"}, object__object_type=self.object_type + ) + record = ObjectRecordFactory.create( + data={"name": "Advies, support en kennis om te weten"}, + object__object_type=self.object_type, + ) + + response = self.client.get( + self.url, {"data_attr": "name__icontains__Advies, support en kennis"} + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + + self.assertEqual(len(data), 1) + self.assertEqual( + data[0]["url"], + f"http://testserver{reverse('object-detail', args=[record.object.uuid])}", + ) + + def test_filter_two_icontains_with_comma(self): + """ + regression test for https://github.com/maykinmedia/objects-api/issues/472 + """ + ObjectRecordFactory.create( + data={"name": "Something important"}, object__object_type=self.object_type + ) + record = ObjectRecordFactory.create( + data={"name": "Advies, support en kennis om te weten"}, + object__object_type=self.object_type, + ) + url = ( + furl(self.url) + .add({"data_attr": "name__icontains__Advies, support en kennis"}) + .add({"data_attr": "name__icontains__om"}) + .url + ) + + response = self.client.get(url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + data = response.json()["results"] + + self.assertEqual(len(data), 1) + self.assertEqual( + data[0]["url"], + f"http://testserver{reverse('object-detail', args=[record.object.uuid])}", + ) + + + class FilterDateTests(TokenAuthMixin, APITestCase): @classmethod def setUpTestData(cls): diff --git a/src/objects/utils/filters.py b/src/objects/utils/filters.py index 61364a6a..14555b76 100644 --- a/src/objects/utils/filters.py +++ b/src/objects/utils/filters.py @@ -1,3 +1,4 @@ +from django import forms from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ @@ -48,3 +49,24 @@ def to_python(self, value): class ObjectTypeFilter(URLModelChoiceFilter): field_class = ObjectTypeField + + +class ManyWidget(forms.Widget): + def value_from_datadict(self, data, files, name): + return data.getlist(name) + + +class ManyCharField(forms.CharField): + widget = ManyWidget + + def to_python(self, value): + if not value: + return [] + + # todo validator if it's list + return value + + +class ManyCharFilter(filters.CharFilter): + # django-filter doesn't support several uses of the same query param out of the box + field_class = ManyCharField