models: Convert Series-Patch relationship to 1:N

Late in the development of the series feature, it was decided that there
were advantages to allowing an N:M relationship between series and
patches. This would allow us to do things like create complete series
where a sole vN patch was sent to a list rather than the full series.
After some time using series in the wild, it's apparent that such
features are very difficult to implement correctly and will likely never
be implemented. As such, it's time to start cleaning up the mess, paving
the way for things like an improved tagging feature.

There are some significant changes to the model required:

- models.py, migrations/0027, migrations/0028, migrations/0029

  The migrations make the following changes:

  1. - Add 'Patch.series_alt' and 'Patch.number' fields.
  2. - Populate the 'Patch.series_alt' and 'Patch.number' fields from
       their 'SeriesPatch' equivalents.
  3. - Remove the 'SeriesPatch' model.
     - Rename 'Patch.series_alt' to 'Patch.series'.
     - Change 'Series.cover_letter' to a 'OneToOneField' since a cover
       letter can no longer be assigned to multiple series.

  Note that the migrations have to be split into multiple parts as the
  combined migration raises an OperationalError as below.

    (1072, "Key column 'series_alt_id' doesn't exist in table")

  This is due to Django's penchant for creating indexes for newly
  created fields, as noted here: https://stackoverflow.com/q/35158530/

Aside from the model changes, there are numerous other changes required:

- admin.py

  Reflect model changes for the 'PatchInline' inline used by
  'SeriesAdmin'

- api/cover.py, api/patch.py

  Update the 'series' field for the cover letter and patch resources to
  reflect the model changes. A 'to_representation' function is added in
  both cases to post-process this field and make it look like a list
  again. This is necessary to avoid breaking clients.

- parser.py

  Update to reflect the replacement of 'SeriesPatch' with 'Patch'.

- signals.py

  Update to filter on changes to 'Patch' instead of 'SeriesPatch'. This
  requires some reworking due to how we set these fields now, as we can
  no longer receive on 'post_save' signals for 'SeriesPatch' and must
  instead watch for 'pre_save' on 'Patch', which is what we do for
  delegate and state changes on same.

- templates/patchwork/*.html

  Remove logic that handled multiple series in favour of the (simpler)
  single series logic.

- tests/*

  Modify the 'create_series_patch' helper to reflect the removal of the
  'SeriesPatch' model. This entire helper will be removed in a future
  change. Improve some tests to cover edge cases that were highlighted
  during development

Unfortunately, all of the above changes must go in at the same time,
otherwise we end up with either (a) broken views, API etc. or (b) split
brain because we need to keep the new single-series fields alongside the
older multi-series fields and models while we rework the views. It's
unfortunate but there's not much to be done here.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Tested-by: Daniel Axtens <dja@axtens.net>

1 files changed, 20 insertions, 20 deletions

diff --git a/patchwork/signals.py b/patchwork/signals.py
index b7b8e6f..536b177 100644
--- a/patchwork/signals.py
+++ b/patchwork/signals.py
@@ -15,7 +15,6 @@ from patchwork.models import Event
 from patchwork.models import Patch
 from patchwork.models import PatchChangeNotification
 from patchwork.models import Series
-from patchwork.models import SeriesPatch
 
 
 @receiver(pre_save, sender=Patch)
@@ -133,39 +132,46 @@ def create_patch_delegated_event(sender, instance, raw, **kwargs):
     create_event(instance, orig_patch.delegate, instance.delegate)
 
 
-@receiver(post_save, sender=SeriesPatch)
-def create_patch_completed_event(sender, instance, created, raw, **kwargs):
-    """Create patch completed event for patches with series."""
+@receiver(pre_save, sender=Patch)
+def create_patch_completed_event(sender, instance, raw, **kwargs):
 
-    def create_event(patch, series):
+    def create_event(patch):
         return Event.objects.create(
             category=Event.CATEGORY_PATCH_COMPLETED,
             project=patch.project,
             patch=patch,
-            series=series)
+            series=patch.series)
 
-    # don't trigger for items loaded from fixtures or existing items
-    if raw or not created:
+    # don't trigger for items loaded from fixtures, new items or items that
+    # (still) don't have a series
+    if raw or not instance.pk or not instance.series:
+        return
+
+    orig_patch = Patch.objects.get(pk=instance.pk)
+
+    # we don't currently allow users to change a series, though this might
+    # change in the future. However, we handle that here nonetheless
+    if orig_patch.series == instance.series:
         return
 
     # if dependencies not met, don't raise event. There's also no point raising
     # events for successors since they'll have the same issue
-    predecessors = SeriesPatch.objects.filter(
+    predecessors = Patch.objects.filter(
         series=instance.series, number__lt=instance.number)
     if predecessors.count() != instance.number - 1:
         return
 
-    create_event(instance.patch, instance.series)
+    create_event(instance)
 
     # if this satisfies dependencies for successor patch, raise events for
     # those
     count = instance.number + 1
-    for successor in SeriesPatch.objects.filter(
+    for successor in Patch.objects.order_by('number').filter(
             series=instance.series, number__gt=instance.number):
         if successor.number != count:
             break
 
-        create_event(successor.patch, successor.series)
+        create_event(successor)
         count += 1
 
 
@@ -204,15 +210,9 @@ def create_series_created_event(sender, instance, created, raw, **kwargs):
     create_event(instance)
 
 
-@receiver(post_save, sender=SeriesPatch)
+@receiver(post_save, sender=Patch)
 def create_series_completed_event(sender, instance, created, raw, **kwargs):
 
-    # NOTE(stephenfin): We subscribe to the SeriesPatch.post_save signal
-    # instead of Series.m2m_changed to minimize the amount of times this is
-    # fired. The m2m_changed signal doesn't support a 'changed' parameter,
-    # which we could use to quick skip the signal when a patch is merely
-    # updated instead of added to the series.
-
     # NOTE(stephenfin): It's actually possible for this event to be fired
     # multiple times for a given series. To trigger this case, you would need
     # to send an additional patch to already exisiting series. This pattern
@@ -229,5 +229,5 @@ def create_series_completed_event(sender, instance, created, raw, **kwargs):
     if raw or not created:
         return
 
-    if instance.series.received_all:
+    if instance.series and instance.series.received_all:
         create_event(instance.series)