From 24e7bf41c4cefd9d8cb5af454f492763661b9f02 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 22 Jul 2026 23:17:47 +0100 Subject: [PATCH] Route infrastructure workspace node edits to the update endpoint The source-manager workspace retired the legacy per-type shells that set the configured-nodes editingNode signal, so every save from the Manage flow fell through to the add endpoint. Editing an existing PVE, PBS or PMG connection then failed with "Authentication credentials required" (or a duplicate-host conflict) and the editor closed anyway, discarding the form. Pass the edited node along with the save call, report save success, and keep the editor open when the save fails. Fixes #1605 Contract-Neutral: #1605 frontend-only fix routing workspace node edits to the update endpoint; no subsystem boundary moved --- .../CredentialSlots/NodeCredentialSlot.tsx | 9 ++- .../__tests__/NodeCredentialSlot.test.tsx | 72 ++++++++++++++++++- .../Settings/proxmoxSettingsModel.ts | 5 +- .../useInfrastructureConfiguredNodesState.ts | 9 ++- 4 files changed, 89 insertions(+), 6 deletions(-) diff --git a/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/NodeCredentialSlot.tsx b/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/NodeCredentialSlot.tsx index 0054553aa1..986ad8603e 100644 --- a/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/NodeCredentialSlot.tsx +++ b/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/NodeCredentialSlot.tsx @@ -52,8 +52,13 @@ export const NodeCredentialSlot: Component = (props) => const [importImpactPreviewError, setImportImpactPreviewError] = createSignal(null); const handleSave = async (nodeData: Partial) => { - await props.settings.saveNode(nodeData); - props.onSaved(); + // The configured-nodes state no longer tracks which node the workspace is + // editing, so the edit target must travel with the save call; otherwise + // edits are misrouted to the add endpoint (#1605). + const saved = await props.settings.saveNode(nodeData, props.editingNode ?? null); + if (saved) { + props.onSaved(); + } }; const modalProps: NodeModalProps = { diff --git a/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/__tests__/NodeCredentialSlot.test.tsx b/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/__tests__/NodeCredentialSlot.test.tsx index 200de29650..81568f5786 100644 --- a/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/__tests__/NodeCredentialSlot.test.tsx +++ b/frontend-modern/src/components/Settings/ConnectionEditor/CredentialSlots/__tests__/NodeCredentialSlot.test.tsx @@ -15,7 +15,7 @@ const createSettings = (): InfrastructurePlatformSettingsProps => savingTemperatureSetting: () => false, handleNodeTemperatureMonitoringChange: vi.fn().mockResolvedValue(undefined), handleTemperatureMonitoringChange: vi.fn().mockResolvedValue(undefined), - saveNode: vi.fn().mockResolvedValue(undefined), + saveNode: vi.fn().mockResolvedValue(true), }) as unknown as InfrastructurePlatformSettingsProps; describe('NodeCredentialSlot', () => { @@ -185,4 +185,74 @@ describe('NodeCredentialSlot', () => { const payload = vi.mocked(settings.saveNode).mock.calls[0][0]; expect(payload).not.toHaveProperty('clusterEndpointOverrides'); }); + + it('passes the edited node to saveNode so edits hit the update path', async () => { + const settings = createSettings(); + const onSaved = vi.fn(); + const editingNode = { + id: 'pve-0', + type: 'pve', + name: 'homelab', + host: 'https://pve1.local:8006', + user: '', + tokenName: 'root@pam!pulse', + hasToken: true, + verifySSL: true, + status: 'connected', + } as unknown as NodeConfigWithStatus; + + render(() => ( + + )); + + fireEvent.submit(screen.getByRole('button', { name: 'Save changes' }).closest('form')!); + + await vi.waitFor(() => { + expect(settings.saveNode).toHaveBeenCalledTimes(1); + }); + expect(vi.mocked(settings.saveNode).mock.calls[0][1]).toBe(editingNode); + await vi.waitFor(() => { + expect(onSaved).toHaveBeenCalledTimes(1); + }); + }); + + it('keeps the editor open when the save fails', async () => { + const settings = createSettings(); + vi.mocked(settings.saveNode).mockResolvedValue(false); + const onSaved = vi.fn(); + const editingNode = { + id: 'pve-0', + type: 'pve', + name: 'homelab', + host: 'https://pve1.local:8006', + user: '', + tokenName: 'root@pam!pulse', + hasToken: true, + verifySSL: true, + status: 'connected', + } as unknown as NodeConfigWithStatus; + + render(() => ( + + )); + + fireEvent.submit(screen.getByRole('button', { name: 'Save changes' }).closest('form')!); + + await vi.waitFor(() => { + expect(settings.saveNode).toHaveBeenCalledTimes(1); + }); + expect(onSaved).not.toHaveBeenCalled(); + }); }); diff --git a/frontend-modern/src/components/Settings/proxmoxSettingsModel.ts b/frontend-modern/src/components/Settings/proxmoxSettingsModel.ts index 08b4281c72..7d15756083 100644 --- a/frontend-modern/src/components/Settings/proxmoxSettingsModel.ts +++ b/frontend-modern/src/components/Settings/proxmoxSettingsModel.ts @@ -67,7 +67,10 @@ export interface InfrastructurePlatformSettingsProps { savingDockerUpdateActions: Accessor; handleDisableDockerUpdateActionsChange: (enabled: boolean) => Promise; handleNodeTemperatureMonitoringChange: (nodeId: string, enabled: boolean | null) => Promise; - saveNode: (nodeData: Partial) => Promise; + saveNode: ( + nodeData: Partial, + existingNode?: NodeConfigWithStatus | null, + ) => Promise; showDeleteNodeModal: Accessor; cancelDeleteNode: () => void; deleteNode: () => Promise; diff --git a/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts b/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts index 0de5895585..9205d11d3a 100644 --- a/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts +++ b/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts @@ -281,9 +281,12 @@ export const useInfrastructureConfiguredNodesState = ({ } }; - const saveNode = async (nodeData: Partial) => { + const saveNode = async ( + nodeData: Partial, + existingNodeOverride?: NodeConfigWithStatus | null, + ): Promise => { try { - const existingNode = editingNode(); + const existingNode = existingNodeOverride ?? editingNode(); if (existingNode?.id) { await NodesAPI.updateNode(existingNode.id, nodeData as NodeConfig); // clusterEndpointOverrides is a write-only payload field: mirror it @@ -322,8 +325,10 @@ export const useInfrastructureConfiguredNodesState = ({ setShowNodeModal(false); setEditingNode(null); + return true; } catch (error) { notificationStore.error(error instanceof Error ? error.message : 'Operation failed'); + return false; } };