We use a modified version of this that allows us to query on multiple
fields.

Signed-off-by: Stephen Finucane <step...@that.guru>
Fixes: #156
---
 patchwork/api/filters.py                           | 103 ++++++++++++---------
 patchwork/tests/api/test_patch.py                  |  15 ++-
 .../improved-rest-filtering-bf68399270a9b245.yaml  |  10 +-
 3 files changed, 82 insertions(+), 46 deletions(-)

diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
index 030f9af3..b30499d0 100644
--- a/patchwork/api/filters.py
+++ b/patchwork/api/filters.py
@@ -19,10 +19,11 @@
 
 from django.contrib.auth.models import User
 from django.core.exceptions import ValidationError
+from django.db.models import Q
 from django_filters import FilterSet
 from django_filters import IsoDateTimeFilter
-from django_filters import ModelChoiceFilter
-from django.forms import ModelChoiceField
+from django_filters import ModelMultipleChoiceFilter
+from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField
 
 from patchwork.models import Bundle
 from patchwork.models import Check
@@ -37,79 +38,96 @@ from patchwork.models import State
 
 # custom fields, filters
 
-class ModelMultiChoiceField(ModelChoiceField):
-
-    def _get_filters(self, value):
-        raise NotImplementedError
-
-    def to_python(self, value):
-        if value in self.empty_values:
-            return None
+class ModelMultipleChoiceField(BaseMultipleChoiceField):
 
+    def _get_filter(self, value):
         try:
-            filters = {'pk': int(value)}
+            return 'pk', int(value)
         except ValueError:
-            filters = {self.alternate_lookup: value}
-
+            return self.alternate_lookup, value
+
+    def _check_values(self, value):
+        """
+        Given a list of possible PK values, returns a QuerySet of the
+        corresponding objects. Raises a ValidationError if a given value is
+        invalid (not a valid PK, not in the queryset, etc.)
+        """
+        # deduplicate given values to avoid creating many querysets or
+        # requiring the database backend deduplicate efficiently.
         try:
-            value = self.queryset.get(**filters)
-        except (ValueError, TypeError, self.queryset.model.DoesNotExist):
-            raise ValidationError(self.error_messages['invalid_choice'],
-                                  code='invalid_choice')
-        return value
+            value = frozenset(value)
+        except TypeError:
+            # list of lists isn't hashable, for example
+            raise ValidationError(
+                self.error_messages['list'],
+                code='list',
+            )
+
+        q_objects = Q()
+
+        for pk in value:
+            key, val = self._get_filter(pk)
+
+            try:
+                # NOTE(stephenfin): In contrast to the Django implementation
+                # of this, we check to ensure each specified key exists and
+                # fail if not. If we don't this, we can end up doing nothing
+                # for the filtering which, to me, seems very confusing
+                self.queryset.get(**{key: val})
+            except (ValueError, TypeError, self.queryset.model.DoesNotExist):
+                raise ValidationError(
+                    self.error_messages['invalid_pk_value'],
+                    code='invalid_pk_value',
+                    params={'pk': val},
+                )
 
+            q_objects |= Q(**{key: val})
 
-class ProjectChoiceField(ModelMultiChoiceField):
+        qs = self.queryset.filter(q_objects)
+
+        return qs
+
+
+class ProjectChoiceField(ModelMultipleChoiceField):
 
     alternate_lookup = 'linkname__iexact'
 
 
-class ProjectFilter(ModelChoiceFilter):
+class ProjectFilter(ModelMultipleChoiceFilter):
 
     field_class = ProjectChoiceField
 
 
-class PersonChoiceField(ModelMultiChoiceField):
+class PersonChoiceField(ModelMultipleChoiceField):
 
     alternate_lookup = 'email__iexact'
 
 
-class PersonFilter(ModelChoiceFilter):
+class PersonFilter(ModelMultipleChoiceFilter):
 
     field_class = PersonChoiceField
 
 
-class StateChoiceField(ModelChoiceField):
+class StateChoiceField(ModelMultipleChoiceField):
 
-    def prepare_value(self, value):
-        if hasattr(value, '_meta'):
-            return value.slug
-        else:
-            return super(StateChoiceField, self).prepare_value(value)
-
-    def to_python(self, value):
-        if value in self.empty_values:
-            return None
+    def _get_filter(self, value):
         try:
-            value = ' '.join(value.split('-'))
-            value = self.queryset.get(name__iexact=value)
-        except (ValueError, TypeError, self.queryset.model.DoesNotExist):
-            raise ValidationError(self.error_messages['invalid_choice'],
-                                  code='invalid_choice')
-        return value
+            return 'pk', int(value)
+        except ValueError:
+            return 'name__iexact', ' '.join(value.split('-'))
 
 
-class StateFilter(ModelChoiceFilter):
+class StateFilter(ModelMultipleChoiceFilter):
 
     field_class = StateChoiceField
 
 
-class UserChoiceField(ModelMultiChoiceField):
+class UserChoiceField(ModelMultipleChoiceField):
 
     alternate_lookup = 'username__iexact'
 
 
-class UserFilter(ModelChoiceFilter):
+class UserFilter(ModelMultipleChoiceFilter):
 
     field_class = UserChoiceField
 
@@ -125,8 +143,7 @@ class TimestampMixin(FilterSet):
 
 class ProjectMixin(FilterSet):
 
-    project = ProjectFilter(to_field_name='linkname',
-                            queryset=Project.objects.all())
+    project = ProjectFilter(queryset=Project.objects.all())
 
 
 class SeriesFilter(ProjectMixin, TimestampMixin, FilterSet):
diff --git a/patchwork/tests/api/test_patch.py 
b/patchwork/tests/api/test_patch.py
index 909c1eb4..373ee0d5 100644
--- a/patchwork/tests/api/test_patch.py
+++ b/patchwork/tests/api/test_patch.py
@@ -71,8 +71,8 @@ class TestPatchAPI(APITestCase):
         self.assertEqual(0, len(resp.data))
 
         person_obj = create_person(email='t...@example.com')
-        state_obj = create_state(name='Under Review')
         project_obj = create_project(linkname='myproject')
+        state_obj = create_state(name='Under Review')
         patch_obj = create_patch(state=state_obj, project=project_obj,
                                  submitter=person_obj)
 
@@ -117,6 +117,19 @@ class TestPatchAPI(APITestCase):
             'submitter': 't...@example.org'})
         self.assertEqual(0, len(resp.data))
 
+        state_obj_b = create_state(name='New')
+        create_patch(state=state_obj_b)
+        state_obj_c = create_state(name='RFC')
+        create_patch(state=state_obj_c)
+
+        resp = self.client.get(self.api_url())
+        self.assertEqual(3, len(resp.data))
+        resp = self.client.get(self.api_url(), [('state', 'under-review')])
+        self.assertEqual(1, len(resp.data))
+        resp = self.client.get(self.api_url(), [('state', 'under-review'),
+                                                ('state', 'new')])
+        self.assertEqual(2, len(resp.data))
+
     def test_detail(self):
         """Validate we can get a specific patch."""
         patch = create_patch(
diff --git a/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml 
b/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml
index b1d12eb6..170e9621 100644
--- a/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml
+++ b/releasenotes/notes/improved-rest-filtering-bf68399270a9b245.yaml
@@ -8,9 +8,15 @@ api:
 
        $ curl /covers/?submitter=step...@that.guru
   - |
-    Bundles can be filtered by owner and checks by user using username. For
-    example:
+    Bundles can be filtered by owner, patches by delegate and checks by user
+    using username. For example:
 
     .. code-block:: shell
 
        $ curl /bundles/?owner=stephenfin
+  - |
+    Filters can now be specified multiple times. For example:
+
+    .. code-block:: shell
+
+       $ curl /patches/?state=under-review&state=rfc
-- 
2.14.3

_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork

Reply via email to