Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

how to make django-filter lookup_expr work for JSONField via declarable fields #1149

Closed
udos opened this issue Nov 14, 2019 · 3 comments
Closed

Comments

@udos
Copy link

udos commented Nov 14, 2019

according docs

you can configure django-filter to apply multiple lookup expressions (lookup_expr) to various fields. this is done via declarable fields as following:

class UserFilter(django_filters.FilterSet):
    class Meta:
        model = User
        fields = {
            'username': ['exact', 'contains'],
            'last_login': ['exact', 'year__gt'],
        }

in my case for field "elevation" (JSONField) unfortunately the lookup_expr are not applied. it always returns the full result.

the code for the filter:

# api/views.py
from django import forms
from django_filters import rest_framework as drf_filters
from segment.models import Segment
from rest_framework import filters


class FloatFilter(drf_filters.Filter):
    field_class = forms.FloatField

class SegmentFilter(drf_filters.FilterSet):
    elevation = FloatFilter(field_name='data__properties__ele__max')

    class Meta:
        model = Segment
        fields = {
            'id': ['lt', 'lte', 'gt', 'gte'],
            'elevation': ['lt', 'lte', 'gt', 'gte'],
        }

which I use in the following viewset:

# api/views.py

class SegmentViewSet(viewsets.ReadOnlyModelViewSet):
    queryset = Segment.objects.all()
    serializer_class = serializers.SegmentSerializer
    filter_backends = [filters.OrderingFilter, drf_filters.DjangoFilterBackend]
    filterset_class = SegmentFilter

the serializer is this:

# api/serializers.py
from drf_queryfields import QueryFieldsMixin
from rest_framework import serializers
from segment.models import Segment


class SegmentSerializer(QueryFieldsMixin, serializers.HyperlinkedModelSerializer):
    data = serializers.DictField()

    class Meta:
        model = Segment
        fields = ['id', 'data']

note: on field "id" the lookup_expr lt, lte, *gt and gte are working.

any ideas why they are not applied on field "elevation"?

@rpkilby
Copy link
Collaborator

rpkilby commented Nov 15, 2019

Hi @udos. I haven't read your issue in full, but it looks like you're running into #1013, which is unrelated to JSONFields. In short, filter generation only works for model fields, not declared filters. Right now this fails silently, but should instead raise an error. The issue goes into more detail.

Beyond that, there are several issues that discuss difficulties with JSON support. If you just want simple value filters, the current recommendation is to declare them. It's verbose, but simple. e.g.,

class SegmentFilter(drf_filters.FilterSet):
    elevation = FloatFilter(field_name='data__properties__ele__max')
    elevation__lt = FloatFilter(field_name='data__properties__ele__max', lookup_expr='lt')
    elevation__gt = FloatFilter(field_name='data__properties__ele__max', lookup_expr='gt')
    elevation__lte = FloatFilter(field_name='data__properties__ele__max', lookup_expr='lte')
    elevation__gte = FloatFilter(field_name='data__properties__ele__max', lookup_expr='gte')

For more info on difficulties supporting JSONFields, see #426 and have a look through the issues.

@udos
Copy link
Author

udos commented Nov 15, 2019

thanks @rpkilby. yes, exactly. it's due to #1013 (I did not spot this :| ). I thought of the workaround to explicitly declare the filters, but I thought I was doing something wrong with the auto generation via fields.

I agree that they should not fail silently. maybe a "currently not supported" warning/error would be nice to have.

@rpkilby
Copy link
Collaborator

rpkilby commented Nov 15, 2019

No worries - it's a confusing bug, especially given the general confusion over Meta.fields.

I agree that they should not fail silently. maybe a "currently not supported" warning/error would be nice to have.

If it weren't for the bug in #1013, you should have gotten a warning indicated that the field/lookup isn't supported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants