[PATCH] REST: Filter projects by 'linkname', not 'name'

[Daniel Axtens]

Hi Stephen,

Merged to master     at 174de19d41d76f8387027e789e5975fc078f2d08
Merged to stable/2.0 at b23f206b941694a03bf3d0a94fcbdb4f3d146f85

Thanks!

Regards,
Daniel

Stephen Finucane <stephen at that.guru> writes:

> Based on a report on the OVS mailing list, it appears that projects are
> being filtered on the 'Project.name' field instead of the
> 'Project.linkname' field. Correct this and add regression tests to
> prevent it happening again.
>
> Signed-off-by: Stephen Finucane <stephen at that.guru>
> Fixes: 08b2115 ("REST: Allow filtering by both project ID and linkname")
> Closes-bug: #117 ("Projects are filtered on the wrong field")
> Cc: Daniel Axtens <dja at axtens.net>
> ---
> daxtens - if you've time (and this looks good), you might handle the
> merge and backport part of this. Everything about the latter should be
> documented in enough detail:
>
>   https://patchwork.readthedocs.io/en/latest/development/releasing/#backporting
>
> Feel free to ping me here on IRC (stephenfin on #freenode) if necessary.
> ---
>  patchwork/api/filters.py         |  2 +-
>  patchwork/tests/test_rest_api.py | 39 +++++++++++++++++++++++++++++++--------
>  2 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/patchwork/api/filters.py b/patchwork/api/filters.py
> index 9966543..314aa47 100644
> --- a/patchwork/api/filters.py
> +++ b/patchwork/api/filters.py
> @@ -50,7 +50,7 @@ class ProjectChoiceField(ModelChoiceField):
>          try:
>              filters = {'pk': int(value)}
>          except ValueError:
> -            filters = {'name__iexact': ' '.join(value.split('-'))}
> +            filters = {'linkname__iexact': ' '.join(value.split('-'))}
>  
>          try:
>              value = self.queryset.get(**filters)
> diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
> index abffd17..14e53b2 100644
> --- a/patchwork/tests/test_rest_api.py
> +++ b/patchwork/tests/test_rest_api.py
> @@ -97,7 +97,23 @@ class TestProjectAPI(APITestCase):
>          self.assertEqual(status.HTTP_200_OK, resp.status_code)
>          self.assertSerialized(project, resp.data)
>  
> -    def test_get_numeric_linkname(self):
> +    def test_get_by_id(self):
> +        """Validate that it's possible to filter by pk."""
> +        project = create_project()
> +
> +        resp = self.client.get(self.api_url(project.pk))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertSerialized(project, resp.data)
> +
> +    def test_get_by_linkname(self):
> +        """Validate that it's possible to filter by linkname."""
> +        project = create_project(linkname='project', name='Sample project')
> +
> +        resp = self.client.get(self.api_url('project'))
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code)
> +        self.assertSerialized(project, resp.data)
> +
> +    def test_get_by_numeric_linkname(self):
>          """Validate we try to do the right thing for numeric linkname"""
>          project = create_project(linkname='12345')
>  
> @@ -326,7 +342,8 @@ class TestPatchAPI(APITestCase):
>          self.assertEqual(0, len(resp.data))
>  
>          state_obj = create_state(name='Under Review')
> -        patch_obj = create_patch(state=state_obj)
> +        project_obj = create_project(linkname='myproject')
> +        patch_obj = create_patch(state=state_obj, project=project_obj)
>  
>          # anonymous user
>          resp = self.client.get(self.api_url())
> @@ -338,12 +355,6 @@ class TestPatchAPI(APITestCase):
>          self.assertNotIn('content', patch_rsp)
>          self.assertNotIn('diff', patch_rsp)
>  
> -        # test filtering by state
> -        resp = self.client.get(self.api_url(), {'state': 'under-review'})
> -        self.assertEqual([patch_obj.id], [x['id'] for x in resp.data])
> -        resp = self.client.get(self.api_url(), {'state': 'missing-state'})
> -        self.assertEqual(0, len(resp.data))
> -
>          # authenticated user
>          user = create_user()
>          self.client.force_authenticate(user=user)
> @@ -353,6 +364,18 @@ class TestPatchAPI(APITestCase):
>          patch_rsp = resp.data[0]
>          self.assertSerialized(patch_obj, patch_rsp)
>  
> +        # test filtering by state
> +        resp = self.client.get(self.api_url(), {'state': 'under-review'})
> +        self.assertEqual([patch_obj.id], [x['id'] for x in resp.data])
> +        resp = self.client.get(self.api_url(), {'state': 'missing-state'})
> +        self.assertEqual(0, len(resp.data))
> +
> +        # test filtering by project
> +        resp = self.client.get(self.api_url(), {'project': 'myproject'})
> +        self.assertEqual([patch_obj.id], [x['id'] for x in resp.data])
> +        resp = self.client.get(self.api_url(), {'project': 'invalidproject'})
> +        self.assertEqual(0, len(resp.data))
> +
>      def test_detail(self):
>          """Validate we can get a specific patch."""
>          patch = create_patch(
> -- 
> 2.13.5