diff --git a/web/app/components/workflow-app/hooks/__tests__/use-nodes-sync-draft.spec.ts b/web/app/components/workflow-app/hooks/__tests__/use-nodes-sync-draft.spec.ts new file mode 100644 index 0000000000..d35e6e3612 --- /dev/null +++ b/web/app/components/workflow-app/hooks/__tests__/use-nodes-sync-draft.spec.ts @@ -0,0 +1,111 @@ +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { useNodesSyncDraft } from '../use-nodes-sync-draft' + +const mockGetNodes = vi.fn() +vi.mock('reactflow', () => ({ + useStoreApi: () => ({ getState: () => ({ getNodes: mockGetNodes, edges: [], transform: [0, 0, 1] }) }), +})) + +vi.mock('@/app/components/workflow/store', () => ({ + useWorkflowStore: () => ({ + getState: () => ({ + appId: 'app-1', + isWorkflowDataLoaded: true, + syncWorkflowDraftHash: 'hash-123', + environmentVariables: [], + conversationVariables: [], + setSyncWorkflowDraftHash: vi.fn(), + setDraftUpdatedAt: vi.fn(), + }), + }), +})) + +vi.mock('@/app/components/base/features/hooks', () => ({ + useFeaturesStore: () => ({ + getState: () => ({ + features: { + opening: { enabled: false, opening_statement: '', suggested_questions: [] }, + suggested: {}, + text2speech: {}, + speech2text: {}, + citation: {}, + moderation: {}, + file: {}, + }, + }), + }), +})) + +vi.mock('@/app/components/workflow/hooks/use-workflow', () => ({ + useNodesReadOnly: () => ({ getNodesReadOnly: () => false }), +})) + +vi.mock('@/app/components/workflow/hooks/use-serial-async-callback', () => ({ + useSerialAsyncCallback: (fn: (...args: unknown[]) => Promise, checkFn: () => boolean) => + (...args: unknown[]) => { + if (!checkFn()) + return fn(...args) + }, +})) + +const mockSyncWorkflowDraft = vi.fn() +vi.mock('@/service/workflow', () => ({ + syncWorkflowDraft: (p: unknown) => mockSyncWorkflowDraft(p), +})) + +vi.mock('@/service/fetch', () => ({ postWithKeepalive: vi.fn() })) +vi.mock('@/config', () => ({ API_PREFIX: '/api' })) + +const mockHandleRefreshWorkflowDraft = vi.fn() +vi.mock('@/app/components/workflow-app/hooks', () => ({ + useWorkflowRefreshDraft: () => ({ handleRefreshWorkflowDraft: mockHandleRefreshWorkflowDraft }), +})) + +describe('useNodesSyncDraft — handleRefreshWorkflowDraft(true) on 409', () => { + beforeEach(() => { + vi.clearAllMocks() + mockGetNodes.mockReturnValue([{ id: 'n1', position: { x: 0, y: 0 }, data: { type: 'start' } }]) + mockSyncWorkflowDraft.mockResolvedValue({ hash: 'new', updated_at: 1 }) + }) + + it('should call handleRefreshWorkflowDraft(true) — not updating canvas — on draft_workflow_not_sync', async () => { + const error = { json: vi.fn().mockResolvedValue({ code: 'draft_workflow_not_sync' }), bodyUsed: false } + mockSyncWorkflowDraft.mockRejectedValue(error) + + const { result } = renderHook(() => useNodesSyncDraft()) + await act(async () => { + await result.current.doSyncWorkflowDraft(false) + }) + await new Promise(r => setTimeout(r, 0)) + + expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledWith(true) + }) + + it('should NOT refresh when notRefreshWhenSyncError=true', async () => { + const error = { json: vi.fn().mockResolvedValue({ code: 'draft_workflow_not_sync' }), bodyUsed: false } + mockSyncWorkflowDraft.mockRejectedValue(error) + + const { result } = renderHook(() => useNodesSyncDraft()) + await act(async () => { + await result.current.doSyncWorkflowDraft(true) + }) + await new Promise(r => setTimeout(r, 0)) + + expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled() + }) + + it('should NOT refresh for a different error code', async () => { + const error = { json: vi.fn().mockResolvedValue({ code: 'other_error' }), bodyUsed: false } + mockSyncWorkflowDraft.mockRejectedValue(error) + + const { result } = renderHook(() => useNodesSyncDraft()) + await act(async () => { + await result.current.doSyncWorkflowDraft(false) + }) + await new Promise(r => setTimeout(r, 0)) + + expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled() + }) +}) diff --git a/web/app/components/workflow-app/hooks/__tests__/use-workflow-init.spec.ts b/web/app/components/workflow-app/hooks/__tests__/use-workflow-init.spec.ts new file mode 100644 index 0000000000..42e4b593ed --- /dev/null +++ b/web/app/components/workflow-app/hooks/__tests__/use-workflow-init.spec.ts @@ -0,0 +1,107 @@ +import { renderHook, waitFor } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { useWorkflowInit } from '../use-workflow-init' + +const mockSetSyncWorkflowDraftHash = vi.fn() +const mockSetDraftUpdatedAt = vi.fn() +const mockSetToolPublished = vi.fn() +const mockSetPublishedAt = vi.fn() +const mockSetLastPublishedHasUserInput = vi.fn() +const mockSetFileUploadConfig = vi.fn() +const mockWorkflowStoreSetState = vi.fn() +const mockWorkflowStoreGetState = vi.fn() + +vi.mock('@/app/components/workflow/store', () => ({ + useStore: (selector: (state: { setSyncWorkflowDraftHash: ReturnType }) => T): T => + selector({ setSyncWorkflowDraftHash: mockSetSyncWorkflowDraftHash }), + useWorkflowStore: () => ({ + setState: mockWorkflowStoreSetState, + getState: mockWorkflowStoreGetState, + }), +})) + +vi.mock('@/app/components/app/store', () => ({ + useStore: (selector: (state: { appDetail: { id: string, name: string, mode: string } }) => T): T => + selector({ appDetail: { id: 'app-1', name: 'Test', mode: 'workflow' } }), +})) + +vi.mock('../use-workflow-template', () => ({ + useWorkflowTemplate: () => ({ nodes: [], edges: [] }), +})) + +vi.mock('@/service/use-workflow', () => ({ + useWorkflowConfig: () => ({ data: null, isLoading: false }), +})) + +const mockFetchWorkflowDraft = vi.fn() +const mockSyncWorkflowDraft = vi.fn() + +vi.mock('@/service/workflow', () => ({ + fetchWorkflowDraft: (...args: unknown[]) => mockFetchWorkflowDraft(...args), + syncWorkflowDraft: (...args: unknown[]) => mockSyncWorkflowDraft(...args), + fetchNodesDefaultConfigs: () => Promise.resolve([]), + fetchPublishedWorkflow: () => Promise.resolve({ created_at: 0, graph: { nodes: [], edges: [] } }), +})) + +const notExistError = () => ({ + json: vi.fn().mockResolvedValue({ code: 'draft_workflow_not_exist' }), + bodyUsed: false, +}) + +const draftResponse = { + id: 'draft-id', + graph: { nodes: [], edges: [] }, + hash: 'server-hash', + created_at: 0, + created_by: { id: '', name: '', email: '' }, + updated_at: 1, + updated_by: { id: '', name: '', email: '' }, + tool_published: false, + environment_variables: [], + conversation_variables: [], + version: '1', + marked_name: '', + marked_comment: '', +} + +describe('useWorkflowInit — hash fix (draft_workflow_not_exist)', () => { + beforeEach(() => { + vi.clearAllMocks() + mockWorkflowStoreGetState.mockReturnValue({ + setDraftUpdatedAt: mockSetDraftUpdatedAt, + setToolPublished: mockSetToolPublished, + setPublishedAt: mockSetPublishedAt, + setLastPublishedHasUserInput: mockSetLastPublishedHasUserInput, + setFileUploadConfig: mockSetFileUploadConfig, + }) + mockFetchWorkflowDraft + .mockRejectedValueOnce(notExistError()) + .mockResolvedValueOnce(draftResponse) + mockSyncWorkflowDraft.mockResolvedValue({ hash: 'new-hash', updated_at: 1 }) + }) + + it('should call setSyncWorkflowDraftHash with hash returned by syncWorkflowDraft', async () => { + renderHook(() => useWorkflowInit()) + await waitFor(() => expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash')) + }) + + it('should store hash BEFORE making the recursive fetchWorkflowDraft call', async () => { + const order: string[] = [] + mockSetSyncWorkflowDraftHash.mockImplementation((h: string) => order.push(`hash:${h}`)) + mockFetchWorkflowDraft + .mockReset() + .mockRejectedValueOnce(notExistError()) + .mockImplementationOnce(async () => { + order.push('fetch:2') + return draftResponse + }) + mockSyncWorkflowDraft.mockResolvedValue({ hash: 'new-hash', updated_at: 1 }) + + renderHook(() => useWorkflowInit()) + + await waitFor(() => expect(order).toContain('fetch:2')) + expect(order).toContain('hash:new-hash') + expect(order.indexOf('hash:new-hash')).toBeLessThan(order.indexOf('fetch:2')) + }) +}) diff --git a/web/app/components/workflow-app/hooks/__tests__/use-workflow-refresh-draft.spec.ts b/web/app/components/workflow-app/hooks/__tests__/use-workflow-refresh-draft.spec.ts new file mode 100644 index 0000000000..2fd06e587b --- /dev/null +++ b/web/app/components/workflow-app/hooks/__tests__/use-workflow-refresh-draft.spec.ts @@ -0,0 +1,80 @@ +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { useWorkflowRefreshDraft } from '../use-workflow-refresh-draft' + +const mockHandleUpdateWorkflowCanvas = vi.fn() +const mockSetSyncWorkflowDraftHash = vi.fn() + +vi.mock('@/app/components/workflow/store', () => ({ + useWorkflowStore: () => ({ + getState: () => ({ + appId: 'app-1', + isWorkflowDataLoaded: true, + debouncedSyncWorkflowDraft: undefined, + setSyncWorkflowDraftHash: mockSetSyncWorkflowDraftHash, + setIsSyncingWorkflowDraft: vi.fn(), + setEnvironmentVariables: vi.fn(), + setEnvSecrets: vi.fn(), + setConversationVariables: vi.fn(), + setIsWorkflowDataLoaded: vi.fn(), + }), + }), +})) + +vi.mock('@/app/components/workflow/hooks', () => ({ + useWorkflowUpdate: () => ({ handleUpdateWorkflowCanvas: mockHandleUpdateWorkflowCanvas }), +})) + +const mockFetchWorkflowDraft = vi.fn() +vi.mock('@/service/workflow', () => ({ + fetchWorkflowDraft: (...args: unknown[]) => mockFetchWorkflowDraft(...args), +})) + +const draftResponse = { + hash: 'server-hash', + graph: { nodes: [{ id: 'n1' }], edges: [], viewport: { x: 1, y: 2, zoom: 1 } }, + environment_variables: [], + conversation_variables: [], +} + +describe('useWorkflowRefreshDraft — notUpdateCanvas parameter', () => { + beforeEach(() => { + vi.clearAllMocks() + mockFetchWorkflowDraft.mockResolvedValue(draftResponse) + }) + + it('should update canvas by default (notUpdateCanvas omitted)', async () => { + const { result } = renderHook(() => useWorkflowRefreshDraft()) + await act(async () => { + result.current.handleRefreshWorkflowDraft() + }) + expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledTimes(1) + }) + + it('should update canvas when notUpdateCanvas=false', async () => { + const { result } = renderHook(() => useWorkflowRefreshDraft()) + await act(async () => { + result.current.handleRefreshWorkflowDraft(false) + }) + expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledTimes(1) + }) + + it('should NOT update canvas when notUpdateCanvas=true', async () => { + // This is the key change: when called from a 409 error during editing, + // canvas must not be overwritten with server state. + const { result } = renderHook(() => useWorkflowRefreshDraft()) + await act(async () => { + result.current.handleRefreshWorkflowDraft(true) + }) + expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled() + }) + + it('should still update hash even when notUpdateCanvas=true', async () => { + const { result } = renderHook(() => useWorkflowRefreshDraft()) + await act(async () => { + result.current.handleRefreshWorkflowDraft(true) + }) + expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('server-hash') + }) +}) diff --git a/web/app/components/workflow-app/hooks/use-nodes-sync-draft.ts b/web/app/components/workflow-app/hooks/use-nodes-sync-draft.ts index 5dc0741324..4f9e529d92 100644 --- a/web/app/components/workflow-app/hooks/use-nodes-sync-draft.ts +++ b/web/app/components/workflow-app/hooks/use-nodes-sync-draft.ts @@ -132,7 +132,7 @@ export const useNodesSyncDraft = () => { if (error && error.json && !error.bodyUsed) { error.json().then((err: any) => { if (err.code === 'draft_workflow_not_sync' && !notRefreshWhenSyncError) - handleRefreshWorkflowDraft() + handleRefreshWorkflowDraft(true) }) } callback?.onError?.() diff --git a/web/app/components/workflow-app/hooks/use-workflow-init.ts b/web/app/components/workflow-app/hooks/use-workflow-init.ts index 8e976937b5..00bff2919f 100644 --- a/web/app/components/workflow-app/hooks/use-workflow-init.ts +++ b/web/app/components/workflow-app/hooks/use-workflow-init.ts @@ -100,6 +100,7 @@ export const useWorkflowInit = () => { }, }).then((res) => { workflowStore.getState().setDraftUpdatedAt(res.updated_at) + setSyncWorkflowDraftHash(res.hash) handleGetInitialWorkflowData() }) } diff --git a/web/app/components/workflow-app/hooks/use-workflow-refresh-draft.ts b/web/app/components/workflow-app/hooks/use-workflow-refresh-draft.ts index fa4a44d894..a7283c0078 100644 --- a/web/app/components/workflow-app/hooks/use-workflow-refresh-draft.ts +++ b/web/app/components/workflow-app/hooks/use-workflow-refresh-draft.ts @@ -8,7 +8,7 @@ export const useWorkflowRefreshDraft = () => { const workflowStore = useWorkflowStore() const { handleUpdateWorkflowCanvas } = useWorkflowUpdate() - const handleRefreshWorkflowDraft = useCallback(() => { + const handleRefreshWorkflowDraft = useCallback((notUpdateCanvas?: boolean) => { const { appId, setSyncWorkflowDraftHash, @@ -31,12 +31,14 @@ export const useWorkflowRefreshDraft = () => { fetchWorkflowDraft(`/apps/${appId}/workflows/draft`) .then((response) => { // Ensure we have a valid workflow structure with viewport - const workflowData: WorkflowDataUpdater = { - nodes: response.graph?.nodes || [], - edges: response.graph?.edges || [], - viewport: response.graph?.viewport || { x: 0, y: 0, zoom: 1 }, + if (!notUpdateCanvas) { + const workflowData: WorkflowDataUpdater = { + nodes: response.graph?.nodes || [], + edges: response.graph?.edges || [], + viewport: response.graph?.viewport || { x: 0, y: 0, zoom: 1 }, + } + handleUpdateWorkflowCanvas(workflowData) } - handleUpdateWorkflowCanvas(workflowData) setSyncWorkflowDraftHash(response.hash) setEnvSecrets((response.environment_variables || []).filter(env => env.value_type === 'secret').reduce((acc, env) => { acc[env.id] = env.value