[PATCH 3/7] REST: Use ModelMultipleChoiceField
Stephen Finucane
stephen at that.guru
Thu Apr 12 02:13:34 AEST 2018
We use a modified version of this that allows us to query on multiple
fields.
Signed-off-by: Stephen Finucane <stephen at 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='test at 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': 'test at 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=stephen at 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
More information about the Patchwork
mailing list