Skip to content

Deprecate DjangoFilterBackend #4507

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

Closed
Closed
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
2 changes: 1 addition & 1 deletion requirements/requirements-optionals.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Optional packages which may be used with REST framework.
markdown==2.6.4
django-guardian==1.4.3
django-filter==0.13.0
django-filter==0.15.0
coreapi==1.32.0
116 changes: 21 additions & 95 deletions rest_framework/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
from __future__ import unicode_literals

import operator
import warnings
from functools import reduce

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.db import models
from django.db.models.constants import LOOKUP_SEP
Expand All @@ -16,50 +16,10 @@
from django.utils.translation import ugettext_lazy as _

from rest_framework.compat import (
crispy_forms, distinct, django_filters, guardian, template_render
distinct, django_filters, guardian, template_render
)
from rest_framework.settings import api_settings

if 'crispy_forms' in settings.INSTALLED_APPS and crispy_forms and django_filters:
# If django-crispy-forms is installed, use it to get a bootstrap3 rendering
# of the DjangoFilterBackend controls when displayed as HTML.
from crispy_forms.helper import FormHelper
from crispy_forms.layout import Layout, Submit

class FilterSet(django_filters.FilterSet):
def __init__(self, *args, **kwargs):
super(FilterSet, self).__init__(*args, **kwargs)
for field in self.form.fields.values():
field.help_text = None

layout_components = list(self.form.fields.keys()) + [
Submit('', _('Submit'), css_class='btn-default'),
]

helper = FormHelper()
helper.form_method = 'GET'
helper.template_pack = 'bootstrap3'
helper.layout = Layout(*layout_components)

self.form.helper = helper

filter_template = 'rest_framework/filters/django_filter_crispyforms.html'

elif django_filters:
# If django-crispy-forms is not installed, use the standard
# 'form.as_p' rendering when DjangoFilterBackend is displayed as HTML.
class FilterSet(django_filters.FilterSet):
def __init__(self, *args, **kwargs):
super(FilterSet, self).__init__(*args, **kwargs)
for field in self.form.fields.values():
field.help_text = None

filter_template = 'rest_framework/filters/django_filter.html'

else:
FilterSet = None
filter_template = None


class BaseFilterBackend(object):
"""
Expand All @@ -80,67 +40,33 @@ class DjangoFilterBackend(BaseFilterBackend):
"""
A filter backend that uses django-filter.
"""
default_filter_set = FilterSet
template = filter_template

def __init__(self):
def __new__(cls, *args, **kwargs):
assert django_filters, 'Using DjangoFilterBackend, but django-filter is not installed'
assert django_filters.VERSION >= (0, 15, 0), 'django-filter 0.15.0 and above is required'

def get_filter_class(self, view, queryset=None):
"""
Return the django-filters `FilterSet` used to filter the queryset.
"""
filter_class = getattr(view, 'filter_class', None)
filter_fields = getattr(view, 'filter_fields', None)

if filter_class:
filter_model = filter_class.Meta.model
warnings.warn(
"'rest_framework.filters.DjangoFilterBackend' has been deprecated "
"in favor of 'django_filters.rest_framework.DjangoFilterBackend'",
DeprecationWarning
)

assert issubclass(queryset.model, filter_model), \
'FilterSet model %s does not match queryset model %s' % \
(filter_model, queryset.model)
from django_filters.rest_framework import DjangoFilterBackend as DFBackend

return filter_class
class DjangoFilterDeprecationBackend(DFBackend):

if filter_fields:
class AutoFilterSet(self.default_filter_set):
class Meta:
model = queryset.model
fields = filter_fields
# get_fields was introduced after the move to django-filter
def get_fields(self, view):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a test on get_fields would fix the patch coverage failure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given my comment here, I'm not really sure how to write a meaningful test.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomchristie a small amount of guidance if you please... :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows introspection of filter fields for schema generation. It's possible that we ought to change the signature prior to 3.5, so that it returns more information that simply the names. Currently undecided on that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So - just leave it as is for now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tomchristie. The 3.5 release seems imminent - did you want to merge this in for 3.5 or hold of until later? Aside from fixing conflicts, anything else I should clean up?

filter_class = getattr(view, 'filter_class', None)
if filter_class:
return list(filter_class().filters.keys())

return AutoFilterSet
filter_fields = getattr(view, 'filter_fields', None)
if filter_fields:
return filter_fields

return None
return []

def filter_queryset(self, request, queryset, view):
filter_class = self.get_filter_class(view, queryset)

if filter_class:
return filter_class(request.query_params, queryset=queryset).qs

return queryset

def to_html(self, request, queryset, view):
filter_class = self.get_filter_class(view, queryset)
if not filter_class:
return None
filter_instance = filter_class(request.query_params, queryset=queryset)
context = {
'filter': filter_instance
}
template = loader.get_template(self.template)
return template_render(template, context)

def get_fields(self, view):
filter_class = getattr(view, 'filter_class', None)
if filter_class:
return list(filter_class().filters.keys())

filter_fields = getattr(view, 'filter_fields', None)
if filter_fields:
return filter_fields

return []
return DjangoFilterDeprecationBackend(*args, **kwargs)


class SearchFilter(BaseFilterBackend):
Expand Down
34 changes: 34 additions & 0 deletions tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import datetime
import unittest
import warnings
from decimal import Decimal

from django.conf.urls import url
Expand Down Expand Up @@ -134,6 +135,39 @@ class IntegrationTestFiltering(CommonFilteringTestCase):
Integration tests for filtered list views.
"""

@unittest.skipUnless(django_filters, 'django-filter not installed')
def test_backend_deprecation(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")

view = FilterFieldsRootView.as_view()
request = factory.get('/')
response = view(request).render()

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data, self.data)

self.assertTrue(issubclass(w[-1].category, DeprecationWarning))
self.assertIn("'rest_framework.filters.DjangoFilterBackend' has been deprecated", str(w[-1].message))

@unittest.skipUnless(django_filters, 'django-filter not installed')
def test_no_df_deprecation(self):
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter("always")

import django_filters.rest_framework

class DFFilterFieldsRootView(FilterFieldsRootView):
filter_backends = (django_filters.rest_framework.DjangoFilterBackend,)

view = DFFilterFieldsRootView.as_view()
request = factory.get('/')
response = view(request).render()

self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(response.data, self.data)
self.assertEqual(len(w), 0)

@unittest.skipUnless(django_filters, 'django-filter not installed')
def test_get_filtered_fields_root_view(self):
"""
Expand Down