Signed-off-by: Stephen Finucane <step...@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@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
