fix(studio): harden organize-imports against long-running queries and hangs
CI / check (push) Successful in 2m33s
CI / deploy (push) Successful in 1m26s
CI / publish-container (push) Successful in 50s

The original GET route used a correlated NOT EXISTS / FIND_IN_SET
subquery over the entire catalog_items table for every recent import
audit entry, causing server timeouts when the audit log or catalog
grew large.  The per-item host-page validation inside the create
action also issued one SELECT + one UPDATE per moved offer.

Changes:
- GET /api/admin/import/organize: replace the correlated subquery
  with a bounded candidate list and a JS-side placed-set check, then
  resolve all needed base items in a single indexed SELECT.  This
  bounds the query cost regardless of catalog or audit log size.
- organizeImportFurni action: validate mover ids in one SELECT, then
  batch every move per group into a single UPDATE with a CASE
  expression instead of one UPDATE per item.
- OrganizeImportsDialog: add a 45-second abort timeout on the fetch
  and a distinct load-error state so the UI never silently hangs.
- Add 'loadError' translation key (en + nl).
This commit is contained in:
openhands committed 2026-09-11 20:24:38 +02:00
1 parent 01a85ebcd0
commit 19dc0347df
5 files changed
+181 -66

No files matched your search

+54 -15
View File
@@ -451,9 +451,37 @@ export async function organizeImportFurni(input: {
.where(inArray(ItemsBase.id, baseIds)); .where(inArray(ItemsBase.id, baseIds));
const baseMap = new Map(bases.map((b) => [b.id, b])); const baseMap = new Map(bases.map((b) => [b.id, b]));
// Verify every offer we're about to move still sits inside the import
// tree in ONE query, so we never hijack an unrelated catalog offer.
const moverIds = [
...new Set(
group.items
.map((r) => Number(r.catalogItemId))
.filter((n) => Number.isFinite(n) && n > 0),
),
];
const validMoveIds = new Set<number>();
if (moverIds.length > 0) {
const [hostRows] = (await db.execute(sql`
SELECT id, page_id FROM catalog_items WHERE id IN (${sql.join(
moverIds.map((id) => sql`${id}`),
sql`, `,
)})
`)) as unknown as [Array<{ id: number; page_id: number }>, unknown];
for (const row of hostRows) {
if (importPageIds.has(Number(row.page_id))) {
validMoveIds.add(Number(row.id));
}
}
}
let nextOrder = 1; let nextOrder = 1;
let moved = 0; let moved = 0;
let added = 0; let added = 0;
const moveCaseOrder: string[] = [];
const moveCaseName: string[] = [];
const moveIds: number[] = [];
for (const row of group.items) { for (const row of group.items) {
const base = baseMap.get(row.itemId); const base = baseMap.get(row.itemId);
if (!base) continue; if (!base) continue;
@@ -461,23 +489,21 @@ export async function organizeImportFurni(input: {
base.publicName || base.itemName || String(row.itemId); base.publicName || base.itemName || String(row.itemId);
if (row.catalogItemId) { if (row.catalogItemId) {
// Verify the offer currently sits inside the import tree before if (!validMoveIds.has(Number(row.catalogItemId))) {
// moving it, so we never hijack an unrelated catalog offer. // Offer no longer lives in the import tree (moved elsewhere or
const [host] = (await db.execute(sql` // deleted) — skip it instead of duplicating it.
SELECT page_id FROM catalog_items WHERE id = ${row.catalogItemId}
`)) as unknown as [Array<{ page_id: number }>, unknown];
const hostPageId = Number(host[0]?.page_id ?? -1);
if (!importPageIds.has(hostPageId)) {
console.error(
`[organizeImportFurni] skipping offer #${row.catalogItemId} on non-import page #${hostPageId}`,
);
continue; continue;
} }
await db.execute(sql` moveCaseOrder.push(
UPDATE catalog_items `WHEN ${Number(row.catalogItemId)} THEN ${nextOrder}`,
SET page_id = ${pageId}, order_number = ${nextOrder}, catalog_name = ${catalogName} );
WHERE id = ${row.catalogItemId} moveCaseName.push(
`); `WHEN ${Number(row.catalogItemId)} THEN '${catalogName.replace(
/['\\]/g,
"\\$&",
)}'`,
);
moveIds.push(Number(row.catalogItemId));
moved++; moved++;
} else { } else {
await insertCatalogItemRow({ await insertCatalogItemRow({
@@ -502,6 +528,19 @@ export async function organizeImportFurni(input: {
nextOrder++; nextOrder++;
} }
if (moveIds.length > 0) {
await db.execute(sql`
UPDATE catalog_items
SET page_id = ${pageId},
order_number = CASE id ${sql.raw(moveCaseOrder.join(" "))} END,
catalog_name = CASE id ${sql.raw(moveCaseName.join(" "))} END
WHERE id IN (${sql.join(
moveIds.map((id) => sql`${id}`),
sql`, `,
)})
`);
}
await logStaffActivity({ await logStaffActivity({
staffId: staff.id, staffId: staff.id,
action: "catalog_page_create", action: "catalog_page_create",
File diff suppressed because it is too large. Load diff
@@ -75,6 +75,7 @@ export function OrganizeImportsDialog({
const router = useRouter(); const router = useRouter();
const [loading, setLoading] = useState(false); const [loading, setLoading] = useState(false);
const [loadError, setLoadError] = useState(false);
const [items, setItems] = useState<OrganizeItem[]>([]); const [items, setItems] = useState<OrganizeItem[]>([]);
const [rootPages, setRootPages] = useState<RootPage[]>([]); const [rootPages, setRootPages] = useState<RootPage[]>([]);
const [importRootPageId, setImportRootPageId] = useState<number | null>(null); const [importRootPageId, setImportRootPageId] = useState<number | null>(null);
@@ -96,10 +97,17 @@ export function OrganizeImportsDialog({
const load = useCallback(async () => { const load = useCallback(async () => {
setLoading(true); setLoading(true);
setLoadError(false);
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), 45000);
try { try {
const [data, tree] = await Promise.all([ const [data, tree] = await Promise.all([
fetch("/api/admin/import/organize").then((r) => r.json()), fetch("/api/admin/import/organize", { signal: controller.signal }).then(
fetch("/api/admin/catalog/tree?parentId=-1").then((r) => r.json()), (r) => r.json(),
),
fetch("/api/admin/catalog/tree?parentId=-1", {
signal: controller.signal,
}).then((r) => r.json()),
]); ]);
setItems(Array.isArray(data?.items) ? data.items : []); setItems(Array.isArray(data?.items) ? data.items : []);
setImportRootPageId(data?.importRootPageId ?? null); setImportRootPageId(data?.importRootPageId ?? null);
@@ -113,8 +121,10 @@ export function OrganizeImportsDialog({
); );
} catch (error) { } catch (error) {
console.error("[OrganizeImports] load failed:", error); console.error("[OrganizeImports] load failed:", error);
toast.error("Failed to load imports"); setLoadError(true);
setItems([]);
} finally { } finally {
clearTimeout(timeout);
setLoading(false); setLoading(false);
} }
}, []); }, []);
@@ -347,7 +357,7 @@ export function OrganizeImportsDialog({
</div> </div>
) : items.length === 0 ? ( ) : items.length === 0 ? (
<div className="flex flex-1 flex-col items-center justify-center gap-3 text-sm text-muted-foreground"> <div className="flex flex-1 flex-col items-center justify-center gap-3 text-sm text-muted-foreground">
{t("empty")} {loadError ? t("loadError") : t("empty")}
<Button <Button
type="button" type="button"
variant="outline" variant="outline"
+1
View File
@@ -2204,6 +2204,7 @@
"itemsBadge": "{count} items", "itemsBadge": "{count} items",
"loading": "Loading imported furniture…", "loading": "Loading imported furniture…",
"empty": "No imported furniture found. Import furniture first.", "empty": "No imported furniture found. Import furniture first.",
"loadError": "Could not load imports. Try again.",
"refresh": "Refresh", "refresh": "Refresh",
"desc": "Imported furniture is grouped into suggested pages. Review each group, then create the pages you approve.", "desc": "Imported furniture is grouped into suggested pages. Review each group, then create the pages you approve.",
"selectAll": "Select all", "selectAll": "Select all",
+1
View File
@@ -1972,6 +1972,7 @@
"itemsBadge": "{count} items", "itemsBadge": "{count} items",
"loading": "Geïmporteerde meubels laden…", "loading": "Geïmporteerde meubels laden…",
"empty": "Geen geïmporteerde meubels gevonden. Importeer eerst meubels.", "empty": "Geen geïmporteerde meubels gevonden. Importeer eerst meubels.",
"loadError": "Kon imports niet laden. Probeer het opnieuw.",
"refresh": "Vernieuwen", "refresh": "Vernieuwen",
"desc": "Geïmporteerde meubels worden gegroepeerd in voorgestelde pagina's. Bekijk elke groep en maak alleen de pagina's aan die je goedkeurt.", "desc": "Geïmporteerde meubels worden gegroepeerd in voorgestelde pagina's. Bekijk elke groep en maak alleen de pagina's aan die je goedkeurt.",
"selectAll": "Alles selecteren", "selectAll": "Alles selecteren",