diff mbox series

[06/10] REST: A check must specify a state

Message ID 20190430060308.10432-7-dja@axtens.net
State Accepted
Headers show
Series Patchwork 2.1.2 review series | expand

Commit Message

Daniel Axtens April 30, 2019, 6:03 a.m. UTC 
The Ozlabs crew noticed that a check without a state caused a
KeyError in data['state']. Mark state as mandatory, check for
it, and add a test.

Reported-by: Russell Currey <ruscur@russell.cc>
Reported-by: Jeremy Kerr <jk@ozlabs.org>
Signed-off-by: Daniel Axtens <dja@axtens.net>
(backported from commit 7a20ccda99e48dab643d1fbd7e170fe3e4c47185
 No Swagger schema changes in the stable backport.)
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 patchwork/api/check.py            |  4 ++++
 patchwork/tests/api/test_check.py | 16 ++++++++++++++++
 2 files changed, 20 insertions(+)

Comments

Stephen Finucane April 30, 2019, 5:50 p.m. UTC | #1 
On Tue, 2019-04-30 at 16:03 +1000, Daniel Axtens wrote:
> The Ozlabs crew noticed that a check without a state caused a
> KeyError in data['state']. Mark state as mandatory, check for
> it, and add a test.
> 
> Reported-by: Russell Currey <ruscur@russell.cc>
> Reported-by: Jeremy Kerr <jk@ozlabs.org>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> (backported from commit 7a20ccda99e48dab643d1fbd7e170fe3e4c47185
>  No Swagger schema changes in the stable backport.)
> Signed-off-by: Daniel Axtens <dja@axtens.net>

Applied.
diff mbox series

Patch

diff --git a/patchwork/api/check.py b/patchwork/api/check.py
index 62e6fd19e761..1498abbbffb2 100644
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -26,6 +26,7 @@  from rest_framework.generics import RetrieveAPIView
 from rest_framework.serializers import CurrentUserDefault
 from rest_framework.serializers import HiddenField
 from rest_framework.serializers import HyperlinkedModelSerializer
+from rest_framework.serializers import ValidationError
 
 from patchwork.api.base import CheckHyperlinkedIdentityField
 from patchwork.api.base import MultipleFieldLookupMixin
@@ -50,6 +51,9 @@  class CheckSerializer(HyperlinkedModelSerializer):
     user = UserSerializer(default=CurrentUserDefault())
 
     def run_validation(self, data):
+        if 'state' not in data or data['state'] == '':
+            raise ValidationError({'state': ["A check must have a state."]})
+
         for val, label in Check.STATE_CHOICES:
             if label != data['state']:
                 continue
diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py
index f5a8eca155a9..e3ad099cf656 100644
--- a/patchwork/tests/api/test_check.py
+++ b/patchwork/tests/api/test_check.py
@@ -147,6 +147,22 @@  class TestCheckAPI(APITestCase):
         self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
         self.assertEqual(0, Check.objects.all().count())
 
+    def test_create_missing_state(self):
+        """Create a check using invalid values.
+
+        Ensure we handle the state being absent.
+        """
+        check = {
+            'target_url': 'http://t.co',
+            'description': 'description',
+            'context': 'context',
+        }
+
+        self.client.force_authenticate(user=self.user)
+        resp = self.client.post(self.api_url(), check)
+        self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
+        self.assertEqual(0, Check.objects.all().count())
+
     def test_create_invalid_patch(self):
         """Ensure we handle non-existent patches."""
         check = {