From 55fa29f565473865f4c262f2b8516bc1899faa82 Mon Sep 17 00:00:00 2001 From: Anso Date: Tue, 21 Jul 2026 13:16:00 -0400 Subject: [PATCH] fix: leave editor after deleting the open stack (#1665) * fix: leave editor after deleting the open stack Clear selection on delete-key identity match, navigate to dashboard only when the editor is visible, and clear mobile detail so the URL writer leaves the deleted stack route instead of a permanent skeleton. * fix: remove duplicate setIsFileLoading in stack actions test fixture --- frontend/src/components/EditorLayout.tsx | 9 + .../hooks/useStackActions.test.ts | 160 +++++++++++++++++- .../EditorLayout/hooks/useStackActions.ts | 23 ++- .../EditorLayout/hooks/useUrlSync.test.ts | 70 ++++++++ 4 files changed, 258 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index 81deb221..3411a01b 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -208,6 +208,10 @@ export default function EditorLayout() { // useViewNavigationState needs onNavigateToDashboard -> resetEditorState // but stackActions isn't created until after navState const resetEditorStateRef = useRef<() => void>(() => {}); + // Mobile state is declared after useStackActions; this ref is assigned once + // pendingDetailStack / mobileView exist so delete-of-open-stack can flip to + // the list surface without reordering the hook graph. + const onDeletedOpenStackRef = useRef<() => void>(() => {}); const navState = useViewNavigationState({ onNavigateToDashboard: () => resetEditorStateRef.current(), @@ -282,6 +286,7 @@ export default function EditorLayout() { return can('stack:edit', 'stack', stackName); }, canOfferVolumeRemoval, + onDeletedOpenStack: () => onDeletedOpenStackRef.current(), }); // Wire the ref now that stackActions is available @@ -375,6 +380,10 @@ export default function EditorLayout() { const [pendingDetailStack, setPendingDetailStack] = useState(null); const [pendingAnatomyTab, setPendingAnatomyTab] = useState<'networking' | 'doctor' | 'dossier' | 'drift' | undefined>(); const [fleetUpdatesIntent, setFleetUpdatesIntent] = useState<{ tab: 'nodes' | 'changelog' } | null>(null); + onDeletedOpenStackRef.current = () => { + setPendingDetailStack(null); + setMobileView('list'); + }; const handleFleetUpdatesIntentConsumed = useCallback(() => setFleetUpdatesIntent(null), []); diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts index f56a9465..559cb860 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.test.ts @@ -102,6 +102,8 @@ function makeOverlay(over: Partial = {}): OverlayState { setPreDeployAdvisory: vi.fn(), openSelfStackProtected: vi.fn(), setDiffPreview: vi.fn(), + stackToDelete: null, + closeDeleteDialog: vi.fn(), ...over, } as unknown as OverlayState; } @@ -116,17 +118,24 @@ function setup(over: { editorState?: Partial; overlay?: Partial; stackList?: Partial; + navState?: Partial; getLastDeployOutputLine?: (stackName: string) => string | undefined; hasUpdateGuard?: boolean; canEditStack?: (stackNameOrFilename: string) => boolean; activeNode?: Parameters[0]['activeNode']; setActiveNode?: Parameters[0]['setActiveNode']; + onDeletedOpenStack?: () => void; } = {}) { const editorState = makeEditorState(over.editorState); const stackListState = makeStackListState(over.stackList); - const navState = { setActiveView: vi.fn() } as unknown as NavState; + const navState = { + activeView: 'editor', + setActiveView: vi.fn(), + ...over.navState, + } as unknown as NavState; const overlayState = makeOverlay(over.overlay); const setActiveNode = over.setActiveNode ?? vi.fn(); + const onDeletedOpenStack = over.onDeletedOpenStack ?? vi.fn(); const { result } = renderHook(() => useStackActions({ @@ -142,9 +151,10 @@ function setup(over: { diffPreviewEnabled: false, hasUpdateGuard: over.hasUpdateGuard ?? false, canEditStack: over.canEditStack ?? (() => true), + onDeletedOpenStack, }), ); - return { result, editorState, stackListState, overlayState, setActiveNode }; + return { result, editorState, stackListState, overlayState, navState, setActiveNode, onDeletedOpenStack }; } describe('useStackActions.saveFile', () => { @@ -182,7 +192,7 @@ describe('useStackActions.saveFile', () => { useStackActions({ editorState, stackListState, - navState: { setActiveView: vi.fn() } as unknown as NavState, + navState: { activeView: 'editor', setActiveView: vi.fn() } as unknown as NavState, overlayState: makeOverlay(), activeNode: { id: 1, type: 'local' } as Parameters[0]['activeNode'], setActiveNode: vi.fn(), @@ -191,6 +201,7 @@ describe('useStackActions.saveFile', () => { getLastDeployOutputLine: () => undefined, diffPreviewEnabled: false, canEditStack: () => true, + onDeletedOpenStack: vi.fn(), }), ); const ok = await result.current.saveFile(); @@ -1207,3 +1218,146 @@ describe('useStackActions.openStackApp', () => { expect(clickCount).toBe(0); }); }); + +describe('useStackActions.deleteStack', () => { + beforeEach(() => { + vi.mocked(apiFetch).mockReset(); + }); + + it('leaves the editor for dashboard when deleting the open stack by filename', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const { result, stackListState, overlayState, navState, onDeletedOpenStack } = setup({ + overlay: { stackToDelete: 'web.yml' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(false); + }); + + expect(apiFetch).toHaveBeenCalledWith('/stacks/web.yml', { method: 'DELETE' }); + expect(stackListState.setSelectedFile).toHaveBeenCalledWith(null); + expect(navState.setActiveView).toHaveBeenCalledWith('dashboard'); + expect(navState.setActiveView).toHaveBeenCalledTimes(1); + expect(onDeletedOpenStack).toHaveBeenCalledTimes(1); + expect(overlayState.closeDeleteDialog).toHaveBeenCalled(); + expect(stackListState.refreshStacks).toHaveBeenCalled(); + }); + + it('clears isFileLoading on delete-leave so the URL writer is not blocked', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const { result, editorState } = setup({ + overlay: { stackToDelete: 'web.yml' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + editorState: { isFileLoading: true }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(false); + }); + + expect(editorState.setIsFileLoading).toHaveBeenCalledWith(false); + }); + + it('leaves the editor when sidebar delete passes a basename', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const { result, stackListState, navState, onDeletedOpenStack } = setup({ + overlay: { stackToDelete: 'web' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(false); + }); + + expect(apiFetch).toHaveBeenCalledWith('/stacks/web', { method: 'DELETE' }); + expect(stackListState.setSelectedFile).toHaveBeenCalledWith(null); + expect(navState.setActiveView).toHaveBeenCalledWith('dashboard'); + expect(onDeletedOpenStack).toHaveBeenCalledTimes(1); + }); + + it('does not navigate when deleting a different stack', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const { result, stackListState, navState, onDeletedOpenStack } = setup({ + overlay: { stackToDelete: 'other.yml' }, + stackList: { + selectedFile: 'web.yml', + files: ['web.yml', 'other.yml'], + }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(false); + }); + + expect(stackListState.setSelectedFile).not.toHaveBeenCalledWith(null); + expect(navState.setActiveView).not.toHaveBeenCalled(); + expect(onDeletedOpenStack).not.toHaveBeenCalled(); + expect(stackListState.refreshStacks).toHaveBeenCalled(); + }); + + it('clears selection without navigating when the matching stack is hidden behind another view', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response(null, { status: 200 })); + const { result, stackListState, navState, onDeletedOpenStack } = setup({ + overlay: { stackToDelete: 'web.yml' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + navState: { activeView: 'resources' }, + }); + + await act(async () => { + await result.current.deleteStack(false); + }); + + expect(stackListState.setSelectedFile).toHaveBeenCalledWith(null); + expect(navState.setActiveView).not.toHaveBeenCalled(); + expect(onDeletedOpenStack).not.toHaveBeenCalled(); + expect(stackListState.refreshStacks).toHaveBeenCalled(); + }); + + it('does not reset or navigate on a non-OK delete response', async () => { + vi.mocked(apiFetch).mockResolvedValue(new Response('boom', { status: 500 })); + const { toast } = await import('@/components/ui/toast-store'); + const { result, stackListState, overlayState, navState, onDeletedOpenStack } = setup({ + overlay: { stackToDelete: 'web.yml' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(false); + }); + + expect(stackListState.setSelectedFile).not.toHaveBeenCalledWith(null); + expect(navState.setActiveView).not.toHaveBeenCalled(); + expect(onDeletedOpenStack).not.toHaveBeenCalled(); + expect(overlayState.closeDeleteDialog).not.toHaveBeenCalled(); + expect(stackListState.refreshStacks).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalled(); + }); + + it('does not navigate on a self-stack-protected response', async () => { + vi.mocked(apiFetch).mockResolvedValue( + new Response(JSON.stringify({ code: 'self_stack_protected' }), { status: 409 }), + ); + const { result, stackListState, overlayState, navState, onDeletedOpenStack } = setup({ + overlay: { stackToDelete: 'web.yml' }, + stackList: { selectedFile: 'web.yml', files: ['web.yml'] }, + navState: { activeView: 'editor' }, + }); + + await act(async () => { + await result.current.deleteStack(false); + }); + + expect(overlayState.openSelfStackProtected).toHaveBeenCalled(); + expect(overlayState.closeDeleteDialog).toHaveBeenCalled(); + expect(stackListState.setSelectedFile).not.toHaveBeenCalledWith(null); + expect(navState.setActiveView).not.toHaveBeenCalled(); + expect(onDeletedOpenStack).not.toHaveBeenCalled(); + expect(stackListState.refreshStacks).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/EditorLayout/hooks/useStackActions.ts b/frontend/src/components/EditorLayout/hooks/useStackActions.ts index db8458e0..3ced55ee 100644 --- a/frontend/src/components/EditorLayout/hooks/useStackActions.ts +++ b/frontend/src/components/EditorLayout/hooks/useStackActions.ts @@ -168,6 +168,13 @@ interface UseStackActionsOptions { canEditStack: (stackNameOrFilename: string) => boolean; /** Fail-closed: true only when active node meta explicitly lists stack-down-remove-volumes. */ canOfferVolumeRemoval?: boolean; + /** + * Mobile (and any shell-owned) cleanup after deleting the stack that is + * currently open in the editor. EditorLayout clears pending detail and + * flips to the stack list surface. Required: the sole production caller + * owns that state, and an optional callback would silently skip it. + */ + onDeletedOpenStack: () => void; } const isRecord = (value: unknown): value is Record => @@ -390,6 +397,7 @@ export function useStackActions(options: UseStackActionsOptions) { hasServiceScopedUpdate = false, canEditStack, canOfferVolumeRemoval = false, + onDeletedOpenStack, } = options; const pendingStackLoadRef = useRef(null); @@ -493,6 +501,10 @@ export function useStackActions(options: UseStackActionsOptions) { // previous node's data. loadFileAbortRef.current?.abort(); loadFileAbortRef.current = null; + // loadFileCore's finally skips clearing loading when the signal is aborted, + // so clear it here. Otherwise useUrlSync's writer stays blocked after a + // delete-leave (or any other reset) that aborts a mid-flight load. + editorState.setIsFileLoading(false); stackListState.setSelectedFile(null); editorState.setContent(''); editorState.setOriginalContent(''); @@ -1767,8 +1779,17 @@ export function useStackActions(options: UseStackActionsOptions) { } toast.success('Stack deleted successfully!'); overlayState.closeDeleteDialog(); - if (stackListState.selectedFile === stackToDelete) { + const selected = stackListState.selectedFile; + const stripExt = (name: string) => name.replace(/\.(yml|yaml)$/, ''); + // Always clear a deleted selection, even when another top-level view is + // visible (Resources, Networking, etc.). Leaving that view is gated on + // the editor being the active surface below. + if (selected != null && stripExt(selected) === stripExt(deleteKey)) { resetEditorState(); + if (navState.activeView === 'editor') { + navState.setActiveView('dashboard'); + onDeletedOpenStack(); + } } await stackListState.refreshStacks(); } catch (error) { diff --git a/frontend/src/components/EditorLayout/hooks/useUrlSync.test.ts b/frontend/src/components/EditorLayout/hooks/useUrlSync.test.ts index 0e7b5a0f..70432483 100644 --- a/frontend/src/components/EditorLayout/hooks/useUrlSync.test.ts +++ b/frontend/src/components/EditorLayout/hooks/useUrlSync.test.ts @@ -554,6 +554,76 @@ describe('useUrlSync', () => { pushSpy.mockRestore(); }); + it('writes dashboard URL after leaving a deleted editor stack', () => { + window.history.replaceState({ senchoIdx: 0 }, '', '/nodes/local/stacks/radarr'); + const pushSpy = vi.spyOn(window.history, 'pushState'); + + const { rerender } = renderHook( + (props) => useUrlSync(props), + { + initialProps: makeOpts({ + activeView: 'editor', + selectedFile: 'radarr', + files: ['radarr'], + }), + }, + ); + + act(() => { + rerender(makeOpts({ + activeView: 'dashboard', + selectedFile: null, + files: [], + })); + }); + + const paths = [ + ...pushSpy.mock.calls.map((call) => String(call[2] ?? '')), + window.location.pathname, + ]; + expect(paths.some((p) => p === '/nodes/local/dashboard')).toBe(true); + expect(window.location.pathname).not.toContain('/stacks/radarr'); + + pushSpy.mockRestore(); + }); + + it('writes mobile stack-list URL after leaving a deleted editor stack', () => { + window.history.replaceState({ senchoIdx: 0 }, '', '/nodes/local/stacks/radarr'); + const pushSpy = vi.spyOn(window.history, 'pushState'); + + const { rerender } = renderHook( + (props) => useUrlSync(props), + { + initialProps: makeOpts({ + activeView: 'editor', + selectedFile: 'radarr', + files: ['radarr'], + isMobile: true, + mobileSurface: 'detail', + }), + }, + ); + + act(() => { + rerender(makeOpts({ + activeView: 'dashboard', + selectedFile: null, + files: [], + isMobile: true, + mobileSurface: 'list', + })); + }); + + const paths = [ + ...pushSpy.mock.calls.map((call) => String(call[2] ?? '')), + window.location.pathname, + ]; + expect(paths.some((p) => p === '/nodes/local/stacks')).toBe(true); + expect(window.location.pathname).not.toContain('/stacks/radarr'); + + pushSpy.mockRestore(); + }); + it('opens Monaco when hydrating /compose deep link', async () => { const loadFileForRoute = vi.fn().mockResolvedValue({ ok: true, envFiles: [] }); const applyEditorRouteState = vi.fn();