Hi Stephen,
Thank you for looking into this issue!
Stephen Finucane a écrit :
This causes two issues. Firstly, on fresh installs you see the following
error message:
"Table 'patchwork.patchwork_state' doesn't exist"
Unfortunately, it does not solve this point because you still query the
DB for a table that does not exist in StateField.__init__() which is
called to define the "state" class attribute of PatchListSerializer,
which happens at module import.
Maybe the states choices could be initialized in a lazy manner?
Secondly, any new states created when the process is running will not be
reflected in the API until the server process is restarted.
This point is solved as far as I can tell.
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 <[email protected]>
Fixes: a2993505 ("REST: Make 'Patch.state' editable")
Closes-bug: #67
Closes-bug: #80
Cc: [email protected]
Cc: [email protected]
---
patchwork/api/base.py | 5 -----
patchwork/api/patch.py | 7 ++++---
patchwork/tests/test_rest_api.py | 14 ++++++++++++++
3 files changed, 18 insertions(+), 8 deletions(-)
[snip]
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)
+
I wonder if we shouldn't check the validation error message here, to
make sure it includes actual states of the DB, and not only those
present at server startup. Something like:
self.assertEqual(['Invalid state. Expected one of: state_0, state_1 '],
resp.json()['state'])
with a `create_state()` at the beginning of test method.
_______________________________________________
Patchwork mailing list
[email protected]
https://lists.ozlabs.org/listinfo/patchwork
