From 9cc3b7c08cb0c0c9ced138a479e21b519a5d7438 Mon Sep 17 00:00:00 2001 From: Julian Appel Date: Thu, 23 Jul 2026 17:22:56 +0200 Subject: [PATCH] Make bulk row moves atomic --- docs/circuit-list-editor-interactions.md | 2 +- docs/circuit-list-editor-known-limitations.md | 1 - .../circuit-device-row.repository.ts | 146 ++++++++++++++ src/domain/services/circuit-write.service.ts | 93 +++------ tests/circuit-write.rules.test.ts | 184 ++++++++++++++---- 5 files changed, 310 insertions(+), 116 deletions(-) diff --git a/docs/circuit-list-editor-interactions.md b/docs/circuit-list-editor-interactions.md index a9c8e6b..7d38d8c 100644 --- a/docs/circuit-list-editor-interactions.md +++ b/docs/circuit-list-editor-interactions.md @@ -80,6 +80,7 @@ Intent is separated by drag source type: - cross-section reorder is rejected - bulk device row move: - supported via multi-selection + drag + - row assignments, optional target creation and reserve-state updates are committed in one SQLite transaction - multi-circuit move: - supported for same-section selected circuit blocks @@ -125,4 +126,3 @@ Current limitations: - session-local only - no persisted history across browser reload -- some multi-step backend flows are not fully transaction-hardened end-to-end diff --git a/docs/circuit-list-editor-known-limitations.md b/docs/circuit-list-editor-known-limitations.md index 70f289b..0d44341 100644 --- a/docs/circuit-list-editor-known-limitations.md +++ b/docs/circuit-list-editor-known-limitations.md @@ -5,7 +5,6 @@ - No global cross-project device library workflow is implemented yet. - Final electrical sizing logic is not implemented yet. - No full norm-compliant voltage-drop and protection-dimensioning calculation flow yet. -- Bulk device-row move flow is command-based but not fully transaction-hardened end-to-end across all affected circuits. - Sorting is view-only until users explicitly apply sorted order. - Cross-section circuit drag-reorder is intentionally blocked. - Legacy consumer and circuit-first paths coexist; migration is transitional and still requires operational discipline. diff --git a/src/db/repositories/circuit-device-row.repository.ts b/src/db/repositories/circuit-device-row.repository.ts index b6a7974..b72174e 100644 --- a/src/db/repositories/circuit-device-row.repository.ts +++ b/src/db/repositories/circuit-device-row.repository.ts @@ -26,6 +26,18 @@ export interface CircuitDeviceRowUpdateInput { overriddenFields?: string; } +export interface CircuitDeviceRowsBulkMoveInput { + rows: Array<{ id: string; expectedCircuitId: string }>; + targetCircuitId?: string; + createTargetCircuit?: { + circuitListId: string; + sectionId: string; + equipmentIdentifier: string; + displayName: string; + sortOrder: number; + }; +} + function toUpdateValues(input: CircuitDeviceRowUpdateInput) { return { linkedProjectDeviceId: input.linkedProjectDeviceId ?? null, @@ -259,4 +271,138 @@ export class CircuitDeviceRowRepository { }) .where(eq(circuitDeviceRows.id, rowId)); } + + moveRowsTransactional(input: CircuitDeviceRowsBulkMoveInput) { + if (input.rows.length === 0) { + throw new Error("Keine Gerätezeilen zum Verschieben angegeben."); + } + if (Boolean(input.targetCircuitId) === Boolean(input.createTargetCircuit)) { + throw new Error("Für die Mehrfachverschiebung muss genau ein Ziel angegeben werden."); + } + + const rowIds = input.rows.map((row) => row.id); + if (new Set(rowIds).size !== rowIds.length) { + throw new Error("Gerätezeilen dürfen nicht mehrfach ausgewählt sein."); + } + + const createdCircuitId = input.createTargetCircuit ? crypto.randomUUID() : undefined; + const targetCircuitId = input.targetCircuitId ?? createdCircuitId!; + + // better-sqlite3 transactions must remain synchronous. Every row assignment, + // reserve-state update and optional target creation is committed or rolled back + // as one operation. + db.transaction((tx) => { + const persistedRows = tx + .select({ id: circuitDeviceRows.id, circuitId: circuitDeviceRows.circuitId }) + .from(circuitDeviceRows) + .where(inArray(circuitDeviceRows.id, rowIds)) + .all(); + if (persistedRows.length !== input.rows.length) { + throw new Error("Eine oder mehrere Gerätezeilen sind ungültig."); + } + + const persistedRowsById = new Map(persistedRows.map((row) => [row.id, row])); + for (const expectedRow of input.rows) { + if (persistedRowsById.get(expectedRow.id)?.circuitId !== expectedRow.expectedCircuitId) { + throw new Error("Eine Gerätezeile wurde vor Abschluss der Verschiebung verändert."); + } + } + + let targetCircuit: { id: string; circuitListId: string }; + if (input.createTargetCircuit) { + tx + .insert(circuits) + .values({ + id: targetCircuitId, + circuitListId: input.createTargetCircuit.circuitListId, + sectionId: input.createTargetCircuit.sectionId, + equipmentIdentifier: input.createTargetCircuit.equipmentIdentifier, + displayName: input.createTargetCircuit.displayName, + sortOrder: input.createTargetCircuit.sortOrder, + isReserve: 0, + }) + .run(); + targetCircuit = { + id: targetCircuitId, + circuitListId: input.createTargetCircuit.circuitListId, + }; + } else { + const [persistedTargetCircuit] = tx + .select({ id: circuits.id, circuitListId: circuits.circuitListId }) + .from(circuits) + .where(eq(circuits.id, targetCircuitId)) + .limit(1) + .all(); + if (!persistedTargetCircuit) { + throw new Error("Der Zielstromkreis ist ungültig."); + } + targetCircuit = persistedTargetCircuit; + } + + const sourceCircuitIds = [...new Set(input.rows.map((row) => row.expectedCircuitId))]; + const sourceCircuits = tx + .select({ id: circuits.id, circuitListId: circuits.circuitListId }) + .from(circuits) + .where(inArray(circuits.id, sourceCircuitIds)) + .all(); + if (sourceCircuits.length !== sourceCircuitIds.length) { + throw new Error("Einer oder mehrere Quellstromkreise sind ungültig."); + } + if (sourceCircuits.some((circuit) => circuit.circuitListId !== targetCircuit.circuitListId)) { + throw new Error("Alle verschobenen Zeilen müssen zur Stromkreisliste des Ziels gehören."); + } + + const movedRowIdSet = new Set(rowIds); + const retainedTargetRows = tx + .select({ id: circuitDeviceRows.id, sortOrder: circuitDeviceRows.sortOrder }) + .from(circuitDeviceRows) + .where(eq(circuitDeviceRows.circuitId, targetCircuitId)) + .all() + .filter((row) => !movedRowIdSet.has(row.id)); + const lastTargetSortOrder = retainedTargetRows.reduce( + (highest, row) => Math.max(highest, row.sortOrder), + 0 + ); + + for (let index = 0; index < input.rows.length; index += 1) { + const row = input.rows[index]; + const result = tx + .update(circuitDeviceRows) + .set({ + circuitId: targetCircuitId, + sortOrder: lastTargetSortOrder + (index + 1) * 10, + }) + .where( + and( + eq(circuitDeviceRows.id, row.id), + eq(circuitDeviceRows.circuitId, row.expectedCircuitId) + ) + ) + .run(); + if (result.changes !== 1) { + throw new Error("Eine Gerätezeile wurde vor Abschluss der Verschiebung verändert."); + } + } + + for (const sourceCircuitId of sourceCircuitIds) { + const remainingRows = tx + .select({ id: circuitDeviceRows.id }) + .from(circuitDeviceRows) + .where(eq(circuitDeviceRows.circuitId, sourceCircuitId)) + .all(); + tx + .update(circuits) + .set({ isReserve: remainingRows.length === 0 ? 1 : 0 }) + .where(eq(circuits.id, sourceCircuitId)) + .run(); + } + tx.update(circuits).set({ isReserve: 0 }).where(eq(circuits.id, targetCircuitId)).run(); + }); + + return { + movedRowIds: rowIds, + targetCircuitId, + createdCircuitId, + }; + } } diff --git a/src/domain/services/circuit-write.service.ts b/src/domain/services/circuit-write.service.ts index baf6fca..229fb41 100644 --- a/src/domain/services/circuit-write.service.ts +++ b/src/domain/services/circuit-write.service.ts @@ -330,7 +330,7 @@ export class CircuitWriteService { circuitListId: sourceCircuit.circuitListId, sectionId: section.id, equipmentIdentifier: nextIdentifier, - displayName: "New circuit", + displayName: "Neuer Stromkreis", sortOrder: nextSortOrder, isReserve: false, }); @@ -436,7 +436,17 @@ export class CircuitWriteService { throw new Error("Invalid target circuit id."); } - // Bulk placeholder move creates exactly one new circuit as common target. + // Bulk placeholder move prepares exactly one new circuit as common target. + // Its actual creation happens together with the row moves in one transaction. + let createTargetCircuit: + | { + circuitListId: string; + sectionId: string; + equipmentIdentifier: string; + displayName: string; + sortOrder: number; + } + | undefined; if (!targetCircuit) { if (!input.targetSectionId || !input.createNewCircuit) { throw new Error("Invalid move target."); @@ -446,86 +456,27 @@ export class CircuitWriteService { const sectionCircuits = await this.circuitRepository.listBySection(section.id); const nextSortOrder = sectionCircuits.length > 0 ? Math.max(...sectionCircuits.map((circuit) => circuit.sortOrder)) + 10 : 10; - const createdId = await this.circuitRepository.create({ + createTargetCircuit = { circuitListId: referenceSourceCircuit.circuitListId, sectionId: section.id, equipmentIdentifier: nextIdentifier, - displayName: "New circuit", + displayName: "Neuer Stromkreis", sortOrder: nextSortOrder, - isReserve: false, - }); - targetCircuit = await this.circuitRepository.findById(createdId); - if (!targetCircuit) { - throw new Error("Failed to create target circuit."); - } + }; } - // Any source circuit emptied by bulk move is preserved as reserve circuit. + const targetCircuitListId = targetCircuit?.circuitListId ?? createTargetCircuit!.circuitListId; for (const sourceCircuit of sourceCircuits.values()) { - if (sourceCircuit.circuitListId !== targetCircuit.circuitListId) { + if (sourceCircuit.circuitListId !== targetCircuitListId) { throw new Error("All moved rows must belong to same circuit list as target."); } } - const targetCount = await this.deviceRowRepository.countByCircuit(targetCircuit.id); - let offset = 1; - for (const row of rows) { - await this.deviceRowRepository.moveToCircuit(row.id, targetCircuit.id, (targetCount + offset) * 10); - offset += 1; - } - - for (const sourceCircuit of sourceCircuits.values()) { - const remaining = await this.deviceRowRepository.countByCircuit(sourceCircuit.id); - if (remaining === 0) { - await this.circuitRepository.update(sourceCircuit.id, { - sectionId: sourceCircuit.sectionId, - equipmentIdentifier: sourceCircuit.equipmentIdentifier, - displayName: sourceCircuit.displayName ?? undefined, - sortOrder: sourceCircuit.sortOrder, - protectionType: sourceCircuit.protectionType ?? undefined, - protectionRatedCurrent: sourceCircuit.protectionRatedCurrent ?? undefined, - protectionCharacteristic: sourceCircuit.protectionCharacteristic ?? undefined, - cableType: sourceCircuit.cableType ?? undefined, - cableCrossSection: sourceCircuit.cableCrossSection ?? undefined, - cableLength: sourceCircuit.cableLength ?? undefined, - rcdAssignment: sourceCircuit.rcdAssignment ?? undefined, - terminalDesignation: sourceCircuit.terminalDesignation ?? undefined, - voltage: sourceCircuit.voltage ?? undefined, - controlRequirement: sourceCircuit.controlRequirement ?? undefined, - status: sourceCircuit.status ?? undefined, - isReserve: true, - remark: sourceCircuit.remark ?? undefined, - }); - } - } - - if (Boolean(targetCircuit.isReserve)) { - await this.circuitRepository.update(targetCircuit.id, { - sectionId: targetCircuit.sectionId, - equipmentIdentifier: targetCircuit.equipmentIdentifier, - displayName: targetCircuit.displayName ?? undefined, - sortOrder: targetCircuit.sortOrder, - protectionType: targetCircuit.protectionType ?? undefined, - protectionRatedCurrent: targetCircuit.protectionRatedCurrent ?? undefined, - protectionCharacteristic: targetCircuit.protectionCharacteristic ?? undefined, - cableType: targetCircuit.cableType ?? undefined, - cableCrossSection: targetCircuit.cableCrossSection ?? undefined, - cableLength: targetCircuit.cableLength ?? undefined, - rcdAssignment: targetCircuit.rcdAssignment ?? undefined, - terminalDesignation: targetCircuit.terminalDesignation ?? undefined, - voltage: targetCircuit.voltage ?? undefined, - controlRequirement: targetCircuit.controlRequirement ?? undefined, - status: targetCircuit.status ?? undefined, - isReserve: false, - remark: targetCircuit.remark ?? undefined, - }); - } - - return { - movedRowIds: rows.map((row) => row.id), - targetCircuitId: targetCircuit.id, - createdCircuitId: input.targetCircuitId ? undefined : targetCircuit.id, - }; + return this.deviceRowRepository.moveRowsTransactional({ + rows: rows.map((row) => ({ id: row.id, expectedCircuitId: row.circuitId })), + targetCircuitId: targetCircuit?.id, + createTargetCircuit, + }); } async getNextIdentifier(sectionId: string) { diff --git a/tests/circuit-write.rules.test.ts b/tests/circuit-write.rules.test.ts index eb9c33a..8c0e62d 100644 --- a/tests/circuit-write.rules.test.ts +++ b/tests/circuit-write.rules.test.ts @@ -1,6 +1,7 @@ import assert from "node:assert/strict"; import { describe, it } from "node:test"; import { CircuitWriteService } from "../src/domain/services/circuit-write.service.js"; +import { CircuitDeviceRowRepository } from "../src/db/repositories/circuit-device-row.repository.js"; import { CircuitRepository } from "../src/db/repositories/circuit.repository.js"; import { db } from "../src/db/client.js"; import { createCircuitSchema } from "../src/shared/validation/circuit.schemas.js"; @@ -424,8 +425,13 @@ describe("circuit write service rules", () => { }); it("moving multiple device rows to a circuit preserves input order and toggles reserve", async () => { - const movedCalls: Array<{ rowId: string; targetCircuitId: string; sortOrder: number }> = []; - const reserveUpdates: Array<{ id: string; isReserve: boolean }> = []; + let transactionalMove: + | { + rows: Array<{ id: string; expectedCircuitId: string }>; + targetCircuitId?: string; + createTargetCircuit?: unknown; + } + | undefined; const service = new CircuitWriteService({ deviceRowRepository: { async findById(rowId: string) { @@ -434,14 +440,12 @@ describe("circuit write service rules", () => { } return { id: "r2", circuitId: "c2" } as never; }, - async countByCircuit(circuitId: string) { - if (circuitId === "c3") { - return 1; - } - return 0; - }, - async moveToCircuit(rowId: string, targetCircuitId: string, sortOrder: number) { - movedCalls.push({ rowId, targetCircuitId, sortOrder }); + moveRowsTransactional(input: typeof transactionalMove) { + transactionalMove = input; + return { + movedRowIds: input!.rows.map((row) => row.id), + targetCircuitId: input!.targetCircuitId!, + }; }, } as never, circuitRepository: { @@ -454,39 +458,48 @@ describe("circuit write service rules", () => { } return { id: "c3", sectionId: "s1", circuitListId: "l1", equipmentIdentifier: "-1F3", sortOrder: 30, isReserve: 1 } as never; }, - async update(id: string, payload: { isReserve: boolean }) { - reserveUpdates.push({ id, isReserve: payload.isReserve }); - }, } as never, }); - await service.moveDeviceRowsBulk({ rowIds: ["r1", "r2"], targetCircuitId: "c3" }); - assert.deepEqual(movedCalls, [ - { rowId: "r1", targetCircuitId: "c3", sortOrder: 20 }, - { rowId: "r2", targetCircuitId: "c3", sortOrder: 30 }, - ]); - assert.deepEqual(reserveUpdates, [ - { id: "c1", isReserve: true }, - { id: "c2", isReserve: true }, - { id: "c3", isReserve: false }, - ]); + const result = await service.moveDeviceRowsBulk({ rowIds: ["r1", "r2"], targetCircuitId: "c3" }); + assert.deepEqual(transactionalMove, { + rows: [ + { id: "r1", expectedCircuitId: "c1" }, + { id: "r2", expectedCircuitId: "c2" }, + ], + targetCircuitId: "c3", + createTargetCircuit: undefined, + }); + assert.deepEqual(result, { + movedRowIds: ["r1", "r2"], + targetCircuitId: "c3", + }); }); it("moving multiple device rows to placeholder creates exactly one new circuit", async () => { - let createCount = 0; + let transactionCount = 0; + let preparedTarget: + | { + circuitListId: string; + sectionId: string; + equipmentIdentifier: string; + displayName: string; + sortOrder: number; + } + | undefined; const service = new CircuitWriteService({ deviceRowRepository: { async findById(rowId: string) { return { id: rowId, circuitId: rowId === "r1" ? "c1" : "c2" } as never; }, - async countByCircuit(circuitId: string) { - if (circuitId === "c-new") { - return 0; - } - return 1; - }, - async moveToCircuit() { - return; + moveRowsTransactional(input: { rows: Array<{ id: string }>; createTargetCircuit?: typeof preparedTarget }) { + transactionCount += 1; + preparedTarget = input.createTargetCircuit; + return { + movedRowIds: input.rows.map((row) => row.id), + targetCircuitId: "c-new", + createdCircuitId: "c-new", + }; }, } as never, circuitRepository: { @@ -494,21 +507,11 @@ describe("circuit write service rules", () => { if (circuitId === "c1" || circuitId === "c2") { return { id: circuitId, sectionId: "s1", circuitListId: "l1", equipmentIdentifier: "-1F1", sortOrder: 10, isReserve: 0 } as never; } - if (circuitId === "c-new") { - return { id: "c-new", sectionId: "s2", circuitListId: "l1", equipmentIdentifier: "-2F8", sortOrder: 40, isReserve: 0 } as never; - } return null as never; }, async listBySection() { return [{ sortOrder: 30 }] as never[]; }, - async create() { - createCount += 1; - return "c-new"; - }, - async update() { - return; - }, } as never, circuitSectionRepository: { async findById() { @@ -527,7 +530,14 @@ describe("circuit write service rules", () => { targetSectionId: "s2", createNewCircuit: true, }); - assert.equal(createCount, 1); + assert.equal(transactionCount, 1); + assert.deepEqual(preparedTarget, { + circuitListId: "l1", + sectionId: "s2", + equipmentIdentifier: "-2F8", + displayName: "Neuer Stromkreis", + sortOrder: 40, + }); assert.equal(result.createdCircuitId, "c-new"); }); @@ -650,6 +660,94 @@ describe("circuit write service rules", () => { } }); + it("bulk device row move uses one synchronous transaction callback", () => { + const repository = new CircuitDeviceRowRepository(); + const originalTransaction = (db as unknown as { transaction: unknown }).transaction; + + let callbackReturnedPromise = false; + let selectCall = 0; + let rowUpdateCount = 0; + const selectedRows = [ + [ + { id: "r1", circuitId: "c1" }, + { id: "r2", circuitId: "c2" }, + ], + [{ id: "c3", circuitListId: "l1" }], + [ + { id: "c1", circuitListId: "l1" }, + { id: "c2", circuitListId: "l1" }, + ], + [{ id: "existing-target-row", sortOrder: 10 }], + [], + [], + ]; + + (db as unknown as { transaction: (cb: (tx: unknown) => unknown) => void }).transaction = (cb) => { + const fakeTx = { + select() { + const rows = selectedRows[selectCall++] ?? []; + const query = { + from() { + return query; + }, + where() { + return query; + }, + limit() { + return query; + }, + all() { + return rows; + }, + }; + return query; + }, + update() { + return { + set() { + return { + where() { + return { + run() { + rowUpdateCount += 1; + return { changes: 1 }; + }, + }; + }, + }; + }, + }; + }, + }; + const callbackResult = cb(fakeTx); + callbackReturnedPromise = Boolean( + callbackResult && typeof (callbackResult as Promise).then === "function" + ); + if (callbackReturnedPromise) { + throw new Error("Transaction function cannot return a promise"); + } + }; + + try { + const result = repository.moveRowsTransactional({ + rows: [ + { id: "r1", expectedCircuitId: "c1" }, + { id: "r2", expectedCircuitId: "c2" }, + ], + targetCircuitId: "c3", + }); + assert.equal(callbackReturnedPromise, false); + assert.equal(rowUpdateCount, 5); + assert.deepEqual(result, { + movedRowIds: ["r1", "r2"], + targetCircuitId: "c3", + createdCircuitId: undefined, + }); + } finally { + (db as unknown as { transaction: unknown }).transaction = originalTransaction; + } + }); + it("tracks local edits on linked device rows as overridden fields", async () => { let savedOverrides: string | undefined; const current = {