diff --git a/src/features/catalogue/CatalogueBrowser.tsx b/src/features/catalogue/CatalogueBrowser.tsx index b67f810..9bc782f 100644 --- a/src/features/catalogue/CatalogueBrowser.tsx +++ b/src/features/catalogue/CatalogueBrowser.tsx @@ -28,7 +28,6 @@ import { import { CatalogueCard } from './CatalogueCard'; import { CatalogueSidebar } from './CatalogueSidebar'; import { CatalogueDetailDrawer } from './CatalogueDetailDrawer'; -import { ImportConfirmDrawer } from './ImportConfirmDrawer'; const PAGE_SIZE = 24; @@ -94,15 +93,6 @@ export function CatalogueBrowser({ const [debounced, setDebounced] = useState(''); const [busy, setBusy] = useState(null); const [justImported, setJustImported] = useState>(new Set()); - /* - The import waiting on a confirmation. - - Importing used to be one click. It still writes the same row — the step - added in front of it asks whether this shelf shows the product's health - score, which is a question the shopkeeper is the right person to answer and - nobody was asking. See ImportConfirmDrawer. - */ - const [pending, setPending] = useState(null); const [open, setOpen] = useState(null); const [category, setCategory] = useState(''); const [page, setPage] = useState(1); @@ -343,8 +333,17 @@ export function CatalogueBrowser({ async function importDirect(product: CatalogueProduct) { if (!tenantid || !locationid) return; - // Asks first. The write itself is unchanged — see confirmImport. - setPending([product]); + + /* + Opens the product rather than importing on the spot. + + The drawer already shows this product's health score and how confident + the match is, and the import now carries a decision about exactly that — + so the question is asked where the answer is visible. A separate + confirmation would be a second panel repeating what this one already + renders. + */ + setOpen(product); } /* @@ -354,34 +353,30 @@ export function CatalogueBrowser({ different rows — the single-product button taking a different route from the bulk action is how a selection quietly imports under a different category. */ - async function confirmImport(showHealthScore: boolean | undefined) { - const products = pending; - if (!products || products.length === 0) return; + /* + The import, once the shopkeeper has answered. - if (products.length === 1 && products[0]) { - const product = products[0]; - const key = catalogueKey(product); - setBusy(key); - try { - await importOne.mutateAsync( - importRowFor( - product, - aisleIdForCategory(categoryNameFor(product), await aisleIds()), - showHealthScore, - ), - ); - setJustImported((set) => new Set(set).add(key)); - } finally { - setBusy(null); - setPending(null); - } - return; - } + `showHealthScore` is undefined for a product with no score — the import then + says nothing about it rather than sending a choice nobody was offered, and + the backend reads an absent field as yes. + */ + async function confirmImport(product: CatalogueProduct, showHealthScore: boolean | undefined) { + if (!tenantid || !locationid) return; + const key = catalogueKey(product); + setBusy(key); try { - await importMany.mutateAsync({ products, showHealthScore }); + await importOne.mutateAsync( + importRowFor( + product, + aisleIdForCategory(categoryNameFor(product), await aisleIds()), + showHealthScore, + ), + ); + setJustImported((set) => new Set(set).add(key)); + setOpen(null); } finally { - setPending(null); + setBusy(null); } } @@ -566,8 +561,16 @@ export function CatalogueBrowser({ size="sm" isLoading={importMany.isPending} isDisabled={importMany.isPending} + /* No per-product question here: a toggle asked once + for forty products is a question nobody answers + honestly. A batch imports showing the score, which + is the default, and any of them can be changed + afterwards from the product. */ onClick={() => - setPending(rows.filter((product) => selection.has(product.id))) + importMany.mutate({ + products: rows.filter((product) => selection.has(product.id)), + showHealthScore: undefined, + }) } /> @@ -619,19 +622,6 @@ export function CatalogueBrowser({ - {/* Asked before anything is written. One drawer for a single card and for - a batch, so the question and the answer cannot diverge between the - two paths. */} - {pending && pending[0] ? ( - setPending(null)} - onConfirm={(showHealthScore) => void confirmImport(showHealthScore)} - /> - ) : null} - {open ? ( { + onImport: (showHealthScore: boolean | undefined) => { const product = open; - if (onImport) setOpen(null); - run(product); + // A caller that owns the import — the shelving flow — takes + // over here and the drawer closes. Otherwise this writes the + // row itself, carrying the shopkeeper's answer. + if (onImport) { + setOpen(null); + onImport(product); + return; + } + void confirmImport(product, showHealthScore); }, } : {})} diff --git a/src/features/catalogue/CatalogueDetailDrawer.tsx b/src/features/catalogue/CatalogueDetailDrawer.tsx index 59d214b..4facb19 100644 --- a/src/features/catalogue/CatalogueDetailDrawer.tsx +++ b/src/features/catalogue/CatalogueDetailDrawer.tsx @@ -44,7 +44,14 @@ export interface CatalogueDetailDrawerProps { isBusy?: boolean; actionLabel?: string; /** Absent when the caller has nowhere to import to yet. */ - onImport?: () => void; + /** + * Import this product. + * + * `showHealthScore` is the shopkeeper's answer to the toggle below the score, + * and `undefined` when this product has none — the import then says nothing + * about it rather than sending a choice nobody was offered. + */ + onImport?: (showHealthScore: boolean | undefined) => void; /** Shown in place of the action when importing is unavailable. */ blockedReason?: string; onClose: () => void; @@ -76,6 +83,19 @@ export function CatalogueDetailDrawer({ const aisle = aisleForCategory(suggested.category); const images = product.images ?? []; + /* + Whether this shop will show the product's health score, and whether there is + one to show. + + `hasScore` is reported up by the panel rather than looked up again here: the + panel already makes that call, already caches it, and already knows the + three ways a product can have no score — not food, not known to the service, + known but unscored. Asking a second time would be a second request and a + second opinion. + */ + const [showScore, setShowScore] = useState(true); + const [hasScore, setHasScore] = useState(false); + const [heroAt, setHeroAt] = useState(0); const [isZoomed, setIsZoomed] = useState(false); const [failed, setFailed] = useState(false); @@ -115,7 +135,7 @@ export function CatalogueDetailDrawer({ variant="primary" icon={} isDisabled={Boolean(isBusy)} - onClick={onImport} + onClick={() => onImport(hasScore ? showScore : undefined)} /> ), } @@ -197,9 +217,19 @@ export function CatalogueDetailDrawer({ `brand` and `image_id` come straight off the catalogue row, so no lookup is needed to find the key. */} + {/* The decision and the thing being decided about, in one place. + + "Show health score?" against a product name is unanswerable; the same + question directly beneath "22/100 — Less healthy, matched at 47% + confidence" answers itself. That is the whole reason the toggle lives + here rather than in a confirmation of its own. */} {product.highlights?.length || product.nutrients?.length ? ( diff --git a/src/features/catalogue/ImportConfirmDrawer.tsx b/src/features/catalogue/ImportConfirmDrawer.tsx deleted file mode 100644 index ac9e05b..0000000 --- a/src/features/catalogue/ImportConfirmDrawer.tsx +++ /dev/null @@ -1,190 +0,0 @@ -import { useState } from 'react'; -import { useQuery } from '@tanstack/react-query'; -import { Check } from 'lucide-react'; -import { nutritionApi } from '@/api/nutrition'; -import type { CatalogueProduct } from '@/api/types'; -import { Drawer } from '@/features/store-admin/Drawer'; -import { DrawerButton, Note, Section } from '@/features/store-admin/drawerKit'; -import { BAND_COLOR, BAND_LABEL, isEdible, present } from '@/features/store-admin/healthScore'; - -/** - * Confirming an import, and deciding whether this shelf shows a health score. - * - * ── Why there is a step here at all ───────────────────────────────────────── - * - * Importing used to be one click. It still writes the same row; the only thing - * added is a question the shopkeeper is the right person to answer. - * - * The score comes from a third party that matches a reference product by name, - * and it publishes how sure it is — 47% and 61% are ordinary. One live record - * reports under 1mg of sodium per 100g for salted crisps, and the rating's own - * "low in sodium" praise is derived from that figure. A shopkeeper holding the - * packet can see that in a second; the matcher cannot. - * - * ── Why the score is shown and not just named ─────────────────────────────── - * - * The toggle is useless without the thing it is deciding about. "Show health - * score?" against a product name is a question nobody can answer; the same - * question beside "22/100 — Less healthy, matched at 47% confidence" answers - * itself. - * - * ── Why the toggle disappears ─────────────────────────────────────────────── - * - * No score, no question. Non-food never gets one — the service has rated - * insecticide 80/100, so an allowlist gates it — and most products are simply - * unscored. Showing a dead toggle on those invites somebody to set it and - * wonder later why nothing changed. - */ -export function ImportConfirmDrawer({ - product, - count, - isBusy, - onCancel, - onConfirm, -}: { - /** The product being imported, or the first of a batch. */ - product: CatalogueProduct; - /** How many products this confirms. 1 for a single card. */ - count: number; - isBusy: boolean; - onCancel: () => void; - /** `showHealthScore` is undefined when the product has no score to show. */ - onConfirm: (showHealthScore: boolean | undefined) => void; -}) { - const [showScore, setShowScore] = useState(true); - - const brand = (product.brand ?? '').trim(); - const imageId = (product.image_id ?? '').trim(); - - /* - The same lookup the product drawer makes, and the same guard in front of it. - - `enabled` keeps a non-food product from asking at all — there is nothing to - ask about — and a batch asks only about the one product on screen, because - forty lookups to populate one toggle is forty requests to a third party for - a question that has one answer. - */ - const isFood = isEdible(product.category); - const query = useQuery({ - queryKey: ['nutrition', brand, imageId], - queryFn: () => nutritionApi.forProduct(brand, imageId), - enabled: Boolean(brand && imageId && isFood), - staleTime: 60 * 60_000, - refetchOnWindowFocus: false, - retry: false, - }); - - const shown = present(query.data ?? null); - const hasScore = isFood && !shown.isEmpty && !shown.isPending && shown.score !== null; - const isChecking = isFood && Boolean(brand && imageId) && query.isLoading; - - const label = count === 1 ? product.product_name : `${count} products`; - - return ( - - - onConfirm(hasScore ? showScore : undefined)} - /> - - } - > - {isChecking ? ( -
- Checking whether this product has one… -
- ) : hasScore ? ( -
-
- - {shown.display} - -
- - {shown.band ? BAND_LABEL[shown.band] : ''} - - out of 100 -
-
- - {/* The confidence, where it is weak. This is the single most useful - thing on this screen: it is the reason the question is being asked - at all, and most matches are below the line. */} - {shown.caveat ? {shown.caveat} : null} - - -
- ) : ( -
- - {isFood - ? 'No health score for this product yet, so there is nothing to show or hide.' - : 'Health scores are only for food and drink, so this product will not have one.'} - -
- )} - - {count > 1 ? ( - }> - {`This choice applies to all ${count} products in this batch.`} - - ) : null} - - ); -} diff --git a/src/features/store-admin/HealthScorePanel.tsx b/src/features/store-admin/HealthScorePanel.tsx index 9be6cea..74a3dce 100644 --- a/src/features/store-admin/HealthScorePanel.tsx +++ b/src/features/store-admin/HealthScorePanel.tsx @@ -1,3 +1,4 @@ +import { useEffect } from 'react'; import { useQuery } from '@tanstack/react-query'; import { AlertTriangle, Check, ExternalLink, Leaf } from 'lucide-react'; import { nutritionApi } from '@/api/nutrition'; @@ -30,15 +31,35 @@ export function HealthScorePanel({ product, category, onToggleShown, + showToggle, + onScoreResolved, }: { product: Product; /** - * Turn the score on or off for this shop. Omitted where the viewer does not - * get to decide — the Nearle staff console passes nothing, because the - * decision being made there is whether the data is fit to publish at all, - * not whether one shopkeeper wants it on their shelf. + * Turn the score on or off for a product ALREADY on the shelf. Omitted where + * the viewer does not get to decide — the Nearle staff console passes + * nothing, because the decision being made there is whether the data is fit + * to publish at all, not whether one shopkeeper wants it on their shelf. */ onToggleShown?: (show: boolean) => void; + /** + * The same control, for a product being imported — where the answer is not + * saved on each click but carried into the import. + * + * Separate from `onToggleShown` because the two are different moments with + * different state: one writes immediately, the other is a form field until + * the import button is pressed. + */ + showToggle?: { isShown: boolean; onChange: (show: boolean) => void }; + /** + * Reports whether this product has a score at all. + * + * The caller needs to know — to decide whether to offer the toggle, and what + * to send on import — and this panel is already the thing that finds out. A + * second lookup in the drawer would be a second request and a second opinion + * about the three different ways a product can have no score. + */ + onScoreResolved?: (hasScore: boolean) => void; /** * The product's category, used to decide whether a nutrition score means * anything for it at all. Passed in because the two callers hold it in @@ -127,6 +148,20 @@ export function HealthScorePanel({ const shown = present(query.data ?? null); + /* + Tell the caller whether there is a score, once the lookup has settled. + + In an effect rather than during render: this sets state in a parent, and + doing that while rendering is the React warning about updating one + component from inside another. Guarded on `isLoading` so a drawer does not + briefly see "no score" for a product that has one and hide its toggle. + */ + const hasScore = !shown.isEmpty && !shown.isPending && shown.score !== null; + useEffect(() => { + if (query.isLoading) return; + onScoreResolved?.(hasScore); + }, [query.isLoading, hasScore, onScoreResolved]); + /* This shop has turned the score off. @@ -269,7 +304,11 @@ export function HealthScorePanel({ {/* Beneath the score, deliberately. The decision is about the thing above it, and a shopkeeper weighing "is this rating fair to my product" should be reading the rating and the confidence while they decide. */} - {onToggleShown ? : null} + {showToggle ? ( + + ) : onToggleShown ? ( + + ) : null} ); }