Skip to content

Declared filters overwritten by AllLookupsFilter #123

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

Merged
merged 2 commits into from
Sep 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 30 additions & 33 deletions rest_framework_filters/filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import warnings

from django.db.models.constants import LOOKUP_SEP
from django.db.models.fields.related import ForeignObjectRel
from django.utils import six

from django_filters import filterset, rest_framework
Expand Down Expand Up @@ -35,7 +34,7 @@ class FilterSetMetaclass(filterset.FilterSetMetaclass):
def __new__(cls, name, bases, attrs):
new_class = super(FilterSetMetaclass, cls).__new__(cls, name, bases, attrs)
fix_filter_field = _get_fix_filter_field(new_class)
opts = new_class._meta
opts = copy.deepcopy(new_class._meta)

# order_by is not compatible.
if opts.order_by:
Expand All @@ -51,38 +50,36 @@ def __new__(cls, name, bases, attrs):
if not opts.model:
return new_class

# Populate our FilterSet fields with all the possible
# filters for the AllLookupsFilter field.
# Determine declared filters and filters to generate lookups from. Declared
# filters have precedence over generated filters and should not be overwritten.
declared_filters, lookups_filters = OrderedDict(), OrderedDict()
for name, f in six.iteritems(new_class.declared_filters):
if isinstance(f, (filters.AllLookupsFilter, filters.RelatedFilter)):
lookups_filters[name] = f

# `AllLookupsFilter` is an exception, as it should be overwritten
if not isinstance(f, filters.AllLookupsFilter):
declared_filters[name] = f

# generate filters for AllLookups/Related filters
# name is the parameter name on the filterset, f.name is the model field's name
for name, f in six.iteritems(lookups_filters):
opts.fields = {f.name: f.lookups or []}
new_filters = new_class.filters_for_model(opts.model, opts)

# filters_for_model generate param names from the model field name
# replace model field name with the parameter name from the filerset
new_class.base_filters.update(OrderedDict(
(param.replace(f.name, name, 1), v)
for param, v in six.iteritems(new_filters)
))

# re-apply declared filters (sans `AllLookupsFilter`s)
new_class.base_filters.update(declared_filters)

# TODO: remove with deprecations
for name, filter_ in six.iteritems(new_class.base_filters.copy()):
if isinstance(filter_, (filters.AllLookupsFilter, filters.RelatedFilter)):
field = filterset.get_model_field(opts.model, filter_.name)

lookups = filter_.lookups or []
if lookups == '__all__':
lookups = utils.lookups_for_field(field)

for lookup_expr in lookups:
if isinstance(filter_, filters.RelatedFilter) and lookup_expr == 'exact':
# Don't replace the RelatedFilter
continue

if isinstance(field, ForeignObjectRel):
f = new_class.filter_for_reverse_field(field, filter_.name)
else:
f = new_class.filter_for_field(field, filter_.name, lookup_expr)
f = fix_filter_field(f)

# compute filter name
filter_name = LOOKUP_SEP.join([name, lookup_expr])

# Don't add "exact" to filter names
_exact = LOOKUP_SEP + 'exact'
if filter_name.endswith(_exact):
filter_name = filter_name[:-len(_exact)]

new_class.base_filters[filter_name] = f

elif name not in new_class.declared_filters:
if name not in new_class.declared_filters:
new_class.base_filters[name] = fix_filter_field(filter_)

return new_class
Expand Down
38 changes: 31 additions & 7 deletions tests/test_filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,21 @@

class LookupsFilterTests(TestCase):
"""
Test basic filter construction for `AllLookupsFilter`, '__all__',
and `RelatedFilter.lookups`.
Test basic filter construction for `AllLookupsFilter`, '__all__', and `RelatedFilter.lookups`.
"""

def test_alllookupsfilter_meta_fields_unmodified(self):
f = []

class F(FilterSet):
id = filters.AllLookupsFilter()

class Meta:
model = Note
fields = f

self.assertIs(F._meta.fields, f)

def test_alllookupsfilter_replaced(self):
# See: https://github.com/philipn/django-rest-framework-filters/issues/118
class F(FilterSet):
Expand Down Expand Up @@ -91,16 +102,29 @@ class Meta:

def test_declared_filter_persistence_with__all__(self):
# ensure that __all__ does not overwrite declared filters.
f = filters.Filter()

class F(FilterSet):
name = filters.ChoiceFilter(lookup_expr='iexact')
name = f

class Meta:
model = Person
fields = {
'name': '__all__',
}
fields = {'name': '__all__'}

self.assertIs(F.base_filters['name'], f)

def test_declared_filter_persistence_with_alllookupsfilter(self):
# ensure that AllLookupsFilter does not overwrite declared filters.
f = filters.Filter()

class F(FilterSet):
id = filters.AllLookupsFilter()
id__in = f

class Meta:
model = Note

self.assertIsInstance(F.base_filters['name'], filters.ChoiceFilter)
self.assertIs(F.base_filters['id__in'], f)


class GetFilterNameTests(TestCase):
Expand Down