mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-06 17:08:10 +00:00
fix(stacks): close dialog and toast on Empty create (F-2)
The Empty branch of the Create Stack dialog left no visual confirmation on a successful POST: no success toast, no busy state on the Create button, and no double-click guard. The first thing a new operator does is create a stack, so a click that looks like a no-op is the day-one impression killer. What changed - Empty handler now mirrors the Git and docker-run patterns: a creatingEmpty busy guard, Loader2 spinner on the Create button, disabled Cancel mid-flight, and a "Stack <name> created." success toast fired synchronously before the editor-load handoff. - The Empty panel is wrapped in a form so the Enter key submits the same path as clicking Create. - Mapped 403 to a clearer permission message; fall back to the backend's error string for other non-OK statuses. - Capture the active node id at handler entry and pass it through onStackCreated. The parent compares against the live node ref and skips the editor load if the user switched nodes mid-create, with an info toast pointing at the prior node. - Tightened the create-a-new-stack E2E (no more silent .catch on the dialog-close assertion, asserts the toast, no longer relies on a page reload to see the sidebar entry) and added a regression spec that the busy guard collapses double-clicks into a single POST.
This commit is contained in:
SaelixCode
+46
-7
@@ -35,18 +35,57 @@ test.describe('Stack management', () => {
|
||||
await page.locator('#create-stack-name').fill(TEST_STACK);
|
||||
await page.locator('[role="dialog"]').getByRole('button', { name: 'Create' }).click();
|
||||
|
||||
// Wait for dialog to close (success) or error message to appear (failure)
|
||||
await Promise.race([
|
||||
page.getByRole('dialog').waitFor({ state: 'hidden', timeout: 8_000 }),
|
||||
page.getByText(/already exists/i).waitFor({ state: 'visible', timeout: 8_000 }),
|
||||
]).catch(() => { });
|
||||
// F-2: the dialog MUST close on a successful create. No silent .catch swallow.
|
||||
await expect(page.getByRole('dialog')).toBeHidden({ timeout: 8_000 });
|
||||
|
||||
// The stack should now exist - refresh and verify via the sidebar
|
||||
// F-2: a success toast MUST appear so the click does not feel like a no-op.
|
||||
await expect(page.getByText(`Stack "${TEST_STACK}" created.`)).toBeVisible({ timeout: 5_000 });
|
||||
|
||||
// F-2: the new stack appears in the sidebar without a manual reload.
|
||||
await expect(
|
||||
page.locator('[role="listbox"]').getByText(TEST_STACK, { exact: true })
|
||||
).toBeVisible({ timeout: 5_000 });
|
||||
});
|
||||
|
||||
test('create dialog: double-clicking Create fires only one POST', async ({ page }) => {
|
||||
const stackName = 'e2e-double-click-stack';
|
||||
|
||||
// Clean any prior leftover and reload so we start from a known sidebar state.
|
||||
await page.evaluate(async (name) => {
|
||||
await fetch(`/api/stacks/${name}`, { method: 'DELETE', credentials: 'include' }).catch(() => { });
|
||||
}, stackName);
|
||||
await page.reload();
|
||||
await loginAs(page);
|
||||
await waitForStacksLoaded(page);
|
||||
|
||||
await expect(page.getByText(TEST_STACK).first()).toBeVisible({ timeout: 5_000 });
|
||||
// Count POST /api/stacks calls (exact path; the regex anchors avoid matching
|
||||
// /api/stacks/<name>/deploy and similar nested endpoints).
|
||||
let postCount = 0;
|
||||
await page.route('**/api/stacks', (route) => {
|
||||
if (route.request().method() === 'POST') postCount += 1;
|
||||
void route.continue();
|
||||
});
|
||||
|
||||
try {
|
||||
await page.getByRole('button', { name: 'Create Stack' }).click();
|
||||
await expect(page.getByRole('dialog')).toBeVisible({ timeout: 5_000 });
|
||||
await page.locator('#create-stack-name').fill(stackName);
|
||||
|
||||
const createBtn = page.locator('[role="dialog"]').getByRole('button', { name: /^Create/ });
|
||||
// Rapid double-click; the busy guard must reject the second click synchronously.
|
||||
await createBtn.click();
|
||||
await createBtn.click({ force: true }).catch(() => { /* second click may land on disabled btn */ });
|
||||
|
||||
await expect(page.getByRole('dialog')).toBeHidden({ timeout: 8_000 });
|
||||
await expect(page.getByText(`Stack "${stackName}" created.`)).toBeVisible({ timeout: 5_000 });
|
||||
expect(postCount).toBe(1);
|
||||
} finally {
|
||||
await page.unroute('**/api/stacks');
|
||||
// Cleanup
|
||||
await page.evaluate(async (name) => {
|
||||
await fetch(`/api/stacks/${name}`, { method: 'DELETE', credentials: 'include' }).catch(() => { });
|
||||
}, stackName);
|
||||
}
|
||||
});
|
||||
|
||||
test('delete the test stack', async ({ page }) => {
|
||||
|
||||
@@ -33,6 +33,7 @@ import { useTrivyStatus } from '@/hooks/useTrivyStatus';
|
||||
import { StackSidebar } from '@/components/sidebar/StackSidebar';
|
||||
import type { StackRowStatus } from '@/components/sidebar/stack-status-utils';
|
||||
import { useComposeDiffPreviewEnabled } from '@/hooks/use-compose-diff-preview-enabled';
|
||||
import { toast } from '@/components/ui/toast-store';
|
||||
|
||||
export default function EditorLayout() {
|
||||
const { isAdmin, can } = useAuth();
|
||||
@@ -91,6 +92,15 @@ export default function EditorLayout() {
|
||||
|
||||
const { nodes, activeNode, setActiveNode } = useNodes();
|
||||
|
||||
// Mirror activeNode.id in a ref so async handlers (e.g. CreateStackDialog's
|
||||
// post-create handoff) can detect a node switch that happened mid-flight.
|
||||
// Closure capture of activeNode would always match the value at handler-creation
|
||||
// time and miss the switch.
|
||||
const activeNodeIdRef = useRef<number | null>(activeNode?.id ?? null);
|
||||
useEffect(() => {
|
||||
activeNodeIdRef.current = activeNode?.id ?? null;
|
||||
}, [activeNode?.id]);
|
||||
|
||||
const overlayState = useOverlayState();
|
||||
const {
|
||||
createDialogOpen, setCreateDialogOpen,
|
||||
@@ -227,8 +237,16 @@ export default function EditorLayout() {
|
||||
<CreateStackDialog
|
||||
open={createDialogOpen}
|
||||
onOpenChange={setCreateDialogOpen}
|
||||
onStackCreated={async (sName) => {
|
||||
onStackCreated={async (sName, sourceNodeId) => {
|
||||
await refreshStacks();
|
||||
// loadFile keeps its own unsaved-changes overlay (intentional safety,
|
||||
// shared with every other "switch to a different stack" code path).
|
||||
// Skip the load if the user switched nodes mid-create so we do not
|
||||
// 404 against a stack name that lives on the previous node.
|
||||
if (sourceNodeId != null && activeNodeIdRef.current !== sourceNodeId) {
|
||||
toast.info(`Stack "${sName}" created on the previous node.`);
|
||||
return;
|
||||
}
|
||||
await stackActions.loadFile(sName);
|
||||
}}
|
||||
onStacksChanged={async () => { await refreshStacks(); }}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useRef, useState, type KeyboardEvent } from 'react';
|
||||
import { useRef, useState, type FormEvent, type KeyboardEvent } from 'react';
|
||||
import { Plus, GitBranch, FileCode2, Loader2, type LucideIcon } from 'lucide-react';
|
||||
import { Modal, ModalHeader, ModalBody, ModalFooter } from '../ui/modal';
|
||||
import { Button } from '../ui/button';
|
||||
@@ -9,12 +9,16 @@ import { Checkbox } from '../ui/checkbox';
|
||||
import { GitSourceFields, type ApplyMode } from '../stack/GitSourceFields';
|
||||
import { apiFetch } from '@/lib/api';
|
||||
import { toast } from '@/components/ui/toast-store';
|
||||
import { useNodes } from '@/context/NodeContext';
|
||||
import { cn } from '@/lib/utils';
|
||||
|
||||
export interface CreateStackDialogProps {
|
||||
open: boolean;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
onStackCreated: (stackName: string) => void | Promise<void>;
|
||||
// sourceNodeId is the active node ID captured at the moment the user clicked
|
||||
// Create. Parent compares against the current active node before navigating
|
||||
// so a mid-flight node switch does not land the user on a 404.
|
||||
onStackCreated: (stackName: string, sourceNodeId: number | null | undefined) => void | Promise<void>;
|
||||
onStacksChanged: () => void | Promise<void>;
|
||||
}
|
||||
|
||||
@@ -30,8 +34,10 @@ const tabId = (m: CreateMode) => `create-stack-tab-${m}`;
|
||||
const panelId = (m: CreateMode) => `create-stack-panel-${m}`;
|
||||
|
||||
export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacksChanged }: CreateStackDialogProps) {
|
||||
const { activeNode } = useNodes();
|
||||
const [createMode, setCreateMode] = useState<CreateMode>('empty');
|
||||
const [newStackName, setNewStackName] = useState('');
|
||||
const [creatingEmpty, setCreatingEmpty] = useState(false);
|
||||
const [dockerRunInput, setDockerRunInput] = useState('');
|
||||
const [convertedYaml, setConvertedYaml] = useState<string | null>(null);
|
||||
const [isConverting, setIsConverting] = useState(false);
|
||||
@@ -66,8 +72,11 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
};
|
||||
|
||||
const handleCreateStack = async () => {
|
||||
if (creatingEmpty) return;
|
||||
if (!newStackName.trim()) return;
|
||||
const stackName = newStackName.trim();
|
||||
const sourceNodeId = activeNode?.id;
|
||||
setCreatingEmpty(true);
|
||||
try {
|
||||
const response = await apiFetch('/stacks', {
|
||||
method: 'POST',
|
||||
@@ -75,21 +84,35 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
});
|
||||
if (!response.ok) {
|
||||
if (response.status === 409) {
|
||||
throw new Error('Stack already exists');
|
||||
} else if (response.status === 400) {
|
||||
throw new Error('Invalid stack name (use alphanumeric characters and hyphens only)');
|
||||
throw new Error('Stack already exists.');
|
||||
}
|
||||
throw new Error('Failed to create stack');
|
||||
if (response.status === 400) {
|
||||
throw new Error('Invalid stack name (use alphanumeric characters and hyphens only).');
|
||||
}
|
||||
if (response.status === 403) {
|
||||
throw new Error('You do not have permission to create stacks.');
|
||||
}
|
||||
const body = await response.json().catch(() => ({}));
|
||||
throw new Error((body as { error?: string })?.error || 'Failed to create stack.');
|
||||
}
|
||||
onOpenChange(false);
|
||||
setNewStackName('');
|
||||
await onStackCreated(stackName);
|
||||
setCreateMode('empty');
|
||||
toast.success(`Stack "${stackName}" created.`);
|
||||
await onStackCreated(stackName, sourceNodeId);
|
||||
} catch (error) {
|
||||
console.error('Failed to create stack:', error);
|
||||
toast.error((error as Error).message || 'Failed to create stack');
|
||||
toast.error((error as Error).message || 'Failed to create stack.');
|
||||
} finally {
|
||||
setCreatingEmpty(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleEmptyFormSubmit = (e: FormEvent<HTMLFormElement>) => {
|
||||
e.preventDefault();
|
||||
void handleCreateStack();
|
||||
};
|
||||
|
||||
const handleCreateStackFromGit = async () => {
|
||||
const stackName = newStackName.trim();
|
||||
if (!stackName) {
|
||||
@@ -104,6 +127,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
toast.error('Only HTTPS repository URLs are supported.');
|
||||
return;
|
||||
}
|
||||
const sourceNodeId = activeNode?.id;
|
||||
setCreatingFromGit(true);
|
||||
const loadingId = toast.loading(gitDeployNow ? 'Fetching, creating, and deploying...' : 'Fetching and creating stack...');
|
||||
try {
|
||||
@@ -154,7 +178,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
}
|
||||
onOpenChange(false);
|
||||
resetCreateFromGitForm();
|
||||
await onStackCreated(stackName);
|
||||
await onStackCreated(stackName, sourceNodeId);
|
||||
} catch (error) {
|
||||
console.error('Failed to create stack from Git:', error);
|
||||
toast.error((error as Error)?.message || 'Failed to create stack from Git.');
|
||||
@@ -209,6 +233,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
toast.error('Convert the command before creating the stack.');
|
||||
return;
|
||||
}
|
||||
const sourceNodeId = activeNode?.id;
|
||||
setCreatingFromDockerRun(true);
|
||||
const loadingId = toast.loading('Creating stack from converted YAML...');
|
||||
let createdStack = false;
|
||||
@@ -245,7 +270,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
onOpenChange(false);
|
||||
resetCreateFromDockerRunForm();
|
||||
setNewStackName('');
|
||||
await onStackCreated(stackName);
|
||||
await onStackCreated(stackName, sourceNodeId);
|
||||
} catch (error) {
|
||||
console.error('Failed to create stack from docker run:', error);
|
||||
const err = error as { message?: string; error?: string; data?: { error?: string } };
|
||||
@@ -265,7 +290,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
}
|
||||
};
|
||||
|
||||
const busy = creatingFromGit || creatingFromDockerRun;
|
||||
const busy = creatingEmpty || creatingFromGit || creatingFromDockerRun;
|
||||
|
||||
return (
|
||||
<Modal
|
||||
@@ -275,6 +300,7 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
onOpenChange(o);
|
||||
if (!o) {
|
||||
setCreateMode('empty');
|
||||
setCreatingEmpty(false);
|
||||
resetCreateFromGitForm();
|
||||
resetCreateFromDockerRunForm();
|
||||
}
|
||||
@@ -289,31 +315,43 @@ export function CreateStackDialog({ open, onOpenChange, onStackCreated, onStacks
|
||||
|
||||
{createMode === 'empty' && (
|
||||
<div role="tabpanel" id={panelId('empty')} aria-labelledby={tabId('empty')}>
|
||||
<ModalBody>
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="create-stack-name">Stack Name</Label>
|
||||
<Input
|
||||
id="create-stack-name"
|
||||
placeholder="Stack name (e.g., myapp)"
|
||||
value={newStackName}
|
||||
onChange={(e) => setNewStackName(e.target.value)}
|
||||
autoFocus
|
||||
/>
|
||||
</div>
|
||||
</ModalBody>
|
||||
<ModalFooter
|
||||
hint="ALPHANUMERIC · HYPHENS"
|
||||
secondary={
|
||||
<Button type="button" variant="ghost" onClick={() => onOpenChange(false)}>
|
||||
Cancel
|
||||
</Button>
|
||||
}
|
||||
primary={
|
||||
<Button onClick={handleCreateStack} disabled={!newStackName.trim()}>
|
||||
Create
|
||||
</Button>
|
||||
}
|
||||
/>
|
||||
<form onSubmit={handleEmptyFormSubmit}>
|
||||
<ModalBody>
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="create-stack-name">Stack Name</Label>
|
||||
<Input
|
||||
id="create-stack-name"
|
||||
placeholder="Stack name (e.g., myapp)"
|
||||
value={newStackName}
|
||||
onChange={(e) => setNewStackName(e.target.value)}
|
||||
disabled={creatingEmpty}
|
||||
autoFocus
|
||||
/>
|
||||
</div>
|
||||
</ModalBody>
|
||||
<ModalFooter
|
||||
hint="ALPHANUMERIC · HYPHENS"
|
||||
secondary={
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
onClick={() => onOpenChange(false)}
|
||||
disabled={creatingEmpty}
|
||||
>
|
||||
Cancel
|
||||
</Button>
|
||||
}
|
||||
primary={
|
||||
<Button type="submit" disabled={creatingEmpty || !newStackName.trim()}>
|
||||
{creatingEmpty ? (
|
||||
<><Loader2 className="w-4 h-4 mr-1.5 animate-spin" strokeWidth={1.5} />Creating</>
|
||||
) : (
|
||||
<><Plus className="w-4 h-4 mr-1.5" strokeWidth={1.5} />Create</>
|
||||
)}
|
||||
</Button>
|
||||
}
|
||||
/>
|
||||
</form>
|
||||
</div>
|
||||
)}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user