From 41c1b3c574ff192a45369b22280d830fc6ee8950 Mon Sep 17 00:00:00 2001 From: Nils Pukropp Date: Wed, 27 May 2026 16:42:59 +0200 Subject: [PATCH] implemented merging --- .../management/commands/cleanup_duplicates.py | 95 +++++----- backend/api/services/flavor.py | 48 +++++ backend/api/tests/test_merge.py | 89 ++++++++++ backend/api/views/admin.py | 29 ++- frontend/src/App.tsx | 2 + frontend/src/pages/admin/AdminFlavors.tsx | 168 ++++++++++++++++++ frontend/src/pages/admin/AdminLayout.tsx | 2 + 7 files changed, 390 insertions(+), 43 deletions(-) create mode 100644 backend/api/services/flavor.py create mode 100644 backend/api/tests/test_merge.py create mode 100644 frontend/src/pages/admin/AdminFlavors.tsx diff --git a/backend/api/management/commands/cleanup_duplicates.py b/backend/api/management/commands/cleanup_duplicates.py index bc72cca..d1615ad 100644 --- a/backend/api/management/commands/cleanup_duplicates.py +++ b/backend/api/management/commands/cleanup_duplicates.py @@ -1,51 +1,62 @@ +import re from django.core.management.base import BaseCommand -from django.db.models import Count +from django.db.models import Count, F, Func, Value, CharField -from api.models import Flavor, Rating +from api.models import Flavor, Category +from api.services.flavor import merge_flavors class Command(BaseCommand): - help = "Identifies and merges duplicate flavors (same name and category)" + 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\)$", + ] + norm = name.strip() + for suffix in suffixes: + norm = re.sub(suffix, "", norm, flags=re.IGNORECASE) + return norm.strip().lower() def handle(self, *args, **options): - # Find duplicates based on name and category - duplicates = ( - Flavor.objects.values("name", "category") - .annotate(name_count=Count("id")) - .filter(name_count__gt=1) - ) + # 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() + total_merged = 0 - self.stdout.write(f"Found {duplicates.count()} sets of duplicates.") + for category in categories: + flavors = Flavor.objects.filter(category=category) + 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 other in others: + merge_flavors(keep, other) + total_merged += 1 - for dup in duplicates: - name = dup["name"] - category_id = dup["category"] - - # Get all flavors in this set, ordered by having an external_id first, then by date - flavor_set = Flavor.objects.filter(name=name, category_id=category_id).order_by( - "-external_id", "created_at" - ) - - # Keep the first one, merge others into it - keep = flavor_set[0] - others = flavor_set[1:] - - self.stdout.write( - f"Merging {len(others)} duplicates into '{keep.name}' (ID: {keep.id}, Ext: {keep.external_id})" - ) - - for other in others: - # Re-assign ratings - for rating in Rating.objects.filter(flavor=other): - try: - rating.flavor = keep - rating.save() - except Exception: - # User already has a rating for the 'keep' flavor - # Delete the duplicate rating instead - rating.delete() - - # Delete the duplicate flavor - other.delete() - - self.stdout.write(self.style.SUCCESS("Finished cleaning up duplicates.")) + self.stdout.write(self.style.SUCCESS(f"Finished cleaning up {total_merged} duplicates.")) diff --git a/backend/api/services/flavor.py b/backend/api/services/flavor.py new file mode 100644 index 0000000..2ea6d2d --- /dev/null +++ b/backend/api/services/flavor.py @@ -0,0 +1,48 @@ +from django.db import transaction +from api.models import Flavor, Rating, Reply + +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. + """ + 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() + + # 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() diff --git a/backend/api/tests/test_merge.py b/backend/api/tests/test_merge.py new file mode 100644 index 0000000..2593fd0 --- /dev/null +++ b/backend/api/tests/test_merge.py @@ -0,0 +1,89 @@ +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 + +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") + + # 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") + + 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) + +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 + + 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) diff --git a/backend/api/views/admin.py b/backend/api/views/admin.py index b38233a..b63903b 100644 --- a/backend/api/views/admin.py +++ b/backend/api/views/admin.py @@ -8,7 +8,8 @@ from rest_framework.decorators import action from rest_framework.request import Request from rest_framework.response import Response -from api.models import Job, Rating, Reply, SystemConfig, Ticket, User +from api.models import Flavor, Job, Rating, Reply, SystemConfig, Ticket, User +from api.services.flavor import merge_flavors from api.serializers import ( AdminUserDetailSerializer, AdminUserListSerializer, @@ -154,6 +155,32 @@ class AdminViewSet(viewsets.ViewSet): _sync_periodic_task(job) return Response(JobSerializer(job).data) + @action(detail=False, methods=["post"]) + def merge_flavors(self, request: Request) -> Response: + keep_id = request.data.get("keep_id") + remove_id = request.data.get("remove_id") + + if not keep_id or not remove_id: + return Response( + {"error": "Both keep_id and remove_id are required"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + try: + keep_flavor = Flavor.objects.get(pk=keep_id) + remove_flavor = Flavor.objects.get(pk=remove_id) + except Flavor.DoesNotExist: + return Response({"error": "One or both flavors not found"}, status=status.HTTP_404_NOT_FOUND) + + if keep_flavor.category_id != remove_flavor.category_id: + return Response( + {"error": "Flavors must belong to the same category"}, + status=status.HTTP_400_BAD_REQUEST, + ) + + merge_flavors(keep_flavor, remove_flavor) + return Response({"status": "Flavors merged successfully"}) + @action(detail=False, methods=["post"]) def send_test_email(self, request: Request) -> Response: me = current_user(request) diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index b61cb15..322fc8f 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -53,6 +53,7 @@ const AdminBanners = lazy(() => import('./pages/admin/AdminBanners')); const AdminJobs = lazy(() => import('./pages/admin/AdminJobs')); const AdminTickets = lazy(() => import('./pages/admin/AdminTickets')); const AdminConfig = lazy(() => import('./pages/admin/AdminConfig')); +const AdminFlavors = lazy(() => import('./pages/admin/AdminFlavors')); const AdminUserDetail = lazy(() => import('./pages/AdminUserDetail')); const AdminRatingDetail = lazy(() => import('./pages/AdminRatingDetail')); const AdminReplyDetail = lazy(() => import('./pages/AdminReplyDetail')); @@ -338,6 +339,7 @@ const App: React.FC = () => { } /> } /> } /> + } /> } /> } /> } /> diff --git a/frontend/src/pages/admin/AdminFlavors.tsx b/frontend/src/pages/admin/AdminFlavors.tsx new file mode 100644 index 0000000..4d8a78c --- /dev/null +++ b/frontend/src/pages/admin/AdminFlavors.tsx @@ -0,0 +1,168 @@ +import React, { useState } from 'react'; +import { + Autocomplete, + Box, + Button, + Card, + CardContent, + Dialog, + DialogActions, + DialogContent, + DialogContentText, + DialogTitle, + Divider, + Grid, + TextField, + Typography, +} from '@mui/material'; +import { useTranslation } from 'react-i18next'; +import { useSnackbar } from '../../contexts/NotificationContext'; +import { useFlavorsList } from '../../api/queries/useFlavorsList'; +import api from '../../lib/api'; +import { useQueryClient } from '@tanstack/react-query'; +import { queryKeys } from '../../api/keys'; + +const AdminFlavors: React.FC = () => { + const { t } = useTranslation(); + const { notify } = useSnackbar(); + const queryClient = useQueryClient(); + const { data: flavors = [], isLoading } = useFlavorsList(); + + const [keepFlavor, setKeepFlavor] = useState(null); + const [removeFlavor, setRemoveFlavor] = useState(null); + const [confirmOpen, setConfirmOpen] = useState(false); + const [isMerging, setIsMerging] = useState(false); + + const handleMerge = async () => { + if (!keepFlavor || !removeFlavor) return; + + setIsMerging(true); + try { + await api.post('admin/merge_flavors/', { + keep_id: keepFlavor.id, + remove_id: removeFlavor.id, + }); + + notify({ message: t('admin.mergeSuccess', { defaultValue: 'Flavors merged successfully' }), severity: 'success' }); + + // Invalidate queries + queryClient.invalidateQueries({ queryKey: queryKeys.flavors({ scope: 'all' }) }); + + // Reset selection + setKeepFlavor(null); + setRemoveFlavor(null); + setConfirmOpen(false); + } catch (err: any) { + const errorMsg = err.response?.data?.error || err.message; + notify({ message: `Error: ${errorMsg}`, severity: 'error' }); + } finally { + setIsMerging(false); + } + }; + + const canMerge = keepFlavor && removeFlavor && keepFlavor.id !== removeFlavor.id; + + return ( + + + {t('admin.manageFlavors', { defaultValue: 'Manage Flavors' })} + + + + + + {t('admin.mergeFlavors', { defaultValue: 'Merge Flavors' })} + + + {t('admin.mergeFlavorsDesc', { + defaultValue: 'Combine two flavors into one. Ratings from the source flavor will be moved to the target flavor. If a user rated both, the more detailed review is kept.' + })} + + + + + `${option.name} (${option.category_name})`} + value={removeFlavor} + onChange={(_, newValue) => setRemoveFlavor(newValue)} + loading={isLoading} + renderInput={(params) => ( + + )} + /> + + + + → + + + + `${option.name} (${option.category_name})`} + value={keepFlavor} + onChange={(_, newValue) => setKeepFlavor(newValue)} + loading={isLoading} + renderInput={(params) => ( + + )} + /> + + + + + + + + + + + + + !isMerging && setConfirmOpen(false)}> + {t('admin.confirmMergeTitle', { defaultValue: 'Confirm Flavor Merge' })} + + + {t('admin.confirmMergeText', { + defaultValue: 'Are you sure you want to merge "{{remove}}" into "{{keep}}"? This action is irreversible. "{{remove}}" will be deleted, and all its ratings will be transferred.', + remove: removeFlavor?.name, + keep: keepFlavor?.name + })} + + + + + + + + + ); +}; + +export default AdminFlavors; diff --git a/frontend/src/pages/admin/AdminLayout.tsx b/frontend/src/pages/admin/AdminLayout.tsx index 91aa53c..1133bb7 100644 --- a/frontend/src/pages/admin/AdminLayout.tsx +++ b/frontend/src/pages/admin/AdminLayout.tsx @@ -16,6 +16,7 @@ import WallpaperIcon from '@mui/icons-material/Wallpaper'; import TerminalIcon from '@mui/icons-material/Terminal'; import HelpOutlineIcon from '@mui/icons-material/HelpOutline'; import TuneIcon from '@mui/icons-material/Tune'; +import LocalDrinkIcon from '@mui/icons-material/LocalDrink'; import { useTranslation } from 'react-i18next'; import { PageShell } from '../../components/ui'; import { useTitle } from '../../hooks/useTitle'; @@ -23,6 +24,7 @@ import { useTitle } from '../../hooks/useTitle'; const SECTIONS = [ { value: 'overview', icon: }, { value: 'users', icon: }, + { value: 'flavors', icon: }, { value: 'banners', icon: }, { value: 'jobs', icon: }, { value: 'tickets', icon: },