Message ID | 20191130164021.57099-1-stephen@that.guru |
---|---|
State | Accepted |
Headers | show |
Series | REST: Exclude filters added in later version | expand |
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 --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."""
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(-)