From 197500464971c5ac28b13ab19dc4d7f87a82b4b8 Mon Sep 17 00:00:00 2001
From: Sym Roe <sym.roe@talusdesign.co.uk>
Date: Mon, 14 Aug 2023 14:07:23 +0100
Subject: [PATCH] Fix for three way merge duplication suggestion error

---
 ynr/apps/duplicates/merge_helpers.py          |  7 +++
 .../duplicates/tests/test_merge_helpers.py    | 47 +++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/ynr/apps/duplicates/merge_helpers.py b/ynr/apps/duplicates/merge_helpers.py
index 8d55675c3..85b094e24 100644
--- a/ynr/apps/duplicates/merge_helpers.py
+++ b/ynr/apps/duplicates/merge_helpers.py
@@ -16,6 +16,13 @@ def alter_duplicate_suggestion_post_merge(source_person, dest_person):
     # Case 2: The secondary person has a DS but we are merging it in to someone
     # else in this case we need to update the ID
 
+    # Remove and suggestions that would be the same after we've merged and
+    # updated the IDs below
+    for suggestion in DuplicateSuggestion.objects.filter(person=dest_person):
+        DuplicateSuggestion.objects.filter(
+            person_id=dest_person, other_person=suggestion.other_person
+        ).delete()
+
     # Any suggestions where the 'person' (the destination of a suggestion) is the
     # source of this merge (the object we area bout to delete), update to change
     # the 'person' to the destination of this merge
diff --git a/ynr/apps/duplicates/tests/test_merge_helpers.py b/ynr/apps/duplicates/tests/test_merge_helpers.py
index 439c6a28f..9228bf8ed 100644
--- a/ynr/apps/duplicates/tests/test_merge_helpers.py
+++ b/ynr/apps/duplicates/tests/test_merge_helpers.py
@@ -2,6 +2,7 @@
 from django.test import TestCase
 from duplicates import merge_helpers
 from duplicates.models import DuplicateSuggestion
+from people.merging import PersonMerger
 from people.tests.factories import PersonFactory
 
 
@@ -151,3 +152,49 @@ def test_regresson_test_three_way_suggestion_merging(self):
         )
         # This should close all open duplicate suggestions
         assert DuplicateSuggestion.objects.count() == 0
+
+    def test_three_way_duplicate_all_orders(self):
+        """
+        `test_regresson_test_three_way_suggestion_merging` above
+        catches some of this bug, but not all of it.
+
+        Turns out the _order_ of the merging matters.
+
+        We need to set up:
+
+        A -> B
+        A -> C
+        B -> C
+
+        And then merge A -> B
+
+        """
+        person_a_id = 35348
+        person_b_id = 75618
+        person_c_id = 108860
+        PersonA = PersonFactory(pk=person_a_id, name="A")
+        PersonB = PersonFactory(pk=person_b_id, name="B")
+        PersonC = PersonFactory(pk=person_c_id, name="C")
+
+        DuplicateSuggestion.objects.create(
+            person=PersonA,
+            other_person=PersonB,
+            user=self.user,
+        )
+        DuplicateSuggestion.objects.create(
+            person=PersonA,
+            other_person=PersonC,
+            user=self.user,
+        )
+        DuplicateSuggestion.objects.create(
+            person=PersonB,
+            other_person=PersonC,
+            user=self.user,
+        )
+
+        self.assertEqual(DuplicateSuggestion.objects.count(), 3)
+        PersonMerger(PersonA, PersonB).merge()
+        qs = DuplicateSuggestion.objects.all()
+        self.assertEqual(qs.count(), 1)
+        self.assertEqual(qs.get().person_id, person_a_id)
+        self.assertEqual(qs.get().other_person_id, person_c_id)