From 2bf9f1bca8e23bbc3549441b5ce6cd420e36c71d Mon Sep 17 00:00:00 2001 From: Pouzor Date: Thu, 27 Aug 2026 00:51:47 +0200 Subject: [PATCH] fix(canvas): keep a parent ahead of its children after a subnet import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subnet import only guaranteed the zone preceded the nodes it pulled in. An arrival that is itself a parent — a Proxmox host with nested VMs, whose children stay put because they already have a parent — could end up listed after those children, which React Flow renders detached with a parent-not-found error. Nothing re-sorts on load, so the broken order survived a save. Reorder the whole array instead: every node now follows its parent, which covers both directions at once. An already-valid list comes back untouched and a parent cycle terminates rather than recursing. ha-relevant: yes --- .../__tests__/canvasStore/zones.test.ts | 34 +++++++++++++++++ frontend/src/stores/canvasStore.ts | 38 ++++++++++++++++--- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/frontend/src/stores/__tests__/canvasStore/zones.test.ts b/frontend/src/stores/__tests__/canvasStore/zones.test.ts index e6af603..2a0424c 100644 --- a/frontend/src/stores/__tests__/canvasStore/zones.test.ts +++ b/frontend/src/stores/__tests__/canvasStore/zones.test.ts @@ -253,6 +253,40 @@ describe('canvasStore — importZoneSubnet', () => { expect(ids.indexOf('z1')).toBeLessThan(ids.indexOf('n2')) }) + it('keeps an arrival that is itself a parent ahead of the children it leaves behind', () => { + // A Proxmox host matches the subnet; its VM does not move, because it + // already has a parent. The host must still be listed before the VM. + useCanvasStore.setState({ + nodes: [ + device('proxmox', '192.168.1.10'), + { ...device('vm1', '10.0.0.1'), parentId: 'proxmox' }, + zone('z1'), + ], + }) + + expect(useCanvasStore.getState().importZoneSubnet('z1', '192.168.1.0/24')).toBe(1) + + const nodes = useCanvasStore.getState().nodes + const ids = nodes.map((n) => n.id) + expect(nodes.find((n) => n.id === 'proxmox')!.parentId).toBe('z1') + expect(nodes.find((n) => n.id === 'vm1')!.parentId).toBe('proxmox') + expect(ids.indexOf('z1')).toBeLessThan(ids.indexOf('proxmox')) + expect(ids.indexOf('proxmox')).toBeLessThan(ids.indexOf('vm1')) + }) + + it('leaves an already-valid order untouched', () => { + useCanvasStore.setState({ + nodes: [zone('z1'), device('a', '10.0.0.1'), device('b', '10.0.0.2'), device('hit', '192.168.1.10')], + }) + useCanvasStore.getState().importZoneSubnet('z1', '192.168.1.0/24') + + // Only the matched node moves in the array; the untouched ones keep their + // relative order. + const ids = useCanvasStore.getState().nodes.map((n) => n.id) + expect(ids.indexOf('a')).toBeLessThan(ids.indexOf('b')) + expect(ids[0]).toBe('z1') + }) + it('is additive: a second subnet keeps the first import inside', () => { useCanvasStore.setState({ nodes: [zone('z1'), device('n1', '192.168.1.10'), device('n2', '10.0.0.5')], diff --git a/frontend/src/stores/canvasStore.ts b/frontend/src/stores/canvasStore.ts index 3a82be6..0d6246f 100644 --- a/frontend/src/stores/canvasStore.ts +++ b/frontend/src/stores/canvasStore.ts @@ -30,6 +30,32 @@ type Clipboard = { nodes: Node[]; edges: Edge[] } /** Resolve a node's effective parent id from either the RF field or domain data. */ const parentIdOf = (n: Node): string | undefined => n.parentId ?? n.data.parent_id ?? undefined +/** + * Reorder so every node follows its parent, which is what React Flow needs to + * resolve nesting — a child listed first renders detached and logs a + * parent-not-found error. Order is otherwise preserved: a list that is already + * valid comes back untouched. A parent cycle terminates instead of recursing. + */ +function orderParentsFirst(nodes: Node[]): Node[] { + const byId = new Map(nodes.map((n) => [n.id, n])) + const emitted = new Set() + const visiting = new Set() + const out: Node[] = [] + + const visit = (n: Node) => { + if (emitted.has(n.id) || visiting.has(n.id)) return + visiting.add(n.id) + const parent = n.parentId ? byId.get(n.parentId) : undefined + if (parent) visit(parent) + visiting.delete(n.id) + emitted.add(n.id) + out.push(n) + } + + for (const n of nodes) visit(n) + return out +} + // Zone subnet import: the grid the arrivals are packed on, in zone-relative // pixels. A cell is one node box plus the gap that follows it. const DEFAULT_ZONE_WIDTH = 360 @@ -1069,13 +1095,13 @@ export const useCanvasStore = create((rawSet, get) => { } }) - // React Flow requires a parent to precede its children in the array. - const others = updated.filter((n) => !moved.has(n.id)) - const children = updated.filter((n) => moved.has(n.id)) - const zoneIdx = others.findIndex((n) => n.id === zoneId) - return { - nodes: [...others.slice(0, zoneIdx + 1), ...children, ...others.slice(zoneIdx + 1)], + // React Flow requires a parent to precede its children in the array. + // Reordering the whole list covers both directions at once: the zone + // ahead of its new children, and an arrival that is itself a parent + // (a Proxmox host with nested VMs, whose children stay put because + // they already have a parent) ahead of the children it left behind. + nodes: orderParentsFirst(updated), hasUnsavedChanges: true, past: [...s.past.slice(-49), { nodes: s.nodes, edges: s.edges }], future: [],