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 <stephen@that.guru>
Related: #379
(cherry picked from commit 646a2f2c80056a33c70efd760446832f90899247)

diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
index f5618d41faa05f6a0d6373e92c116c29dc1dc939..af15df1ce9bc1571b9d411700c6784b10358ce6d 100644 (file)
--- 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 6b497abaf24421c5e19927e000f27c4dd3171c60..babc972ace35278b9517ee2d243d1abc05e70b21 100644 (file)
--- 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 7bdbe66997c01d392cea57039074f8443c6f3df4..db60c789c0b8ab95e582ed0e42dde960ceabb41a 100644 (file)
--- 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 951786767c742bea4922ff327acee957d1f617e8..bc1bda463c9b258e5bf680477e6789cd1c386550 100644 (file)
--- 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 3a2f0fbdd3a44a0a1884925f5fc4aa4c40cb4691..0771104a4713eb35bb4a649a49e325bb34e0b18a 100644 (file)
--- 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 5bac7789064a5cbb6f26b00bbb1ff8c901e639ac..090b6dc019de6d6cdc2400b22bad04260bbb4d7e 100644 (file)
--- 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):