Deprecate MethodFilter (#113)

Author: Ryan P Kilby

rest_framework_filters/filters.py

@@ -84,6 +84,11 @@ class MethodFilter(Filter):
     def __init__(self, *args, **kwargs):
         self.action = kwargs.pop('action', '')
         super(MethodFilter, self).__init__(*args, **kwargs)
+        warnings.warn(
+            'MethodFilter is deprecated and no longer necessary. See: '
+            'https://github.com/philipn/django-rest-framework-filters/issues/109',
+            DeprecationWarning, stacklevel=2
+        )
 
     def resolve_action(self):
         """

tests/test_deprecations.py

@@ -154,3 +154,19 @@ class Meta:
             # Generate another warning for field
             F({'id__in': '1'}).qs
             self.assertEqual(len(w), 2)
+
+
+class MethodFilterDeprecationTests(TestCase):
+
+    def test_notification(self):
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+
+            class F(FilterSet):
+                username__in = filters.MethodFilter()
+
+                class Meta:
+                    model = User
+                    fields = []
+
+            self.assertEqual(len(w), 1)

tests/test_filterset.py

@@ -2,8 +2,6 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
-import datetime
-
 import django
 from django.test import TestCase
 
@@ -22,7 +20,6 @@
     NoteFilterWithRelatedAll,
     NoteFilterWithRelatedAllDifferentFilterName,
     PostFilter,
-    CoverFilterWithRelatedMethodFilter,
     CoverFilterWithRelated,
     # PageFilterWithRelated,
     TagFilter,
@@ -556,44 +553,6 @@ def test_non_filter_subset(self):
         self.assertEqual(len(filterset_class.base_filters), 0)
 
 
-class MethodFilterTests(TestCase):
-
-    @classmethod
-    def setUpTestData(cls):
-        user = User.objects.create(username="user1", email="user1@example.org")
-
-        note1 = Note.objects.create(title="Test 1", content="Test content 1", author=user)
-        note2 = Note.objects.create(title="Test 2", content="Test content 2", author=user)
-
-        post1 = Post.objects.create(note=note1, content="Test content in post 1")
-        post2 = Post.objects.create(note=note2, content="Test content in post 2", date_published=datetime.date.today())
-
-        Cover.objects.create(post=post1, comment="Cover 1")
-        Cover.objects.create(post=post2, comment="Cover 2")
-
-    def test_method_filter(self):
-        GET = {
-            'is_published': 'true'
-        }
-        filterset = PostFilter(GET, queryset=Post.objects.all())
-        results = list(filterset.qs)
-        self.assertEqual(len(results), 1)
-        self.assertEqual(results[0].content, "Test content in post 2")
-
-    def test_related_method_filter(self):
-        """
-        Missing MethodFilter filter methods are silently ignored, returning
-        the unfiltered queryset.
-        """
-        GET = {
-            'post__is_published': 'true'
-        }
-        filterset = CoverFilterWithRelatedMethodFilter(GET, queryset=Cover.objects.all())
-        results = list(filterset.qs)
-        self.assertEqual(len(results), 1)
-        self.assertEqual(results[0].comment, "Cover 2")
-
-
 class FilterOverrideTests(TestCase):
 
     def test_declared_filters(self):

tests/test_regressions.py

@@ -19,14 +19,16 @@
 from rest_framework.renderers import JSONRenderer
 
 from .testapp.models import (
-    User, Person,
+    User, Person, Note, Post, Cover,
 )
 
 from .testapp.filters import (
     UserFilter,
     AllLookupsPersonDateFilter,
     InSetLookupPersonIDFilter,
     InSetLookupPersonNameFilter,
+    PostFilter,
+    CoverFilterWithRelatedMethodFilter,
 )
 
 
@@ -284,3 +286,44 @@ def test_isnull_override(self):
         results = list(filterset.qs)
         self.assertEqual(len(results), 1)
         self.assertEqual(results[0].username, 'user2')
+
+
+class FilterMethodTests(TestCase):
+    """
+    Old test case for MethodFilter. Ensure that the new Filter.method remains compatible.
+    """
+
+    @classmethod
+    def setUpTestData(cls):
+        user = User.objects.create(username="user1", email="user1@example.org")
+
+        note1 = Note.objects.create(title="Test 1", content="Test content 1", author=user)
+        note2 = Note.objects.create(title="Test 2", content="Test content 2", author=user)
+
+        post1 = Post.objects.create(note=note1, content="Test content in post 1")
+        post2 = Post.objects.create(note=note2, content="Test content in post 2", date_published=datetime.date.today())
+
+        Cover.objects.create(post=post1, comment="Cover 1")
+        Cover.objects.create(post=post2, comment="Cover 2")
+
+    def test_method_filter(self):
+        GET = {
+            'is_published': 'true'
+        }
+        filterset = PostFilter(GET, queryset=Post.objects.all())
+        results = list(filterset.qs)
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0].content, "Test content in post 2")
+
+    def test_related_method_filter(self):
+        """
+        Missing MethodFilter filter methods are silently ignored, returning
+        the unfiltered queryset.
+        """
+        GET = {
+            'post__is_published': 'true'
+        }
+        filterset = CoverFilterWithRelatedMethodFilter(GET, queryset=Cover.objects.all())
+        results = list(filterset.qs)
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0].comment, "Cover 2")

tests/testapp/filters.py

@@ -68,31 +68,23 @@ class Meta:
 
 
 class PostFilter(FilterSet):
-    # Used for Related filter and MethodFilter tests
+    # Used for Related filter and Filter.method regression tests
     note = RelatedFilter(NoteFilterWithRelatedAll, name='note')
     date_published = filters.AllLookupsFilter()
-    is_published = filters.MethodFilter()
+    is_published = filters.BooleanFilter(name='date_published', method='filter_is_published')
 
     class Meta:
         model = Post
 
-    def filter_is_published(self, name, qs, value):
+    def filter_is_published(self, qs, name, value):
         """
         `is_published` is based on the actual `date_published`.
         If the publishing date is null, then the post is not published.
         """
-        # convert value to boolean
-        null = value.lower() != 'true'
-
-        # The lookup name will end with `is_published`, but could be
-        # preceded by a related lookup path.
-        if LOOKUP_SEP in name:
-            rel, _ = name.rsplit(LOOKUP_SEP, 1)
-            name = LOOKUP_SEP.join([rel, 'date_published__isnull'])
-        else:
-            name = 'date_published__isnull'
-
-        return qs.filter(**{name: null})
+        isnull = not value
+        lookup_expr = LOOKUP_SEP.join([name, 'isnull'])
+
+        return qs.filter(**{lookup_expr: isnull})
 
 
 class CoverFilterWithRelatedMethodFilter(FilterSet):