[RFC 09/11] REST: Add query parameters to filter patches by

[Finucane, Stephen]

On 10 May 17:30, Andy Doan wrote:
> On 05/09/2016 09:06 AM, Finucane, Stephen wrote:
> >On 15 Apr 13:24, Andy Doan wrote:
> 
> >>+    def test_query_filters(self):
> >>+        """Test out our filtering support."""
> >>+        patches = create_patches(4)
> >>+        project = Project.objects.create(
> >>+            linkname='foo', name='Foo', listid='foo.example.com')
> >>+        patches[0].project = project
> >>+        patches[0].save()
> >>+        resp = self.client.get('/api/1.0/patches/?project=Foo')
> >
> >I have a feeling we've discussed this before (and it doesn't really
> >belong in this patch), but why are we not nesting patches under
> >projects? Seems like a clear hierarchy to me.
> 
> You thought that by nesting patches under a project that you should
> be able to get sequential ids like:
> 
>  projects/foo/1
>  projects/foo/2
> 
> but "2" might be a patch in project bar. And creating a per-project
> unique patch ID isn't a fool-proof as you'd think it should be.

Yup, that was it. Cheers :)

> >>diff --git a/patchwork/views/rest_api.py b/patchwork/views/rest_api.py
> 
> >>+    query_filters = {
> >>+        'project': 'project__name',
> >>+        'submitter': 'submitter__name',
> >>+        'delegate': 'delegate__name',
> >>+        'state': 'state__name',
> >>+        'since': 'date__gt',
> >>+        'until': 'date__lt',
> >>+    }
> >>+
> >
> >This seems to be a reinvention of parts of 'filters.py'. Could that
> >code be integrated here in any way?
> 
> Its probably possible. Would probably require some re-work of
> filters.py to support things like since and until and the default
> listing needs to be all patches.
> 
> I see a couple of ways forward:
> 
>  * leave this patch out for now and revisit
>  * leave the patch as is
>  * take the time now to integrate nicely with filters.py
> 
> let me know what you'd like to see.

If it doesn't cost us anything, I would favour leaving this feature out
for now and reviewing as a separate series (option 1). I'd be concerned
about bringing more tech debt on our heads by choosing another option.
In addition, there's probably more things we can do here - e.g. adding
'updated_at' and 'created_at' fields to some models, that would make
things easier. Are you OK with this?

Stephen