From 7d6fef5fed2406450dbf355481af65b5c3749190 Mon Sep 17 00:00:00 2001 From: ericsocrat Date: Tue, 17 Mar 2026 22:28:14 +0100 Subject: [PATCH] fix(admin): clarify legacy null-country submissions in review UI - Add nullLabel prop to CountryChip for null-country fallback rendering - Show 'No country' chip with muted styling for legacy submissions - Add legacy help text: 'Submitted before country tracking was added' - Add GS1 informational hint for null-country with known GS1 barcode - Add 'No country' filter option in country dropdown (__none__ sentinel) - Use effectiveCountry variable to simplify GS1 mismatch logic - Add 3 new admin i18n keys to en/pl/de locales - Update 1 existing test + add 6 new tests (63/63 passing) --- frontend/messages/de.json | 5 +- frontend/messages/en.json | 5 +- frontend/messages/pl.json | 5 +- .../app/app/admin/submissions/page.test.tsx | 129 +++++++++++++++++- .../src/app/app/admin/submissions/page.tsx | 66 +++++---- .../components/common/CountryChip.test.tsx | 10 +- .../src/components/common/CountryChip.tsx | 21 ++- 7 files changed, 206 insertions(+), 35 deletions(-) diff --git a/frontend/messages/de.json b/frontend/messages/de.json index 049e265f..c67f6d36 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -1096,7 +1096,10 @@ "allCountries": "Alle Länder", "gs1Mismatch": "GS1-Barcode deutet auf {gs1Country}, aber Einreichung zielt auf {effectiveCountry}", "regionMismatch": "Gescannt in {scanCountry}, aber vorgeschlagen für {suggestedCountry}", - "crossCountryProducts": "Gleiche EAN existiert in {countries} ({count} Produkt(e))" + "crossCountryProducts": "Gleiche EAN existiert in {countries} ({count} Produkt(e))", + "noCountry": "Kein Land", + "noCountryHint": "Eingereicht vor der Länder-Erfassung", + "gs1InfoHint": "GS1-Barcode registriert in {gs1Country} (informativ)" }, "monitoring": { "title": "Systemüberwachung", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index fce59017..1944a0d3 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -1096,7 +1096,10 @@ "allCountries": "All countries", "gs1Mismatch": "GS1 barcode suggests {gs1Country} but submission targets {effectiveCountry}", "regionMismatch": "Scanned in {scanCountry} but suggested for {suggestedCountry}", - "crossCountryProducts": "Same EAN exists in {countries} ({count} product(s))" + "crossCountryProducts": "Same EAN exists in {countries} ({count} product(s))", + "noCountry": "No country", + "noCountryHint": "Submitted before country tracking was added", + "gs1InfoHint": "GS1 barcode registered in {gs1Country} (informational)" }, "monitoring": { "title": "System Monitoring", diff --git a/frontend/messages/pl.json b/frontend/messages/pl.json index 0b944438..5fab5bdf 100644 --- a/frontend/messages/pl.json +++ b/frontend/messages/pl.json @@ -1096,7 +1096,10 @@ "allCountries": "Wszystkie kraje", "gs1Mismatch": "Kod GS1 wskazuje na {gs1Country}, ale zgłoszenie dotyczy {effectiveCountry}", "regionMismatch": "Zeskanowano w {scanCountry}, ale zasugerowano dla {suggestedCountry}", - "crossCountryProducts": "Ten sam EAN istnieje w {countries} ({count} produkt(ów))" + "crossCountryProducts": "Ten sam EAN istnieje w {countries} ({count} produkt(ów))", + "noCountry": "Brak kraju", + "noCountryHint": "Zgłoszone przed dodaniem śledzenia krajów", + "gs1InfoHint": "Kod GS1 zarejestrowany w {gs1Country} (informacyjnie)" }, "monitoring": { "title": "Monitoring systemu", diff --git a/frontend/src/app/app/admin/submissions/page.test.tsx b/frontend/src/app/app/admin/submissions/page.test.tsx index 75eae21a..19a10d0a 100644 --- a/frontend/src/app/app/admin/submissions/page.test.tsx +++ b/frontend/src/app/app/admin/submissions/page.test.tsx @@ -26,8 +26,12 @@ vi.mock("@/components/common/skeletons", () => ({ })); vi.mock("@/components/common/CountryChip", () => ({ - CountryChip: ({ country }: { country: string | null }) => - country ? {country} : null, + CountryChip: ({ country, nullLabel }: { country: string | null; nullLabel?: string }) => + country ? ( + {country} + ) : nullLabel ? ( + {nullLabel} + ) : null, })); // ─── Helpers ──────────────────────────────────────────────────────────────── @@ -581,7 +585,7 @@ describe("AdminSubmissionsPage", () => { expect(chip).toHaveTextContent("DE"); }); - it("does not render country chip when both countries are null", async () => { + it("shows fallback country chip when both countries are null", async () => { mockCallRpc.mockImplementation((_client: unknown, fnName: string) => { if (fnName === "api_admin_get_submissions") { return Promise.resolve({ @@ -606,7 +610,9 @@ describe("AdminSubmissionsPage", () => { await waitFor(() => { expect(screen.getByText("Unknown Origin")).toBeInTheDocument(); }); - expect(screen.queryByTestId("country-chip")).not.toBeInTheDocument(); + const chip = screen.getByTestId("country-chip"); + expect(chip).toBeInTheDocument(); + expect(chip).toHaveAttribute("data-null-country"); }); it("renders country filter dropdown", async () => { @@ -857,4 +863,119 @@ describe("AdminSubmissionsPage", () => { }); expect(screen.queryByTestId("cross-country-badge")).not.toBeInTheDocument(); }); + + // ─── Legacy Null-Country UX Tests ───────────────────────────────────────── + + it("shows legacy help text for null-country submission", async () => { + mockCallRpc.mockImplementation((_client: unknown, fnName: string) => { + if (fnName === "api_admin_get_submissions") { + return Promise.resolve({ + ok: true, + data: { + submissions: [ + makeSubmission({ + scan_country: null, + suggested_country: null, + product_name: "Legacy Product", + }), + ], + page: 1, + pages: 1, + total: 1, + }, + }); + } + return Promise.resolve({ ok: true, data: {} }); + }); + render(, { wrapper: createWrapper() }); + await waitFor(() => { + expect(screen.getByTestId("no-country-info")).toBeInTheDocument(); + }); + }); + + it("shows GS1 informational hint when country is null but gs1_hint exists", async () => { + mockCallRpc.mockImplementation((_client: unknown, fnName: string) => { + if (fnName === "api_admin_get_submissions") { + return Promise.resolve({ + ok: true, + data: { + submissions: [ + makeSubmission({ + scan_country: null, + suggested_country: null, + gs1_hint: { code: "DE", name: "Germany", confidence: "high", prefix: "400" }, + product_name: "GS1 Hint Legacy", + }), + ], + page: 1, + pages: 1, + total: 1, + }, + }); + } + return Promise.resolve({ ok: true, data: {} }); + }); + render(, { wrapper: createWrapper() }); + await waitFor(() => { + expect(screen.getByTestId("gs1-info-hint")).toBeInTheDocument(); + }); + expect(screen.queryByTestId("gs1-mismatch-badge")).not.toBeInTheDocument(); + }); + + it("renders No country option in country filter", async () => { + render(, { wrapper: createWrapper() }); + await waitFor(() => { + expect(screen.getByTestId("country-filter")).toBeInTheDocument(); + }); + expect(screen.getByText("No country")).toBeInTheDocument(); + }); + + it("sends p_country __none__ when No country filter is selected", async () => { + render(, { wrapper: createWrapper() }); + const user = userEvent.setup(); + + await waitFor(() => { + expect(screen.getByTestId("country-filter")).toBeInTheDocument(); + }); + + await user.selectOptions(screen.getByTestId("country-filter"), "__none__"); + + await waitFor(() => { + expect(mockCallRpc).toHaveBeenCalledWith( + expect.anything(), + "api_admin_get_submissions", + expect.objectContaining({ p_country: "__none__" }), + ); + }); + }); + + it("suppresses mismatch badges for null-country submissions", async () => { + mockCallRpc.mockImplementation((_client: unknown, fnName: string) => { + if (fnName === "api_admin_get_submissions") { + return Promise.resolve({ + ok: true, + data: { + submissions: [ + makeSubmission({ + scan_country: null, + suggested_country: null, + gs1_hint: { code: "DE", name: "Germany", confidence: "high", prefix: "400" }, + product_name: "No Mismatch Legacy", + }), + ], + page: 1, + pages: 1, + total: 1, + }, + }); + } + return Promise.resolve({ ok: true, data: {} }); + }); + render(, { wrapper: createWrapper() }); + await waitFor(() => { + expect(screen.getByText("No Mismatch Legacy")).toBeInTheDocument(); + }); + expect(screen.queryByTestId("gs1-mismatch-badge")).not.toBeInTheDocument(); + expect(screen.queryByTestId("region-mismatch-badge")).not.toBeInTheDocument(); + }); }); diff --git a/frontend/src/app/app/admin/submissions/page.tsx b/frontend/src/app/app/admin/submissions/page.tsx index fb49c84e..a023c4b7 100644 --- a/frontend/src/app/app/admin/submissions/page.tsx +++ b/frontend/src/app/app/admin/submissions/page.tsx @@ -9,31 +9,31 @@ import { CountryChip } from "@/components/common/CountryChip"; import { EmptyStateIllustration } from "@/components/common/EmptyStateIllustration"; import { SubmissionsSkeleton } from "@/components/common/skeletons"; import { Breadcrumbs } from "@/components/layout/Breadcrumbs"; -import { useTranslation } from "@/lib/i18n"; import { COUNTRIES } from "@/lib/constants"; +import { useTranslation } from "@/lib/i18n"; import { callRpc } from "@/lib/rpc"; import { createClient } from "@/lib/supabase/client"; import { showToast } from "@/lib/toast"; import type { - AdminBatchRejectResponse, - AdminReviewResponse, - AdminSubmission, - AdminSubmissionsResponse, - AdminVelocityResponse, - RpcResult, + AdminBatchRejectResponse, + AdminReviewResponse, + AdminSubmission, + AdminSubmissionsResponse, + AdminVelocityResponse, + RpcResult, } from "@/lib/types"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { - Activity, - Ban, - CheckCircle, - Clock, - FileText, - Link2, - RefreshCw, - ShieldAlert, - ShieldCheck, - XCircle, + Activity, + Ban, + CheckCircle, + Clock, + FileText, + Link2, + RefreshCw, + ShieldAlert, + ShieldCheck, + XCircle, } from "lucide-react"; import { useCallback, useMemo, useState } from "react"; @@ -284,6 +284,7 @@ export default function AdminSubmissionsPage() { {c.code} ))} + @@ -408,6 +409,7 @@ function AdminSubmissionCard({ const { t } = useTranslation(); const date = new Date(submission.created_at).toLocaleString(); const canReview = submission.status === "pending"; + const effectiveCountry = submission.suggested_country ?? submission.scan_country; return (
  • @@ -432,10 +434,9 @@ function AdminSubmissionCard({
    {submission.user_trust_score !== null && submission.user_trust_score !== undefined && ( @@ -459,6 +460,13 @@ function AdminSubmissionCard({

    )} + {/* Legacy help text for null-country submissions */} + {!effectiveCountry && ( +

    + ℹ {t("admin.noCountryHint")} +

    + )} + {submission.existing_product_match && (

    ⚠ Possible duplicate — existing product #{submission.existing_product_match.product_id}{" "} @@ -470,18 +478,26 @@ function AdminSubmissionCard({ {submission.gs1_hint && submission.gs1_hint.code !== "UNKNOWN" && submission.gs1_hint.code !== "STORE" && - (submission.suggested_country ?? submission.scan_country) && - submission.gs1_hint.code !== - (submission.suggested_country ?? submission.scan_country) && ( + effectiveCountry && + submission.gs1_hint.code !== effectiveCountry && (

    ⚠ {t("admin.gs1Mismatch", { gs1Country: submission.gs1_hint.name, - effectiveCountry: - (submission.suggested_country ?? submission.scan_country)!, + effectiveCountry: effectiveCountry, })}

    )} + {/* GS1 informational hint for legacy null-country submissions */} + {!effectiveCountry && + submission.gs1_hint && + submission.gs1_hint.code !== "UNKNOWN" && + submission.gs1_hint.code !== "STORE" && ( +

    + ℹ {t("admin.gs1InfoHint", { gs1Country: submission.gs1_hint.name })} +

    + )} + {/* Region mismatch badge (#929) */} {submission.scan_country && submission.suggested_country && diff --git a/frontend/src/components/common/CountryChip.test.tsx b/frontend/src/components/common/CountryChip.test.tsx index 9e940366..85935006 100644 --- a/frontend/src/components/common/CountryChip.test.tsx +++ b/frontend/src/components/common/CountryChip.test.tsx @@ -1,5 +1,5 @@ -import { describe, it, expect, vi } from "vitest"; import { render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; import { CountryChip } from "./CountryChip"; // ─── Mocks ──────────────────────────────────────────────────────────────────── @@ -24,6 +24,14 @@ describe("CountryChip", () => { expect(container.innerHTML).toBe(""); }); + it("renders fallback chip with nullLabel when country is null", () => { + render(); + const chip = screen.getByRole("img"); + expect(chip).toBeTruthy(); + expect(chip.getAttribute("aria-label")).toBe("No country"); + expect(screen.getByText("No country")).toBeTruthy(); + }); + // ─── SVG flag rendering ──────────────────────────────────────────────── it("renders SVG flag for PL (not emoji)", () => { diff --git a/frontend/src/components/common/CountryChip.tsx b/frontend/src/components/common/CountryChip.tsx index ea963475..b01072b5 100644 --- a/frontend/src/components/common/CountryChip.tsx +++ b/frontend/src/components/common/CountryChip.tsx @@ -10,12 +10,14 @@ import { useTranslation } from "@/lib/i18n"; // ─── Types ────────────────────────────────────────────────────────────────── interface CountryChipProps { - /** ISO 3166-1 alpha-2 country code. Null → render nothing. */ + /** ISO 3166-1 alpha-2 country code. Null → render nothing (unless nullLabel provided). */ country: string | null; /** Show full country name instead of 2-letter code. */ showLabel?: boolean; /** Badge size variant. */ size?: "sm" | "md"; + /** Label to show when country is null. If omitted, null renders nothing. */ + nullLabel?: string; className?: string; } @@ -95,11 +97,26 @@ export function CountryChip({ country, showLabel = false, size = "md", + nullLabel, className = "", }: Readonly) { const { t } = useTranslation(); - if (!country) return null; + if (!country && !nullLabel) return null; + + if (!country) { + const cfg = SIZE_CONFIG[size]; + return ( + + + {nullLabel} + + ); + } const meta = COUNTRIES.find((c) => c.code === country); const name = meta?.name ?? country;