Merge pull request #97 from rpkilby/fix-related-alllookups

Temporarily fix some issues with all lookups

Author: Ryan P Kilby

CHANGELOG.rst

@@ -1,11 +1,18 @@
-Unreleased
-----------
+Unreleased:
+-----------
 
-* Fixes #79, enabling compatibility with ``django.contrib.postgres``
-* Adds basic infinite recursion prevention for chainable transforms
+v0.8.1:
+-------
+
+* Fix bug where AllLookupsFilter would override a declared filter of the same name
+* #84 Fix AllLookupsFilter compatibility with ``ForeignObject`` related fields
+* #82 Fix AllLookupsFilter compatibility with mixin FilterSets
+* #81 Fix bug where FilterSet modified ``ViewSet.filter_fields``
+* #79 Prevent infinite recursion for chainable transforms, fixing compatiblity
+  w/ ``django.contrib.postgres``
 
-v0.8.0
-------
+v0.8.0:
+-------
 
 This release is tied to a major update of django-filter (more details in #66),
 which fixes how lookup expressions are resolved. 'in', 'range', and 'isnull'
@@ -15,8 +22,8 @@ This has the following effects:
   * Deprecates ArrayDecimalField/InSetNumberFilter
   * Deprecates ArrayCharField/InSetCharFilter
   * Deprecates FilterSet.fix_filter_field
-  * Deprecates ALL_LOOKUPS in favor of '__all__' constant.
-  * AllLookupsFilter now generates only valid lookup expressions.
+  * Deprecates ALL_LOOKUPS in favor of '__all__' constant
+  * AllLookupsFilter now generates only valid lookup expressions
 
 * #2 'range' lookup types do not work
 * #15 Date lookup types do not work (year, day, ...)
@@ -52,7 +59,7 @@ v0.5.0:
 * #31 Fix timezone-aware datetime handling
 * #36 Fix '__in' filter to work with strings
 * #33 Fix RelatedFilter handling to not override existing isnull filters
-* #35 Fix python 3.5 compatibility issue.
+* #35 Fix python 3.5 compatibility issue
 * Drops support for Django 1.6 and below
 
 v0.4.0:

rest_framework_filters/filterset.py

@@ -99,11 +99,14 @@ def convert__all__(attrs):
         """
         Extract Meta.fields and convert any fields w/ `__all__`
         to a declared AllLookupsFilter.
+
+        This is a temporary hack to fix #87.
         """
         meta = attrs.get('Meta', None)
+        model = getattr(meta, 'model', None)
         fields = getattr(meta, 'fields', None)
 
-        if isinstance(fields, dict):
+        if model and isinstance(fields, dict):
             for name, lookups in six.iteritems(fields.copy()):
                 if lookups == filters.ALL_LOOKUPS:
                     warnings.warn(
@@ -114,8 +117,10 @@ def convert__all__(attrs):
                     lookups = '__all__'
 
                 if lookups == '__all__':
-                    del fields[name]
-                    attrs[name] = filters.AllLookupsFilter()
+                    # Modifying fields is incorrect. The correct behavior will
+                    # require hooking into filters_for_model
+                    field = model._meta.get_field(name)
+                    fields[name] = utils.lookups_for_field(field)
 
 
 class FilterSet(six.with_metaclass(FilterSetMetaclass, filterset.FilterSet)):

rest_framework_filters/utils.py

@@ -4,6 +4,7 @@
 import django
 from django.db.models.constants import LOOKUP_SEP
 from django.db.models.expressions import Expression
+from django.db.models.fields.related import ForeignObject
 from django.db.models.lookups import Transform
 from django.utils import six
 
@@ -12,6 +13,11 @@ def lookups_for_field(model_field):
     """
     Generates a list of all possible lookup expressions for a model field.
     """
+    # This is a hack to work around:
+    # https://github.com/django/django/pull/6906
+    if isinstance(model_field, ForeignObject):
+        return ['exact', 'gt', 'gte', 'lt', 'lte', 'in', 'isnull']
+
     lookups = []
 
     for expr, lookup in six.iteritems(class_lookups(model_field)):

tests/test_filterset.py

@@ -7,6 +7,7 @@
 from django.test import TestCase
 
 from rest_framework_filters import FilterSet, filters
+from django_filters.filters import BaseInFilter
 
 from .testapp.models import (
     User, Note, Post, Cover, Page, A, B, C, Person, Tag, BlogPost,
@@ -156,7 +157,48 @@ def test_relatedfilter(self):
         note = list(f)[0]
         self.assertEqual(note.title, "Hello Test 4")
 
-    def test_relatedfilter_combined_with_alllookups(self):
+    def test_alllookupsfilter_for_relation(self):
+        # See: https://github.com/philipn/django-rest-framework-filters/issues/84
+        class F(FilterSet):
+            class Meta:
+                model = Note
+                fields = {
+                    'author': '__all__',
+                }
+
+        self.assertIsInstance(F.base_filters['author'], filters.ModelChoiceFilter)
+        self.assertIsInstance(F.base_filters['author__in'], BaseInFilter)
+
+    def test_relatedfilter_combined_with__all__(self):
+        # ensure that related filter is compatible with __all__ lookups.
+        class F(FilterSet):
+            author = filters.RelatedFilter(UserFilter)
+
+            class Meta:
+                model = Note
+                fields = {
+                    'author': '__all__',
+                }
+
+        self.assertIsInstance(F.base_filters['author'], filters.RelatedFilter)
+        self.assertIsInstance(F.base_filters['author__in'], BaseInFilter)
+
+    def test_filter_persistence_with__all__(self):
+        # ensure that __all__ does not overwrite declared filters.
+        class F(FilterSet):
+            name = filters.ChoiceFilter(lookup_expr='iexact')
+
+            class Meta:
+                model = Person
+                fields = {
+                    'name': '__all__',
+                }
+
+        self.assertIsInstance(F.base_filters['name'], filters.ChoiceFilter)
+
+    def test_relatedfilter_for_related_alllookups(self):
+        # ensure that filters work for AllLookupsFilter across a RelatedFilter.
+
         # Test that the default exact filter works
         GET = {
             'author': User.objects.get(username='user2').pk,
@@ -195,7 +237,7 @@ def test_relatedfilter_combined_with_alllookups(self):
         f = NoteFilterWithRelatedAll(GET, queryset=Note.objects.all())
         self.assertEqual(len(list(f)), 4)
 
-    def test_relatedfilter_combined_with_alllookups_and_different_filter_name(self):
+    def test_relatedfilter_for_related_alllookups_and_different_filter_name(self):
         # Test that the default exact filter works
         GET = {
             'writer': User.objects.get(username='user2').pk,
@@ -294,7 +336,7 @@ def test_m2m_relation(self):
         }
         f = BlogPostFilter(GET, queryset=BlogPost.objects.all())
         self.assertEqual(len(list(f)), 2)
-        titles = set([p.title for p in f])
+        titles = set([person.title for person in f])
         self.assertEqual(titles, set(["First post", "Second post"]))
 
     def test_nonexistent_related_field(self):

tests/test_utils.py

@@ -5,7 +5,7 @@
 
 from rest_framework_filters import utils
 
-from .testapp.models import Person
+from .testapp.models import Person, Note
 
 
 class LookupsForFieldTests(TestCase):
@@ -29,6 +29,15 @@ def test_transformed_field(self):
         self.assertIn('year__exact', lookups)
         self.assertIn('date__year__exact', lookups)
 
+    def test_relation_field(self):
+        # ForeignObject relations are special cased currently
+        model_field = Note._meta.get_field('author')
+        lookups = utils.lookups_for_field(model_field)
+
+        self.assertIn('exact', lookups)
+        self.assertIn('in', lookups)
+        self.assertNotIn('regex', lookups)
+
 
 @unittest.skipIf(django.VERSION < (1, 9), "version does not support transformed lookup expressions")
 class LookupsForTransformTests(TestCase):