diff mbox series

REST: Exclude filters added in later version

Message ID 20191130164021.57099-1-stephen@that.guru
State Accepted
Headers show
Series REST: Exclude filters added in later version | expand

Commit Message

Stephen Finucane Nov. 30, 2019, 4:40 p.m. UTC
If a person requests API version 1.1, they should get the exact same
behavior regardless of the base Patchwork version. We already do this
for fields in the output, so now extend this to filters in the
querystring.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Cc: Daniel Axtens <dja@axtens.net>
---
 patchwork/api/filters.py          | 36 +++++++++++++++++++++++++------
 patchwork/tests/api/test_patch.py |  9 ++++++++
 2 files changed, 38 insertions(+), 7 deletions(-)

Comments

Stephen Finucane Nov. 30, 2019, 4:48 p.m. UTC | #1
On Sat, 2019-11-30 at 16:40 +0000, Stephen Finucane wrote:
> If a person requests API version 1.1, they should get the exact same
> behavior regardless of the base Patchwork version. We already do this
> for fields in the output, so now extend this to filters in the
> querystring.
> 
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Cc: Daniel Axtens <dja@axtens.net>

I'm going to apply this just so I can keep moving with the stack of
patches that have built up during my absence, but if you spot anything
that looks off about this let me know and we can address the issues or
straight up revert.

Stephen

> ---
>  patchwork/api/filters.py          | 36 +++++++++++++++++++++++++------
>  patchwork/tests/api/test_patch.py |  9 ++++++++
>  2 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
> index 4184ee82..6b4d84c7 100644
> --- a/patchwork/api/filters.py
> +++ b/patchwork/api/filters.py
> @@ -13,6 +13,7 @@ from django_filters import ModelMultipleChoiceFilter
>  from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField
>  from django.forms.widgets import MultipleHiddenInput
>  
> +from patchwork.api import utils
>  from patchwork.compat import NAME_FIELD
>  from patchwork.models import Bundle
>  from patchwork.models import Check
> @@ -136,14 +137,32 @@ class UserFilter(ModelMultipleChoiceFilter):
>  
>  # filter sets
>  
> -class TimestampMixin(FilterSet):
> +
> +class BaseFilterSet(FilterSet):
> +
> +    @property
> +    def form(self):
> +        form = super(BaseFilterSet, self).form
> +
> +        for version in getattr(self.Meta, 'versioned_fields', {}):
> +            if utils.has_version(self.request, version):
> +                continue
> +
> +            for field in self.Meta.versioned_fields[version]:
> +                if field in form.fields:
> +                    del form.fields[field]
> +
> +        return form
> +
> +
> +class TimestampMixin(BaseFilterSet):
>  
>      # TODO(stephenfin): These should filter on a 'updated_at' field instead
>      before = IsoDateTimeFilter(lookup_expr='lt', **{NAME_FIELD: 'date'})
>      since = IsoDateTimeFilter(lookup_expr='gte', **{NAME_FIELD: 'date'})
>  
>  
> -class SeriesFilterSet(TimestampMixin, FilterSet):
> +class SeriesFilterSet(TimestampMixin, BaseFilterSet):
>  
>      submitter = PersonFilter(queryset=Person.objects.all())
>      project = ProjectFilter(queryset=Project.objects.all())
> @@ -153,7 +172,7 @@ class SeriesFilterSet(TimestampMixin, FilterSet):
>          fields = ('submitter', 'project')
>  
>  
> -class CoverLetterFilterSet(TimestampMixin, FilterSet):
> +class CoverLetterFilterSet(TimestampMixin, BaseFilterSet):
>  
>      project = ProjectFilter(queryset=Project.objects.all())
>      # NOTE(stephenfin): We disable the select-based HTML widgets for these
> @@ -167,7 +186,7 @@ class CoverLetterFilterSet(TimestampMixin, FilterSet):
>          fields = ('project', 'series', 'submitter')
>  
>  
> -class PatchFilterSet(TimestampMixin, FilterSet):
> +class PatchFilterSet(TimestampMixin, BaseFilterSet):
>  
>      project = ProjectFilter(queryset=Project.objects.all())
>      # NOTE(stephenfin): We disable the select-based HTML widgets for these
> @@ -187,9 +206,12 @@ class PatchFilterSet(TimestampMixin, FilterSet):
>          # which seems to rather defeat the point of using django-filters.
>          fields = ('project', 'series', 'submitter', 'delegate',
>                    'state', 'archived', 'hash')
> +        versioned_fields = {
> +            '1.2': ('hash', ),
> +        }
>  
>  
> -class CheckFilterSet(TimestampMixin, FilterSet):
> +class CheckFilterSet(TimestampMixin, BaseFilterSet):
>  
>      user = UserFilter(queryset=User.objects.all())
>  
> @@ -198,7 +220,7 @@ class CheckFilterSet(TimestampMixin, FilterSet):
>          fields = ('user', 'state', 'context')
>  
>  
> -class EventFilterSet(TimestampMixin, FilterSet):
> +class EventFilterSet(TimestampMixin, BaseFilterSet):
>  
>      # NOTE(stephenfin): We disable the select-based HTML widgets for these
>      # filters as the resulting query is _huge_
> @@ -217,7 +239,7 @@ class EventFilterSet(TimestampMixin, FilterSet):
>          fields = ('project', 'category', 'series', 'patch', 'cover')
>  
>  
> -class BundleFilterSet(FilterSet):
> +class BundleFilterSet(BaseFilterSet):
>  
>      project = ProjectFilter(queryset=Project.objects.all())
>      owner = UserFilter(queryset=User.objects.all())
> diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
> index 4afc15a9..bf3ef9f8 100644
> --- a/patchwork/tests/api/test_patch.py
> +++ b/patchwork/tests/api/test_patch.py
> @@ -188,6 +188,15 @@ class TestPatchAPI(utils.APITestCase):
>              'hash': 'da638d0746a115000bf890fada1f02679aa282e8'})
>          self.assertEqual(0, len(resp.data))
>  
> +    def test_list_filter_hash_version_1_1(self):
> +        """Filter patches by hash using API v1.1."""
> +        self._create_patch()
> +
> +        # we still see the patch since the hash field is ignored
> +        resp = self.client.get(self.api_url(version='1.1'),
> +                               {'hash': 'garbagevalue'})
> +        self.assertEqual(1, len(resp.data))
> +
>      @utils.store_samples('patch-list-1-0')
>      def test_list_version_1_0(self):
>          """List patches using API v1.0."""
diff mbox series

Patch

diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
index 4184ee82..6b4d84c7 100644
--- a/patchwork/api/filters.py
+++ b/patchwork/api/filters.py
@@ -13,6 +13,7 @@  from django_filters import ModelMultipleChoiceFilter
 from django.forms import ModelMultipleChoiceField as BaseMultipleChoiceField
 from django.forms.widgets import MultipleHiddenInput
 
+from patchwork.api import utils
 from patchwork.compat import NAME_FIELD
 from patchwork.models import Bundle
 from patchwork.models import Check
@@ -136,14 +137,32 @@  class UserFilter(ModelMultipleChoiceFilter):
 
 # filter sets
 
-class TimestampMixin(FilterSet):
+
+class BaseFilterSet(FilterSet):
+
+    @property
+    def form(self):
+        form = super(BaseFilterSet, self).form
+
+        for version in getattr(self.Meta, 'versioned_fields', {}):
+            if utils.has_version(self.request, version):
+                continue
+
+            for field in self.Meta.versioned_fields[version]:
+                if field in form.fields:
+                    del form.fields[field]
+
+        return form
+
+
+class TimestampMixin(BaseFilterSet):
 
     # TODO(stephenfin): These should filter on a 'updated_at' field instead
     before = IsoDateTimeFilter(lookup_expr='lt', **{NAME_FIELD: 'date'})
     since = IsoDateTimeFilter(lookup_expr='gte', **{NAME_FIELD: 'date'})
 
 
-class SeriesFilterSet(TimestampMixin, FilterSet):
+class SeriesFilterSet(TimestampMixin, BaseFilterSet):
 
     submitter = PersonFilter(queryset=Person.objects.all())
     project = ProjectFilter(queryset=Project.objects.all())
@@ -153,7 +172,7 @@  class SeriesFilterSet(TimestampMixin, FilterSet):
         fields = ('submitter', 'project')
 
 
-class CoverLetterFilterSet(TimestampMixin, FilterSet):
+class CoverLetterFilterSet(TimestampMixin, BaseFilterSet):
 
     project = ProjectFilter(queryset=Project.objects.all())
     # NOTE(stephenfin): We disable the select-based HTML widgets for these
@@ -167,7 +186,7 @@  class CoverLetterFilterSet(TimestampMixin, FilterSet):
         fields = ('project', 'series', 'submitter')
 
 
-class PatchFilterSet(TimestampMixin, FilterSet):
+class PatchFilterSet(TimestampMixin, BaseFilterSet):
 
     project = ProjectFilter(queryset=Project.objects.all())
     # NOTE(stephenfin): We disable the select-based HTML widgets for these
@@ -187,9 +206,12 @@  class PatchFilterSet(TimestampMixin, FilterSet):
         # which seems to rather defeat the point of using django-filters.
         fields = ('project', 'series', 'submitter', 'delegate',
                   'state', 'archived', 'hash')
+        versioned_fields = {
+            '1.2': ('hash', ),
+        }
 
 
-class CheckFilterSet(TimestampMixin, FilterSet):
+class CheckFilterSet(TimestampMixin, BaseFilterSet):
 
     user = UserFilter(queryset=User.objects.all())
 
@@ -198,7 +220,7 @@  class CheckFilterSet(TimestampMixin, FilterSet):
         fields = ('user', 'state', 'context')
 
 
-class EventFilterSet(TimestampMixin, FilterSet):
+class EventFilterSet(TimestampMixin, BaseFilterSet):
 
     # NOTE(stephenfin): We disable the select-based HTML widgets for these
     # filters as the resulting query is _huge_
@@ -217,7 +239,7 @@  class EventFilterSet(TimestampMixin, FilterSet):
         fields = ('project', 'category', 'series', 'patch', 'cover')
 
 
-class BundleFilterSet(FilterSet):
+class BundleFilterSet(BaseFilterSet):
 
     project = ProjectFilter(queryset=Project.objects.all())
     owner = UserFilter(queryset=User.objects.all())
diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
index 4afc15a9..bf3ef9f8 100644
--- a/patchwork/tests/api/test_patch.py
+++ b/patchwork/tests/api/test_patch.py
@@ -188,6 +188,15 @@  class TestPatchAPI(utils.APITestCase):
             'hash': 'da638d0746a115000bf890fada1f02679aa282e8'})
         self.assertEqual(0, len(resp.data))
 
+    def test_list_filter_hash_version_1_1(self):
+        """Filter patches by hash using API v1.1."""
+        self._create_patch()
+
+        # we still see the patch since the hash field is ignored
+        resp = self.client.get(self.api_url(version='1.1'),
+                               {'hash': 'garbagevalue'})
+        self.assertEqual(1, len(resp.data))
+
     @utils.store_samples('patch-list-1-0')
     def test_list_version_1_0(self):
         """List patches using API v1.0."""