diff --git a/src/components/builder/hooks/use-builder-clipboard.ts b/src/components/builder/hooks/use-builder-clipboard.ts index f5910e4..8a16fd9 100644 --- a/src/components/builder/hooks/use-builder-clipboard.ts +++ b/src/components/builder/hooks/use-builder-clipboard.ts @@ -15,14 +15,16 @@ type ClipboardPayload = { connections: Connection[] } -type SaveToHistory = (snapshot?: Workflow) => void +type SaveToHistory = () => void + +type MutateWorkflow = (id: string) => Promise export function useBuilderClipboard(options: { workflowId: string | null selectedNodeIds: string[] workflow: Workflow | null | undefined saveToHistory: SaveToHistory - mutateWorkflow: (id: string) => void + mutateWorkflow: MutateWorkflow safeFetch: SafeFetch toast: ToastFn }) { @@ -66,7 +68,6 @@ export function useBuilderClipboard(options: { toast({ title: "Nothing to paste", variant: "destructive" }) return } - const previous = workflow const response = await safeFetch(`/api/workflows/${workflowId}/paste`, { method: "POST", headers: { "Content-Type": "application/json" }, @@ -74,8 +75,8 @@ export function useBuilderClipboard(options: { }) if (response.ok) { const result = await response.json() - saveToHistory(previous) - mutateWorkflow(workflowId) + saveToHistory() + await mutateWorkflow(workflowId) toast({ title: `Pasted ${result.nodeIds?.length ?? 0} node(s)` }) } else { toast({ title: "Nothing to paste", variant: "destructive" }) @@ -85,7 +86,6 @@ export function useBuilderClipboard(options: { const duplicateNodeIds = useCallback( async (nodeIds: string[], successTitle: string) => { if (!nodeIds.length || !workflowId || !workflow) return - const previous = workflow const copied = await copyNodeIds(nodeIds) if (!copied || !clipboardRef.current) return const pasteRes = await safeFetch(`/api/workflows/${workflowId}/paste`, { @@ -97,8 +97,8 @@ export function useBuilderClipboard(options: { }), }) if (pasteRes.ok) { - saveToHistory(previous) - mutateWorkflow(workflowId) + saveToHistory() + await mutateWorkflow(workflowId) toast({ title: successTitle }) } }, diff --git a/src/components/builder/hooks/use-builder-graph-mutations.ts b/src/components/builder/hooks/use-builder-graph-mutations.ts index 4eda492..7c633b2 100644 --- a/src/components/builder/hooks/use-builder-graph-mutations.ts +++ b/src/components/builder/hooks/use-builder-graph-mutations.ts @@ -20,14 +20,16 @@ type ToastFn = (props: { variant?: "default" | "destructive" }) => void -type SaveToHistory = (snapshot?: Workflow) => void +type SaveToHistory = () => void + +type MutateWorkflow = (id: string) => Promise export function useBuilderGraphMutations(options: { workflowId: string | null workflow: Workflow | null | undefined edges: Edge[] saveToHistory: SaveToHistory - mutateWorkflow: (id: string) => void + mutateWorkflow: MutateWorkflow safeFetch: SafeFetch toast: ToastFn screenToFlowPosition: (position: XYPosition) => XYPosition @@ -62,14 +64,15 @@ export function useBuilderGraphMutations(options: { const handleNodeDeleteById = useCallback( async (nodeId: string) => { if (!workflowId || !workflow) return - const previous = workflow const response = await safeFetch(`/api/workflows/${workflowId}/nodes/${nodeId}`, { method: "DELETE", }) if (response.ok) { - saveToHistory(previous) + saveToHistory() + await mutateWorkflow(workflowId) + return } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) }, [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch], ) @@ -77,18 +80,17 @@ export function useBuilderGraphMutations(options: { const handleAssignToFrame = useCallback( async (nodeId: string, frameId: string) => { if (!workflowId || !workflow) return - const previous = workflow const response = await safeFetch(`/api/workflows/${workflowId}/nodes/${nodeId}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ parentId: frameId }), }) if (!response.ok) { - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) return } - saveToHistory(previous) - mutateWorkflow(workflowId) + saveToHistory() + await mutateWorkflow(workflowId) toast({ title: "Node added to frame" }) }, [workflowId, workflow, saveToHistory, mutateWorkflow, toast, safeFetch], @@ -97,18 +99,17 @@ export function useBuilderGraphMutations(options: { const handleRemoveFromFrame = useCallback( async (nodeId: string) => { if (!workflowId || !workflow) return - const previous = workflow const response = await safeFetch(`/api/workflows/${workflowId}/nodes/${nodeId}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ parentId: null }), }) if (!response.ok) { - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) return } - saveToHistory(previous) - mutateWorkflow(workflowId) + saveToHistory() + await mutateWorkflow(workflowId) toast({ title: "Node removed from frame" }) }, [workflowId, workflow, saveToHistory, mutateWorkflow, toast, safeFetch], @@ -119,16 +120,17 @@ export function useBuilderGraphMutations(options: { if (!workflowId || !workflow) return const node = workflow.nodes.find((n) => n.id === nodeId) if (!node) return - const previous = workflow const response = await safeFetch(`/api/workflows/${workflowId}/nodes/${nodeId}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ data: { ...node.data, label: newLabel } }), }) if (response.ok) { - saveToHistory(previous) + saveToHistory() + await mutateWorkflow(workflowId) + return } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) }, [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch], ) @@ -145,17 +147,18 @@ export function useBuilderGraphMutations(options: { onEdgesChange(changes) const removeChanges = changes.filter((c) => c.type === "remove") as { id: string }[] if (removeChanges.length > 0 && workflowId && workflow) { - const previous = workflow const updatedEdges = edges.filter((e) => !removeChanges.some((r) => r.id === e.id)) void safeFetch(`/api/workflows/${workflowId}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ connections: reactFlowEdgesToConnections(updatedEdges) }), - }).then((response) => { + }).then(async (response) => { if (response.ok) { - saveToHistory(previous) + saveToHistory() + await mutateWorkflow(workflowId) + return } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) }) } }, @@ -165,7 +168,6 @@ export function useBuilderGraphMutations(options: { const handleConnect = useCallback( async (connection: ReactFlowConnection) => { if (!workflowId || !workflow || !connection.source || !connection.target) return - const previous = workflow const response = await safeFetch(`/api/workflows/${workflowId}/connections`, { method: "POST", headers: { "Content-Type": "application/json" }, @@ -177,9 +179,11 @@ export function useBuilderGraphMutations(options: { }), }) if (response.ok) { - saveToHistory(previous) + saveToHistory() + await mutateWorkflow(workflowId) + return } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) }, [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch], ) @@ -187,7 +191,6 @@ export function useBuilderGraphMutations(options: { const handleNodeDragStop = useCallback( async (_: React.MouseEvent, node: Node) => { if (!workflowId || !workflow) return - const previous = workflow const snappedPosition = { x: Math.round(node.position.x / GRID_SIZE) * GRID_SIZE, y: Math.round(node.position.y / GRID_SIZE) * GRID_SIZE, @@ -198,9 +201,11 @@ export function useBuilderGraphMutations(options: { body: JSON.stringify({ position: snappedPosition }), }) if (response.ok) { - saveToHistory(previous) + saveToHistory() + await mutateWorkflow(workflowId) + return } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) }, [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch], ) @@ -208,7 +213,6 @@ export function useBuilderGraphMutations(options: { const handleAddNode = useCallback( async (type: NodeType, position?: Position) => { if (!workflowId || !workflow) return - const previous = workflow let posX: number let posY: number @@ -263,9 +267,11 @@ export function useBuilderGraphMutations(options: { body: JSON.stringify(nodePayload), }) if (response.ok) { - saveToHistory(previous) + saveToHistory() + await mutateWorkflow(workflowId) + return } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) }, [workflowId, workflow, saveToHistory, mutateWorkflow, screenToFlowPosition, safeFetch], ) @@ -280,12 +286,11 @@ export function useBuilderGraphMutations(options: { clearTimeout(layoutTransitionTimeoutRef.current) layoutTransitionTimeoutRef.current = null } - const previous = workflow setIsLayoutTransitioning(true) const response = await safeFetch(`/api/workflows/${workflowId}/auto-layout`, { method: "POST" }) if (response.ok) { - saveToHistory(previous) - mutateWorkflow(workflowId) + saveToHistory() + await mutateWorkflow(workflowId) toast({ title: "Layout applied successfully" }) layoutTransitionTimeoutRef.current = setTimeout(() => { layoutTransitionTimeoutRef.current = null @@ -301,16 +306,17 @@ export function useBuilderGraphMutations(options: { async (nodeIds: string | string[] | null) => { const ids = Array.isArray(nodeIds) ? nodeIds : nodeIds ? [nodeIds] : [] if (!ids.length || !workflowId || !workflow) return - const previous = workflow const results = await Promise.all( ids.map((nodeId) => safeFetch(`/api/workflows/${workflowId}/nodes/${nodeId}`, { method: "DELETE" }), ), ) if (results.every((response) => response.ok)) { - saveToHistory(previous) + saveToHistory() + await mutateWorkflow(workflowId) + return } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) }, [workflowId, workflow, saveToHistory, mutateWorkflow, safeFetch], ) diff --git a/src/components/builder/hooks/use-builder-history.ts b/src/components/builder/hooks/use-builder-history.ts index 7606ba5..0b53878 100644 --- a/src/components/builder/hooks/use-builder-history.ts +++ b/src/components/builder/hooks/use-builder-history.ts @@ -79,19 +79,30 @@ export function useBuilderHistory(options: { const canUndo = canUndoBit === "1" const canRedo = canRedoBit === "1" - const mutateWorkflow = useCallback((id: string) => { - mutate(`/api/workflows/${id}`) + const mutateWorkflow = useCallback(async (id: string) => { + const fresh = await mutate( + `/api/workflows/${id}`, + async () => { + const response = await fetch(`/api/workflows/${id}`) + if (!response.ok) { + throw new Error(await response.text()) + } + return (await response.json()) as Workflow + }, + { revalidate: true }, + ) + if (fresh) { + lastPersistedRef.current = fresh + lastSyncedUpdatedAtRef.current = workflowUpdatedAtMs(fresh) + } }, []) - const saveToHistory = useCallback( - (snapshot?: Workflow) => { - const toSave = snapshot ?? workflow - if (toSave && workflowId) { - getHistoryManager(workflowId).saveState(toSave) - } - }, - [workflow, workflowId], - ) + const saveToHistory = useCallback(() => { + const toSave = lastPersistedRef.current ?? workflow + if (toSave && workflowId) { + getHistoryManager(workflowId).saveState(toSave) + } + }, [workflow, workflowId]) const applyHistoryTransition = useCallback( async (direction: "undo" | "redo") => { @@ -158,7 +169,6 @@ export function useBuilderHistory(options: { if (transitionInFlightRef.current) return transitionInFlightRef.current = true setIsHistoryTransitioning(true) - const previous = workflow try { const response = await safeFetch(`/api/workflows/${workflowId}`, { method: "PATCH", @@ -169,13 +179,13 @@ export function useBuilderHistory(options: { }), }) if (!response.ok) return - getHistoryManager(workflowId).saveState(previous) + saveToHistory() lastPersistedRef.current = { - ...previous, + ...workflow, nodes: version.nodes, connections: version.connections, } - mutateWorkflow(workflowId) + void mutateWorkflow(workflowId) toast({ title: "Version restored", description: `Restored ${version.name}`, @@ -187,7 +197,7 @@ export function useBuilderHistory(options: { setIsHistoryTransitioning(false) } }, - [workflowId, workflow, mutateWorkflow, safeFetch, toast], + [workflowId, workflow, mutateWorkflow, safeFetch, toast, saveToHistory], ) return { diff --git a/src/lib/history-manager.ts b/src/lib/history-manager.ts index 47f113c..b0649c7 100644 --- a/src/lib/history-manager.ts +++ b/src/lib/history-manager.ts @@ -31,8 +31,14 @@ export class HistoryManager { } saveState(workflow: Workflow) { + const snapshot = JSON.parse(JSON.stringify(workflow)) as Workflow + const top = this.undoStack[this.undoStack.length - 1] + if (top && JSON.stringify(top.workflow) === JSON.stringify(snapshot)) { + return + } + this.undoStack.push({ - workflow: JSON.parse(JSON.stringify(workflow)), + workflow: snapshot, timestamp: Date.now(), }) diff --git a/tests/components/builder-canvas.test.tsx b/tests/components/builder-canvas.test.tsx index 76248f4..a0ec20e 100644 --- a/tests/components/builder-canvas.test.tsx +++ b/tests/components/builder-canvas.test.tsx @@ -426,7 +426,25 @@ describe("BuilderCanvas", () => { resolvePatch = resolve }) - global.fetch = vi.fn().mockReturnValue(deferredPatch) as unknown as typeof fetch + global.fetch = vi + .fn() + .mockReturnValueOnce(deferredPatch) + .mockResolvedValue({ + ok: true, + status: 200, + json: async () => mockWorkflow, + clone() { + return this + }, + text: async () => "", + }) as unknown as typeof fetch + + vi.mocked(mutate).mockImplementation(async (_key, fetcher) => { + if (typeof fetcher === "function") { + return fetcher() + } + return undefined + }) render() @@ -453,10 +471,14 @@ describe("BuilderCanvas", () => { }) await waitFor(() => { - expect(mutate).toHaveBeenCalledWith("/api/workflows/wf-1") + expect(mutate).toHaveBeenCalledWith( + "/api/workflows/wf-1", + expect.any(Function), + { revalidate: true }, + ) }) - expect(global.fetch).toHaveBeenCalledTimes(1) + expect(global.fetch).toHaveBeenCalledTimes(2) expect(getHistoryManager("wf-1").canUndo()).toBe(false) expect(getHistoryManager("wf-1").canRedo()).toBe(true) }) diff --git a/tests/lib/history-manager.test.ts b/tests/lib/history-manager.test.ts index 1450321..e6bc0d7 100644 --- a/tests/lib/history-manager.test.ts +++ b/tests/lib/history-manager.test.ts @@ -107,6 +107,17 @@ describe("HistoryManager", () => { expect(restored?.nodes).toEqual([makeNode("n1")]) }) + it("skips duplicate consecutive snapshots", () => { + const v0 = makeWorkflow() + manager.saveState(v0) + manager.saveState(v0) + expect(manager.canUndo()).toBe(true) + + const withTwoNodes = makeWorkflow({ nodes: [makeNode("n1"), makeNode("n2")] }) + expect(manager.undo(withTwoNodes)?.nodes).toEqual([]) + expect(manager.canUndo()).toBe(false) + }) + it("notifies subscribers when stacks change", () => { const seen: number[] = [] const unsubscribe = manager.subscribe(() => {