Merge pull request #145 from rpkilby/multiwidget-caveat

Document MultiWidget incompatibility

Author: Ryan P Kilby

README.rst

@@ -24,6 +24,10 @@ Using ``django-rest-framework-filters``, we can easily do stuff like::
     /api/article?author__first_name__icontains=john
     /api/article?is_published!=true
 
+.. contents::
+    **Table of Contents**
+    :local:
+    :depth: 2
 
 Features
 --------
@@ -305,8 +309,8 @@ To work around this, you have the following options:
             model = Product
 
 
-Can I mix and match `django-filter` and `django-rest-framework-filters`?
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Can I mix and match ``django-filter`` and ``django-rest-framework-filters``?
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 Yes you can. ``django-rest-framework-filters`` is simply an extension of ``django-filter``. Note
 that ``RelatedFilter`` and other ``django-rest-framework-filters`` features are designed to work
@@ -332,6 +336,49 @@ and ``FilterSet``s from either package are compatible with the other's DRF backe
         drf = rest_framework_filters.RelatedFilter(filterset=DRFFilter)
 
 
+Caveats & Limitations
+~~~~~~~~~~~~~~~~~~~~~
+
+``MultiWidget`` is incompatible
+"""""""""""""""""""""""""""""""
+
+djangorestframework-filters is not compatible with form widgets that parse query names that differ from the filter's
+attribute name. Although this only practically applies to ``MultiWidget``, it is a general limitation that affects
+custom widgets that also have this behavior. Affected filters include ``RangeFilter``, ``DateTimeFromToRangeFilter``,
+``DateFromToRangeFilter``, ``TimeRangeFilter``, and ``NumericRangeFilter``.
+
+To demonstrate the incompatiblity, take the following filterset:
+
+.. code-block:: python
+
+    class PostFilter(FilterSet):
+        publish_date = filters.DateFromToRangeFilter()
+
+The above filter allows users to perform a ``range`` query on the publication date. The filter class internally uses
+``MultiWidget`` to separately parse the upper and lower bound values. The incompatibility lies in that ``MultiWidget``
+appends an index to its inner widget names. Instead of parsing ``publish_date``, it expects ``publish_date_0`` and
+``publish_date_1``. It is possible to fix this by including the attribute name in the querystring, although this is
+not recommended.
+
+.. code-block::
+
+    ?publish_date_0=2016-01-01&publish_date_1=2016-02-01&publish_date=
+
+``MultiWidget`` is also discouraged since:
+
+* ``core-api`` field introspection fails for similar reasons
+* ``_0`` and ``_1`` are less API-friendly than ``_min`` and ``_max``
+
+The recommended solutions are to either:
+
+* Create separate filters for each of the sub-widgets (such as ``publish_date_min`` and ``publish_date_max``).
+* Use a CSV-based filter such as those derived from ``BaseCSVFilter``/``BaseInFilter``/``BaseRangeFilter``. eg,
+
+.. code-block::
+
+    ?publish_date__range=2016-01-01,2016-02-01
+
+
 License
 -------
 Copyright (c) 2013-2015 Philip Neustrom <philipn@gmail.com>,

tests/test_filtering.py

@@ -6,6 +6,7 @@
 
 from rest_framework_filters.compat import set_many
 from rest_framework_filters import FilterSet, filters
+from django_filters import FilterSet as DFFilterSet
 
 from .testapp.models import (
     User, Note, Post, Cover, A, B, C, Person, Tag, BlogPost,
@@ -324,3 +325,46 @@ def test_related_filters_caching(self):
         self.assertIn('_related_filters', PostFilter.__dict__)
 
         self.assertEqual(len(filters), 1)
+
+
+class MiscTests(TestCase):
+    def test_multiwidget_incompatibility(self):
+        Person.objects.create(name='A')
+
+        # test django-filter functionality
+        class PersonFilter(DFFilterSet):
+            date_joined = filters.DateFromToRangeFilter(name='date_joined')
+
+            class Meta:
+                model = Person
+                fields = ['date_joined']
+
+        # Test from ... to 2016-01-01
+        GET = {
+            'date_joined_1': '2016-01-01',
+        }
+        f = PersonFilter(GET, queryset=Person.objects.all())
+        self.assertEqual(f.qs.count(), 0)
+
+        # test drf-filters caveat
+        class PersonFilter(FilterSet):
+            date_joined = filters.DateFromToRangeFilter(name='date_joined')
+
+            class Meta:
+                model = Person
+                fields = ['date_joined']
+
+        # Test from ... to 2016-01-01, failure case
+        GET = {
+            'date_joined_1': '2016-01-01',
+        }
+        f = PersonFilter(GET, queryset=Person.objects.all())
+        self.assertEqual(f.qs.count(), 1)
+
+        # Test from ... to 2016-01-01, "fix"
+        GET = {
+            'date_joined_1': '2016-01-01',
+            'date_joined': '',
+        }
+        f = PersonFilter(GET, queryset=Person.objects.all())
+        self.assertEqual(f.qs.count(), 0)

tests/test_performance.py

@@ -21,7 +21,7 @@ class PerformanceTests(TestCase):
     def setUpTestData(cls):
         cls.client = Client()
         cls.iterations = range(1000)
-        cls.threshold = 1.2
+        cls.threshold = 1.3
 
     def test_sanity(self):
         # sanity check to ensure our request are behaving as expected