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

Stephen Finucane stephen at that.guru
Thu Aug 31 18:48:51 AEST 2017


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



More information about the Patchwork mailing list