migrations: Don't attempt to rehome patches

Migration 0039 attempts to move patches that have ended up in an
arbitrary series due to race conditions into the correct series.
However, there are a number of race conditions that can occur here that
make this particularly tricky to do. Given that series are really just
arbitary metadata, it's not really necessary to do this...so don't.
Instead, just delete the series references that identical message IDs
and below to the same project, allowing us to add the uniqueness
constraint and prevent the issue bubbling up in the future. This means
we're still left with orphaned series but these could be fixed manually,
if necessary.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Tested-by: Ali Alnubani <alialnu@mellanox.com>
Closes: #340

1 files changed, 46 insertions, 64 deletions

diff --git a/patchwork/migrations/0039_unique_series_references.py b/patchwork/migrations/0039_unique_series_references.py
index 99b10fc..3a10172 100644
--- a/patchwork/migrations/0039_unique_series_references.py
+++ b/patchwork/migrations/0039_unique_series_references.py
@@ -3,62 +3,6 @@ from django.db.models import Count
 import django.db.models.deletion
 
 
-def merge_duplicate_series(apps, schema_editor):
-    SeriesReference = apps.get_model('patchwork', 'SeriesReference')
-    Patch = apps.get_model('patchwork', 'Patch')
-
-    msgid_seriesrefs = {}
-
-    # find all SeriesReference that share a msgid but point to different series
-    # and decide which of the series is going to be the authoritative one
-    msgid_counts = (
-        SeriesReference.objects.values('msgid')
-        .annotate(count=Count('msgid'))
-        .filter(count__gt=1)
-    )
-    for msgid_count in msgid_counts:
-        msgid = msgid_count['msgid']
-        chosen_ref = None
-        for series_ref in SeriesReference.objects.filter(msgid=msgid):
-            if series_ref.series.cover_letter:
-                if chosen_ref:
-                    # I don't think this can happen, but explode if it does
-                    raise Exception(
-                        "Looks like you've got two or more series that share "
-                        "some patches but do not share a cover letter. Unable "
-                        "to auto-resolve."
-                    )
-
-                # if a series has a cover letter, that's the one we'll group
-                # everything under
-                chosen_ref = series_ref
-
-        if not chosen_ref:
-            # if none of the series have cover letters, simply use the last
-            # one (hint: this relies on Python's weird scoping for for loops
-            # where 'series_ref' is still accessible outside the loop)
-            chosen_ref = series_ref
-
-        msgid_seriesrefs[msgid] = chosen_ref
-
-    # reassign any patches referring to non-authoritative series to point to
-    # the authoritative one, and delete the other series; we do this separately
-    # to allow us a chance to raise the exception above if necessary
-    for msgid, chosen_ref in msgid_seriesrefs.items():
-        for series_ref in SeriesReference.objects.filter(msgid=msgid):
-            if series_ref == chosen_ref:
-                continue
-
-            # update the patches to point to our chosen series instead, on the
-            # assumption that all other metadata is correct
-            for patch in Patch.objects.filter(series=series_ref.series):
-                patch.series = chosen_ref.series
-                patch.save()
-
-            # delete the other series (which will delete the series ref)
-            series_ref.series.delete()
-
-
 def copy_project_field(apps, schema_editor):
     if connection.vendor == 'postgresql':
         schema_editor.execute(
@@ -67,7 +11,7 @@ def copy_project_field(apps, schema_editor):
               SET project_id = patchwork_series.project_id
             FROM patchwork_series
               WHERE patchwork_seriesreference.series_id = patchwork_series.id
-        """
+            """
         )
     elif connection.vendor == 'mysql':
         schema_editor.execute(
@@ -75,7 +19,7 @@ def copy_project_field(apps, schema_editor):
             UPDATE patchwork_seriesreference, patchwork_series
               SET patchwork_seriesreference.project_id = patchwork_series.project_id
             WHERE patchwork_seriesreference.series_id = patchwork_series.id
-        """  # noqa
+            """  # noqa
         )
     else:
         SeriesReference = apps.get_model('patchwork', 'SeriesReference')
@@ -87,14 +31,49 @@ def copy_project_field(apps, schema_editor):
             series_ref.save()
 
 
+def delete_duplicate_series(apps, schema_editor):
+    if connection.vendor == 'postgresql':
+        schema_editor.execute(
+            """
+            DELETE
+            FROM
+              patchwork_seriesreference a
+                USING patchwork_seriesreference b
+            WHERE
+              a.id < b.id
+              AND a.project_id = b.project_id
+              AND a.msgid = b.msgid
+            """
+        )
+    elif connection.vendor == 'mysql':
+        schema_editor.execute(
+            """
+            DELETE a FROM patchwork_seriesreference a
+            INNER JOIN patchwork_seriesreference b
+            WHERE
+              a.id < b.id
+              AND a.project_id = b.project_id
+              AND a.msgid = b.msgid
+            """
+        )
+    else:
+        Project = apps.get_model('patchwork', 'Project')
+        SeriesReference = apps.get_model('patchwork', 'SeriesReference')
+
+        for project in Project.objects.all():
+            (
+                SeriesReference.objects.filter(project=project)
+                .annotate(count=Count('msgid'))
+                .filter(count__gt=1)
+                .delete()
+            )
+
+
 class Migration(migrations.Migration):
 
     dependencies = [('patchwork', '0038_state_slug')]
 
     operations = [
-        migrations.RunPython(
-            merge_duplicate_series, migrations.RunPython.noop, atomic=False
-        ),
         migrations.AddField(
             model_name='seriesreference',
             name='project',
@@ -104,12 +83,12 @@ class Migration(migrations.Migration):
                 to='patchwork.Project',
             ),
         ),
-        migrations.AlterUniqueTogether(
-            name='seriesreference', unique_together={('project', 'msgid')}
-        ),
         migrations.RunPython(
             copy_project_field, migrations.RunPython.noop, atomic=False
         ),
+        migrations.RunPython(
+            delete_duplicate_series, migrations.RunPython.noop, atomic=False
+        ),
         migrations.AlterField(
             model_name='seriesreference',
             name='project',
@@ -118,4 +97,7 @@ class Migration(migrations.Migration):
                 to='patchwork.Project',
             ),
         ),
+        migrations.AlterUniqueTogether(
+            name='seriesreference', unique_together={('project', 'msgid')}
+        ),
     ]