implemented more rail guards

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.
+        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."))
 
-        categories = Category.objects.all()
         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 Flavor.objects.filter(category=category):
+                grouped.setdefault(self.normalize_name(f.name, strip_suffixes), []).append(f)
 
-            for f in flavors:
-                norm = self.normalize_name(f.name)
-                if norm not in grouped:
-                    grouped[norm] = []
-                grouped[norm].append(f)
+            for flavor_set in grouped.values():
+                if len(flavor_set) <= 1:
+                    continue
 
-            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 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:]
 
-                    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})"
-                    )
+                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."))

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 incoming in Rating.objects.filter(flavor=remove_flavor):
+            existing = Rating.objects.filter(user=incoming.user, flavor=keep_flavor).first()
 
-        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 existing is None:
+                # No conflict — move the rating wholesale.
+                incoming.flavor = keep_flavor
+                incoming.save(update_fields=["flavor"])
+                continue
 
-            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 ""
+            # 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"])
 
-                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()
+            # 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()
 
-                # Move ALL replies from the rating being deleted to the one being kept
-                Reply.objects.filter(rating=rating).update(rating=existing_rating)
+        # 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
 
-                # Delete the redundant rating
-                rating.delete()
-
-        # 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"])
-
-        # 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"])

backend/api/tests/test_merge.py

@@ -1,9 +1,12 @@
-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")
@@ -25,8 +28,10 @@ class FlavorMergeTests(TestCase):
 
     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!")
@@ -43,8 +48,10 @@ class FlavorMergeTests(TestCase):
         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")
 
@@ -56,6 +63,37 @@ class FlavorMergeTests(TestCase):
         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")
@@ -73,7 +111,7 @@ class CleanupDuplicatesTests(TestCase):
 
         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)