[PATCH] REST: Don't iterate through models at class level

Stephen Finucane stephen at that.guru
Tue Feb 7 06:48:37 AEDT 2017


This causes two issues. Firstly, on fresh installs you see the following
error message:

  "Table 'patchwork.patchwork_state' doesn't exist"

Secondly, any new states created when the process is running will not be
reflected in the API until the server process is restarted.

Resolve this issue by moving the step into a method, thus ensuring it's
continuously refreshed. It doesn't seem possible to add tests to prevent
this regressing but some similarly useful tests are included to at least
validate the behavior of that field.

Signed-off-by: Stephen Finucane <stephen at that.guru>
Fixes: a2993505 ("REST: Make 'Patch.state' editable")
Closes-bug: #67
Closes-bug: #80
Cc: denis at laxalde.org
Cc: steve at asksteved.com
---
 patchwork/api/base.py            |  5 -----
 patchwork/api/patch.py           |  7 ++++---
 patchwork/tests/test_rest_api.py | 14 ++++++++++++++
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/patchwork/api/base.py b/patchwork/api/base.py
index 13a8432..dbd8148 100644
--- a/patchwork/api/base.py
+++ b/patchwork/api/base.py
@@ -23,11 +23,6 @@ from rest_framework import permissions
 from rest_framework.pagination import PageNumberPagination
 from rest_framework.response import Response
 
-from patchwork.models import State
-
-STATE_CHOICES = ['-'.join(x.name.lower().split(' '))
-                 for x in State.objects.all()]
-
 
 class LinkHeaderPagination(PageNumberPagination):
     """Provide pagination based on rfc5988.
diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index e8fb0ef..2030dd9 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -28,7 +28,6 @@ from rest_framework.serializers import HyperlinkedModelSerializer
 from rest_framework.serializers import SerializerMethodField
 
 from patchwork.api.base import PatchworkPermission
-from patchwork.api.base import STATE_CHOICES
 from patchwork.api.filters import PatchFilter
 from patchwork.models import Patch
 from patchwork.models import State
@@ -38,7 +37,9 @@ class StateField(ChoiceField):
     """Avoid the need for a state endpoint."""
 
     def __init__(self, *args, **kwargs):
-        kwargs['choices'] = STATE_CHOICES
+        self.state_choices = ['-'.join(x.name.lower().split(' '))
+            for x in State.objects.all()]
+        kwargs['choices'] = self.state_choices
         super(StateField, self).__init__(*args, **kwargs)
 
     def to_internal_value(self, data):
@@ -47,7 +48,7 @@ class StateField(ChoiceField):
             return State.objects.get(name__iexact=data)
         except State.DoesNotExist:
             raise ValidationError('Invalid state. Expected one of: %s ' %
-                                  ', '.join(STATE_CHOICES))
+                                  ', '.join(self.state_choices))
 
     def to_representation(self, obj):
         return '-'.join(obj.name.lower().split())
diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
index cc1fcef..ace84cf 100644
--- a/patchwork/tests/test_rest_api.py
+++ b/patchwork/tests/test_rest_api.py
@@ -398,6 +398,20 @@ class TestPatchAPI(APITestCase):
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
         self.assertEqual(Patch.objects.get(id=patch.id).state, state)
 
+    def test_update_invalid(self):
+        """Ensure StateField is working as expected."""
+        project = create_project()
+        patch = create_patch(project=project)
+        user = create_maintainer(project)
+
+        self.client.force_authenticate(user=user)
+        resp = self.client.get(self.api_url(patch.id))
+        self.assertEqual(status.HTTP_200_OK, resp.status_code)
+
+        # invalid state
+        resp = self.client.patch(self.api_url(patch.id), {'state': 'foobar'})
+        self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
+
     def test_delete(self):
         """Ensure deletions are always rejected."""
         project = create_project()
-- 
2.9.3



More information about the Patchwork mailing list