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 <rus...@russell.cc>
Reported-by: Jeremy Kerr <j...@ozlabs.org>
Signed-off-by: Daniel Axtens <d...@axtens.net>
---
Also needs to go to stable.
---
docs/api/schemas/latest/patchwork.yaml | 2 ++
docs/api/schemas/patchwork.j2 | 2 ++
docs/api/schemas/v1.0/patchwork.yaml | 2 ++
docs/api/schemas/v1.1/patchwork.yaml | 2 ++
patchwork/api/check.py | 4 ++++
patchwork/tests/api/test_check.py | 17 +++++++++++++++++
6 files changed, 29 insertions(+)
diff --git a/docs/api/schemas/latest/patchwork.yaml
b/docs/api/schemas/latest/patchwork.yaml
index e3ba69c5c64f..724b05ebf1b3 100644
--- a/docs/api/schemas/latest/patchwork.yaml
+++ b/docs/api/schemas/latest/patchwork.yaml
@@ -1316,6 +1316,8 @@ components:
nullable: true
CheckCreate:
type: object
+ required:
+ - state
properties:
state:
title: State
diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
index 7d3486387ede..5e2f5e4ddc74 100644
--- a/docs/api/schemas/patchwork.j2
+++ b/docs/api/schemas/patchwork.j2
@@ -1319,6 +1319,8 @@ components:
nullable: true
CheckCreate:
type: object
+ required:
+ - state
properties:
state:
title: State
diff --git a/docs/api/schemas/v1.0/patchwork.yaml
b/docs/api/schemas/v1.0/patchwork.yaml
index 11e3ae30adc0..02f3a1561b7b 100644
--- a/docs/api/schemas/v1.0/patchwork.yaml
+++ b/docs/api/schemas/v1.0/patchwork.yaml
@@ -1311,6 +1311,8 @@ components:
nullable: true
CheckCreate:
type: object
+ required:
+ - state
properties:
state:
title: State
diff --git a/docs/api/schemas/v1.1/patchwork.yaml
b/docs/api/schemas/v1.1/patchwork.yaml
index 4e81ac33d9b2..0c086edaa776 100644
--- a/docs/api/schemas/v1.1/patchwork.yaml
+++ b/docs/api/schemas/v1.1/patchwork.yaml
@@ -1316,6 +1316,8 @@ components:
nullable: true
CheckCreate:
type: object
+ required:
+ - state
properties:
state:
title: State
diff --git a/patchwork/api/check.py b/patchwork/api/check.py
index 4d2181d0a04b..2c3fe445872e 100644
--- a/patchwork/api/check.py
+++ b/patchwork/api/check.py
@@ -7,6 +7,7 @@ from django.http import Http404
from django.http.request import QueryDict
from django.shortcuts import get_object_or_404
from rest_framework.exceptions import PermissionDenied
+from rest_framework.exceptions import ValidationError
from rest_framework.generics import ListCreateAPIView
from rest_framework.generics import RetrieveAPIView
from rest_framework.serializers import CurrentUserDefault
@@ -36,6 +37,9 @@ class CheckSerializer(HyperlinkedModelSerializer):
user = UserSerializer(default=CurrentUserDefault())
def run_validation(self, data):
+ if 'state' not in data:
+ 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 1cfdff6e757b..24451aba09ad 100644
--- a/patchwork/tests/api/test_check.py
+++ b/patchwork/tests/api/test_check.py
@@ -151,6 +151,23 @@ class TestCheckAPI(utils.APITestCase):
self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code)
self.assertEqual(0, Check.objects.all().count())
+ @utils.store_samples('check-create-error-missing-state')
+ 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())
+
@utils.store_samples('check-create-error-not-found')
def test_create_invalid_patch(self):
"""Ensure we handle non-existent patches."""
--
2.19.1
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
