feat(catalog): make the live catalog self-correcting and honest about failure
Gitea Actions Runner Test / test-job (push) Successful in 0s
CI / check (push) Successful in 29s
CI / tests-unit (push) Successful in 1m34s
CI / tests-integration (push) Failing after 1m34s
CI / tests-ui (push) Successful in 2m19s
CI / preflight (push) Skipped
CI / deploy (push) Skipped
Gitea Actions Runner Test / test-job (push) Successful in 0s
CI / check (push) Successful in 29s
CI / tests-unit (push) Successful in 1m34s
CI / tests-integration (push) Failing after 1m34s
CI / tests-ui (push) Successful in 2m19s
CI / preflight (push) Skipped
CI / deploy (push) Skipped
The previous commit made imports update the Studio without a reload, but the guarantee only held inside the tab that started the import and only as long as every read succeeded. Four holes were left, and this closes them. A session that mounted the tree before an import kept the pre-import tree for the rest of its life, because ensureCatalogTreeLoaded() was a once-per-session no-op. It now asks the server whether what it holds is still current. The answer is a revision: sendCatalogUpdate() already runs after every catalog write, so it bumps one, and clients read it on mount, on focus, on a 20s poll and from other tabs over a BroadcastChannel. An import that finishes in another tab, another browser or the job worker now lands here too. A failed read used to be swallowed, which is the worst outcome available: the rail kept showing pre-import counts as if they were current and nothing said so. The snapshot now carries the error, the rail shows it with a retry, and the previous tree stays on screen because stale beats empty. Every settled import pulled the entire flat tree, which is the one payload that grows with the size of the catalog. The revision doubles as the ETag on mode=full, so an unchanged catalog answers 304 and the poll costs a file read. An import could also report success for an offer the hotel will never sell: a hidden or disabled page, an item_ids that misses the furni id, a zero amount. importSingleFurni reads its own row back and reports each of those as a warning, where the import report already is, instead of leaving it to surface as "the import did not work" in the client. Finally, the catalog items table no longer falls back to router.refresh() — onRefresh is now required, so every mutation ends in a refresh of the caller's own data instead of a route re-render that threw away editor state and scroll position. useServerAction keeps its default, because 47 callers across the app depend on it. The 750-line CatalogTree in catalog-tree.tsx was dead code that kept its own stale tree and three more router.refresh() calls; only CatalogIcon and LAYOUT_COLORS are still imported, so the rest is gone. Tests: the store now covers revisions, 304s, probe failures and error recovery; a jsdom test mounts a consumer and asserts the tree updates in place with no navigation; the old organize-imports e2e asserted nothing about the endpoints the code actually calls, and is replaced by one that asserts a cross-tab write lands in the mounted categories without a reload. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This commit is contained in:
1 parent
28ce0f911c
commit
9550b3d66f
18 files changed
+1016
-973
No files matched your search
@@ -4,6 +4,7 @@ import path from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
import { and, eq, type SQL, sql } from "drizzle-orm";
|
||||
import { CatalogPages, db, execResult, ItemsBase, queryRows } from "@/lib/db";
|
||||
import { offerPurchasabilityProblems } from "@/lib/furni/offer-purchasability";
|
||||
import { officialHabboEnrichmentWarning } from "@/lib/habbo-gamedata-hotel";
|
||||
import { logger } from "@/lib/logger";
|
||||
import { logServerError } from "@/lib/server-log";
|
||||
@@ -525,6 +526,55 @@ export async function allocateCatalogItemId<T>(
|
||||
|
||||
/** Test seam: drop the cached id counter between tests. */
|
||||
|
||||
/**
|
||||
* Read a written offer back and report everything that would keep the hotel from
|
||||
* selling it. Best-effort by design: a failed probe must never fail an import
|
||||
* that already wrote its files, so this returns no problems on error.
|
||||
*/
|
||||
async function catalogOfferProblems(
|
||||
catalogItemId: number,
|
||||
furniId: number,
|
||||
): Promise<string[]> {
|
||||
try {
|
||||
const rows = await queryRows<{
|
||||
item_ids: string;
|
||||
amount: number;
|
||||
cost_credits: number;
|
||||
cost_points: number;
|
||||
offer_id: number;
|
||||
haveOffer: string;
|
||||
page_enabled: string;
|
||||
page_visible: string;
|
||||
}>(sql`
|
||||
SELECT ci.item_ids, ci.amount, ci.cost_credits, ci.cost_points, ci.offer_id, ci.haveOffer,
|
||||
cp.enabled AS page_enabled, cp.visible AS page_visible
|
||||
FROM catalog_items ci
|
||||
LEFT JOIN catalog_pages cp ON cp.id = ci.page_id
|
||||
WHERE ci.id = ${catalogItemId}
|
||||
LIMIT 1
|
||||
`);
|
||||
const row = rows[0];
|
||||
if (!row) return ["catalog entry disappeared right after writing it"];
|
||||
return offerPurchasabilityProblems({
|
||||
pageEnabled: String(row.page_enabled ?? "0"),
|
||||
pageVisible: String(row.page_visible ?? "0"),
|
||||
itemIds: String(row.item_ids ?? ""),
|
||||
amount: Number(row.amount),
|
||||
costCredits: Number(row.cost_credits),
|
||||
costPoints: Number(row.cost_points),
|
||||
offerId: Number(row.offer_id),
|
||||
haveOffer: String(row.haveOffer ?? "0"),
|
||||
furniId,
|
||||
});
|
||||
} catch (error) {
|
||||
logger.warn("[import-furni] Cannot verify the imported offer", {
|
||||
error: (error as Error).message,
|
||||
catalogItemId,
|
||||
});
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
export async function importSingleFurni(params: {
|
||||
id: number;
|
||||
classname: string;
|
||||
@@ -1149,6 +1199,25 @@ export async function importSingleFurni(params: {
|
||||
warnings.push("Catalog entry creation failed");
|
||||
}
|
||||
|
||||
if (catalogItemId !== null) {
|
||||
// The write above is not proof the hotel will sell anything: a hidden or
|
||||
// disabled page, an `item_ids` that misses the furni id or a zero amount
|
||||
// all import "successfully" and show the player nothing. Read the row back
|
||||
// and say so here, where the import report already shows up, instead of
|
||||
// letting it surface as "the import did not work" in the client.
|
||||
const problems = await catalogOfferProblems(catalogItemId, itemId);
|
||||
for (const problem of problems) {
|
||||
warnings.push(`Offer not purchasable: ${problem}`);
|
||||
}
|
||||
if (problems.length > 0) {
|
||||
logger.warn("[import-furni] Imported offer is not purchasable", {
|
||||
classname,
|
||||
catalogItemId,
|
||||
problems,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
ok: true,
|
||||
itemId,
|
||||
|
||||
Reference in new issue
Block a user