[PATCH 04/17] REST: Ensure submission exists for comment listing
Daniel Axtens
dja at axtens.net
Fri Nov 9 00:19:41 AEDT 2018
LGTM.
Regards,
Daniel
Stephen Finucane <stephen at that.guru> writes:
> Signed-off-by: Stephen Finucane <stephen at that.guru>
> Closes: #225
> ---
> patchwork/api/comment.py | 5 +++++
> patchwork/tests/api/test_comment.py | 13 +++++++++++++
> releasenotes/notes/issue-225-94215600c1b23f6e.yaml | 6 ++++++
> 3 files changed, 24 insertions(+)
> create mode 100644 releasenotes/notes/issue-225-94215600c1b23f6e.yaml
>
> diff --git a/patchwork/api/comment.py b/patchwork/api/comment.py
> index 214a9438..57b37111 100644
> --- a/patchwork/api/comment.py
> +++ b/patchwork/api/comment.py
> @@ -5,6 +5,7 @@
>
> import email.parser
>
> +from django.http import Http404
> from rest_framework.generics import ListAPIView
> from rest_framework.serializers import SerializerMethodField
>
> @@ -12,6 +13,7 @@ from patchwork.api.base import BaseHyperlinkedModelSerializer
> from patchwork.api.base import PatchworkPermission
> from patchwork.api.embedded import PersonSerializer
> from patchwork.models import Comment
> +from patchwork.models import Submission
>
>
> class CommentListSerializer(BaseHyperlinkedModelSerializer):
> @@ -64,6 +66,9 @@ class CommentList(ListAPIView):
> lookup_url_kwarg = 'pk'
>
> def get_queryset(self):
> + if not Submission.objects.filter(pk=self.kwargs['pk']).exists():
> + raise Http404
> +
> return Comment.objects.filter(
> submission=self.kwargs['pk']
> ).select_related('submitter')
> diff --git a/patchwork/tests/api/test_comment.py b/patchwork/tests/api/test_comment.py
> index a0aec594..06fe2de3 100644
> --- a/patchwork/tests/api/test_comment.py
> +++ b/patchwork/tests/api/test_comment.py
> @@ -5,6 +5,7 @@
>
> import unittest
>
> +from django.http import Http404
> from django.conf import settings
> from django.urls import NoReverseMatch
> from django.urls import reverse
> @@ -61,6 +62,12 @@ class TestCoverComments(APITestCase):
> with self.assertRaises(NoReverseMatch):
> self.client.get(self.api_url(cover_obj, version='1.0'))
>
> + def test_list_invalid_cover(self):
> + """Ensure we get a 404 for a non-existent cover letter."""
> + resp = self.client.get(
> + reverse('api-cover-comment-list', kwargs={'pk': '99999'}))
> + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
> +
>
> @unittest.skipUnless(settings.ENABLE_REST_API, 'requires ENABLE_REST_API')
> class TestPatchComments(APITestCase):
> @@ -99,3 +106,9 @@ class TestPatchComments(APITestCase):
> # check we can't access comments using the old version of the API
> with self.assertRaises(NoReverseMatch):
> self.client.get(self.api_url(patch_obj, version='1.0'))
> +
> + def test_list_invalid_patch(self):
> + """Ensure we get a 404 for a non-existent patch."""
> + resp = self.client.get(
> + reverse('api-patch-comment-list', kwargs={'pk': '99999'}))
> + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code)
> diff --git a/releasenotes/notes/issue-225-94215600c1b23f6e.yaml b/releasenotes/notes/issue-225-94215600c1b23f6e.yaml
> new file mode 100644
> index 00000000..035e38d8
> --- /dev/null
> +++ b/releasenotes/notes/issue-225-94215600c1b23f6e.yaml
> @@ -0,0 +1,6 @@
> +---
> +fixes:
> + - |
> + Showing comments for a non-existant patch or cover letter was returning an
> + empty response instead of a HTTP 404. This issue is resolved for both
> + resources.
> --
> 2.17.2
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list