diff --git a/src/api/catalogue.ts b/src/api/catalogue.ts index 11bd0a2..b750c64 100644 --- a/src/api/catalogue.ts +++ b/src/api/catalogue.ts @@ -101,7 +101,37 @@ export const catalogueApi = { api.list(`${WEB}/products/getimportedcatalogueproducts`, { tenantid }), }; -/** Key for the imported-refs lookup. Both halves, always. */ +/** + * Key for the imported-refs lookup. + * + * `image_id` when there is one, and the brand-qualified id only as a fallback. + * The order matters: the catalogue is rebuilt by scrape and renumbered every + * time, so a tick placed by `catalogueid` lands on whatever product now holds + * that number — or, far more often, on nothing. Eleven of the nineteen links on + * the platform were in that state on 2026-08-31, which showed rows a shop + * really held as NOT imported and invited someone to import them again. + * + * `image_id` is the key the catalogue itself deduplicates on and survives both + * a renumber and a rename. + */ export function catalogueKey(ref: CatalogueRef | CatalogueProduct): string { + const imageId = 'catalogueid' in ref ? ref.imageid : ref.image_id; + if (imageId) return `img:${imageId}`; return 'catalogueid' in ref ? `${ref.brand}:${ref.catalogueid}` : `${ref.brand}:${ref.id}`; } + +/** + * Every key one imported ref can be recognised by. + * + * A ref carries both halves during the changeover — the stable key it has just + * acquired, and the id it was imported under years of scrapes ago. Emitting + * both means a browse screen keeps matching products that have not been + * relinked yet, instead of showing a shop's own stock as missing until someone + * runs the repair. + */ +export function catalogueKeysOf(ref: CatalogueRef): string[] { + const keys: string[] = []; + if (ref.imageid) keys.push(`img:${ref.imageid}`); + if (ref.catalogueid) keys.push(`${ref.brand}:${ref.catalogueid}`); + return keys; +} diff --git a/src/api/catalogueKey.test.ts b/src/api/catalogueKey.test.ts new file mode 100644 index 0000000..ba1e2c7 --- /dev/null +++ b/src/api/catalogueKey.test.ts @@ -0,0 +1,85 @@ +/** + * Which key a catalogue product is recognised by. + * + * This decides whether the browse screen shows a product as already imported. + * Getting it wrong is not cosmetic: a shop's own stock shown as missing gets + * imported a second time, and the shop ends up with duplicates. + * + * The reason it changed: `catalogueid` is renumbered by every re-scrape. + * Pepsico's live ids run 3, 6, 9 … 27, 30 — there is no 19, 25 or 26 — so on + * 2026-08-31 eleven of the nineteen links on the platform pointed at rows that + * no longer existed. `image_id` is the key the catalogue itself deduplicates on + * and survives both a renumber and a rename. + */ +import assert from 'node:assert/strict'; +import { test } from 'node:test'; +import { catalogueKey, catalogueKeysOf } from './catalogue'; + +test('a product with a stable key is identified by it, not by its id', () => { + assert.equal( + catalogueKey({ brand: 'pepsico', id: 27, image_id: 'cheetos_chips_2d6bf74f' } as never), + 'img:cheetos_chips_2d6bf74f', + ); + assert.equal( + catalogueKey({ brand: 'pepsico', catalogueid: 27, imageid: 'cheetos_chips_2d6bf74f' } as never), + 'img:cheetos_chips_2d6bf74f', + ); +}); + +/* +The two sides have to agree. A ref from Fiesta and a product from the catalogue +describe the same thing under different field names — `imageid` and `image_id` — +and if they produced different keys the tick would never appear at all. +*/ +test('a ref and a catalogue row agree on the key', () => { + const fromFiesta = catalogueKey({ + brand: 'pepsico', + catalogueid: 27, + imageid: 'cheetos_chips_2d6bf74f', + } as never); + const fromCatalogue = catalogueKey({ + brand: 'pepsico', + id: 27, + image_id: 'cheetos_chips_2d6bf74f', + } as never); + assert.equal(fromFiesta, fromCatalogue); +}); + +// The fallback still has to work: products imported before the column existed +// carry only the id, and they are genuinely imported. +test('without a stable key the brand-qualified id is used', () => { + assert.equal(catalogueKey({ brand: 'dabur', catalogueid: 19 } as never), 'dabur:19'); + assert.equal(catalogueKey({ brand: 'dabur', id: 19 } as never), 'dabur:19'); +}); + +// Brand-qualified, never bare. Each brand is its own table with its own +// sequence, so dabur 19 and pepsico 19 both exist and a bare id would tick the +// wrong product. +test('the fallback key keeps the brand, because ids repeat across brands', () => { + assert.notEqual( + catalogueKey({ brand: 'dabur', catalogueid: 19 } as never), + catalogueKey({ brand: 'pepsico', catalogueid: 19 } as never), + ); +}); + +/* +During the changeover a ref carries both. Emitting only the stable key would +make every not-yet-relinked product read as missing the moment this shipped — +turning a silent problem into a visible one on every shop at once. +*/ +test('a ref is recognised by both keys while the changeover runs', () => { + assert.deepEqual( + catalogueKeysOf({ brand: 'pepsico', catalogueid: 27, imageid: 'cheetos_chips_2d6bf74f' }), + ['img:cheetos_chips_2d6bf74f', 'pepsico:27'], + ); +}); + +test('a ref with only an id still yields its one key', () => { + assert.deepEqual(catalogueKeysOf({ brand: 'dabur', catalogueid: 19 }), ['dabur:19']); +}); + +// A ref with neither yields nothing rather than a key like "undefined:0" that +// would collide with every other broken ref and tick unrelated products. +test('a ref with nothing to match on yields no keys at all', () => { + assert.deepEqual(catalogueKeysOf({ brand: 'dabur', catalogueid: 0 }), []); +}); diff --git a/src/api/types.ts b/src/api/types.ts index c77822a..d826fca 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -170,6 +170,14 @@ export interface CatalogueBrand { export interface CatalogueRef { brand: string; catalogueid: number; + /** + * The catalogue's own stable key, when the product carries one. + * + * Preferred over `catalogueid` for matching: the id is renumbered by every + * re-scrape. Empty for a product imported before the column existed, and + * filled in by a re-import or by `products/relinkcatalogue`. + */ + imageid?: string; } /* ──────────────────────────────────────────────────────────────────────────── diff --git a/src/features/catalogue/CatalogueBrowser.tsx b/src/features/catalogue/CatalogueBrowser.tsx index d685fb3..bc07117 100644 --- a/src/features/catalogue/CatalogueBrowser.tsx +++ b/src/features/catalogue/CatalogueBrowser.tsx @@ -11,7 +11,7 @@ import { TextInput } from '@astryxdesign/core/TextInput'; import { Token } from '@astryxdesign/core/Token'; import { VStack } from '@astryxdesign/core/VStack'; import { Funnel, PackageSearch, Search, SearchX } from 'lucide-react'; -import { catalogueKey } from '@/api/catalogue'; +import { catalogueKey, catalogueKeysOf } from '@/api/catalogue'; import { productsApi } from '@/api/products'; import type { CatalogueProduct, ImportCatalogueProductRequest } from '@/api/types'; import { queryKeys } from '@/queries/keys'; @@ -188,7 +188,10 @@ export function CatalogueBrowser({ const knownTotal = !category && !debounced ? brandTotal : undefined; const importedKeys = useMemo(() => { - const set = new Set((imported.data ?? []).map(catalogueKey)); + // Both keys per ref, not one. During the changeover a ref can carry the + // stable key it has just acquired AND the id it was imported under, and a + // product that has not been relinked yet is still genuinely imported. + const set = new Set((imported.data ?? []).flatMap(catalogueKeysOf)); for (const key of justImported) set.add(key); return set; }, [imported.data, justImported]); diff --git a/src/features/nearle-admin/import/shelve.ts b/src/features/nearle-admin/import/shelve.ts index a46e585..d1ce2fa 100644 --- a/src/features/nearle-admin/import/shelve.ts +++ b/src/features/nearle-admin/import/shelve.ts @@ -34,6 +34,13 @@ export interface ShelveResult { skipped: number; /** Kept so the caller can say which ones, and why. */ plan: StockPlan; + /** + * Brands the catalogue could not be read for, if any. + * + * Named rather than counted, because the answer is always "ask about this + * brand" and a number does not say which. + */ + failedBrands: string[]; } /** @@ -57,11 +64,23 @@ export async function shelveBatch( // One catalogue read per BRAND rather than per product. A 500-row sheet would // otherwise open 500 requests from a shop's connection. + // + // A brand that cannot be read does NOT fail the batch. It used to: a sheet of + // twenty products naming one brand the catalogue could not resolve threw on + // the first lookup and shelved nothing, so nineteen products the shop was + // entitled to sell stayed unpriced because of the twentieth. Its products now + // fall through to `unresolved`, which is already reported product by product, + // and the brand is named so the cause is not left to guesswork. const brands = [...new Set(plan.matched.map((entry) => entry.product.brand))]; const catalogueIds = new Map(); + const failedBrands: string[] = []; for (const brand of brands) { - for (const [imageId, id] of await catalogueApi.idsByImageId(brand)) { - catalogueIds.set(imageId, id); + try { + for (const [imageId, id] of await catalogueApi.idsByImageId(brand)) { + catalogueIds.set(imageId, id); + } + } catch { + failedBrands.push(brand); } } @@ -77,5 +96,10 @@ export async function shelveBatch( // the stock ledger entry together. if (requests.length > 0) await productsApi.importFromCatalogue(requests); - return { shelved: requests.length, skipped: unresolved.length + unpriced.length, plan }; + return { + shelved: requests.length, + skipped: unresolved.length + unpriced.length, + plan, + failedBrands, + }; } diff --git a/src/features/uploads/UploadsPanel.tsx b/src/features/uploads/UploadsPanel.tsx index 8e4c9e5..4e326ff 100644 --- a/src/features/uploads/UploadsPanel.tsx +++ b/src/features/uploads/UploadsPanel.tsx @@ -417,9 +417,17 @@ function ShelveAction({ skipped: result.skipped, }); setNote( - result.skipped > 0 - ? `${result.shelved} on the shelf. ${result.skipped} were left out — the sheet did not price them.` - : `${result.shelved} priced, shelved and stocked.`, + [ + `${result.shelved} priced, shelved and stocked.`, + result.skipped > 0 ? `${result.skipped} were left out.` : "", + // Named, because "left out" alone sends someone looking at their + // spreadsheet for a problem that is in the catalogue. + result.failedBrands.length > 0 + ? `The catalogue could not be read for: ${result.failedBrands.join(", ")}.` + : "", + ] + .filter(Boolean) + .join(" "), ); onDone(); } catch (cause) {