From: Stephen Finucane Date: Sun, 29 Nov 2020 12:50:22 +0000 (+0000) Subject: REST: Null out previous, current relation info X-Git-Tag: v2.2.3~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ea8b10264f66833da921055947f8431cf831179;p=thirdparty%2Fpatchwork.git REST: Null out previous, current relation info These fields don't work like we expect them to. Because we're linking to a non-idempotent entity, an instance of 'relation', what we're storing in either of these fields is subject to change as patches are added and removed. This makes the information pretty much useless after the fact. It's best to just state the patch and request that people query the information themselves if necessary. We don't want to remove the field entirely from the API - that would be a truly breaking change - so instead we null it out like we do for patch tags. In a v2 API (i.e. a major version bump) we can remove this entirely. A small bug with the schema generation is corrected. Signed-off-by: Stephen Finucane Related: #379 (cherry picked from commit 646a2f2c80056a33c70efd760446832f90899247) --- diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2 index f5618d41..af15df1c 100644 --- a/docs/api/schemas/patchwork.j2 +++ b/docs/api/schemas/patchwork.j2 @@ -1821,7 +1821,7 @@ components: current_state: title: Current state type: string -{% if version >= (1, 1) %} +{% if version >= (1, 2) %} EventPatchRelationChanged: allOf: - $ref: '#/components/schemas/EventBase' @@ -1837,9 +1837,11 @@ components: previous_relation: title: Previous relation type: string + nullable: true current_relation: title: Current relation type: string + nullable: true {% endif %} EventPatchDelegated: allOf: diff --git a/docs/api/schemas/v1.1/patchwork.yaml b/docs/api/schemas/v1.1/patchwork.yaml index 6b497aba..babc972a 100644 --- a/docs/api/schemas/v1.1/patchwork.yaml +++ b/docs/api/schemas/v1.1/patchwork.yaml @@ -1551,24 +1551,6 @@ components: current_state: title: Current state type: string - EventPatchRelationChanged: - allOf: - - $ref: '#/components/schemas/EventBase' - - type: object - properties: - category: - enum: - - patch-relation-changed - payload: - properties: - patch: - $ref: '#/components/schemas/PatchEmbedded' - previous_relation: - title: Previous relation - type: string - current_relation: - title: Current relation - type: string EventPatchDelegated: allOf: - $ref: '#/components/schemas/EventBase' diff --git a/docs/api/schemas/v1.2/patchwork.yaml b/docs/api/schemas/v1.2/patchwork.yaml index 7bdbe669..db60c789 100644 --- a/docs/api/schemas/v1.2/patchwork.yaml +++ b/docs/api/schemas/v1.2/patchwork.yaml @@ -1768,9 +1768,11 @@ components: previous_relation: title: Previous relation type: string + nullable: true current_relation: title: Current relation type: string + nullable: true EventPatchDelegated: allOf: - $ref: '#/components/schemas/EventBase' diff --git a/patchwork/api/event.py b/patchwork/api/event.py index 95178676..bc1bda46 100644 --- a/patchwork/api/event.py +++ b/patchwork/api/event.py @@ -33,10 +33,8 @@ class EventSerializer(ModelSerializer): current_delegate = UserSerializer() created_check = SerializerMethodField() created_check = CheckSerializer() - previous_relation = PatchSerializer( - source='previous_relation.patches', many=True, default=None) - current_relation = PatchSerializer( - source='current_relation.patches', many=True, default=None) + previous_relation = SerializerMethodField() + current_relation = SerializerMethodField() _category_map = { Event.CATEGORY_COVER_CREATED: ['cover'], @@ -53,6 +51,12 @@ class EventSerializer(ModelSerializer): Event.CATEGORY_SERIES_COMPLETED: ['series'], } + def get_previous_relation(self, instance): + return None + + def get_current_relation(self, instance): + return None + def to_representation(self, instance): data = super(EventSerializer, self).to_representation(instance) payload = OrderedDict() @@ -72,10 +76,12 @@ class EventSerializer(ModelSerializer): class Meta: model = Event - fields = ('id', 'category', 'project', 'date', 'actor', 'patch', - 'series', 'cover', 'previous_state', 'current_state', - 'previous_delegate', 'current_delegate', 'created_check', - 'previous_relation', 'current_relation',) + fields = ( + 'id', 'category', 'project', 'date', 'actor', 'patch', + 'series', 'cover', 'previous_state', 'current_state', + 'previous_delegate', 'current_delegate', 'created_check', + 'previous_relation', 'current_relation', + ) read_only_fields = fields versioned_fields = { '1.2': ('actor', ), diff --git a/patchwork/signals.py b/patchwork/signals.py index 3a2f0fbd..0771104a 100644 --- a/patchwork/signals.py +++ b/patchwork/signals.py @@ -137,14 +137,12 @@ def create_patch_delegated_event(sender, instance, raw, **kwargs): @receiver(pre_save, sender=Patch) def create_patch_relation_changed_event(sender, instance, raw, **kwargs): - def create_event(patch, before, after): + def create_event(patch): return Event.objects.create( category=Event.CATEGORY_PATCH_RELATION_CHANGED, project=patch.project, actor=getattr(patch, '_edited_by', None), - patch=patch, - previous_relation=before, - current_relation=after) + patch=patch) # don't trigger for items loaded from fixtures or new items if raw or not instance.pk: @@ -155,7 +153,7 @@ def create_patch_relation_changed_event(sender, instance, raw, **kwargs): if orig_patch.related == instance.related: return - create_event(instance, orig_patch.related, instance.related) + create_event(instance) @receiver(pre_save, sender=Patch) diff --git a/patchwork/tests/test_events.py b/patchwork/tests/test_events.py index 5bac7789..090b6dc0 100644 --- a/patchwork/tests/test_events.py +++ b/patchwork/tests/test_events.py @@ -190,8 +190,8 @@ class PatchChangedTest(_BaseTestCase): self.assertEqual( events[1].category, Event.CATEGORY_PATCH_RELATION_CHANGED) self.assertEqual(events[1].project, patches[1].project) - self.assertEqual(events[1].previous_relation, None) - self.assertEqual(events[1].current_relation, relation) + self.assertIsNone(events[1].previous_relation) + self.assertIsNone(events[1].current_relation) # add the third patch @@ -203,8 +203,8 @@ class PatchChangedTest(_BaseTestCase): self.assertEqual( events[1].category, Event.CATEGORY_PATCH_RELATION_CHANGED) self.assertEqual(events[1].project, patches[1].project) - self.assertEqual(events[1].previous_relation, None) - self.assertEqual(events[1].current_relation, relation) + self.assertIsNone(events[1].previous_relation) + self.assertIsNone(events[1].current_relation) # drop the third patch @@ -216,8 +216,8 @@ class PatchChangedTest(_BaseTestCase): self.assertEqual( events[2].category, Event.CATEGORY_PATCH_RELATION_CHANGED) self.assertEqual(events[2].project, patches[1].project) - self.assertEqual(events[2].previous_relation, relation) - self.assertEqual(events[2].current_relation, None) + self.assertIsNone(events[2].previous_relation) + self.assertIsNone(events[2].current_relation) class CheckCreatedTest(_BaseTestCase):