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> (backported from commit 7a20ccda99e48dab643d1fbd7e170fe3e4c47185 No Swagger schema changes in the stable backport.) Signed-off-by: Daniel Axtens <d...@axtens.net> --- patchwork/api/check.py | 4 ++++ patchwork/tests/api/test_check.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+) 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 = { -- 2.19.1 _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork