Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for three way merge duplication suggestion error #2161

Merged
merged 1 commit into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ynr/apps/duplicates/merge_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions ynr/apps/duplicates/tests/test_merge_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
Loading