diff --git a/frontend/src/components/blueprints/BlueprintDetail.test.tsx b/frontend/src/components/blueprints/BlueprintDetail.test.tsx index 9bf51774..884dcf7c 100644 --- a/frontend/src/components/blueprints/BlueprintDetail.test.tsx +++ b/frontend/src/components/blueprints/BlueprintDetail.test.tsx @@ -7,7 +7,7 @@ * of them, so the sheet can never issue a request the API answers with 403. */ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import type { BlueprintSummary } from '@/lib/blueprintsApi'; vi.mock('@/lib/blueprintsApi', async (importOriginal) => { @@ -57,6 +57,79 @@ beforeEach(() => { vi.mocked(getBlueprint).mockResolvedValue(summary()); }); +describe('BlueprintDetail data fetching', () => { + it('does not refetch when the parent re-renders with new callback identities', async () => { + const { rerender } = render( + {}} onChanged={noop} canEdit distinctLabels={[]} />, + ); + + // Let the initial load settle so the body content is on screen. + expect(await screen.findByText('Show compose source')).toBeInTheDocument(); + const callsAfterLoad = vi.mocked(getBlueprint).mock.calls.length; + + // A parent re-render (e.g. the Fleet view's polling) hands the open sheet a + // brand-new onOpenChange closure every render. Before the fix that closure was + // a refresh dependency, so the load effect re-ran on every parent render and + // flickered the body through its loading skeleton. It must now keep showing the + // data it already has instead of refetching. + rerender( + {}} onChanged={noop} canEdit distinctLabels={[]} />, + ); + rerender( + {}} onChanged={noop} canEdit distinctLabels={[]} />, + ); + await Promise.resolve(); + + expect(vi.mocked(getBlueprint)).toHaveBeenCalledTimes(callsAfterLoad); + }); + + it('refetches when blueprintId changes while the sheet stays open', async () => { + const { rerender } = render( + , + ); + expect(await screen.findByText('Show compose source')).toBeInTheDocument(); + const callsAfterLoad = vi.mocked(getBlueprint).mock.calls.length; + + // Opening a different blueprint without closing the sheet must load the new one, + // so blueprintId has to stay a refresh dependency. + rerender( + , + ); + await screen.findByText('Show compose source'); + + expect(vi.mocked(getBlueprint)).toHaveBeenCalledTimes(callsAfterLoad + 1); + expect(vi.mocked(getBlueprint)).toHaveBeenLastCalledWith(2); + }); + + it('keeps the loaded body on screen while a refresh is in flight', async () => { + let settleRefresh = () => {}; + vi.mocked(getBlueprint) + .mockResolvedValueOnce(summary()) + .mockImplementationOnce( + () => new Promise((resolve) => { settleRefresh = () => resolve(summary()); }), + ); + + render( + , + ); + expect(await screen.findByText('Show compose source')).toBeInTheDocument(); + const callsAfterLoad = vi.mocked(getBlueprint).mock.calls.length; + + // Applying reloads the blueprint. The populated body must stay mounted instead + // of collapsing to the loading skeleton while that reload is in flight. + fireEvent.click(screen.getByRole('button', { name: /apply now/i })); + await waitFor(() => expect(vi.mocked(getBlueprint).mock.calls.length).toBe(callsAfterLoad + 1)); + + // The deployment table only renders in the loaded body branch, never in the + // skeleton, so its presence proves the skeleton did not take over the refresh. + expect(screen.getByText('Show compose source')).toBeInTheDocument(); + expect(screen.getByTestId('deployment-table')).toBeInTheDocument(); + + settleRefresh(); + await screen.findByText('Show compose source'); + }); +}); + describe('BlueprintDetail action gating', () => { it('shows the Apply / Edit / Delete actions for an admin (canEdit)', async () => { render( diff --git a/frontend/src/components/blueprints/BlueprintDetail.tsx b/frontend/src/components/blueprints/BlueprintDetail.tsx index 686e6997..a31f0c89 100644 --- a/frontend/src/components/blueprints/BlueprintDetail.tsx +++ b/frontend/src/components/blueprints/BlueprintDetail.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState, useCallback } from 'react'; +import { useEffect, useRef, useState, useCallback } from 'react'; import { Pencil, Pin, Play, Power, Trash2 } from 'lucide-react'; import { SystemSheet, SheetSection } from '@/components/ui/system-sheet'; import { Modal, ModalDestructiveHeader, ModalBody, ModalFooter } from '@/components/ui/modal'; @@ -48,6 +48,12 @@ export function BlueprintDetail({ blueprintId, open, onOpenChange, onChanged, ca const [deleteConfirmText, setDeleteConfirmText] = useState(''); const { nodes } = useNodes(); + // Hold the latest onOpenChange without making it a refresh dependency. Parents + // pass a fresh closure on every render, so binding refresh to it would re-run the + // load effect on each parent render and flicker the body through its skeleton. + const onOpenChangeRef = useRef(onOpenChange); + useEffect(() => { onOpenChangeRef.current = onOpenChange; }, [onOpenChange]); + const refresh = useCallback(async () => { setLoading(true); try { @@ -55,11 +61,11 @@ export function BlueprintDetail({ blueprintId, open, onOpenChange, onChanged, ca setSummary(result); } catch (err) { toast.error(err instanceof Error ? err.message : 'Failed to load blueprint'); - onOpenChange(false); + onOpenChangeRef.current(false); } finally { setLoading(false); } - }, [blueprintId, onOpenChange]); + }, [blueprintId]); useEffect(() => { if (open) { @@ -245,7 +251,7 @@ export function BlueprintDetail({ blueprintId, open, onOpenChange, onChanged, ca footerContext={footerContext} size="lg" > - {loading || !blueprint || !summary ? ( + {!blueprint || !summary ? (
diff --git a/frontend/src/components/blueprints/StateReviewDialog.test.tsx b/frontend/src/components/blueprints/StateReviewDialog.test.tsx new file mode 100644 index 00000000..34135e0e --- /dev/null +++ b/frontend/src/components/blueprints/StateReviewDialog.test.tsx @@ -0,0 +1,30 @@ +/** + * The fresh-deploy action triggers a remote deploy that can take several seconds. + * The dialog must show an in-progress indicator while it runs, otherwise the click + * looks like it did nothing. + */ +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { StateReviewDialog } from './StateReviewDialog'; + +const baseProps = { + open: true, + onOpenChange: () => {}, + blueprintName: 'heimdall', + nodeName: 'sencho-test-01', + onAccept: () => {}, +}; + +describe('StateReviewDialog', () => { + it('offers the fresh deploy action when idle', () => { + render(); + expect(screen.getByText('Deploy fresh')).toBeInTheDocument(); + expect(screen.queryByText(/deploying/i)).not.toBeInTheDocument(); + }); + + it('shows an in-progress indicator and disables the action while deploying', () => { + render(); + expect(screen.getByText(/deploying/i)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /deploying/i })).toBeDisabled(); + }); +}); diff --git a/frontend/src/components/blueprints/StateReviewDialog.tsx b/frontend/src/components/blueprints/StateReviewDialog.tsx index bb9968c7..bc1a45f3 100644 --- a/frontend/src/components/blueprints/StateReviewDialog.tsx +++ b/frontend/src/components/blueprints/StateReviewDialog.tsx @@ -1,4 +1,4 @@ -import { Database, Play } from 'lucide-react'; +import { Database, Loader2, Play } from 'lucide-react'; import { Modal, ModalHeader, ModalBody, ModalFooter } from '@/components/ui/modal'; import { Button } from '@/components/ui/button'; @@ -31,11 +31,20 @@ export function StateReviewDialog({ type="button" onClick={() => onAccept('fresh')} disabled={busy} - className="w-full text-left rounded-lg border border-card-border border-t-card-border-top bg-card hover:border-t-card-border-hover transition-colors p-3 cursor-pointer" + className="w-full text-left rounded-lg border border-card-border border-t-card-border-top bg-card hover:border-t-card-border-hover transition-colors p-3 cursor-pointer disabled:opacity-60 disabled:cursor-not-allowed" >
- - Deploy fresh + {busy ? ( + <> + + Deploying… + + ) : ( + <> + + Deploy fresh + + )}

Create empty named volumes on this node. The container starts with whatever default state its image carries.