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

diff --git a/docs/api/schemas/patchwork.j2 b/docs/api/schemas/patchwork.j2
index c55f6f45a2a945a5807b68d5b25739e32e6c6a70..af20743dd4c6dfdea63b081f56837cf88f243eb5 100644 (file)
--- a/docs/api/schemas/patchwork.j2
+++ b/docs/api/schemas/patchwork.j2
@@ -1765,7 +1765,7 @@ components:
                 current_state:
                   title: Current state
                   type: string
-{% if version >= (1, 1) %}
+{% if version >= (1, 2) %}
     EventPatchRelationChanged:
       allOf:
         - $ref: '#/components/schemas/EventBase'
@@ -1781,9 +1781,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 5acd33a7a19e79b8eaa0af32e9555685833f5695..3b75c548ff9ed5d4d5d79bf385a6bbe8ae974e6e 100644 (file)
--- a/docs/api/schemas/v1.1/patchwork.yaml
+++ b/docs/api/schemas/v1.1/patchwork.yaml
@@ -1510,24 +1510,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 712d137ed04038ae218b7045a5009ec0a818b037..17d948a2f4285c2a50b3d3f7f31407c29d8f2048 100644 (file)
--- a/docs/api/schemas/v1.2/patchwork.yaml
+++ b/docs/api/schemas/v1.2/patchwork.yaml
@@ -1712,9 +1712,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 75bf870837d9bac8017c70fe1387d099b8eaeaa8..71f95937c2bb2f64128d5cb445bbd8897a9b835c 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 4db9909f6063096bc9d41b0fe707c9afea23a0d9..dc08129c2f47926d19fdaf8aa6f04cff18950c7d 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):