philipn / django-rest-framework-filters

Better filtering for Django REST Framework
Other
848 stars 131 forks source link

Support `__in` for field with `choices` defined. #65

Closed ghost closed 8 years ago

ghost commented 8 years ago

The lookup type in is supported for CharField and NumberFiled though if a choices argument is passed to the field it won't work.

See fix_filter_field is using a condition on the type of f

    @classmethod
    def fix_filter_field(cls, f):
        """
        Fix the filter field based on the lookup type.
        """

        lookup_type = f.lookup_type
        if lookup_type == 'isnull':
            return filters.BooleanFilter(name=f.name, lookup_type='isnull')
        if lookup_type == 'in' and type(f) == filters.NumberFilter:
            return filters.InSetNumberFilter(name=f.name, lookup_type='in')
        if lookup_type == 'in' and type(f) == filters.CharFilter:
            return filters.InSetCharFilter(name=f.name, lookup_type='in')
        return f

Though if the field is provided with a choices attribute, it won't work. see https://github.com/alex/django-filter/blob/69767e12d0265fc4d0286db80aec88acd8783cf9/django_filters/filterset.py#L444 where it gets set to ChoiceFilter instead.

    if f.choices:
        default['choices'] = f.choices
        return ChoiceFilter(**default)

Right now the work around we are using is

class SomeFilter(filters.FilterSet):
    a_field = filters.AllLookupsFilter()
    a_field_in = filters.InSetCharFilter(name='a_field', lookup_type='in')

I have tried to play around a bit to find a fix but I could not come up with something sane in a timely matter. Maybe some one with more knowledge can weigh in and help out.

rpkilby commented 8 years ago

Hi @jackdbernier. If this is a one off problem, I would try something like the following:

class SomeFilter(filters.FilterSet):
    # Notice the double underscores separating `a_field` and `in`
    a_field__in = filters.InSetCharFilter(name='a_field', lookup_type='in', choices=choices)

    class Meta:
        model = SomeModel
        # I recommend explicitly declaring relevant lookups, instead of using AllLookupsFilter.
        fields = {
            'a_field': ['exact', 'gt', 'gte', 'lt', 'lte', 'etc...'],
        }

If this is a common issue for you, you could also override the behavior of fix_filter_field by subclassing FilterSet.

rpkilby commented 8 years ago

btw - I'm currently working on a patch for django-filter that should negate the need for a fix in drf-filters (fix_filter_field would no longer be necessary).

rpkilby commented 8 years ago

Hi @jackdbernier, django-filter v0.13.0 now supports in & range csv lookups, as well as date transforms. The upcoming release of drf-filters (v0.8.0) should be released soon, and no longer relies on fix_filter_field.

ghost commented 8 years ago

Great I'll give a try at the new version. Thanks.

evenicoulddoit commented 8 years ago

As a follow up to this here's one way of solving the problem:

import rest_framework_filters as filters

from models import Foo

class FooFilter(filters.FilterSet):
    class Meta:
        model = Foo
        fields = {
            'bar': ['exact', 'in'],
        }

Then you can use bar__in=baz,zulu as well as bar__in!=baz,zulu as expected.

I did need to use the fields dictionary though to explicitly provide the lookup types. Perhaps that means some follow-up work would just solve this if the "default all" approach used by django-rest-framework-filters included in?

rpkilby commented 8 years ago

@evenicoulddoit - AllLookupsFilter should include in filtering. What version of drf-filters are you using?

evenicoulddoit commented 8 years ago

@rpkilby djangorestframework-filters==0.8.0

sean-bennett112 commented 8 years ago

Not sure if this should be in the same issue or a separate one, but I can confirm just running into this as well. Using 0.8.0, it doesn't appear that in is included in AllLookupsFilter.

Rather, when I send up ?id__in=2,3,4,5,6, for example, I weirdly only get returned the entity with id 6.

I'm running into this in my test suite when using

self.client.get(self.api_url, data={'id__in': [2, 3, 4, 5, 6]})
class DeviceFilter(FilterSet):
    organization = RelatedFilter(organization_filter, name='organization')
    project = RelatedFilter(project_filter, name='project')

    id = AllLookupsFilter(name='id')
    created = AllLookupsFilter(name='created')
    updated = AllLookupsFilter(name='updated')
    name = AllLookupsFilter(name='name')

    class Meta:
        model = Device
sean-bennett112 commented 8 years ago

Actually ignore that, this was an error on my part. Passing the list in as a string 2,3,4,5,6 fixes everything for me.