From cede541813d22e5258eb13dec6b5e9fe320035c7 Mon Sep 17 00:00:00 2001 From: openhands Date: Wed, 30 Sep 2026 20:06:48 +0200 Subject: [PATCH] fix(catalog): route item-table writes to the catalog they belong to The items table is shared between both catalogs, but its four mutating actions were normal-only: moving, reordering, creating and updating a Builder Club offer wrote to catalog_items, so a BC edit either landed in the wrong catalog or hit an unknown column. Pass the catalog from the table through the actions and let the server resolve it. BC rows have no price, points or currency column, so the BC commands strip those fields instead of rejecting them. Moving and reordering now share one command that locks the category and writes the table for the same catalog, and BC writes revalidate the BC route. --- src/actions/catalog-items.ts | 75 ++++++++++++---- .../catalog-items-table.tsx | 86 +++++++++++++------ src/features/catalog/server/offer-commands.ts | 44 +++++++++- 3 files changed, 155 insertions(+), 50 deletions(-) diff --git a/src/actions/catalog-items.ts b/src/actions/catalog-items.ts index 0a370c08..30d438c2 100644 --- a/src/actions/catalog-items.ts +++ b/src/actions/catalog-items.ts @@ -11,7 +11,9 @@ import { } from "@/features/catalog/server/item-deletes"; import { createOfferCommand, + moveBcOffersCommand, moveOffersCommand, + reorderBcOffersCommand, reorderOffersCommand, updateOfferCommand, } from "@/features/catalog/server/offer-commands"; @@ -66,24 +68,40 @@ export async function insertCatalogItemRow(data: { ); } -export async function createCatalogItem(data: { - pageId: number; - itemIds: string; - catalogName: string; - costCredits: number; - costPoints: number; - pointsType: number; - amount: number; - orderNumber: number; - offerId: number; - limitedSells: number; - limitedStack: number; - extradata: string; - songId: number; - haveOffer: "0" | "1"; - clubOnly: "0" | "1"; -}) { +export async function createCatalogItem( + data: { + pageId: number; + itemIds: string; + catalogName: string; + costCredits: number; + costPoints: number; + pointsType: number; + amount: number; + orderNumber: number; + offerId: number; + limitedSells: number; + limitedStack: number; + extradata: string; + songId: number; + haveOffer: "0" | "1"; + clubOnly: "0" | "1"; + }, + // The table is shared between both catalogs, and a BC offer has no price or + // currency columns at all. The catalog decides which command runs rather than + // letting the table send normal-only fields at a BC row. + catalog: "normal" | "bc" = "normal", +) { const staff = await requirePermission(PERMS.CATALOG_EDIT); + if (catalog === "bc") { + const { createBcItem } = await import("@/actions/catalog-bc"); + return createBcItem({ + pageId: data.pageId, + itemIds: data.itemIds, + catalogName: data.catalogName, + orderNumber: data.orderNumber, + extradata: data.extradata, + }); + } return await withCatalogExport(async () => { let catalogName = data.catalogName.trim(); if (!catalogName) { @@ -297,32 +315,42 @@ export async function restoreDeletedCatalogItems({ export async function moveCatalogItems({ ids, targetPageId, + catalog = "normal", }: { ids: number[]; targetPageId: number; + catalog?: "normal" | "bc"; }) { await requirePermission(PERMS.CATALOG_EDIT); return await withCatalogExport(async () => { if (ids.length === 0) { return { ok: true as const, data: {} }; } - await moveOffersCommand(ids, targetPageId); + // Moving BC offers has to lock a BC category and write the BC table; the + // normal command would move a row in the wrong catalog. + if (catalog === "bc") await moveBcOffersCommand(ids, targetPageId); + else await moveOffersCommand(ids, targetPageId); await sendCatalogUpdate(); revalidatePath("/admin/catalog"); + if (catalog === "bc") revalidatePath("/admin/catalog/builder-club"); return { ok: true as const, data: {} }; }); } export async function reorderCatalogItems({ orders, + catalog = "normal", }: { orders: Array<{ id: number; orderNumber: number }>; + catalog?: "normal" | "bc"; }) { await requirePermission(PERMS.CATALOG_EDIT); return await withCatalogExport(async () => { - await reorderOffersCommand(orders); + if (catalog === "bc") await reorderBcOffersCommand(orders); + else await reorderOffersCommand(orders); await sendCatalogUpdate(); revalidatePath("/admin/catalog"); + if (catalog === "bc") revalidatePath("/admin/catalog/builder-club"); return { ok: true as const, data: {} }; }); } @@ -331,12 +359,21 @@ export async function updateCatalogItem({ id, catalogFields, baseItem, + catalog = "normal", }: { id: number; catalogFields: Record; baseItem?: { id: number; fields: Record }; + catalog?: "normal" | "bc"; }) { const staff = await requirePermission(PERMS.CATALOG_EDIT); + // BC rows have no price, points or currency column. The table is shared, so + // the catalog is resolved here rather than trusting the caller to strip + // fields the BC command would reject anyway. + if (catalog === "bc") { + const { updateBcItem } = await import("@/actions/catalog-bc"); + return updateBcItem({ id, ...catalogFields }); + } return await withCatalogExport(async () => { try { await updateOfferCommand({ id, catalogFields, baseItem }, staff.id); diff --git a/src/components/admin/catalog/catalog-items-table/catalog-items-table.tsx b/src/components/admin/catalog/catalog-items-table/catalog-items-table.tsx index 0b493b6c..d75acfc2 100644 --- a/src/components/admin/catalog/catalog-items-table/catalog-items-table.tsx +++ b/src/components/admin/catalog/catalog-items-table/catalog-items-table.tsx @@ -471,6 +471,7 @@ export function CatalogItemsTable({ () => updateCatalogItem({ id: item.id, + catalog, catalogFields: { catalogName: item.catalogName, itemIds: item.itemIds, @@ -514,6 +515,7 @@ export function CatalogItemsTable({ const result = await updateCatalogItem({ id: item.id, + catalog, catalogFields: { catalogName: item.catalogName, itemIds: item.itemIds, @@ -600,6 +602,7 @@ export function CatalogItemsTable({ moveCatalogItems({ ids: [...selected], targetPageId: moveTargetPageId, + catalog, }), { successMessage: `Moved ${selected.size} item(s) to page #${moveTargetPageId}.`, @@ -666,7 +669,7 @@ export function CatalogItemsTable({ async function handleSaveOrder() { const orders = itemOrder.map((id, idx) => ({ id, orderNumber: idx + 1 })); - run(() => reorderCatalogItems({ orders }), { + run(() => reorderCatalogItems({ orders, catalog }), { successMessage: "Item order saved.", errorMessage: "Failed to save order.", onSuccess: () => onRefresh(), @@ -697,12 +700,15 @@ export function CatalogItemsTable({ async function handleAddItem() { run( () => - createCatalogItem({ - ...newItem, - pageId, - haveOffer: newItem.haveOffer as "0" | "1", - clubOnly: newItem.clubOnly as "0" | "1", - }), + createCatalogItem( + { + ...newItem, + pageId, + haveOffer: newItem.haveOffer as "0" | "1", + clubOnly: newItem.clubOnly as "0" | "1", + }, + catalog, + ), { successMessage: "Item added.", errorMessage: "Failed to add item", @@ -723,7 +729,12 @@ export function CatalogItemsTable({ }); if (!ok) return; run( - () => deleteCatalogItems({ ids: [id], requestKey: crypto.randomUUID() }), + () => + deleteCatalogItems({ + ids: [id], + requestKey: crypto.randomUUID(), + catalog, + }), { successMessage: "Item deleted.", errorMessage: "Failed to delete item.", @@ -760,7 +771,12 @@ export function CatalogItemsTable({ }); } run( - () => moveCatalogItems({ ids: [moveOneId], targetPageId: moveOneTarget }), + () => + moveCatalogItems({ + ids: [moveOneId], + targetPageId: moveOneTarget, + catalog, + }), { successMessage: `Item #${moveOneId} moved to page #${moveOneTarget}.`, errorMessage: "Failed to move item.", @@ -776,23 +792,26 @@ export function CatalogItemsTable({ async function handleDuplicateItem(item: CatalogItemData) { run( () => - createCatalogItem({ - pageId, - itemIds: item.itemIds, - catalogName: item.catalogName, - costCredits: item.costCredits, - costPoints: item.costPoints, - pointsType: item.pointsType, - amount: item.amount, - limitedSells: 0, - limitedStack: item.limitedStack, - orderNumber: item.orderNumber + 1, - offerId: item.offerId, - songId: item.songId, - haveOffer: item.haveOffer as "0" | "1", - clubOnly: item.clubOnly as "0" | "1", - extradata: item.extradata, - }), + createCatalogItem( + { + pageId, + itemIds: item.itemIds, + catalogName: item.catalogName, + costCredits: item.costCredits, + costPoints: item.costPoints, + pointsType: item.pointsType, + amount: item.amount, + limitedSells: 0, + limitedStack: item.limitedStack, + orderNumber: item.orderNumber + 1, + offerId: item.offerId, + songId: item.songId, + haveOffer: item.haveOffer as "0" | "1", + clubOnly: item.clubOnly as "0" | "1", + extradata: item.extradata, + }, + catalog, + ), { successMessage: `Item #${item.id} duplicated.`, errorMessage: "Failed to duplicate item.", @@ -807,6 +826,7 @@ export function CatalogItemsTable({ () => updateCatalogItem({ id: editingItem.id, + catalog, catalogFields: { catalogName: editingItem.catalogName, itemIds: editingItem.itemIds, @@ -1338,7 +1358,12 @@ export function CatalogItemsTable({ ) } className="h-8 text-xs w-16" - title="Credits" + disabled={catalog === "bc"} + title={ + catalog === "bc" + ? "A Builder Club offer has no price of its own" + : "Credits" + } /> c @@ -1356,7 +1381,12 @@ export function CatalogItemsTable({ ) } className="h-8 text-xs w-16" - title="Points" + disabled={catalog === "bc"} + title={ + catalog === "bc" + ? "A Builder Club offer has no price of its own" + : "Points" + } /> {pts.label.charAt(0)} diff --git a/src/features/catalog/server/offer-commands.ts b/src/features/catalog/server/offer-commands.ts index 48883b9b..1077732a 100644 --- a/src/features/catalog/server/offer-commands.ts +++ b/src/features/catalog/server/offer-commands.ts @@ -217,18 +217,56 @@ export async function reorderOffersCommand( }); } export async function moveOffersCommand(ids: number[], targetPageId: number) { + return moveOffersToPage(ids, targetPageId, "normal"); +} + +/** BC offers move between BC categories; the lock and the write have to agree. */ +export async function moveBcOffersCommand(ids: number[], targetPageId: number) { + return moveOffersToPage(ids, targetPageId, "bc"); +} + +async function moveOffersToPage( + ids: number[], + targetPageId: number, + kind: OfferKind, +) { distinctOfferIds(ids); offerIdSchema.parse(targetPageId); if (!ids.length) return; + const table = offerTable(kind); return db.transaction(async (tx) => { - await lockPage(tx, targetPageId); - await lockedOffers(tx, ids); + await lockPage(tx, targetPageId, kind); + await lockedOffers(tx, ids, kind); await tx.execute( - sql`UPDATE ${CatalogItems} SET page_id=${String(targetPageId)} WHERE id IN (${sql.join(ids, sql`, `)})`, + sql`UPDATE ${table} SET page_id=${String(targetPageId)} WHERE id IN (${sql.join(ids, sql`, `)})`, ); }); } +/** Same as the normal reorder, against the BC offers table. */ +export async function reorderBcOffersCommand( + orders: Array<{ id: number; orderNumber: number }>, +) { + const ids = distinctOfferIds(orders.map((row) => row.id)); + for (const row of orders) offerInteger.parse(row.orderNumber); + if (!ids.length) return; + return db.transaction(async (tx) => { + const offers = await lockedOffers(tx, ids, "bc"); + const pageId = offers[0]?.pageId; + if (pageId !== undefined) { + const nums = orders.map((r) => r.orderNumber); + if (new Set(nums).size !== nums.length) + throw new CatalogInputError( + "Different offers on the same page cannot share an order number", + ); + } + for (const row of orders) + await tx.execute( + sql`UPDATE ${CatalogItemsBc} SET order_number=${row.orderNumber} WHERE id=${row.id}`, + ); + }); +} + /** Call the allocator before entering this command; only the insert uses its transaction. */ export async function createOfferCommand( kind: OfferKind,