Skip to content

Commit

Permalink
[1.1] Deprecations for 2.0 (#792)
Browse files Browse the repository at this point in the history
* Deprecate the 'Meta.together' option

* 'Filter.name' => 'Filter.field_name'

* Add strictness migration note
  • Loading branch information
Ryan P Kilby authored and carltongibson committed Oct 19, 2017
1 parent 91002d5 commit 9a1c122
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 28 deletions.
51 changes: 33 additions & 18 deletions django_filters/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@ class Filter(object):
creation_counter = 0
field_class = forms.Field

def __init__(self, name=None, label=None, method=None, lookup_expr='exact',
def __init__(self, field_name=None, label=None, method=None, lookup_expr='exact',
distinct=False, exclude=False, **kwargs):
self.name = name
self.field_name = field_name
if field_name is None and 'name' in kwargs:
deprecate("`Filter.name` has been renamed to `Filter.field_name`.")
self.field_name = kwargs.pop('name')
self.label = label
self.method = method
self.lookup_expr = lookup_expr
Expand Down Expand Up @@ -129,12 +132,24 @@ def fset(self, value):
return locals()
method = property(**method())

def name():
def fget(self):
deprecate("`Filter.name` has been renamed to `Filter.field_name`.")
return self.field_name

def fset(self, value):
deprecate("`Filter.name` has been renamed to `Filter.field_name`.")
self.field_name = value

return locals()
name = property(**name())

def label():
def fget(self):
if self._label is None and hasattr(self, 'parent'):
model = self.parent._meta.model
self._label = label_for_filter(
model, self.name, self.lookup_expr, self.exclude
model, self.field_name, self.lookup_expr, self.exclude
)
return self._label

Expand Down Expand Up @@ -194,7 +209,7 @@ def filter(self, qs, value):
return qs
if self.distinct:
qs = qs.distinct()
qs = self.get_method(qs)(**{'%s__%s' % (self.name, lookup): value})
qs = self.get_method(qs)(**{'%s__%s' % (self.field_name, lookup): value})
return qs


Expand All @@ -217,7 +232,7 @@ def filter(self, qs, value):
if value != self.null_value:
return super(ChoiceFilter, self).filter(qs, value)

qs = self.get_method(qs)(**{'%s__%s' % (self.name, self.lookup_expr): None})
qs = self.get_method(qs)(**{'%s__%s' % (self.field_name, self.lookup_expr): None})
return qs.distinct() if self.distinct else qs


Expand Down Expand Up @@ -302,9 +317,9 @@ def filter(self, qs, value):

def get_filter_predicate(self, v):
try:
return {self.name: getattr(v, self.field.to_field_name)}
return {self.field_name: getattr(v, self.field.to_field_name)}
except (AttributeError, TypeError):
return {self.name: v}
return {self.field_name: v}


class TypedMultipleChoiceFilter(MultipleChoiceFilter):
Expand Down Expand Up @@ -413,13 +428,13 @@ class NumericRangeFilter(Filter):
def filter(self, qs, value):
if value:
if value.start is not None and value.stop is not None:
lookup = '%s__%s' % (self.name, self.lookup_expr)
lookup = '%s__%s' % (self.field_name, self.lookup_expr)
return self.get_method(qs)(**{lookup: (value.start, value.stop)})
else:
if value.start is not None:
qs = self.get_method(qs)(**{'%s__startswith' % self.name: value.start})
qs = self.get_method(qs)(**{'%s__startswith' % self.field_name: value.start})
if value.stop is not None:
qs = self.get_method(qs)(**{'%s__endswith' % self.name: value.stop})
qs = self.get_method(qs)(**{'%s__endswith' % self.field_name: value.stop})
if self.distinct:
qs = qs.distinct()
return qs
Expand All @@ -431,13 +446,13 @@ class RangeFilter(Filter):
def filter(self, qs, value):
if value:
if value.start is not None and value.stop is not None:
lookup = '%s__range' % self.name
lookup = '%s__range' % self.field_name
return self.get_method(qs)(**{lookup: (value.start, value.stop)})
else:
if value.start is not None:
qs = self.get_method(qs)(**{'%s__gte' % self.name: value.start})
qs = self.get_method(qs)(**{'%s__gte' % self.field_name: value.start})
if value.stop is not None:
qs = self.get_method(qs)(**{'%s__lte' % self.name: value.stop})
qs = self.get_method(qs)(**{'%s__lte' % self.field_name: value.stop})
if self.distinct:
qs = qs.distinct()
return qs
Expand Down Expand Up @@ -489,7 +504,7 @@ def filter(self, qs, value):
value = ''

assert value in self.options
qs = self.options[value][1](qs, self.name)
qs = self.options[value][1](qs, self.field_name)
if self.distinct:
qs = qs.distinct()
return qs
Expand All @@ -511,7 +526,7 @@ class AllValuesFilter(ChoiceFilter):
@property
def field(self):
qs = self.model._default_manager.distinct()
qs = qs.order_by(self.name).values_list(self.name, flat=True)
qs = qs.order_by(self.field_name).values_list(self.field_name, flat=True)
self.extra['choices'] = [(o, o) for o in qs]
return super(AllValuesFilter, self).field

Expand All @@ -520,7 +535,7 @@ class AllValuesMultipleFilter(MultipleChoiceFilter):
@property
def field(self):
qs = self.model._default_manager.distinct()
qs = qs.order_by(self.name).values_list(self.name, flat=True)
qs = qs.order_by(self.field_name).values_list(self.field_name, flat=True)
self.extra['choices'] = [(o, o) for o in qs]
return super(AllValuesMultipleFilter, self).field

Expand Down Expand Up @@ -695,7 +710,7 @@ def __call__(self, qs, value):
if value in EMPTY_VALUES:
return qs

return self.method(qs, self.f.name, value)
return self.method(qs, self.f.field_name, value)

@property
def method(self):
Expand All @@ -711,7 +726,7 @@ def method(self):
# otherwise, method is the name of a method on the parent FilterSet.
assert hasattr(instance, 'parent'), \
"Filter '%s' must have a parent FilterSet to find '.%s()'" % \
(instance.name, instance.method)
(instance.field_name, instance.method)

parent = instance.parent
method = getattr(parent, instance.method, None)
Expand Down
19 changes: 11 additions & 8 deletions django_filters/filterset.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
UUIDFilter
)
from .utils import (
deprecate,
get_all_model_fields,
get_model_field,
resolve_field,
Expand Down Expand Up @@ -75,6 +76,8 @@ def __init__(self, options=None):

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

if hasattr(options, 'together'):
deprecate('The `Meta.together` option has been deprecated in favor of overriding `Form.clean`.', 1)
self.together = getattr(options, 'together', None)


Expand All @@ -96,10 +99,10 @@ def get_declared_filters(cls, bases, attrs):
if isinstance(obj, Filter)
]

# Default the `filter.name` to the attribute name on the filterset
# Default the `filter.field_name` to the attribute name on the filterset
for filter_name, f in filters:
if getattr(f, 'name', None) is None:
f.name = filter_name
if getattr(f, 'field_name', None) is None:
f.field_name = filter_name

filters.sort(key=lambda x: x[1].creation_counter)

Expand Down Expand Up @@ -338,11 +341,11 @@ def get_filters(cls):
return filters

@classmethod
def filter_for_field(cls, f, name, lookup_expr='exact'):
def filter_for_field(cls, f, field_name, lookup_expr='exact'):
f, lookup_type = resolve_field(f, lookup_expr)

default = {
'name': name,
'field_name': field_name,
'lookup_expr': lookup_expr,
}

Expand All @@ -353,16 +356,16 @@ def filter_for_field(cls, f, name, lookup_expr='exact'):
"%s resolved field '%s' with '%s' lookup to an unrecognized field "
"type %s. Try adding an override to 'Meta.filter_overrides'. See: "
"https://django-filter.readthedocs.io/en/develop/ref/filterset.html#customise-filter-generation-with-filter-overrides"
) % (cls.__name__, name, lookup_expr, f.__class__.__name__)
) % (cls.__name__, field_name, lookup_expr, f.__class__.__name__)

return filter_class(**default)

@classmethod
def filter_for_reverse_field(cls, f, name):
def filter_for_reverse_field(cls, f, field_name):
rel = remote_field(f.field)
queryset = f.field.model._default_manager.all()
default = {
'name': name,
'field_name': field_name,
'queryset': queryset,
}
if rel.multiple:
Expand Down
38 changes: 36 additions & 2 deletions docs/guide/migration.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,40 @@
================
===============
Migration Guide
===============

----------------
Migrating to 2.0
----------------

Removal of the ``Meta.together`` option
---------------------------------------

The ``Meta.together`` has been deprecated in favor of userland implementations
that override the ``clean`` method of the ``Meta.form`` class. An example will
be provided in a "recipes" secion in future docs.

``Filter.name`` renamed to ``Filter.field_name``
------------------------------------------------

The filter ``name`` has been renamed to ``field_name`` as a way to disambiguate
the filter's attribute name on its FilterSet class from the ``field_name`` used
for filtering purposes.

FilterSet strictness has been removed
-------------------------------------

Strictness handling has been removed from the ``FilterSet`` and added to the
view layer. As a result, the ``FILTERS_STRICTNESS`` setting, ``Meta.strict``
option, and ``strict`` argument for the ``FilterSet`` initializer have all
been removed.

To alter strictness behavior, the appropriate view code should be overridden.
More details will be provided in future docs.


----------------
Migrating to 1.0
================
----------------

The 1.0 release of django-filter introduces several API changes and refinements
that break forwards compatibility. Below is a list of deprecations and
Expand Down
54 changes: 54 additions & 0 deletions tests/test_deprecations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@

import warnings

from django.test import TestCase

from django_filters import FilterSet, filters


class TogetherOptionDeprecationTests(TestCase):

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

class F(FilterSet):
class Meta:
together = ['a', 'b']

self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, DeprecationWarning))
self.assertIn('The `Meta.together` option has been deprecated', str(w[0].message))


class FilterNameDeprecationTests(TestCase):

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

class F(FilterSet):
foo = filters.CharFilter(name='foo')

self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, DeprecationWarning))
self.assertIn("`Filter.name` has been renamed to `Filter.field_name`.", str(w[0].message))

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

filters.CharFilter().name

self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, DeprecationWarning))
self.assertIn("`Filter.name` has been renamed to `Filter.field_name`.", str(w[0].message))

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

filters.CharFilter().name = 'bar'

self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, DeprecationWarning))
self.assertIn("`Filter.name` has been renamed to `Filter.field_name`.", str(w[0].message))

0 comments on commit 9a1c122

Please sign in to comment.