From a3e5f7fce21357d909d345e1bbe0be4ed74dd8c3 Mon Sep 17 00:00:00 2001 From: Abhimanyu Yadav <122007096+Abhi1992002@users.noreply.github.com> Date: Wed, 12 Nov 2025 19:46:30 +0530 Subject: [PATCH] fix(frontend): consolidate graph save logic to prevent duplicate event listeners (#11367) ## Summary This PR fixes an issue where multiple keyboard save event listeners were being registered when the same save hook was used in multiple components, causing the graph to be saved multiple times (3x) when using Ctrl/Cmd+S. ## Changes - **Created a centralized `useSaveGraph` hook** in `/hooks/useSaveGraph.ts` that encapsulates all graph saving logic - **Refactored `useNewSaveControl`** to use the new centralized hook instead of duplicating save logic - **Updated `useRunGraph` and `useScheduleGraph`** to use the centralized `useSaveGraph` hook directly - **Simplified the save control component** by removing redundant logic and using cleaner naming conventions ## Problem The previous implementation had the save logic duplicated in `useNewSaveControl`, and when this hook was used in multiple places (NewSaveControl component, RunGraph, ScheduleGraph), each instance would register its own keyboard event listener for Ctrl/Cmd+S. This caused: - Multiple save requests being sent simultaneously - "Unique constraint failed on the fields: ('id', 'version')" errors from the backend - Poor performance due to unnecessary re-renders ## Solution By centralizing the save logic in a dedicated `useSaveGraph` hook: - Save logic is now in one place, making it easier to maintain - Components can use the save functionality without registering duplicate event listeners - The keyboard shortcut listener is only registered once in the `useNewSaveControl` hook - Other components (RunGraph, ScheduleGraph) can call `saveGraph` directly without side effects ## Testing - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] I have tested my changes according to the test plan: - [x] Verified Ctrl/Cmd+S saves the graph only once - [x] Tested save functionality from Save Control popup - [x] Confirmed Run Graph and Schedule Graph still save before execution - [x] Verified no duplicate save requests in network tab - [x] Checked that save toast notifications appear correctly --- .../components/RunGraph/useRunGraph.ts | 6 +- .../ScheduleGraph/useScheduleGraph.ts | 6 +- .../NewSaveControl/NewSaveControl.tsx | 11 +- .../NewSaveControl/useNewSaveControl.ts | 151 ++++------------- .../(platform)/build/hooks/useSaveGraph.ts | 156 ++++++++++++++++++ .../frontend/src/hooks/useAgentGraph.tsx | 1 + 6 files changed, 196 insertions(+), 135 deletions(-) create mode 100644 autogpt_platform/frontend/src/app/(platform)/build/hooks/useSaveGraph.ts diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/RunGraph/useRunGraph.ts b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/RunGraph/useRunGraph.ts index bbdb44123b..54057f5166 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/RunGraph/useRunGraph.ts +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/RunGraph/useRunGraph.ts @@ -2,16 +2,16 @@ import { usePostV1ExecuteGraphAgent, usePostV1StopGraphExecution, } from "@/app/api/__generated__/endpoints/graphs/graphs"; -import { useNewSaveControl } from "../../../NewControlPanel/NewSaveControl/useNewSaveControl"; import { useToast } from "@/components/molecules/Toast/use-toast"; import { parseAsInteger, parseAsString, useQueryStates } from "nuqs"; import { GraphExecutionMeta } from "@/app/(platform)/library/agents/[id]/components/OldAgentLibraryView/use-agent-runs"; import { useGraphStore } from "@/app/(platform)/build/stores/graphStore"; import { useShallow } from "zustand/react/shallow"; import { useState } from "react"; +import { useSaveGraph } from "@/app/(platform)/build/hooks/useSaveGraph"; export const useRunGraph = () => { - const { onSubmit: onSaveGraph, isLoading: isSaving } = useNewSaveControl({ + const { saveGraph, isSaving } = useSaveGraph({ showToast: false, }); const { toast } = useToast(); @@ -67,7 +67,7 @@ export const useRunGraph = () => { }); const handleRunGraph = async () => { - await onSaveGraph(undefined); + await saveGraph(undefined); if (hasInputs() || hasCredentials()) { setOpenRunInputDialog(true); diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/ScheduleGraph/useScheduleGraph.ts b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/ScheduleGraph/useScheduleGraph.ts index 152ad3904c..4334bf6229 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/ScheduleGraph/useScheduleGraph.ts +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/BuilderActions/components/ScheduleGraph/useScheduleGraph.ts @@ -1,10 +1,10 @@ import { useGraphStore } from "@/app/(platform)/build/stores/graphStore"; import { useShallow } from "zustand/react/shallow"; -import { useNewSaveControl } from "../../../NewControlPanel/NewSaveControl/useNewSaveControl"; import { useState } from "react"; +import { useSaveGraph } from "@/app/(platform)/build/hooks/useSaveGraph"; export const useScheduleGraph = () => { - const { onSubmit: onSaveGraph } = useNewSaveControl({ + const { saveGraph } = useSaveGraph({ showToast: false, }); const hasInputs = useGraphStore(useShallow((state) => state.hasInputs)); @@ -15,7 +15,7 @@ export const useScheduleGraph = () => { const [openCronSchedulerDialog, setOpenCronSchedulerDialog] = useState(false); const handleScheduleGraph = async () => { - await onSaveGraph(undefined); + await saveGraph(undefined); if (hasInputs() || hasCredentials()) { setOpenScheduleInputDialog(true); } else { diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/NewSaveControl.tsx b/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/NewSaveControl.tsx index 78dcd0874d..beae5c1705 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/NewSaveControl.tsx +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/NewSaveControl.tsx @@ -19,10 +19,9 @@ import { Input } from "@/components/atoms/Input/Input"; import { Button } from "@/components/atoms/Button/Button"; export const NewSaveControl = () => { - const { form, onSubmit, isLoading, graphVersion } = useNewSaveControl({ - showToast: true, - }); + const { form, isSaving, graphVersion, handleSave } = useNewSaveControl(); const { saveControlOpen, setSaveControlOpen } = useControlPanelStore(); + return ( @@ -49,7 +48,7 @@ export const NewSaveControl = () => { >
- +
{ className="w-full dark:bg-slate-700 dark:text-slate-100 dark:hover:bg-slate-800" data-id="save-control-save-agent" data-testid="save-control-save-agent-button" - disabled={isLoading} - loading={isLoading} + disabled={isSaving} + loading={isSaving} > Save Agent diff --git a/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/useNewSaveControl.ts b/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/useNewSaveControl.ts index fbfcedd03f..0da548f838 100644 --- a/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/useNewSaveControl.ts +++ b/autogpt_platform/frontend/src/app/(platform)/build/components/NewControlPanel/NewSaveControl/useNewSaveControl.ts @@ -1,22 +1,12 @@ -import { useEffect } from "react"; +import { useCallback, useEffect } from "react"; import { useForm } from "react-hook-form"; import { zodResolver } from "@hookform/resolvers/zod"; import * as z from "zod"; -import { useToast } from "@/components/molecules/Toast/use-toast"; -import { useQueryClient } from "@tanstack/react-query"; import { parseAsInteger, parseAsString, useQueryStates } from "nuqs"; -import { - getGetV1GetSpecificGraphQueryKey, - useGetV1GetSpecificGraph, - usePostV1CreateNewGraph, - usePutV1UpdateGraphVersion, -} from "@/app/api/__generated__/endpoints/graphs/graphs"; +import { useGetV1GetSpecificGraph } from "@/app/api/__generated__/endpoints/graphs/graphs"; import { GraphModel } from "@/app/api/__generated__/models/graphModel"; -import { useNodeStore } from "../../../stores/nodeStore"; -import { useEdgeStore } from "../../../stores/edgeStore"; -import { Graph } from "@/app/api/__generated__/models/graph"; import { useControlPanelStore } from "../../../stores/controlPanelStore"; -import { graphsEquivalent } from "./helpers"; +import { useSaveGraph } from "../../../hooks/useSaveGraph"; const formSchema = z.object({ name: z.string().min(1, "Name is required").max(100), @@ -25,16 +15,23 @@ const formSchema = z.object({ type SaveableGraphFormValues = z.infer; -export const useNewSaveControl = ({ - showToast = true, -}: { - showToast?: boolean; -}) => { +export const useNewSaveControl = () => { const { setSaveControlOpen } = useControlPanelStore(); - const { toast } = useToast(); - const queryClient = useQueryClient(); - const [{ flowID, flowVersion }, setQueryStates] = useQueryStates({ + const onSuccess = (graph: GraphModel) => { + setSaveControlOpen(false); + form.reset({ + name: graph.name, + description: graph.description, + }); + }; + + const { saveGraph, isSaving } = useSaveGraph({ + showToast: true, + onSuccess, + }); + + const [{ flowID, flowVersion }] = useQueryStates({ flowID: parseAsString, flowVersion: parseAsInteger, }); @@ -50,69 +47,6 @@ export const useNewSaveControl = ({ }, ); - const { mutateAsync: createNewGraph, isPending: isCreating } = - usePostV1CreateNewGraph({ - mutation: { - onSuccess: (response) => { - const data = response.data as GraphModel; - form.reset({ - name: data.name, - description: data.description, - }); - setSaveControlOpen(false); - setQueryStates({ - flowID: data.id, - flowVersion: data.version, - }); - if (showToast) { - toast({ - title: "All changes saved successfully!", - }); - } - }, - onError: (error) => { - toast({ - title: (error.detail as string) ?? "An unexpected error occurred.", - description: "An unexpected error occurred.", - variant: "destructive", - }); - }, - }, - }); - - const { mutateAsync: updateGraph, isPending: isUpdating } = - usePutV1UpdateGraphVersion({ - mutation: { - onSuccess: (response) => { - const data = response.data as GraphModel; - form.reset({ - name: data.name, - description: data.description, - }); - setSaveControlOpen(false); - setQueryStates({ - flowID: data.id, - flowVersion: data.version, - }); - if (showToast) { - toast({ - title: "All changes saved successfully!", - }); - } - queryClient.invalidateQueries({ - queryKey: getGetV1GetSpecificGraphQueryKey(data.id), - }); - }, - onError: (error) => { - toast({ - title: (error.detail as string) ?? "An unexpected error occurred.", - description: "An unexpected error occurred.", - variant: "destructive", - }); - }, - }, - }); - const form = useForm({ resolver: zodResolver(formSchema), defaultValues: { @@ -121,47 +55,18 @@ export const useNewSaveControl = ({ }, }); - const onSubmit = async (values: SaveableGraphFormValues | undefined) => { - const graphNodes = useNodeStore.getState().getBackendNodes(); - const graphLinks = useEdgeStore.getState().getBackendLinks(); + const handleSave = useCallback( + (values: SaveableGraphFormValues) => { + saveGraph(values); + }, + [saveGraph], + ); - if (graph && graph.id) { - const data: Graph = { - id: graph.id, - name: - values?.name || graph.name || `New Agent ${new Date().toISOString()}`, - description: values?.description ?? graph.description ?? "", - nodes: graphNodes, - links: graphLinks, - }; - if (graphsEquivalent(graph, data)) { - if (showToast) { - toast({ - title: "No changes to save", - description: "The graph is the same as the saved version.", - variant: "default", - }); - } - return; - } - await updateGraph({ graphId: graph.id, data: data }); - } else { - const data: Graph = { - name: values?.name || `New Agent ${new Date().toISOString()}`, - description: values?.description || "", - nodes: graphNodes, - links: graphLinks, - }; - await createNewGraph({ data: { graph: data } }); - } - }; - - // Handle Ctrl+S / Cmd+S keyboard shortcut useEffect(() => { const handleKeyDown = async (event: KeyboardEvent) => { if ((event.ctrlKey || event.metaKey) && event.key === "s") { event.preventDefault(); - await onSubmit(form.getValues()); + handleSave(form.getValues()); } }; @@ -170,7 +75,7 @@ export const useNewSaveControl = ({ return () => { window.removeEventListener("keydown", handleKeyDown); }; - }, [onSubmit]); + }, [handleSave]); useEffect(() => { if (graph) { @@ -183,8 +88,8 @@ export const useNewSaveControl = ({ return { form, - isLoading: isCreating || isUpdating, + isSaving: isSaving, graphVersion: graph?.version, - onSubmit, + handleSave, }; }; diff --git a/autogpt_platform/frontend/src/app/(platform)/build/hooks/useSaveGraph.ts b/autogpt_platform/frontend/src/app/(platform)/build/hooks/useSaveGraph.ts new file mode 100644 index 0000000000..1f531b5f73 --- /dev/null +++ b/autogpt_platform/frontend/src/app/(platform)/build/hooks/useSaveGraph.ts @@ -0,0 +1,156 @@ +// Creating this hook, because we are using same saving stuff at multiple places in our builder + +import { useCallback } from "react"; +import { useToast } from "@/components/molecules/Toast/use-toast"; +import { useQueryClient } from "@tanstack/react-query"; +import { parseAsInteger, parseAsString, useQueryStates } from "nuqs"; +import { + getGetV1GetSpecificGraphQueryKey, + useGetV1GetSpecificGraph, + usePostV1CreateNewGraph, + usePutV1UpdateGraphVersion, +} from "@/app/api/__generated__/endpoints/graphs/graphs"; +import { GraphModel } from "@/app/api/__generated__/models/graphModel"; +import { Graph } from "@/app/api/__generated__/models/graph"; +import { useNodeStore } from "../stores/nodeStore"; +import { useEdgeStore } from "../stores/edgeStore"; +import { graphsEquivalent } from "../components/NewControlPanel/NewSaveControl/helpers"; + +export type SaveGraphOptions = { + showToast?: boolean; + onSuccess?: (graph: GraphModel) => void; + onError?: (error: any) => void; +}; + +export const useSaveGraph = ({ + showToast = true, + onSuccess, + onError, +}: SaveGraphOptions) => { + const { toast } = useToast(); + const queryClient = useQueryClient(); + + const [{ flowID, flowVersion }, setQueryStates] = useQueryStates({ + flowID: parseAsString, + flowVersion: parseAsInteger, + }); + + const { data: graph } = useGetV1GetSpecificGraph( + flowID ?? "", + flowVersion !== null ? { version: flowVersion } : {}, + { + query: { + select: (res) => res.data as GraphModel, + enabled: !!flowID, + }, + }, + ); + + const { mutateAsync: createNewGraph, isPending: isCreating } = + usePostV1CreateNewGraph({ + mutation: { + onSuccess: (response) => { + const data = response.data as GraphModel; + setQueryStates({ + flowID: data.id, + flowVersion: data.version, + }); + queryClient.refetchQueries({ + queryKey: getGetV1GetSpecificGraphQueryKey(data.id), + }); + onSuccess?.(data); + if (showToast) { + toast({ + title: "Graph saved successfully", + description: "The graph has been saved successfully.", + variant: "default", + }); + } + }, + onError: (error) => { + onError?.(error); + }, + }, + }); + + const { mutateAsync: updateGraph, isPending: isUpdating } = + usePutV1UpdateGraphVersion({ + mutation: { + onSuccess: (response) => { + const data = response.data as GraphModel; + setQueryStates({ + flowID: data.id, + flowVersion: data.version, + }); + queryClient.refetchQueries({ + queryKey: getGetV1GetSpecificGraphQueryKey(data.id), + }); + onSuccess?.(data); + if (showToast) { + toast({ + title: "Graph saved successfully", + description: "The graph has been saved successfully.", + variant: "default", + }); + } + }, + onError: (error) => { + onError?.(error); + toast({ + title: "Error saving graph", + description: + (error as any).message ?? "An unexpected error occurred.", + variant: "destructive", + }); + }, + }, + }); + + const saveGraph = useCallback( + async (values?: { name?: string; description?: string }) => { + const graphNodes = useNodeStore.getState().getBackendNodes(); + const graphLinks = useEdgeStore.getState().getBackendLinks(); + + if (graph && graph.id) { + const data: Graph = { + id: graph.id, + name: + values?.name || + graph.name || + `New Agent ${new Date().toISOString()}`, + description: values?.description ?? graph.description ?? "", + nodes: graphNodes, + links: graphLinks, + }; + + if (graphsEquivalent(graph, data)) { + if (showToast) { + toast({ + title: "No changes to save", + description: "The graph is the same as the saved version.", + variant: "default", + }); + } + return; + } + + await updateGraph({ graphId: graph.id, data: data }); + } else { + const data: Graph = { + name: values?.name || `New Agent ${new Date().toISOString()}`, + description: values?.description || "", + nodes: graphNodes, + links: graphLinks, + }; + + await createNewGraph({ data: { graph: data } }); + } + }, + [graph, toast, createNewGraph, updateGraph], + ); + + return { + saveGraph, + isSaving: isCreating || isUpdating, + }; +}; diff --git a/autogpt_platform/frontend/src/hooks/useAgentGraph.tsx b/autogpt_platform/frontend/src/hooks/useAgentGraph.tsx index fd2768642a..689294483b 100644 --- a/autogpt_platform/frontend/src/hooks/useAgentGraph.tsx +++ b/autogpt_platform/frontend/src/hooks/useAgentGraph.tsx @@ -766,6 +766,7 @@ export default function useAgentGraph( ]); const saveAgent = useCallback(async () => { + console.log("saveAgent"); setIsSaving(true); try { await _saveAgent();