forked from jappel/leistungsbilanz-ts
Fix code-review findings across domain, persistence, server and frontend
Full-codebase review turned up five real correctness/security bugs and
a dozen smaller inconsistencies; all are fixed here with matching test
coverage:
- BMK uniqueness silently allowed German-umlaut duplicates ("Ä1" vs
"ä1") because the DB's normalized index only folds ASCII case. Added
a shared Unicode-aware pre-check used by every circuit/component
insert and rename path (one of which had no pre-check at all).
- CircuitDeviceRow.simultaneityFactor had no upper bound at the row
level (command model and snapshot/restore schema), unlike every
sibling entity, letting a bad value silently corrupt power totals.
- Grid cell editing silently misread German thousands-separator input
("1.500" parsed as 1.5); "." is now rejected outright with a clear
message instead of guessing.
- The editor's shared command runner (runCommand/applyHistory) had no
re-entrancy guard, so a double click/drop could fire the same
command twice and race a BMK collision or revision conflict. Added a
synchronous ref guard plus isSaving on the buttons that lacked it.
- GET .../next-identifier leaked circuit-numbering state for sections
in other projects (no ownership check, 400 instead of 404). Moved
under /projects/:projectId and scoped it.
Also: added the missing circuits.section_id / circuit_device_rows.
circuit_id indexes (migration 0006), gave FormModal a focus trap /
Escape-to-close / focus restore and rebuilt ProjectSettingsModal on
top of it instead of duplicated markup, removed dead code (3 orphaned
domain model files, an unused persistence helper, a wrapper only used
by its own test), pointed the project page at GET /projects/:id
instead of listing+filtering client-side, closed the gap between the
documented 18 MB CSV limit and the ~17.17 MiB actually enforced, added
missing upper bounds on several free-text fields, filled in nine
missing German labels in the revision timeline, replaced a
key-order-fragile JSON.stringify equality check with a real field
comparison, made an implicit sort-order assumption in three
renumbering helpers explicit, cleared the sidebar's target selection
when it no longer resolves after a tree reload, and fixed
updateGlobalDevice to check-then-write instead of write-then-check.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
fa96be2d42
commit
b45dc5002d
48 changed files with 3263 additions and 526 deletions
|
|
@ -268,6 +268,10 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
const [editingCell, setEditingCell] = useState<EditingCell | null>(null);
|
||||
const [activeSectionId, setActiveSectionId] = useState<string | null>(null);
|
||||
const [isSaving, setIsSaving] = useState(false);
|
||||
// Synchronous re-entry guard: isSaving is React state and only reflects
|
||||
// reality after the next render, so a second click/drop fired within the
|
||||
// same tick could otherwise race past it and double-submit a command.
|
||||
const commandInFlightRef = useRef(false);
|
||||
const [componentEditorIntent, setComponentEditorIntent] =
|
||||
useState<StructureComponentEditorIntent | null>(null);
|
||||
const [circuitGroupEditorIntent, setCircuitGroupEditorIntent] =
|
||||
|
|
@ -723,6 +727,27 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
);
|
||||
}, [data]);
|
||||
|
||||
// Clears the sidebar's target selection once it no longer resolves to a
|
||||
// real section/circuit (e.g. deleted, moved or renumbered elsewhere)
|
||||
// instead of silently holding a stale id after a tree reload.
|
||||
useEffect(() => {
|
||||
if (!data) {
|
||||
return;
|
||||
}
|
||||
if (
|
||||
targetSectionId &&
|
||||
!data.sections.some((section) => section.id === targetSectionId)
|
||||
) {
|
||||
setTargetSectionId(null);
|
||||
}
|
||||
if (
|
||||
targetCircuitId &&
|
||||
!circuitOptions.some((option) => option.id === targetCircuitId)
|
||||
) {
|
||||
setTargetCircuitId(null);
|
||||
}
|
||||
}, [data, circuitOptions, targetSectionId, targetCircuitId]);
|
||||
|
||||
const allCircuits = useMemo(
|
||||
() => data?.sections.flatMap((section) => section.circuits) ?? [],
|
||||
[data]
|
||||
|
|
@ -1046,6 +1071,10 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
|
||||
// Runs a normal command. The server records it in project-wide history.
|
||||
async function runCommand(command: HistoryCommand) {
|
||||
if (commandInFlightRef.current) {
|
||||
return;
|
||||
}
|
||||
commandInFlightRef.current = true;
|
||||
try {
|
||||
setError(null);
|
||||
setIsSaving(true);
|
||||
|
|
@ -1056,6 +1085,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
await loadTree({ showLoading: false });
|
||||
setError(message);
|
||||
} finally {
|
||||
commandInFlightRef.current = false;
|
||||
setIsSaving(false);
|
||||
}
|
||||
}
|
||||
|
|
@ -1063,6 +1093,10 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
// Applies the next eligible project-wide history operation. Selection is only
|
||||
// a best-effort local hint; command eligibility and data changes stay server-owned.
|
||||
async function applyHistory(mode: "undo" | "redo") {
|
||||
if (commandInFlightRef.current) {
|
||||
return;
|
||||
}
|
||||
commandInFlightRef.current = true;
|
||||
try {
|
||||
setError(null);
|
||||
setHistoryBusy(true);
|
||||
|
|
@ -1088,6 +1122,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
await loadTree({ showLoading: false });
|
||||
setError(message);
|
||||
} finally {
|
||||
commandInFlightRef.current = false;
|
||||
setIsSaving(false);
|
||||
setHistoryBusy(false);
|
||||
}
|
||||
|
|
@ -1744,7 +1779,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
if (!section) {
|
||||
throw new Error("Bereich wurde nicht gefunden.");
|
||||
}
|
||||
const next = await getNextCircuitIdentifier(sectionId);
|
||||
const next = await getNextCircuitIdentifier(projectId, sectionId);
|
||||
const sortOrder =
|
||||
section.circuits.length > 0 ? Math.max(...section.circuits.map((circuit) => circuit.sortOrder)) + 10 : 10;
|
||||
const isDeviceField = deviceFieldKeys.has(key);
|
||||
|
|
@ -1933,7 +1968,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
await runCommand({
|
||||
label: "Stromkreis hinzufügen",
|
||||
redo: async () => {
|
||||
const next = await getNextCircuitIdentifier(sectionId);
|
||||
const next = await getNextCircuitIdentifier(projectId, sectionId);
|
||||
const sortOrder = getInsertionSortOrder(section.circuits, afterCircuitId);
|
||||
const circuit = createCircuitSnapshot({
|
||||
sectionId,
|
||||
|
|
@ -2127,7 +2162,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
if (!section) {
|
||||
throw new Error("Der Zielbereich ist ungültig.");
|
||||
}
|
||||
const next = await getNextCircuitIdentifier(sectionId);
|
||||
const next = await getNextCircuitIdentifier(projectId, sectionId);
|
||||
const sortOrder =
|
||||
section.circuits.length > 0 ? Math.max(...section.circuits.map((circuit) => circuit.sortOrder)) + 10 : 10;
|
||||
const circuit = createCircuitSnapshot(
|
||||
|
|
@ -2538,7 +2573,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
await runCommand({
|
||||
label: newCircuitLabel,
|
||||
redo: async () => {
|
||||
const next = await getNextCircuitIdentifier(intent.sectionId);
|
||||
const next = await getNextCircuitIdentifier(projectId, intent.sectionId);
|
||||
const sortOrder =
|
||||
intent.targetCircuitId && intent.placement
|
||||
? getAdjacentInsertionSortOrder(
|
||||
|
|
@ -3699,6 +3734,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={
|
||||
isSaving ||
|
||||
!buildCircuitGroupReorderAssignments(
|
||||
data.sections,
|
||||
section.id,
|
||||
|
|
@ -3716,6 +3752,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={
|
||||
isSaving ||
|
||||
!buildCircuitGroupReorderAssignments(
|
||||
data.sections,
|
||||
section.id,
|
||||
|
|
@ -3733,6 +3770,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={
|
||||
isSaving ||
|
||||
hasActiveSortOrFilter ||
|
||||
!section.category ||
|
||||
!canRenumberCircuitGroups(
|
||||
|
|
@ -3758,6 +3796,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
<button
|
||||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={isSaving}
|
||||
title={
|
||||
canDeleteCircuitGroup(section)
|
||||
? "Leere Stromkreisgruppe entfernen"
|
||||
|
|
@ -3815,13 +3854,18 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
>
|
||||
Gruppen-FI hinzufügen
|
||||
</button>
|
||||
<button type="button" tabIndex={-1} onClick={() => void handleAddReserveCircuit(section.id)}>
|
||||
<button
|
||||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={isSaving}
|
||||
onClick={() => void handleAddReserveCircuit(section.id)}
|
||||
>
|
||||
Stromkreis hinzufügen
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={hasActiveSortOrFilter}
|
||||
disabled={hasActiveSortOrFilter || isSaving}
|
||||
onClick={() => void handleRenumberSection(section.id)}
|
||||
title={hasActiveSortOrFilter ? "Vor der Neunummerierung Sortierung und Filter zurücksetzen." : undefined}
|
||||
>
|
||||
|
|
@ -4426,6 +4470,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
<button
|
||||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={isSaving}
|
||||
onClick={() => void handleAddManualDevice(row.circuit!, row.sectionId)}
|
||||
>
|
||||
Manuelles Gerät hinzufügen
|
||||
|
|
@ -4433,6 +4478,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
<button
|
||||
type="button"
|
||||
tabIndex={-1}
|
||||
disabled={isSaving}
|
||||
onClick={() => void handleDeleteCircuit(row.circuit!.id)}
|
||||
>
|
||||
Stromkreis löschen
|
||||
|
|
@ -4440,7 +4486,7 @@ export function CircuitTreeEditor(props: { projectId: string; circuitListId: str
|
|||
</>
|
||||
) : null}
|
||||
{row.device ? (
|
||||
<button type="button" tabIndex={-1} onClick={() => void handleDeleteDevice(row.device!.id)}>
|
||||
<button type="button" tabIndex={-1} disabled={isSaving} onClick={() => void handleDeleteDevice(row.device!.id)}>
|
||||
Gerät löschen
|
||||
</button>
|
||||
) : null}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue