diff --git a/e2e/smoke.spec.ts b/e2e/smoke.spec.ts index c3f21cf..659e806 100644 --- a/e2e/smoke.spec.ts +++ b/e2e/smoke.spec.ts @@ -81,6 +81,30 @@ test('Registrieren, Set anlegen, Wunschliste teilen und reservieren', async ({ p await visitorContext.close(); }); +test('Dublettenpruefung warnt vor einem fast gleichen Set', async ({ page }) => { + await page.goto('/anmelden'); + await page.getByLabel('E-Mail').fill('test@bricktrack.local'); + await page.getByLabel('Passwort').fill('smoketest-passwort'); + await page.getByRole('button', { name: 'Anmelden', exact: true }).click(); + await expect(page.getByRole('heading', { name: 'Hallo Florian' })).toBeVisible(); + + // Dasselbe Set noch einmal, mit Tippfehler im Namen und anderer Nummer. + await page.goto('/katalog/neu'); + await page.getByLabel('Setnummer').fill('60399'); + await page + .getByRole('textbox', { name: 'Name', exact: true }) + .fill('Familienhaus und Elektroauo'); + await page.getByRole('button', { name: 'Set anlegen' }).click(); + + await expect(page.getByText('Gibt es das vielleicht schon?')).toBeVisible(); + await expect(page.getByText('Sehr ähnlicher Name')).toBeVisible(); + + // Zurueck ins Formular: nichts wurde angelegt. + await page.getByRole('button', { name: 'Zurück zum Formular' }).click(); + await page.goto('/katalog?q=60399'); + await expect(page.getByText('0 Sets')).toBeVisible(); +}); + test('Einladung: zweiter Elternteil bekommt ein eigenes Konto', async ({ page, browser }) => { // The admin from the first test is still the only account, so log in. await page.goto('/anmelden'); diff --git a/src/app/(app)/katalog/duplicate-actions.ts b/src/app/(app)/katalog/duplicate-actions.ts new file mode 100644 index 0000000..f18cfcb --- /dev/null +++ b/src/app/(app)/katalog/duplicate-actions.ts @@ -0,0 +1,100 @@ +'use server'; + +import { and, eq, ne } from 'drizzle-orm'; +import { db } from '@/db'; +import { manufacturer, set } from '@/db/schema'; +import { actionData, type ActionResult } from '@/lib/action-result'; +import { requireUser } from '@/lib/session'; +import { isSimilarName } from '@/lib/similarity'; + +export type DuplicateCandidate = { + id: string; + setNumber: string; + name: string; + manufacturerName: string; + /** Why we are showing it. */ + reason: 'same-number' | 'similar-name' | 'number-elsewhere'; +}; + +/** + * Looks for sets that the one being created might be a duplicate of. + * + * Three cases, in descending order of certainty: + * - the same number at the same manufacturer (that is a real duplicate and is + * rejected on save anyway), + * - a suspiciously similar name at the same manufacturer, + * - the same number at a different manufacturer, which usually means the + * wrong manufacturer was selected. + */ +export async function findDuplicateCandidates(input: { + manufacturerId: string; + setNumber: string; + name: string; + excludeSetId?: string; +}): Promise> { + await requireUser(); + + const setNumber = input.setNumber?.trim() ?? ''; + const name = input.name?.trim() ?? ''; + if (!input.manufacturerId || (setNumber === '' && name === '')) { + return actionData({ candidates: [] }); + } + + const rows = db + .select({ + id: set.id, + setNumber: set.setNumber, + name: set.name, + manufacturerId: set.manufacturerId, + manufacturerName: manufacturer.name, + }) + .from(set) + .innerJoin(manufacturer, eq(manufacturer.id, set.manufacturerId)) + .where(input.excludeSetId ? ne(set.id, input.excludeSetId) : undefined) + .all(); + + const candidates: DuplicateCandidate[] = []; + + for (const row of rows) { + const sameManufacturer = row.manufacturerId === input.manufacturerId; + const sameNumber = setNumber !== '' && row.setNumber.toLowerCase() === setNumber.toLowerCase(); + + if (sameManufacturer && sameNumber) { + candidates.push({ ...row, reason: 'same-number' }); + continue; + } + if (sameManufacturer && name !== '' && isSimilarName(row.name, name)) { + candidates.push({ ...row, reason: 'similar-name' }); + continue; + } + if (!sameManufacturer && sameNumber) { + candidates.push({ ...row, reason: 'number-elsewhere' }); + } + } + + // Certain matches first, and never more than a handful. + const order: Record = { + 'same-number': 0, + 'similar-name': 1, + 'number-elsewhere': 2, + }; + candidates.sort((a, b) => order[a.reason] - order[b.reason]); + + return actionData({ candidates: candidates.slice(0, 5) }); +} + +/** Used by the import to point at an existing entry instead of failing blindly. */ +export async function findSetByNumber( + manufacturerId: string, + setNumber: string, +): Promise> { + await requireUser(); + + const row = db + .select({ id: set.id }) + .from(set) + .where(and(eq(set.manufacturerId, manufacturerId), eq(set.setNumber, setNumber))) + .get(); + + return actionData(row ?? null); +} diff --git a/src/components/katalog/set-form.tsx b/src/components/katalog/set-form.tsx index 2a95a80..0b2ac7e 100644 --- a/src/components/katalog/set-form.tsx +++ b/src/components/katalog/set-form.tsx @@ -1,13 +1,28 @@ 'use client'; import { useRouter } from 'next/navigation'; -import { useState, useTransition } from 'react'; +import { useRef, useState, useTransition } from 'react'; +import Link from 'next/link'; import { Loader2, Save } from 'lucide-react'; import { toast } from 'sonner'; import { createSet, updateSet } from '@/app/(app)/katalog/actions'; +import { + findDuplicateCandidates, + type DuplicateCandidate, +} from '@/app/(app)/katalog/duplicate-actions'; import { centsToInput } from '@/lib/money'; import { setImageSrc } from '@/lib/images'; import { Button } from '@/components/ui/button'; +import { + AlertDialog, + AlertDialogAction, + AlertDialogCancel, + AlertDialogContent, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogHeader, + AlertDialogTitle, +} from '@/components/ui/alert-dialog'; import { Checkbox } from '@/components/ui/checkbox'; import { Field } from '@/components/ui/field'; import { Input } from '@/components/ui/input'; @@ -46,6 +61,8 @@ export function SetForm({ manufacturers, themes, set, defaultEan }: Props) { const [manufacturerId, setManufacturerId] = useState( set?.manufacturerId ?? manufacturers[0]?.id ?? '', ); + const [duplicates, setDuplicates] = useState(null); + const pendingForm = useRef(null); const existingImage = set ? setImageSrc(set) : null; const availableThemes = themes.filter((theme) => theme.manufacturerId === manufacturerId); @@ -54,32 +71,70 @@ export function SetForm({ manufacturers, themes, set, defaultEan }: Props) { event.preventDefault(); const formData = new FormData(event.currentTarget); - startTransition(async () => { - if (set) { - const result = await updateSet(set.id, formData); - if (!result.ok) { - toast.error(result.error); + // Ask before creating something that already exists. Editing skips this: + // there the entry is the one we would be warning about. + if (!set) { + startTransition(async () => { + const check = await findDuplicateCandidates({ + manufacturerId: String(formData.get('manufacturerId') ?? ''), + setNumber: String(formData.get('setNumber') ?? ''), + name: String(formData.get('name') ?? ''), + }); + + if (check.ok && check.data.candidates.length > 0) { + pendingForm.current = formData; + setDuplicates(check.data.candidates); return; } - toast.success('Set gespeichert.'); - router.push(`/katalog/${set.id}`); - router.refresh(); - return; - } - const result = await createSet(formData); - if (!result.ok) { - toast.error(result.error); - return; - } - toast.success('Set angelegt.'); - router.push(`/katalog/${result.data.id}`); - router.refresh(); + await save(formData); + }); + return; + } + + startTransition(() => save(formData)); + } + + async function save(formData: FormData) { + return new Promise((resolve) => { + startTransition(async () => { + if (set) { + const result = await updateSet(set.id, formData); + if (!result.ok) { + toast.error(result.error); + resolve(); + return; + } + toast.success('Set gespeichert.'); + router.push(`/katalog/${set.id}`); + router.refresh(); + resolve(); + return; + } + + const result = await createSet(formData); + if (!result.ok) { + toast.error(result.error); + resolve(); + return; + } + toast.success('Set angelegt.'); + router.push(`/katalog/${result.data.id}`); + router.refresh(); + resolve(); + }); }); } + const REASON_LABELS: Record = { + 'same-number': 'Gleiche Setnummer beim gleichen Hersteller', + 'similar-name': 'Sehr ähnlicher Name', + 'number-elsewhere': 'Gleiche Setnummer bei einem anderen Hersteller', + }; + return ( -
+ <> +
-
+ + + { + if (!open) { + setDuplicates(null); + pendingForm.current = null; + } + }} + > + + + Gibt es das vielleicht schon? + + Diese Einträge sehen ähnlich aus. Schau kurz nach, bevor der Katalog zwei Mal + dasselbe Set enthält. + + + +
    + {(duplicates ?? []).map((candidate) => ( +
  • + + {candidate.setNumber} · {candidate.name} + +

    + {candidate.manufacturerName} — {REASON_LABELS[candidate.reason]} +

    +
    • + ))} +
+ + + Zurück zum Formular + { + const formData = pendingForm.current; + setDuplicates(null); + pendingForm.current = null; + if (formData) void save(formData); + }} + > + Trotzdem anlegen + + + + + ); } diff --git a/src/lib/similarity.ts b/src/lib/similarity.ts new file mode 100644 index 0000000..364758c --- /dev/null +++ b/src/lib/similarity.ts @@ -0,0 +1,77 @@ +/** + * Name comparison for the duplicate check when a set is created. + * + * Pure and free of server imports so it can be unit tested - and it needs to + * be, because "similar enough" is a judgement call that is easy to get wrong + * in both directions. + */ + +/** + * Lowercases, strips punctuation and collapses whitespace, so that + * "Feuerwehr-Auto" and "feuerwehr auto" are the same string. + */ +export function normalizeName(value: string): string { + return value + .toLowerCase() + .replace(/ä/g, 'ae') + .replace(/ö/g, 'oe') + .replace(/ü/g, 'ue') + .replace(/ß/g, 'ss') + .replace(/[^a-z0-9]+/g, ' ') + .trim(); +} + +/** Classic Levenshtein distance, iterative with a single row. */ +export function levenshtein(a: string, b: string): number { + if (a === b) return 0; + if (a.length === 0) return b.length; + if (b.length === 0) return a.length; + + let previous = Array.from({ length: b.length + 1 }, (_, index) => index); + + for (let i = 1; i <= a.length; i += 1) { + const current = [i]; + for (let j = 1; j <= b.length; j += 1) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + current[j] = Math.min( + current[j - 1]! + 1, // insertion + previous[j]! + 1, // deletion + previous[j - 1]! + cost, // substitution + ); + } + previous = current; + } + + return previous[b.length]!; +} + +/** 1 means identical, 0 means nothing in common. */ +export function similarity(a: string, b: string): number { + const left = normalizeName(a); + const right = normalizeName(b); + if (left === right) return 1; + if (left.length === 0 || right.length === 0) return 0; + + const longest = Math.max(left.length, right.length); + return 1 - levenshtein(left, right) / longest; +} + +/** + * Above this, two names are close enough to be worth asking about. Chosen so + * that a typo or a different spelling triggers, while two genuinely different + * sets of the same theme do not. + */ +export const SIMILAR_NAME_THRESHOLD = 0.82; + +export function isSimilarName(a: string, b: string): boolean { + const left = normalizeName(a); + const right = normalizeName(b); + + // One name fully contained in the other ("Feuerwehrauto" vs "Feuerwehrauto + // mit Leiter") is a strong hint even when the length differs a lot. + if (left.length >= 6 && right.length >= 6 && (left.includes(right) || right.includes(left))) { + return true; + } + + return similarity(a, b) >= SIMILAR_NAME_THRESHOLD; +} diff --git a/tests/unit/similarity.test.ts b/tests/unit/similarity.test.ts new file mode 100644 index 0000000..eda70fa --- /dev/null +++ b/tests/unit/similarity.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest'; +import { isSimilarName, levenshtein, normalizeName, similarity } from '@/lib/similarity'; + +describe('Namen normalisieren', () => { + it('schreibt Umlaute aus und wirft Satzzeichen weg', () => { + expect(normalizeName('Feuerwehr-Auto')).toBe('feuerwehr auto'); + expect(normalizeName('Grüne Kiste!')).toBe('gruene kiste'); + expect(normalizeName('Straßenbahn')).toBe('strassenbahn'); + }); + + it('macht aus mehreren Leerzeichen eines', () => { + expect(normalizeName(' Lego City ')).toBe('lego city'); + }); +}); + +describe('Levenshtein', () => { + it('ist null bei identischen Strings', () => { + expect(levenshtein('haus', 'haus')).toBe(0); + }); + + it('zaehlt einzelne Aenderungen', () => { + expect(levenshtein('haus', 'maus')).toBe(1); + expect(levenshtein('haus', 'hausen')).toBe(2); + expect(levenshtein('', 'haus')).toBe(4); + }); +}); + +describe('Aehnlichkeit', () => { + it('erkennt identische Namen', () => { + expect(similarity('Feuerwehrauto', 'feuerwehrauto')).toBe(1); + }); + + it('sinkt mit dem Unterschied', () => { + expect(similarity('Feuerwehrauto', 'Polizeiauto')).toBeLessThan(0.7); + }); +}); + +describe('Dublettenverdacht', () => { + it('schlaegt bei Tippfehlern an', () => { + expect(isSimilarName('Familienhaus und Elektroauto', 'Familienhaus und Elektroauo')).toBe(true); + expect(isSimilarName('Feuerwehreinsatz', 'Feuerwehr-Einsatz')).toBe(true); + }); + + it('schlaegt an, wenn ein Name im anderen steckt', () => { + expect(isSimilarName('Feuerwehrauto', 'Feuerwehrauto mit Leiter')).toBe(true); + }); + + it('schlaegt nicht bei verschiedenen Sets desselben Themas an', () => { + expect(isSimilarName('Feuerwehreinsatz', 'Polizeistation')).toBe(false); + expect(isSimilarName('Kipplaster', 'McLaren P1')).toBe(false); + // Zwei echte, unterschiedliche City-Sets. + expect(isSimilarName('Familienhaus und Elektroauto', 'Feuerwehreinsatz')).toBe(false); + }); + + it('laesst sich von kurzen Namen nicht zu Unsinn verleiten', () => { + // Ohne die Mindestlaenge wuerde "Bus" in "Autobus" jede Dublette melden. + expect(isSimilarName('Bus', 'Autobus')).toBe(false); + }); +});