[PATCH 06/10] REST: Fix issues with comment-related events
Stephen Finucane
stephen at that.guru
Sat Oct 1 02:19:17 AEST 2022
When we introduced this functionality, we missed the fact that these
resources use nested-style URLs that need to be specially handled. Fix
this now.
Signed-off-by: Stephen Finucane <stephen at that.guru>
Fixes: e3d8f7548 ("REST: Add 'patch-comment-created', 'cover-comment-created' events")
Cc: Siddhesh Poyarekar <sipoyare at redhat.com>
Cc: DJ Delorie <dj at redhat.com>
Cc: Carlos O'Donell <carlos at redhat.com>
---
patchwork/api/base.py | 34 +++++++++++++++++++++++++++++++
patchwork/api/embedded.py | 10 +++++++--
patchwork/api/event.py | 25 ++++++++++++++++++++---
patchwork/tests/api/test_event.py | 30 +++++++++++++++++----------
4 files changed, 83 insertions(+), 16 deletions(-)
diff --git patchwork/api/base.py patchwork/api/base.py
index 6268f67d..3ed4182c 100644
--- patchwork/api/base.py
+++ patchwork/api/base.py
@@ -139,6 +139,40 @@ class CheckHyperlinkedIdentityField(HyperlinkedIdentityField):
)
+class CoverCommentHyperlinkedIdentityField(HyperlinkedIdentityField):
+ def get_url(self, obj, view_name, request, format):
+ # Unsaved objects will not yet have a valid URL.
+ if obj.pk is None:
+ return None
+
+ return self.reverse(
+ view_name,
+ kwargs={
+ 'cover_id': obj.cover.id,
+ 'comment_id': obj.id,
+ },
+ request=request,
+ format=format,
+ )
+
+
+class PatchCommentHyperlinkedIdentityField(HyperlinkedIdentityField):
+ def get_url(self, obj, view_name, request, format):
+ # Unsaved objects will not yet have a valid URL.
+ if obj.pk is None:
+ return None
+
+ return self.reverse(
+ view_name,
+ kwargs={
+ 'patch_id': obj.patch.id,
+ 'comment_id': obj.id,
+ },
+ request=request,
+ format=format,
+ )
+
+
class BaseHyperlinkedModelSerializer(HyperlinkedModelSerializer):
def to_representation(self, instance):
data = super(BaseHyperlinkedModelSerializer, self).to_representation(
diff --git patchwork/api/embedded.py patchwork/api/embedded.py
index c41511fe..485ed6f7 100644
--- patchwork/api/embedded.py
+++ patchwork/api/embedded.py
@@ -17,6 +17,8 @@ from rest_framework.serializers import SerializerMethodField
from patchwork.api.base import BaseHyperlinkedModelSerializer
from patchwork.api.base import CheckHyperlinkedIdentityField
+from patchwork.api.base import CoverCommentHyperlinkedIdentityField
+from patchwork.api.base import PatchCommentHyperlinkedIdentityField
from patchwork import models
@@ -127,6 +129,9 @@ class CoverSerializer(SerializedRelatedField):
class CoverCommentSerializer(SerializedRelatedField):
class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer):
+
+ url = CoverCommentHyperlinkedIdentityField('api-cover-comment-detail')
+
class Meta:
model = models.CoverComment
fields = (
@@ -136,7 +141,6 @@ class CoverCommentSerializer(SerializedRelatedField):
'msgid',
'list_archive_url',
'date',
- 'name',
)
read_only_fields = fields
versioned_fields = {
@@ -177,6 +181,9 @@ class PatchSerializer(SerializedRelatedField):
class PatchCommentSerializer(SerializedRelatedField):
class _Serializer(MboxMixin, WebURLMixin, BaseHyperlinkedModelSerializer):
+
+ url = PatchCommentHyperlinkedIdentityField('api-patch-comment-detail')
+
class Meta:
model = models.PatchComment
fields = (
@@ -186,7 +193,6 @@ class PatchCommentSerializer(SerializedRelatedField):
'msgid',
'list_archive_url',
'date',
- 'name',
)
read_only_fields = fields
versioned_fields = {
diff --git patchwork/api/event.py patchwork/api/event.py
index c1b09ab9..6d08b6ee 100644
--- patchwork/api/event.py
+++ patchwork/api/event.py
@@ -19,6 +19,7 @@ from patchwork.api.embedded import ProjectSerializer
from patchwork.api.embedded import SeriesSerializer
from patchwork.api.embedded import UserSerializer
from patchwork.api.filters import EventFilterSet
+from patchwork.api import utils
from patchwork.models import Event
@@ -140,7 +141,7 @@ class EventList(ListAPIView):
ordering = '-date'
def get_queryset(self):
- return Event.objects.all().prefetch_related(
+ events = Event.objects.all().prefetch_related(
'project',
'patch__project',
'series__project',
@@ -150,6 +151,24 @@ class EventList(ListAPIView):
'previous_delegate',
'current_delegate',
'created_check',
- 'cover_comment',
- 'patch_comment',
)
+ # NOTE(stephenfin): We need to exclude comment-related events because
+ # until API v1.3, we didn't have an comment detail API to point to.
+ # This goes against our pledge to version events in the docs but must
+ # be done.
+ # TODO(stephenfin): Make this more generic.
+ if utils.has_version(self.request, '1.3'):
+ events = events.prefetch_related(
+ 'cover_comment',
+ 'cover_comment__cover__project',
+ 'patch_comment',
+ 'patch_comment__patch__project',
+ )
+ else:
+ events = events.exclude(
+ category__in=[
+ Event.CATEGORY_COVER_COMMENT_CREATED,
+ Event.CATEGORY_PATCH_COMMENT_CREATED,
+ ]
+ )
+ return events
diff --git patchwork/tests/api/test_event.py patchwork/tests/api/test_event.py
index 9708f96b..7ca09c2e 100644
--- patchwork/tests/api/test_event.py
+++ patchwork/tests/api/test_event.py
@@ -12,8 +12,10 @@ from patchwork.models import Event
from patchwork.tests.api import utils
from patchwork.tests.utils import create_check
from patchwork.tests.utils import create_cover
+from patchwork.tests.utils import create_cover_comment
from patchwork.tests.utils import create_maintainer
from patchwork.tests.utils import create_patch
+from patchwork.tests.utils import create_patch_comment
from patchwork.tests.utils import create_series
from patchwork.tests.utils import create_state
@@ -70,7 +72,7 @@ class TestEventAPI(APITestCase):
# patch-created, patch-completed, series-completed
patch = create_patch(series=series)
# cover-created
- create_cover(series=series)
+ cover = create_cover(series=series)
# check-created
create_check(patch=patch)
# patch-delegated, patch-state-changed
@@ -81,6 +83,9 @@ class TestEventAPI(APITestCase):
patch.state = state
self.assertTrue(patch.is_editable(actor))
patch.save()
+ # patch-cover-created, cover-comment-created
+ create_patch_comment(patch=patch, submitter=patch.submitter)
+ create_cover_comment(cover=cover, submitter=cover.submitter)
return Event.objects.all()
@@ -91,7 +96,9 @@ class TestEventAPI(APITestCase):
resp = self.client.get(self.api_url())
self.assertEqual(status.HTTP_200_OK, resp.status_code)
- self.assertEqual(8, len(resp.data), [x['category'] for x in resp.data])
+ self.assertEqual(
+ 10, len(resp.data), [x['category'] for x in resp.data]
+ )
for event_rsp in resp.data:
event_obj = events.get(category=event_rsp['category'])
self.assertSerialized(event_obj, event_rsp)
@@ -104,7 +111,7 @@ class TestEventAPI(APITestCase):
resp = self.client.get(self.api_url(), {'project': project.pk})
# All but one event belongs to the same project
- self.assertEqual(8, len(resp.data))
+ self.assertEqual(10, len(resp.data))
resp = self.client.get(self.api_url(), {'project': 'invalidproject'})
self.assertEqual(0, len(resp.data))
@@ -132,9 +139,9 @@ class TestEventAPI(APITestCase):
patch = events.get(category='patch-created').patch
resp = self.client.get(self.api_url(), {'patch': patch.pk})
- # There should be five - patch-created, patch-completed, check-created,
- # patch-state-changed and patch-delegated
- self.assertEqual(5, len(resp.data))
+ # There should be six - patch-created, patch-completed, check-created,
+ # patch-state-changed, patch-delegated and patch-comment-created
+ self.assertEqual(6, len(resp.data))
resp = self.client.get(self.api_url(), {'patch': 999999})
self.assertEqual(0, len(resp.data))
@@ -145,8 +152,8 @@ class TestEventAPI(APITestCase):
cover = events.get(category='cover-created').cover
resp = self.client.get(self.api_url(), {'cover': cover.pk})
- # There should only be one - cover-created
- self.assertEqual(1, len(resp.data))
+ # There should be two - cover-created and cover-comment-created
+ self.assertEqual(2, len(resp.data))
resp = self.client.get(self.api_url(), {'cover': 999999})
self.assertEqual(0, len(resp.data))
@@ -170,7 +177,7 @@ class TestEventAPI(APITestCase):
# The final two events (patch-delegated, patch-state-changed)
# have an actor set
- actor = events[0].actor
+ actor = events.get(category='patch-delegated').actor
resp = self.client.get(self.api_url(), {'actor': actor.pk})
self.assertEqual(2, len(resp.data))
@@ -185,14 +192,15 @@ class TestEventAPI(APITestCase):
resp = self.client.get(
self.api_url(version='1.1'), {'actor': 'foo-bar'}
)
- self.assertEqual(len(events), len(resp.data))
+ # we don't see the two comment-related fields
+ self.assertEqual(len(events) - 2, len(resp.data))
def test_list_bug_335(self):
"""Ensure we retrieve the embedded series project once."""
for _ in range(3):
self._create_events()
- with self.assertNumQueries(27):
+ with self.assertNumQueries(33):
self.client.get(self.api_url())
def test_order_by_date_default(self):
--
2.37.3
More information about the Patchwork
mailing list