fix on shelf
This commit is contained in:
@@ -101,7 +101,37 @@ export const catalogueApi = {
|
|||||||
api.list<CatalogueRef>(`${WEB}/products/getimportedcatalogueproducts`, { tenantid }),
|
api.list<CatalogueRef>(`${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 {
|
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}`;
|
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;
|
||||||
|
}
|
||||||
|
|||||||
85
src/api/catalogueKey.test.ts
Normal file
85
src/api/catalogueKey.test.ts
Normal file
@@ -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 }), []);
|
||||||
|
});
|
||||||
@@ -170,6 +170,14 @@ export interface CatalogueBrand {
|
|||||||
export interface CatalogueRef {
|
export interface CatalogueRef {
|
||||||
brand: string;
|
brand: string;
|
||||||
catalogueid: number;
|
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;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* ────────────────────────────────────────────────────────────────────────────
|
/* ────────────────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ import { TextInput } from '@astryxdesign/core/TextInput';
|
|||||||
import { Token } from '@astryxdesign/core/Token';
|
import { Token } from '@astryxdesign/core/Token';
|
||||||
import { VStack } from '@astryxdesign/core/VStack';
|
import { VStack } from '@astryxdesign/core/VStack';
|
||||||
import { Funnel, PackageSearch, Search, SearchX } from 'lucide-react';
|
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 { productsApi } from '@/api/products';
|
||||||
import type { CatalogueProduct, ImportCatalogueProductRequest } from '@/api/types';
|
import type { CatalogueProduct, ImportCatalogueProductRequest } from '@/api/types';
|
||||||
import { queryKeys } from '@/queries/keys';
|
import { queryKeys } from '@/queries/keys';
|
||||||
@@ -188,7 +188,10 @@ export function CatalogueBrowser({
|
|||||||
const knownTotal = !category && !debounced ? brandTotal : undefined;
|
const knownTotal = !category && !debounced ? brandTotal : undefined;
|
||||||
|
|
||||||
const importedKeys = useMemo(() => {
|
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);
|
for (const key of justImported) set.add(key);
|
||||||
return set;
|
return set;
|
||||||
}, [imported.data, justImported]);
|
}, [imported.data, justImported]);
|
||||||
|
|||||||
@@ -34,6 +34,13 @@ export interface ShelveResult {
|
|||||||
skipped: number;
|
skipped: number;
|
||||||
/** Kept so the caller can say which ones, and why. */
|
/** Kept so the caller can say which ones, and why. */
|
||||||
plan: StockPlan;
|
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
|
// One catalogue read per BRAND rather than per product. A 500-row sheet would
|
||||||
// otherwise open 500 requests from a shop's connection.
|
// 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 brands = [...new Set(plan.matched.map((entry) => entry.product.brand))];
|
||||||
const catalogueIds = new Map<string, number>();
|
const catalogueIds = new Map<string, number>();
|
||||||
|
const failedBrands: string[] = [];
|
||||||
for (const brand of brands) {
|
for (const brand of brands) {
|
||||||
for (const [imageId, id] of await catalogueApi.idsByImageId(brand)) {
|
try {
|
||||||
catalogueIds.set(imageId, id);
|
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.
|
// the stock ledger entry together.
|
||||||
if (requests.length > 0) await productsApi.importFromCatalogue(requests);
|
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,
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -417,9 +417,17 @@ function ShelveAction({
|
|||||||
skipped: result.skipped,
|
skipped: result.skipped,
|
||||||
});
|
});
|
||||||
setNote(
|
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();
|
onDone();
|
||||||
} catch (cause) {
|
} catch (cause) {
|
||||||
|
|||||||
Reference in New Issue
Block a user