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

Move filter options to Meta class #459

Merged
merged 4 commits into from
Aug 23, 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
40 changes: 27 additions & 13 deletions django_filters/filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,12 @@ def __init__(self, options=None):
self.fields = getattr(options, 'fields', None)
self.exclude = getattr(options, 'exclude', None)

self.filter_overrides = getattr(options, 'filter_overrides', {})

self.order_by = getattr(options, 'order_by', False)
self.order_by_field = getattr(options, 'order_by_field', ORDER_BY_FIELD)

self.strict = getattr(options, 'strict', STRICTNESS.RETURN_NO_RESULTS)

self.form = getattr(options, 'form', forms.Form)

Expand All @@ -175,6 +180,10 @@ def __new__(cls, name, bases, attrs):
opts = new_class._meta = FilterSetOptions(
getattr(new_class, 'Meta', None))

if hasattr(new_class, 'filter_overrides'):
deprecate('filter_overrides has been moved to the Meta class.')
new_class._meta.filter_overrides = new_class.filter_overrides

# TODO: replace with deprecations
# if opts.model and opts.fields:
if opts.model:
Expand All @@ -188,6 +197,14 @@ def __new__(cls, name, bases, attrs):
raise TypeError("Meta.fields contains a field that isn't defined "
"on this FilterSet: {}".format(not_defined))

if hasattr(new_class, 'strict'):
deprecate('strict has been deprecated. Use Meta.strict instead.')
new_class._meta.strict = new_class.strict

if hasattr(new_class, 'order_by_field'):
deprecate('order_by_field has been moved to the Meta class.')
new_class._meta.order_by_field = new_class.order_by_field

new_class.declared_filters = declared_filters
new_class.base_filters = filters
return new_class
Expand Down Expand Up @@ -284,20 +301,16 @@ def __new__(cls, name, bases, attrs):


class BaseFilterSet(object):
filter_overrides = {}
order_by_field = ORDER_BY_FIELD
# What to do on on validation errors
strict = STRICTNESS.RETURN_NO_RESULTS

def __init__(self, data=None, queryset=None, prefix=None, strict=None):
self.is_bound = data is not None
self.data = data or {}
if queryset is None:
queryset = self._meta.model._default_manager.all()
self.queryset = queryset
self.form_prefix = prefix
if strict is not None:
self.strict = strict

# What to do on on validation errors
self.strict = self._meta.strict if strict is None else strict

self.filters = copy.deepcopy(self.base_filters)
# propagate the model being used through the filters
Expand Down Expand Up @@ -360,8 +373,8 @@ def qs(self):
qs = filter_.filter(qs, value)

if self._meta.order_by:
order_field = self.form.fields[self.order_by_field]
data = self.form[self.order_by_field].data
order_field = self.form.fields[self._meta.order_by_field]
data = self.form[self._meta.order_by_field].data
ordered_value = None
try:
ordered_value = order_field.clean(data)
Expand All @@ -371,7 +384,7 @@ def qs(self):
# With a None-queryset, ordering must be enforced (#84).
if (ordered_value in EMPTY_VALUES and
self.strict == STRICTNESS.RETURN_NO_RESULTS):
ordered_value = self.form.fields[self.order_by_field].choices[0][0]
ordered_value = self.form.fields[self._meta.order_by_field].choices[0][0]

if ordered_value:
qs = qs.order_by(*self.get_order_by(ordered_value))
Expand All @@ -386,7 +399,7 @@ def form(self):
fields = OrderedDict([
(name, filter_.field)
for name, filter_ in six.iteritems(self.filters)])
fields[self.order_by_field] = self.ordering_field
fields[self._meta.order_by_field] = self.ordering_field
Form = type(str('%sForm' % self.__class__.__name__),
(self._meta.form,), fields)
if self._meta.together:
Expand Down Expand Up @@ -447,7 +460,7 @@ def filters_for_model(cls, model, opts):
fields = opts.fields
if fields is None:
DEFAULTS = dict(FILTER_FOR_DBFIELD_DEFAULTS)
DEFAULTS.update(cls.filter_overrides)
DEFAULTS.update(opts.filter_overrides)
fields = get_all_model_fields(model, field_types=DEFAULTS.keys())

return filters_for_model(
Expand Down Expand Up @@ -494,7 +507,8 @@ def filter_for_reverse_field(cls, f, name):
@classmethod
def filter_for_lookup(cls, f, lookup_type):
DEFAULTS = dict(FILTER_FOR_DBFIELD_DEFAULTS)
DEFAULTS.update(cls.filter_overrides)
if hasattr(cls, '_meta'):
DEFAULTS.update(cls._meta.filter_overrides)

data = try_dbfield(DEFAULTS.get, f.__class__) or {}
filter_class = data.get('filter_class')
Expand Down
30 changes: 30 additions & 0 deletions docs/migration.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,33 @@ the ``Meta.exclude`` attribute.
class Meta:
model = User
exclude = ['password']


Move FilterSet options to Meta class
------------------------------------
Details: https://github.com/carltongibson/django-filter/issues/430

Several ``FilterSet`` options have been moved to the ``Meta`` class to prevent
potential conflicts with declared filter names. This includes:

* ``filter_overrides``
* ``strict``
* ``order_by_field``

.. code-block:: python

# 0.x
class UserFilter(FilterSet):
filter_overrides = {}
strict = STRICTNESS.RAISE_VALIDATION_ERROR
order_by_field = 'order'
...

# 1.0
class UserFilter(FilterSet):
...

class Meta:
filter_overrides = {}
strict = STRICTNESS.RAISE_VALIDATION_ERROR
order_by_field = 'order'
83 changes: 80 additions & 3 deletions tests/test_deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,91 @@ def test_fields_not_set_with_override(self):
warnings.simplefilter("always")

class F(FilterSet):
filter_overrides = {
SubnetMaskField: {'filter_class': CharFilter}
}

class Meta:
model = NetworkSetting
filter_overrides = {
SubnetMaskField: {'filter_class': CharFilter},
}

self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
self.assertIn("Not setting Meta.fields with Meta.model is undocumented behavior", str(w[-1].message))

self.assertEqual(list(F.base_filters.keys()), ['ip', 'mask'])


class StrictnessDeprecationTests(TestCase):
def test_notification(self):

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")

class F(FilterSet):
strict = False

self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))

def test_passthrough(self):
with warnings.catch_warnings(record=True):
warnings.simplefilter("always")

class F(FilterSet):
strict = False

self.assertEqual(F._meta.strict, False)


class FilterOverridesDeprecationTests(TestCase):

def test_notification(self):

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")

class F(FilterSet):
filter_overrides = {}

self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))

def test_passthrough(self):
with warnings.catch_warnings(record=True):
warnings.simplefilter("always")

class F(FilterSet):
filter_overrides = {
SubnetMaskField: {'filter_class': CharFilter},
}

class Meta:
model = NetworkSetting
fields = '__all__'

self.assertDictEqual(F._meta.filter_overrides, {
SubnetMaskField: {'filter_class': CharFilter},
})

self.assertEqual(list(F.base_filters.keys()), ['ip', 'mask'])


class OrderByFieldDeprecationTests(TestCase):
def test_notification(self):

with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")

class F(FilterSet):
order_by_field = 'field'

self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[-1].category, DeprecationWarning))

def test_passthrough(self):
with warnings.catch_warnings(record=True):
warnings.simplefilter("always")

class F(FilterSet):
order_by_field = 'field'

self.assertEqual(F._meta.order_by_field, 'field')
2 changes: 1 addition & 1 deletion tests/test_filtering.py
Original file line number Diff line number Diff line change
Expand Up @@ -857,11 +857,11 @@ def test_filtering_without_strict(self):

class F(FilterSet):
username = AllValuesFilter()
strict = False

class Meta:
model = User
fields = ['username']
strict = False

self.assertEqual(list(F().qs), list(User.objects.all()))
self.assertEqual(list(F({'username': 'alex'}).qs),
Expand Down
35 changes: 17 additions & 18 deletions tests/test_filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,15 @@ def test_filter_for_RANGE_lookup(self):

def test_isnull_with_filter_overrides(self):
class OFilterSet(FilterSet):
filter_overrides = {
models.BooleanField: {
'filter_class': BooleanFilter,
'extra': lambda f: {
'widget': BooleanWidget,
class Meta:
filter_overrides = {
models.BooleanField: {
'filter_class': BooleanFilter,
'extra': lambda f: {
'widget': BooleanWidget,
},
},
},
}
}

f = Article._meta.get_field('author')
result, params = OFilterSet.filter_for_lookup(f, 'isnull')
Expand Down Expand Up @@ -483,13 +484,14 @@ class Meta:

def test_custom_field_gets_filter_from_override(self):
class F(FilterSet):
filter_overrides = {
SubnetMaskField: {'filter_class': CharFilter}}

class Meta:
model = NetworkSetting
fields = '__all__'

filter_overrides = {
SubnetMaskField: {'filter_class': CharFilter}
}

self.assertEqual(list(F.base_filters.keys()), ['ip', 'mask'])

def test_filterset_for_proxy_model(self):
Expand Down Expand Up @@ -609,26 +611,24 @@ class Meta:

def test_ordering_on_unknown_value_results_in_default_ordering_without_strict(self):
class F(FilterSet):
strict = STRICTNESS.IGNORE

class Meta:
model = User
fields = ['username', 'status']
order_by = ['status']
strict = STRICTNESS.IGNORE

self.assertFalse(F.strict)
self.assertFalse(F._meta.strict)
f = F({'o': 'username'}, queryset=self.qs)
self.assertQuerysetEqual(
f.qs, ['alex', 'jacob', 'aaron', 'carl'], lambda o: o.username)

def test_ordering_on_unknown_value_results_in_default_ordering_with_strict_raise(self):
class F(FilterSet):
strict = STRICTNESS.RAISE_VALIDATION_ERROR

class Meta:
model = User
fields = ['username', 'status']
order_by = ['status']
strict = STRICTNESS.RAISE_VALIDATION_ERROR

f = F({'o': 'username'}, queryset=self.qs)
with self.assertRaises(ValidationError) as excinfo:
Expand Down Expand Up @@ -683,16 +683,15 @@ class Meta:

def test_ordering_with_overridden_field_name(self):
"""
Set the `order_by_field` on the queryset and ensure that the
Set the `order_by_field` on the filterset and ensure that the
field name is respected.
"""
class F(FilterSet):
order_by_field = 'order'

class Meta:
model = User
fields = ['username', 'status']
order_by = ['status']
order_by_field = 'order'

f = F({'order': 'status'}, queryset=self.qs)
self.assertQuerysetEqual(
Expand Down
13 changes: 5 additions & 8 deletions tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,16 +215,15 @@ class Meta:

def test_ordering_with_overridden_field_name(self):
"""
Set the `order_by_field` on the queryset and ensure that the
Set the `order_by_field` on the filterset and ensure that the
field name is respected.
"""
class F(FilterSet):
order_by_field = 'order'

class Meta:
model = User
fields = ['username', 'status']
order_by = ['status']
order_by_field = 'order'

f = F().form
self.assertNotIn('o', f.fields)
Expand All @@ -233,16 +232,15 @@ class Meta:

def test_ordering_with_overridden_field_name_and_descending(self):
"""
Set the `order_by_field` on the queryset and ensure that the
Set the `order_by_field` on the filterset and ensure that the
field name is respected.
"""
class F(FilterSet):
order_by_field = 'order'

class Meta:
model = User
fields = ['username', 'status']
order_by = ['status', '-status']
order_by_field = 'order'

f = F().form
self.assertNotIn('o', f.fields)
Expand All @@ -251,12 +249,11 @@ class Meta:

def test_ordering_with_overridden_field_name_and_using_all_fields(self):
class F(FilterSet):
order_by_field = 'order'

class Meta:
model = User
fields = ['username', 'status']
order_by = True
order_by_field = 'order'

f = F().form
self.assertIn('order', f.fields)
Expand Down