REST: Make 'Patch.state' editable

The 'Patch.state' field is exposed by the API, but is not editable.
Tests exist that suggest the field is editable, but they lie as they
don't actually validate the field is changed (it hasn't). Make this
field editable, using a custom field type to allow said user to submit a
string representation of the state rather than an integer id.

This has the side effect of changing the output representation of the
'state' field for the '/patches' endpoint to a slugified representation,
i.e.:

    "state": "under-review",

Instead of:

    "state": "Under review",

The same slugified representation will be used in PATCH requests. This
seems more consistent with how APIs generally do this stuff making it a
"good thing" (TM).

Signed-off-by: Stephen Finucane <stephen@that.guru>
Reviewed-by: Andy Doan <andy.doan@linaro.org>
Reviewed-by: Daniel Axtens <dja@axtens.net>

diff --git a/patchwork/api/base.py b/patchwork/api/base.py
index dbd8148d033f3563162ae5c1865dd55f68051b5d..13a84322580a32f3b55e5c53323effdc6f5abeb7 100644 (file)
--- a/patchwork/api/base.py
+++ b/patchwork/api/base.py
@@ -23,6 +23,11 @@ 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 842745033d17da561911c9c2a1c4efb9f4682c3a..f818f2041917292a7cc1de6a3eab57ed0257e272 100644 (file)
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -20,25 +20,45 @@
 import email.parser
 
 from django.core.urlresolvers import reverse
-from rest_framework.serializers import HyperlinkedModelSerializer
+from rest_framework.exceptions import ValidationError
 from rest_framework.generics import ListAPIView
 from rest_framework.generics import RetrieveUpdateAPIView
+from rest_framework.serializers import ChoiceField
+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.models import Patch
+from patchwork.models import State
+
+
+class StateField(ChoiceField):
+    """Avoid the need for a state endpoint."""
+
+    def __init__(self, *args, **kwargs):
+        kwargs['choices'] = STATE_CHOICES
+        super(StateField, self).__init__(*args, **kwargs)
+
+    def to_internal_value(self, data):
+        data = ' '.join(data.split('-'))
+        try:
+            return State.objects.get(name__iexact=data)
+        except State.DoesNotExist:
+            raise ValidationError('Invalid state. Expected one of: %s ' %
+                                  ', '.join(STATE_CHOICES))
+
+    def to_representation(self, obj):
+        return '-'.join(obj.name.lower().split())
 
 
 class PatchListSerializer(HyperlinkedModelSerializer):
     mbox = SerializerMethodField()
-    state = SerializerMethodField()
+    state = StateField()
     tags = SerializerMethodField()
     check = SerializerMethodField()
     checks = SerializerMethodField()
 
-    def get_state(self, instance):
-        return instance.state.name
-
     def get_mbox(self, instance):
         request = self.context.get('request')
         return request.build_absolute_uri(instance.get_mbox_url())

diff --git a/patchwork/tests/test_rest_api.py b/patchwork/tests/test_rest_api.py
index 469fd267950c142217e7a7157d5a45d0fe266bd8..d76494508c239cd491d8361f567b4530952afbbf 100644 (file)
--- a/patchwork/tests/test_rest_api.py
+++ b/patchwork/tests/test_rest_api.py
@@ -31,6 +31,7 @@ from patchwork.tests.utils import create_maintainer
 from patchwork.tests.utils import create_patch
 from patchwork.tests.utils import create_person
 from patchwork.tests.utils import create_project
+from patchwork.tests.utils import create_state
 from patchwork.tests.utils import create_user
 
 if settings.ENABLE_REST_API:
@@ -368,19 +369,22 @@ class TestPatchAPI(APITestCase):
         # A maintainer can update
         project = create_project()
         patch = create_patch(project=project)
+        state = create_state()
         user = create_maintainer(project)
         self.client.force_authenticate(user=user)
 
+        self.assertNotEqual(Patch.objects.get(id=patch.id).state, state)
         resp = self.client.patch(self.api_url(patch.id),
-                                 {'state': 2})
+                                 {'state': state.name})
         self.assertEqual(status.HTTP_200_OK, resp.status_code)
+        self.assertEqual(Patch.objects.get(id=patch.id).state, state)
 
         # A normal user can't
         user = create_user()
         self.client.force_authenticate(user=user)
 
         resp = self.client.patch(self.api_url(patch.id),
-                                 {'state': 2})
+                                 {'state': state.name})
         self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code)
 
     def test_delete(self):