diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 1a485a9..9ed350e 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -650,7 +650,7 @@ export default function App() { onClose={() => setAddNodeOpen(false)} onSubmit={handleAddNode} title="Add Node" - parentCandidates={nodes.map((n) => ({ id: n.id, label: n.data.label ?? n.id, type: n.data.type }))} + parentCandidates={nodes.map((n) => ({ id: n.id, label: n.data.label ?? n.id, type: n.data.type, container_mode: n.data.container_mode }))} /> {/* key forces re-mount when editing a different node, resetting form state */} @@ -677,7 +677,7 @@ export default function App() { } return nodes .filter((n) => !descendants.has(n.id)) - .map((n) => ({ id: n.id, label: n.data.label ?? n.id, type: n.data.type })) + .map((n) => ({ id: n.id, label: n.data.label ?? n.id, type: n.data.type, container_mode: n.data.container_mode })) })()} currentNodeId={editNodeId ?? undefined} /> diff --git a/frontend/src/components/modals/NodeModal.tsx b/frontend/src/components/modals/NodeModal.tsx index 529f076..f7e6b95 100644 --- a/frontend/src/components/modals/NodeModal.tsx +++ b/frontend/src/components/modals/NodeModal.tsx @@ -55,6 +55,8 @@ interface ParentCandidate { id: string label: string type: NodeType + /** True when the node has container mode on, so any node can nest inside it. */ + container_mode?: boolean } interface NodeModalProps { @@ -98,12 +100,14 @@ export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node' const selectedType = (form.type ?? 'generic') as NodeType const canUseContainerMode = CONTAINER_MODE_TYPES.includes(selectedType) const validParentTypes = getValidParentTypes(selectedType) + // A parent is valid either by the type rules (lxc/vm/docker_container) or + // because the candidate is a container-mode node (any child can nest in it). + const isValidParent = (p: ParentCandidate) => + validParentTypes.includes(p.type) || p.container_mode === true let safeParentId = form.parent_id - if (validParentTypes.length === 0) { - safeParentId = undefined - } else if (safeParentId) { + if (safeParentId) { const parent = parentCandidates.find((n) => n.id === safeParentId) - if (!parent || !validParentTypes.includes(parent.type)) safeParentId = undefined + if (!parent || !isValidParent(parent)) safeParentId = undefined } onSubmit({ ...form, @@ -130,7 +134,12 @@ export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node' setForm((f) => { const next: Partial = { ...f, type: t } if (ZIGBEE_TYPES.includes(t)) next.check_method = 'none' as CheckMethod - if (getValidParentTypes(t).length === 0) next.parent_id = undefined + // Drop the parent only if it's no longer a valid target for the + // new type — keep container-mode parents (any node can nest). + const parent = parentCandidates.find((n) => n.id === f.parent_id) + if (f.parent_id && !(parent && (getValidParentTypes(t).includes(parent.type) || parent.container_mode === true))) { + next.parent_id = undefined + } return next }) }}> @@ -349,9 +358,12 @@ export function NodeModal({ open, onClose, onSubmit, initial, title = 'Add Node' {(() => { const childType = (form.type ?? 'generic') as NodeType const validParentTypes = getValidParentTypes(childType) - if (validParentTypes.length === 0) return null + // Candidates: type-based parents (lxc/vm/docker_container) plus any + // container-mode node. The current parent is always kept so an + // already-nested node can be re-targeted or detached here. const validParents = parentCandidates.filter( - (n) => n.id !== currentNodeId && validParentTypes.includes(n.type), + (n) => n.id !== currentNodeId && + (validParentTypes.includes(n.type) || n.container_mode === true || n.id === form.parent_id), ) if (validParents.length === 0) return null return ( diff --git a/frontend/src/components/modals/__tests__/NodeModal.test.tsx b/frontend/src/components/modals/__tests__/NodeModal.test.tsx index 1895e49..9f187ba 100644 --- a/frontend/src/components/modals/__tests__/NodeModal.test.tsx +++ b/frontend/src/components/modals/__tests__/NodeModal.test.tsx @@ -353,6 +353,48 @@ describe('NodeModal', () => { expect(screen.getByText('Parent Container')).toBeDefined() }) + it('renders Parent Container for a plain node when a container-mode candidate exists', () => { + renderModal({ + initial: { ...BASE, type: 'server' }, + parentCandidates: [{ id: 'px1', label: 'PVE', type: 'proxmox', container_mode: true }], + }) + expect(screen.getByText('Parent Container')).toBeDefined() + }) + + it('still hides Parent Container for a plain node when the candidate is not in container mode', () => { + renderModal({ + initial: { ...BASE, type: 'server' }, + parentCandidates: [{ id: 'px1', label: 'PVE', type: 'proxmox', container_mode: false }], + }) + expect(screen.queryByText('Parent Container')).toBeNull() + }) + + it('renders Parent Container for an already-nested plain node so it can be detached', () => { + renderModal({ + initial: { ...BASE, type: 'server', parent_id: 'px1' }, + parentCandidates: [{ id: 'px1', label: 'PVE', type: 'proxmox', container_mode: true }], + }) + expect(screen.getByText('Parent Container')).toBeDefined() + }) + + it('keeps a container-mode parent_id on submit for a plain node', () => { + const { onSubmit } = renderModal({ + initial: { ...BASE, type: 'server', parent_id: 'px1' }, + parentCandidates: [{ id: 'px1', label: 'PVE', type: 'proxmox', container_mode: true }], + }) + fireEvent.click(screen.getByRole('button', { name: 'Add' })) + expect((onSubmit.mock.calls[0][0] as Partial).parent_id).toBe('px1') + }) + + it('drops a parent_id that is not a valid container on submit', () => { + const { onSubmit } = renderModal({ + initial: { ...BASE, type: 'server', parent_id: 'px1' }, + parentCandidates: [{ id: 'px1', label: 'PVE', type: 'proxmox', container_mode: false }], + }) + fireEvent.click(screen.getByRole('button', { name: 'Add' })) + expect((onSubmit.mock.calls[0][0] as Partial).parent_id).toBeUndefined() + }) + // ── Appearance ──────────────────────────────────────────────────────── it('renders 3 color swatch labels (border, background, icon)', () => { diff --git a/frontend/src/components/panels/DetailPanel.tsx b/frontend/src/components/panels/DetailPanel.tsx index d31aed7..6508675 100644 --- a/frontend/src/components/panels/DetailPanel.tsx +++ b/frontend/src/components/panels/DetailPanel.tsx @@ -21,7 +21,7 @@ type PropForm = { key: string; value: string; icon: string | null; visible: bool const EMPTY_PROP: PropForm = { key: '', value: '', icon: null, visible: true } export function DetailPanel({ onEdit }: DetailPanelProps) { - const { nodes, selectedNodeId, selectedNodeIds, setSelectedNode, deleteNode, updateNode, snapshotHistory, createGroup, ungroup, removeFromGroup, setNodeParent } = useCanvasStore() + const { nodes, selectedNodeId, selectedNodeIds, setSelectedNode, deleteNode, updateNode, snapshotHistory, createGroup, ungroup, removeFromGroup } = useCanvasStore() const serviceStatuses = useCanvasStore((s) => s.serviceStatuses) const [addingForNode, setAddingForNode] = useState(null) @@ -255,30 +255,6 @@ export function DetailPanel({ onEdit }: DetailPanelProps) { {data.last_seen && } - {/* Container membership — only when the node is nested. Lets the user move - it to another container or detach it ("None"). */} - {node.parentId && ( -
- - -
- )} - {/* Properties section */}
diff --git a/frontend/src/components/panels/__tests__/GroupPanels.test.tsx b/frontend/src/components/panels/__tests__/GroupPanels.test.tsx index 707622e..ca73963 100644 --- a/frontend/src/components/panels/__tests__/GroupPanels.test.tsx +++ b/frontend/src/components/panels/__tests__/GroupPanels.test.tsx @@ -43,7 +43,6 @@ const mockStore = { createGroup: vi.fn(), ungroup: vi.fn(), removeFromGroup: vi.fn(), - setNodeParent: vi.fn(), } function setupStore(overrides = {}) { @@ -139,50 +138,6 @@ describe('MultiSelectPanel', () => { }) }) -describe('DetailPanel container selector', () => { - beforeEach(() => vi.clearAllMocks()) - - function makeContainer(id: string, label: string) { - return { id, type: 'proxmox', position: { x: 0, y: 0 }, data: { label, type: 'proxmox', status: 'online', services: [], container_mode: true } } - } - - it('does not show the container selector for a top-level node', () => { - const n1 = makeNode('n1') - setupStore({ nodes: [n1], selectedNodeId: 'n1', selectedNodeIds: ['n1'] }) - renderPanel() - expect(screen.queryByLabelText('Container')).toBeNull() - }) - - it('shows the container selector for a nested node, defaulting to its parent', () => { - const px = makeContainer('px1', 'Proxmox') - const child = makeNode('n1', { parentId: 'px1' }) - setupStore({ nodes: [px, child], selectedNodeId: 'n1', selectedNodeIds: ['n1'] }) - renderPanel() - expect((screen.getByLabelText('Container') as HTMLSelectElement).value).toBe('px1') - }) - - it('detaches the node when "None" is selected', () => { - const setNodeParent = vi.fn() - const px = makeContainer('px1', 'Proxmox') - const child = makeNode('n1', { parentId: 'px1' }) - setupStore({ nodes: [px, child], selectedNodeId: 'n1', selectedNodeIds: ['n1'], setNodeParent }) - renderPanel() - fireEvent.change(screen.getByLabelText('Container'), { target: { value: '' } }) - expect(setNodeParent).toHaveBeenCalledWith('n1', null) - }) - - it('re-parents the node when another container is selected', () => { - const setNodeParent = vi.fn() - const px1 = makeContainer('px1', 'Proxmox A') - const px2 = makeContainer('px2', 'Proxmox B') - const child = makeNode('n1', { parentId: 'px1' }) - setupStore({ nodes: [px1, px2, child], selectedNodeId: 'n1', selectedNodeIds: ['n1'], setNodeParent }) - renderPanel() - fireEvent.change(screen.getByLabelText('Container'), { target: { value: 'px2' } }) - expect(setNodeParent).toHaveBeenCalledWith('n1', 'px2') - }) -}) - describe('GroupDetailPanel', () => { beforeEach(() => vi.clearAllMocks()) diff --git a/frontend/src/stores/__tests__/canvasStore.test.ts b/frontend/src/stores/__tests__/canvasStore.test.ts index 1160541..bda3b00 100644 --- a/frontend/src/stores/__tests__/canvasStore.test.ts +++ b/frontend/src/stores/__tests__/canvasStore.test.ts @@ -651,89 +651,6 @@ describe('canvasStore', () => { expect(useCanvasStore.getState().hasUnsavedChanges).toBe(true) }) - // ── setNodeParent ─────────────────────────────────────────────────────────── - - it('setNodeParent(null) detaches a nested node and restores absolute coords', () => { - const container = { ...makeNode('px1', { type: 'proxmox', container_mode: true }), position: { x: 100, y: 50 } } - const child = { ...makeNode('n1'), position: { x: 20, y: 30 }, parentId: 'px1', extent: 'parent' as const, data: { label: 'n1', type: 'server' as const, status: 'unknown' as const, services: [], parent_id: 'px1' } } - useCanvasStore.setState({ nodes: [container, child] }) - - useCanvasStore.getState().setNodeParent('n1', null) - - const moved = useCanvasStore.getState().nodes.find((n) => n.id === 'n1') - expect(moved?.parentId).toBeUndefined() - expect(moved?.extent).toBeUndefined() - expect(moved?.data.parent_id).toBeUndefined() - // 100+20, 50+30 - expect(moved?.position).toEqual({ x: 120, y: 80 }) - }) - - it('setNodeParent attaches a top-level node to a container with relative coords', () => { - const container = { ...makeNode('px1', { type: 'proxmox', container_mode: true }), position: { x: 76, y: 52 } } - const child = { ...makeNode('n1'), position: { x: 300, y: 200 } } - useCanvasStore.setState({ nodes: [container, child] }) - - useCanvasStore.getState().setNodeParent('n1', 'px1') - - const moved = useCanvasStore.getState().nodes.find((n) => n.id === 'n1') - expect(moved?.parentId).toBe('px1') - expect(moved?.extent).toBe('parent') - expect(moved?.data.parent_id).toBe('px1') - expect(moved?.position).toEqual({ x: 224, y: 148 }) - }) - - it('setNodeParent moves a node from one container to another via absolute coords', () => { - const px1 = { ...makeNode('px1', { type: 'proxmox', container_mode: true }), position: { x: 100, y: 100 } } - const px2 = { ...makeNode('px2', { type: 'proxmox', container_mode: true }), position: { x: 500, y: 100 } } - const child = { ...makeNode('n1'), position: { x: 20, y: 20 }, parentId: 'px1', extent: 'parent' as const } - useCanvasStore.setState({ nodes: [px1, px2, child] }) - - useCanvasStore.getState().setNodeParent('n1', 'px2') - - const moved = useCanvasStore.getState().nodes.find((n) => n.id === 'n1') - // abs = 100+20=120 ; relative to px2 = 120-500 → clamped to 8 - expect(moved?.parentId).toBe('px2') - expect(moved?.position).toEqual({ x: 8, y: 20 }) - // parent must precede child - const nodes = useCanvasStore.getState().nodes - expect(nodes.findIndex((n) => n.id === 'px2')).toBeLessThan(nodes.findIndex((n) => n.id === 'n1')) - }) - - it('setNodeParent is a no-op when target is not a container', () => { - const notContainer = { ...makeNode('s1', { type: 'server' }), position: { x: 0, y: 0 } } - const child = { ...makeNode('n1'), position: { x: 50, y: 50 } } - useCanvasStore.setState({ nodes: [notContainer, child] }) - useCanvasStore.getState().markSaved() - - useCanvasStore.getState().setNodeParent('n1', 's1') - - expect(useCanvasStore.getState().nodes.find((n) => n.id === 'n1')?.parentId).toBeUndefined() - expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false) - }) - - it('setNodeParent is a no-op when the parent is unchanged', () => { - const container = { ...makeNode('px1', { type: 'proxmox', container_mode: true }), position: { x: 0, y: 0 } } - const child = { ...makeNode('n1'), position: { x: 10, y: 10 }, parentId: 'px1', extent: 'parent' as const } - useCanvasStore.setState({ nodes: [container, child] }) - useCanvasStore.getState().markSaved() - - useCanvasStore.getState().setNodeParent('n1', 'px1') - - expect(useCanvasStore.getState().hasUnsavedChanges).toBe(false) - }) - - it('setNodeParent snapshots history and marks unsaved', () => { - const container = { ...makeNode('px1', { type: 'proxmox', container_mode: true }), position: { x: 0, y: 0 } } - const child = { ...makeNode('n1'), position: { x: 10, y: 10 }, parentId: 'px1', extent: 'parent' as const } - useCanvasStore.setState({ nodes: [container, child] }) - useCanvasStore.getState().markSaved() - - useCanvasStore.getState().setNodeParent('n1', null) - - expect(useCanvasStore.getState().past).toHaveLength(1) - expect(useCanvasStore.getState().hasUnsavedChanges).toBe(true) - }) - // ── removeFromGroup ───────────────────────────────────────────────────────── it('removeFromGroup releases the child to absolute coords and keeps the group', () => { diff --git a/frontend/src/stores/canvasStore.ts b/frontend/src/stores/canvasStore.ts index ff89745..c3ecec6 100644 --- a/frontend/src/stores/canvasStore.ts +++ b/frontend/src/stores/canvasStore.ts @@ -70,7 +70,6 @@ interface CanvasState { ungroup: (groupId: string) => void addToGroup: (groupId: string, childId: string) => void addToContainer: (containerId: string, childId: string) => void - setNodeParent: (childId: string, parentId: string | null) => void removeFromGroup: (groupId: string, childId: string) => void markSaved: () => void markUnsaved: () => void @@ -673,72 +672,6 @@ export const useCanvasStore = create((set) => ({ } }), - // Re-parent a node from the detail-panel selector: attach it to a container - // (parentId = container id) or detach it back to the canvas (parentId = null). - // Handles container→container moves by going through absolute coordinates. - setNodeParent: (childId, parentId) => - set((state) => { - const child = state.nodes.find((n) => n.id === childId) - if (!child) return state - const currentParentId = child.parentId - if ((parentId ?? undefined) === currentParentId) return state - if (parentId === childId) return state - - const newParent = parentId ? state.nodes.find((n) => n.id === parentId) : null - // Only container_mode nodes may receive children via the selector. - if (parentId && (!newParent || newParent.data.container_mode !== true)) return state - - // Child's absolute position (its stored position is relative to its - // current parent, if any). - const curParent = currentParentId ? state.nodes.find((n) => n.id === currentParentId) : null - const absX = child.position.x + (curParent?.position.x ?? 0) - const absY = child.position.y + (curParent?.position.y ?? 0) - - const updatedNodes = state.nodes.map((n) => { - if (n.id !== childId) return n - if (!newParent) { - return { - ...n, - parentId: undefined, - extent: undefined, - position: { x: absX, y: absY }, - data: { ...n.data, parent_id: undefined }, - } - } - return { - ...n, - parentId: newParent.id, - extent: 'parent' as const, - position: { - x: Math.max(8, absX - newParent.position.x), - y: Math.max(8, absY - newParent.position.y), - }, - selected: false, - data: { ...n.data, parent_id: newParent.id }, - } - }) - - // When attaching, React Flow needs the parent before the child. - let nodes = updatedNodes - if (newParent) { - const others = updatedNodes.filter((n) => n.id !== childId) - const movedChild = updatedNodes.find((n) => n.id === childId)! - const parentIdx = others.findIndex((n) => n.id === newParent.id) - nodes = [ - ...others.slice(0, parentIdx + 1), - movedChild, - ...others.slice(parentIdx + 1), - ] - } - - return { - nodes, - hasUnsavedChanges: true, - past: [...state.past.slice(-49), { nodes: state.nodes, edges: state.edges }], - future: [], - } - }), - // Release a single child from a group back to the canvas. Group stays. removeFromGroup: (groupId, childId) => set((state) => {