diff --git a/backend/api/management/commands/cleanup_duplicates.py b/backend/api/management/commands/cleanup_duplicates.py index d1615ad..4bc41ca 100644 --- a/backend/api/management/commands/cleanup_duplicates.py +++ b/backend/api/management/commands/cleanup_duplicates.py @@ -1,62 +1,73 @@ import re -from django.core.management.base import BaseCommand -from django.db.models import Count, F, Func, Value, CharField -from api.models import Flavor, Category +from django.core.management.base import BaseCommand + +from api.models import Category, Flavor from api.services.flavor import merge_flavors +SUFFIXES = [ + r"\s+Energy$", + r"\s+Hydration$", + r"\s+Iced\s+Tea$", + r"\s+\(Legacy\)$", +] + class Command(BaseCommand): help = "Identifies and merges duplicate flavors (normalized name and category)" - def normalize_name(self, name: str) -> str: - """Strip common suffixes and normalize for comparison.""" - # Strip common suffixes - suffixes = [ - r"\s+Energy$", - r"\s+Hydration$", - r"\s+Iced\s+Tea$", - r"\s+\(Legacy\)$", - ] + def add_arguments(self, parser): + parser.add_argument( + "--dry-run", + action="store_true", + help="Report merges without performing them.", + ) + parser.add_argument( + "--no-normalize", + action="store_true", + help="Match exact names only; do not strip suffixes like ' Energy'.", + ) + + def normalize_name(self, name: str, strip_suffixes: bool = True) -> str: + """Lowercase, trim, and (optionally) strip common product suffixes.""" norm = name.strip() - for suffix in suffixes: - norm = re.sub(suffix, "", norm, flags=re.IGNORECASE) + if strip_suffixes: + for suffix in SUFFIXES: + norm = re.sub(suffix, "", norm, flags=re.IGNORECASE) return norm.strip().lower() def handle(self, *args, **options): - # We need to find duplicates based on normalized names within each category - # Since we can't easily do normalization in a single SQL query with complex regex, - # we'll fetch all flavors and group them in Python. - - categories = Category.objects.all() + dry_run: bool = options.get("dry_run", False) + strip_suffixes: bool = not options.get("no_normalize", False) + + if dry_run: + self.stdout.write(self.style.WARNING("DRY RUN — no changes will be made.")) + total_merged = 0 - for category in categories: - flavors = Flavor.objects.filter(category=category) + for category in Category.objects.all(): grouped: dict[str, list[Flavor]] = {} - - for f in flavors: - norm = self.normalize_name(f.name) - if norm not in grouped: - grouped[norm] = [] - grouped[norm].append(f) - - for norm_name, flavor_set in grouped.items(): - if len(flavor_set) > 1: - # Sort: external_id first (current catalog), then created_at - # We want to keep the one with external_id - flavor_set.sort(key=lambda x: (x.external_id is None, x.created_at)) - - keep = flavor_set[0] - others = flavor_set[1:] - - self.stdout.write( - f"Merging {len(others)} duplicates into '{keep.name}' " - f"(Category: {category.name}, ID: {keep.id})" - ) - + for f in Flavor.objects.filter(category=category): + grouped.setdefault(self.normalize_name(f.name, strip_suffixes), []).append(f) + + for flavor_set in grouped.values(): + if len(flavor_set) <= 1: + continue + + # Keep the catalog row (has external_id) and the oldest on ties. + flavor_set.sort(key=lambda x: (x.external_id is None, x.created_at)) + keep, others = flavor_set[0], flavor_set[1:] + + names = ", ".join(f"'{o.name}' (ID {o.id})" for o in others) + self.stdout.write( + f"{'[dry-run] would merge' if dry_run else 'Merging'} {names} " + f"into '{keep.name}' (Category: {category.name}, ID: {keep.id})" + ) + + if not dry_run: for other in others: merge_flavors(keep, other) - total_merged += 1 + total_merged += len(others) - self.stdout.write(self.style.SUCCESS(f"Finished cleaning up {total_merged} duplicates.")) + verb = "Would merge" if dry_run else "Finished cleaning up" + self.stdout.write(self.style.SUCCESS(f"{verb} {total_merged} duplicates.")) diff --git a/backend/api/services/flavor.py b/backend/api/services/flavor.py index 2ea6d2d..ec9bd45 100644 --- a/backend/api/services/flavor.py +++ b/backend/api/services/flavor.py @@ -1,48 +1,54 @@ from django.db import transaction -from api.models import Flavor, Rating, Reply + +from api.models import Flavor, Notification, Rating, Reply + + +def _quality(rating: Rating) -> tuple[int, object]: + """Rank a rating for conflict resolution: longest comment wins, newer breaks ties.""" + return (len(rating.comment or ""), rating.created_at) + def merge_flavors(keep_flavor: Flavor, remove_flavor: Flavor) -> None: """ Merges remove_flavor into keep_flavor. - Re-assigns all ratings and ensures integrity by keeping the most detailed - review if a user has rated both. + + Re-assigns all ratings. When a user rated both flavors, the higher-quality + review survives (longest comment, newer on ties) and its replies + + notifications are re-pointed to the surviving row. """ if keep_flavor.id == remove_flavor.id: return with transaction.atomic(): - # Re-assign or resolve ratings - remove_ratings = Rating.objects.filter(flavor=remove_flavor) - - for rating in remove_ratings: - # Check if user already rated the target flavor - existing_rating = Rating.objects.filter(user=rating.user, flavor=keep_flavor).first() - - if not existing_rating: - # Safe to just move it - rating.flavor = keep_flavor - rating.save() - else: - # Conflict Resolution: Keep the longest comment - keep_comment = existing_rating.comment or "" - remove_comment = rating.comment or "" - - if len(remove_comment) > len(keep_comment): - # Update the one we are keeping to have the better content from the one being removed - existing_rating.score = rating.score - existing_rating.comment = rating.comment - existing_rating.save() - - # Move ALL replies from the rating being deleted to the one being kept - Reply.objects.filter(rating=rating).update(rating=existing_rating) - - # Delete the redundant rating - rating.delete() + for incoming in Rating.objects.filter(flavor=remove_flavor): + existing = Rating.objects.filter(user=incoming.user, flavor=keep_flavor).first() - # Transfer external_id if keep_flavor doesn't have one - if not keep_flavor.external_id and remove_flavor.external_id: - keep_flavor.external_id = remove_flavor.external_id - keep_flavor.save(update_fields=["external_id"]) + if existing is None: + # No conflict — move the rating wholesale. + incoming.flavor = keep_flavor + incoming.save(update_fields=["flavor"]) + continue + + # Conflict: keep one row (unique_together user+flavor). If the + # incoming review is better, copy its content + timestamp onto the + # surviving row so the comment and its date stay consistent. + if _quality(incoming) > _quality(existing): + existing.score = incoming.score + existing.comment = incoming.comment + existing.created_at = incoming.created_at # auto_now_add ignores updates + existing.save(update_fields=["score", "comment", "created_at"]) + + # Preserve community feedback attached to the discarded rating. + Reply.objects.filter(rating=incoming).update(rating=existing) + Notification.objects.filter(rating=incoming).update(rating=existing) + incoming.delete() + + # Carry over the Shopify link if the kept flavor lacks one. external_id + # is unique, so the donor row must be gone before we assign it. + inherited_external_id = remove_flavor.external_id if not keep_flavor.external_id else None - # Finally, delete the flavor remove_flavor.delete() + + if inherited_external_id is not None: + keep_flavor.external_id = inherited_external_id + keep_flavor.save(update_fields=["external_id"]) diff --git a/backend/api/tests/test_merge.py b/backend/api/tests/test_merge.py index 2593fd0..8fdab3f 100644 --- a/backend/api/tests/test_merge.py +++ b/backend/api/tests/test_merge.py @@ -1,89 +1,127 @@ -from django.test import TestCase -from api.models import Category, Flavor, Rating, Reply, User -from api.services.flavor import merge_flavors -from api.management.commands.cleanup_duplicates import Command from io import StringIO +from django.test import TestCase + +from api.management.commands.cleanup_duplicates import Command +from api.models import Category, Flavor, Notification, Rating, Reply, User +from api.services.flavor import merge_flavors + + class FlavorMergeTests(TestCase): def setUp(self): self.cat = Category.objects.create(name="Energy", slug="energy") self.user1 = User.objects.create_user(username="user1") self.user2 = User.objects.create_user(username="user2") - + self.keep = Flavor.objects.create(name="Dolphin Daiquiri", category=self.cat) self.remove = Flavor.objects.create(name="Dolphin Daiquiri Energy", category=self.cat) - + def test_basic_merge(self): # User 1 rates 'remove' r1 = Rating.objects.create(user=self.user1, flavor=self.remove, score=8, comment="Good!") - + merge_flavors(self.keep, self.remove) - + self.assertFalse(Flavor.objects.filter(id=self.remove.id).exists()) r1.refresh_from_db() self.assertEqual(r1.flavor, self.keep) - + def test_merge_with_conflict_keeps_longest_comment(self): # User 1 rates both - r_short = Rating.objects.create(user=self.user1, flavor=self.keep, score=5, comment="Short") - r_long = Rating.objects.create(user=self.user1, flavor=self.remove, score=9, comment="Very long and detailed review") - + Rating.objects.create(user=self.user1, flavor=self.keep, score=5, comment="Short") + r_long = Rating.objects.create( + user=self.user1, flavor=self.remove, score=9, comment="Very long and detailed review" + ) + # Add a reply to the one being deleted reply = Reply.objects.create(user=self.user2, rating=r_long, text="Nice review!") - + merge_flavors(self.keep, self.remove) - + self.assertEqual(Rating.objects.filter(user=self.user1, flavor=self.keep).count(), 1) kept_rating = Rating.objects.get(user=self.user1, flavor=self.keep) self.assertEqual(kept_rating.comment, "Very long and detailed review") self.assertEqual(kept_rating.score, 9) - + # Verify reply moved reply.refresh_from_db() self.assertEqual(reply.rating, kept_rating) def test_merge_with_conflict_preserves_replies_from_discarded(self): - r_keep = Rating.objects.create(user=self.user1, flavor=self.keep, score=10, comment="Best ever!") - r_remove = Rating.objects.create(user=self.user1, flavor=self.remove, score=8, comment="Meh") - + Rating.objects.create(user=self.user1, flavor=self.keep, score=10, comment="Best ever!") + r_remove = Rating.objects.create( + user=self.user1, flavor=self.remove, score=8, comment="Meh" + ) + reply_on_remove = Reply.objects.create(user=self.user2, rating=r_remove, text="I disagree") - + merge_flavors(self.keep, self.remove) - + kept_rating = Rating.objects.get(user=self.user1, flavor=self.keep) self.assertEqual(kept_rating.comment, "Best ever!") - + reply_on_remove.refresh_from_db() self.assertEqual(reply_on_remove.rating, kept_rating) + def test_merge_conflict_repoints_notifications(self): + # User 1 rated both; the discarded rating has a notification attached. + Rating.objects.create(user=self.user1, flavor=self.keep, score=10, comment="Best ever!") + r_remove = Rating.objects.create( + user=self.user1, flavor=self.remove, score=8, comment="Meh" + ) + + notif = Notification.objects.create( + recipient=self.user1, + actor=self.user2, + notification_type="reply", + rating=r_remove, + ) + + merge_flavors(self.keep, self.remove) + + kept_rating = Rating.objects.get(user=self.user1, flavor=self.keep) + notif.refresh_from_db() + self.assertEqual(notif.rating, kept_rating) + + def test_merge_transfers_external_id_when_keep_lacks_one(self): + self.assertIsNone(self.keep.external_id) + self.remove.external_id = 99887766 + self.remove.save(update_fields=["external_id"]) + + merge_flavors(self.keep, self.remove) + + self.keep.refresh_from_db() + self.assertEqual(self.keep.external_id, 99887766) + + class CleanupDuplicatesTests(TestCase): def setUp(self): self.cat = Category.objects.create(name="Energy", slug="energy") self.other_cat = Category.objects.create(name="Iced Tea", slug="iced-tea") - + def test_normalization_logic(self): f1 = Flavor.objects.create(name="Dolphin Daiquiri", category=self.cat) f2 = Flavor.objects.create(name="Dolphin Daiquiri Energy", category=self.cat) f3 = Flavor.objects.create(name="Dolphin Daiquiri", category=self.other_cat) - + out = StringIO() cmd = Command() cmd.stdout = out cmd.handle() - + self.assertTrue(Flavor.objects.filter(id=f1.id).exists()) self.assertFalse(Flavor.objects.filter(id=f2.id).exists()) - self.assertTrue(Flavor.objects.filter(id=f3.id).exists()) # Different category - + self.assertTrue(Flavor.objects.filter(id=f3.id).exists()) # Different category + def test_various_suffixes(self): Flavor.objects.create(name="Berry", category=self.cat) Flavor.objects.create(name="Berry Energy", category=self.cat) Flavor.objects.create(name="Berry Hydration", category=self.cat) Flavor.objects.create(name="Berry Iced Tea", category=self.cat) Flavor.objects.create(name="Berry (Legacy)", category=self.cat) - + cmd = Command() cmd.stdout = StringIO() cmd.handle() - + self.assertEqual(Flavor.objects.filter(category=self.cat).count(), 1)