From b2d0afb14c72aa4c19f53fdd06aa2347258216c2 Mon Sep 17 00:00:00 2001 From: FFXN Date: Wed, 28 Jan 2026 17:59:41 +0800 Subject: [PATCH] Revert "Merge branch 'deploy/dev' into feat/knowledgebase-summaryIndex" This reverts commit ce304f2da78cd7e61cc4d5a2d865ad97f9f4aab5, reversing changes made to 334888dac95ad79672c2ca2b02ce0765906c134b. --- .../workflow/nodes/document_extractor/node.py | 25 - ...8-562dcce7d77c_add_summaryindex_feature.py | 69 -- .../components/chunk-card-list/index.spec.tsx | 8 +- .../hooks/use-nodes-sync-draft.spec.ts | 705 ------------------ .../hooks/use-nodes-sync-draft.ts | 2 +- .../hooks/use-workflow-refresh-draft.spec.ts | 556 -------------- .../hooks/use-workflow-refresh-draft.ts | 14 +- web/tsconfig.json | 2 +- 8 files changed, 12 insertions(+), 1369 deletions(-) delete mode 100644 api/migrations/versions/2026_01_12_1358-562dcce7d77c_add_summaryindex_feature.py delete mode 100644 web/app/components/workflow-app/hooks/use-nodes-sync-draft.spec.ts delete mode 100644 web/app/components/workflow-app/hooks/use-workflow-refresh-draft.spec.ts diff --git a/api/core/workflow/nodes/document_extractor/node.py b/api/core/workflow/nodes/document_extractor/node.py index 25dd98f48a..14ebd1f9ae 100644 --- a/api/core/workflow/nodes/document_extractor/node.py +++ b/api/core/workflow/nodes/document_extractor/node.py @@ -62,21 +62,6 @@ class DocumentExtractorNode(Node[DocumentExtractorNodeData]): inputs = {"variable_selector": variable_selector} process_data = {"documents": value if isinstance(value, list) else [value]} - # Ensure storage_key is loaded for File objects - files_to_check = value if isinstance(value, list) else [value] - files_needing_storage_key = [ - f for f in files_to_check if isinstance(f, File) and not f.storage_key and f.related_id - ] - if files_needing_storage_key: - from sqlalchemy.orm import Session - - from extensions.ext_database import db - from factories.file_factory import StorageKeyLoader - - with Session(bind=db.engine) as session: - storage_key_loader = StorageKeyLoader(session, tenant_id=self.tenant_id) - storage_key_loader.load_storage_keys(files_needing_storage_key) - try: if isinstance(value, list): extracted_text_list = list(map(_extract_text_from_file, value)) @@ -430,16 +415,6 @@ def _download_file_content(file: File) -> bytes: response.raise_for_status() return response.content else: - # Check if storage_key is set - if not file.storage_key: - raise FileDownloadError(f"File storage_key is missing for file: {file.filename}") - - # Check if file exists before downloading - from extensions.ext_storage import storage - - if not storage.exists(file.storage_key): - raise FileDownloadError(f"File not found in storage: {file.storage_key}") - return file_manager.download(file) except Exception as e: raise FileDownloadError(f"Error downloading file: {str(e)}") from e diff --git a/api/migrations/versions/2026_01_12_1358-562dcce7d77c_add_summaryindex_feature.py b/api/migrations/versions/2026_01_12_1358-562dcce7d77c_add_summaryindex_feature.py deleted file mode 100644 index 40fe419ef6..0000000000 --- a/api/migrations/versions/2026_01_12_1358-562dcce7d77c_add_summaryindex_feature.py +++ /dev/null @@ -1,69 +0,0 @@ -"""add SummaryIndex feature - -Revision ID: 562dcce7d77c -Revises: 03ea244985ce -Create Date: 2026-01-12 13:58:40.584802 - -""" -from alembic import op -import models as models -import sqlalchemy as sa - - -# revision identifiers, used by Alembic. -revision = '562dcce7d77c' -down_revision = '03ea244985ce' -branch_labels = None -depends_on = None - - -def upgrade(): - # ### commands auto generated by Alembic - please adjust! ### - op.create_table('document_segment_summary', - sa.Column('id', models.types.StringUUID(), nullable=False), - sa.Column('dataset_id', models.types.StringUUID(), nullable=False), - sa.Column('document_id', models.types.StringUUID(), nullable=False), - sa.Column('chunk_id', models.types.StringUUID(), nullable=False), - sa.Column('summary_content', models.types.LongText(), nullable=True), - sa.Column('summary_index_node_id', sa.String(length=255), nullable=True), - sa.Column('summary_index_node_hash', sa.String(length=255), nullable=True), - sa.Column('status', sa.String(length=32), server_default=sa.text("'generating'"), nullable=False), - sa.Column('error', models.types.LongText(), nullable=True), - sa.Column('enabled', sa.Boolean(), server_default=sa.text('true'), nullable=False), - sa.Column('disabled_at', sa.DateTime(), nullable=True), - sa.Column('disabled_by', models.types.StringUUID(), nullable=True), - sa.Column('created_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.Column('updated_at', sa.DateTime(), server_default=sa.text('CURRENT_TIMESTAMP'), nullable=False), - sa.PrimaryKeyConstraint('id', name='document_segment_summary_pkey') - ) - with op.batch_alter_table('document_segment_summary', schema=None) as batch_op: - batch_op.create_index('document_segment_summary_chunk_id_idx', ['chunk_id'], unique=False) - batch_op.create_index('document_segment_summary_dataset_id_idx', ['dataset_id'], unique=False) - batch_op.create_index('document_segment_summary_document_id_idx', ['document_id'], unique=False) - batch_op.create_index('document_segment_summary_status_idx', ['status'], unique=False) - - with op.batch_alter_table('datasets', schema=None) as batch_op: - batch_op.add_column(sa.Column('summary_index_setting', models.types.AdjustedJSON(), nullable=True)) - - with op.batch_alter_table('documents', schema=None) as batch_op: - batch_op.add_column(sa.Column('need_summary', sa.Boolean(), server_default=sa.text('false'), nullable=True)) - - # ### end Alembic commands ### - - -def downgrade(): - # ### commands auto generated by Alembic - please adjust! ### - with op.batch_alter_table('documents', schema=None) as batch_op: - batch_op.drop_column('need_summary') - - with op.batch_alter_table('datasets', schema=None) as batch_op: - batch_op.drop_column('summary_index_setting') - - with op.batch_alter_table('document_segment_summary', schema=None) as batch_op: - batch_op.drop_index('document_segment_summary_status_idx') - batch_op.drop_index('document_segment_summary_document_id_idx') - batch_op.drop_index('document_segment_summary_dataset_id_idx') - batch_op.drop_index('document_segment_summary_chunk_id_idx') - - op.drop_table('document_segment_summary') - # ### end Alembic commands ### diff --git a/web/app/components/rag-pipeline/components/chunk-card-list/index.spec.tsx b/web/app/components/rag-pipeline/components/chunk-card-list/index.spec.tsx index 1864ad0c6e..ca5fae25c7 100644 --- a/web/app/components/rag-pipeline/components/chunk-card-list/index.spec.tsx +++ b/web/app/components/rag-pipeline/components/chunk-card-list/index.spec.tsx @@ -398,7 +398,7 @@ describe('ChunkCard', () => { const { rerender } = render( , @@ -411,7 +411,7 @@ describe('ChunkCard', () => { rerender( , @@ -428,7 +428,7 @@ describe('ChunkCard', () => { const { rerender } = render( , @@ -500,7 +500,7 @@ describe('ChunkCard', () => { render( , diff --git a/web/app/components/workflow-app/hooks/use-nodes-sync-draft.spec.ts b/web/app/components/workflow-app/hooks/use-nodes-sync-draft.spec.ts deleted file mode 100644 index 626cbb4fc4..0000000000 --- a/web/app/components/workflow-app/hooks/use-nodes-sync-draft.spec.ts +++ /dev/null @@ -1,705 +0,0 @@ -/** - * Test Suite for useNodesSyncDraft Hook - * - * PURPOSE: - * This hook handles syncing workflow draft to the server. The key fix being tested - * is the error handling behavior when `draft_workflow_not_sync` error occurs. - * - * MULTI-TAB PROBLEM SCENARIO: - * 1. User opens the same workflow in Tab A and Tab B (both have hash: v1) - * 2. Tab A saves successfully, server returns new hash: v2 - * 3. Tab B tries to save with old hash: v1, server returns 400 error with code - * 'draft_workflow_not_sync' - * 4. BEFORE FIX: handleRefreshWorkflowDraft() was called without args, which fetched - * draft AND overwrote canvas - user lost unsaved changes in Tab B - * 5. AFTER FIX: handleRefreshWorkflowDraft(true) is called, which fetches draft but - * only updates hash (notUpdateCanvas=true), preserving user's canvas changes - * - * TESTING STRATEGY: - * We don't simulate actual tab switching UI behavior. Instead, we mock the API to - * return `draft_workflow_not_sync` error and verify: - * - The hook calls handleRefreshWorkflowDraft(true) - not handleRefreshWorkflowDraft() - * - This ensures canvas data is preserved while hash is updated for retry - * - * This is behavior-driven testing - we verify "what the code does when receiving - * specific API errors" rather than simulating complete user interaction flows. - * True multi-tab integration testing would require E2E frameworks like Playwright. - */ - -import { act, renderHook, waitFor } from '@testing-library/react' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useNodesSyncDraft } from './use-nodes-sync-draft' - -// Mock reactflow store -const mockGetNodes = vi.fn() - -type MockEdge = { - id: string - source: string - target: string - data: Record -} - -const mockStoreState: { - getNodes: ReturnType - edges: MockEdge[] - transform: number[] -} = { - getNodes: mockGetNodes, - edges: [], - transform: [0, 0, 1], -} -vi.mock('reactflow', () => ({ - useStoreApi: () => ({ - getState: () => mockStoreState, - }), -})) - -// Mock features store -const mockFeaturesState = { - features: { - opening: { enabled: false, opening_statement: '', suggested_questions: [] }, - suggested: {}, - text2speech: {}, - speech2text: {}, - citation: {}, - moderation: {}, - file: {}, - }, -} -vi.mock('@/app/components/base/features/hooks', () => ({ - useFeaturesStore: () => ({ - getState: () => mockFeaturesState, - }), -})) - -// Mock workflow service -const mockSyncWorkflowDraft = vi.fn() -vi.mock('@/service/workflow', () => ({ - syncWorkflowDraft: (...args: unknown[]) => mockSyncWorkflowDraft(...args), -})) - -// Mock useNodesReadOnly -const mockGetNodesReadOnly = vi.fn() -vi.mock('@/app/components/workflow/hooks/use-workflow', () => ({ - useNodesReadOnly: () => ({ - getNodesReadOnly: mockGetNodesReadOnly, - }), -})) - -// Mock useSerialAsyncCallback - pass through the callback -vi.mock('@/app/components/workflow/hooks/use-serial-async-callback', () => ({ - useSerialAsyncCallback: (callback: (...args: unknown[]) => unknown) => callback, -})) - -// Mock workflow store -const mockSetSyncWorkflowDraftHash = vi.fn() -const mockSetDraftUpdatedAt = vi.fn() - -const createMockWorkflowStoreState = (overrides = {}) => ({ - appId: 'test-app-id', - conversationVariables: [], - environmentVariables: [], - syncWorkflowDraftHash: 'current-hash-123', - isWorkflowDataLoaded: true, - setSyncWorkflowDraftHash: mockSetSyncWorkflowDraftHash, - setDraftUpdatedAt: mockSetDraftUpdatedAt, - ...overrides, -}) - -const mockWorkflowStoreGetState = vi.fn() -vi.mock('@/app/components/workflow/store', () => ({ - useWorkflowStore: () => ({ - getState: mockWorkflowStoreGetState, - }), -})) - -// Mock useWorkflowRefreshDraft (THE KEY DEPENDENCY FOR THIS TEST) -const mockHandleRefreshWorkflowDraft = vi.fn() -vi.mock('.', () => ({ - useWorkflowRefreshDraft: () => ({ - handleRefreshWorkflowDraft: mockHandleRefreshWorkflowDraft, - }), -})) - -// Mock API_PREFIX -vi.mock('@/config', () => ({ - API_PREFIX: '/api', -})) - -// Create a mock error response that mimics the actual API error -const createMockErrorResponse = (code: string) => { - const errorBody = { code, message: 'Draft not in sync' } - let bodyUsed = false - - return { - json: vi.fn().mockImplementation(() => { - bodyUsed = true - return Promise.resolve(errorBody) - }), - get bodyUsed() { - return bodyUsed - }, - } -} - -describe('useNodesSyncDraft', () => { - beforeEach(() => { - vi.clearAllMocks() - mockGetNodesReadOnly.mockReturnValue(false) - mockGetNodes.mockReturnValue([ - { id: 'node-1', type: 'start', data: { type: 'start' } }, - { id: 'node-2', type: 'llm', data: { type: 'llm' } }, - ]) - mockStoreState.edges = [ - { id: 'edge-1', source: 'node-1', target: 'node-2', data: {} }, - ] - mockWorkflowStoreGetState.mockReturnValue(createMockWorkflowStoreState()) - mockSyncWorkflowDraft.mockResolvedValue({ - hash: 'new-hash-456', - updated_at: Date.now(), - }) - }) - - afterEach(() => { - vi.resetAllMocks() - }) - - describe('doSyncWorkflowDraft function', () => { - it('should return doSyncWorkflowDraft function', () => { - const { result } = renderHook(() => useNodesSyncDraft()) - - expect(result.current.doSyncWorkflowDraft).toBeDefined() - expect(typeof result.current.doSyncWorkflowDraft).toBe('function') - }) - - it('should return syncWorkflowDraftWhenPageClose function', () => { - const { result } = renderHook(() => useNodesSyncDraft()) - - expect(result.current.syncWorkflowDraftWhenPageClose).toBeDefined() - expect(typeof result.current.syncWorkflowDraftWhenPageClose).toBe('function') - }) - }) - - describe('successful sync', () => { - it('should call syncWorkflowDraft service on successful sync', async () => { - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSyncWorkflowDraft).toHaveBeenCalledWith({ - url: '/apps/test-app-id/workflows/draft', - params: expect.objectContaining({ - hash: 'current-hash-123', - graph: expect.objectContaining({ - nodes: expect.any(Array), - edges: expect.any(Array), - viewport: expect.any(Object), - }), - }), - }) - }) - - it('should update syncWorkflowDraftHash on success', async () => { - mockSyncWorkflowDraft.mockResolvedValue({ - hash: 'new-hash-789', - updated_at: 1234567890, - }) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-789') - }) - - it('should update draftUpdatedAt on success', async () => { - const updatedAt = 1234567890 - mockSyncWorkflowDraft.mockResolvedValue({ - hash: 'new-hash', - updated_at: updatedAt, - }) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSetDraftUpdatedAt).toHaveBeenCalledWith(updatedAt) - }) - - it('should call onSuccess callback on success', async () => { - const onSuccess = vi.fn() - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onSuccess }) - }) - - expect(onSuccess).toHaveBeenCalled() - }) - - it('should call onSettled callback after success', async () => { - const onSettled = vi.fn() - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onSettled }) - }) - - expect(onSettled).toHaveBeenCalled() - }) - }) - - describe('sync error handling - draft_workflow_not_sync (THE KEY FIX)', () => { - /** - * This is THE KEY TEST for the bug fix. - * - * SCENARIO: Multi-tab editing - * 1. User opens workflow in Tab A and Tab B - * 2. Tab A saves draft successfully, gets new hash - * 3. Tab B tries to save with old hash - * 4. Server returns 400 with code 'draft_workflow_not_sync' - * - * BEFORE FIX: - * - handleRefreshWorkflowDraft() was called without arguments - * - This would fetch draft AND overwrite the canvas - * - User loses their unsaved changes in Tab B - * - * AFTER FIX: - * - handleRefreshWorkflowDraft(true) is called - * - This fetches draft but DOES NOT overwrite canvas - * - Only hash is updated for the next sync attempt - * - User's unsaved changes are preserved - */ - it('should call handleRefreshWorkflowDraft with notUpdateCanvas=true when draft_workflow_not_sync error occurs', async () => { - const mockError = createMockErrorResponse('draft_workflow_not_sync') - mockSyncWorkflowDraft.mockRejectedValue(mockError) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - // THE KEY ASSERTION: handleRefreshWorkflowDraft must be called with true - await waitFor(() => { - expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledWith(true) - }) - }) - - it('should NOT call handleRefreshWorkflowDraft when notRefreshWhenSyncError is true', async () => { - const mockError = createMockErrorResponse('draft_workflow_not_sync') - mockSyncWorkflowDraft.mockRejectedValue(mockError) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - // First parameter is notRefreshWhenSyncError - await result.current.doSyncWorkflowDraft(true) - }) - - // Wait a bit for async operations - await new Promise(resolve => setTimeout(resolve, 100)) - - expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled() - }) - - it('should call onError callback when draft_workflow_not_sync error occurs', async () => { - const mockError = createMockErrorResponse('draft_workflow_not_sync') - mockSyncWorkflowDraft.mockRejectedValue(mockError) - const onError = vi.fn() - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onError }) - }) - - expect(onError).toHaveBeenCalled() - }) - - it('should call onSettled callback after error', async () => { - const mockError = createMockErrorResponse('draft_workflow_not_sync') - mockSyncWorkflowDraft.mockRejectedValue(mockError) - const onSettled = vi.fn() - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onSettled }) - }) - - expect(onSettled).toHaveBeenCalled() - }) - }) - - describe('other error handling', () => { - it('should NOT call handleRefreshWorkflowDraft for non-draft_workflow_not_sync errors', async () => { - const mockError = createMockErrorResponse('some_other_error') - mockSyncWorkflowDraft.mockRejectedValue(mockError) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - // Wait a bit for async operations - await new Promise(resolve => setTimeout(resolve, 100)) - - expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled() - }) - - it('should handle error without json method', async () => { - const mockError = new Error('Network error') - mockSyncWorkflowDraft.mockRejectedValue(mockError) - - const { result } = renderHook(() => useNodesSyncDraft()) - const onError = vi.fn() - - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onError }) - }) - - expect(onError).toHaveBeenCalled() - expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled() - }) - - it('should handle error with bodyUsed already true', async () => { - const mockError = { - json: vi.fn(), - bodyUsed: true, - } - mockSyncWorkflowDraft.mockRejectedValue(mockError) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - // Should not call json() when bodyUsed is true - expect(mockError.json).not.toHaveBeenCalled() - expect(mockHandleRefreshWorkflowDraft).not.toHaveBeenCalled() - }) - }) - - describe('read-only mode', () => { - it('should not sync when nodes are read-only', async () => { - mockGetNodesReadOnly.mockReturnValue(true) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSyncWorkflowDraft).not.toHaveBeenCalled() - }) - - it('should not sync on page close when nodes are read-only', () => { - mockGetNodesReadOnly.mockReturnValue(true) - - // Mock sendBeacon - const mockSendBeacon = vi.fn() - Object.defineProperty(navigator, 'sendBeacon', { - value: mockSendBeacon, - writable: true, - }) - - const { result } = renderHook(() => useNodesSyncDraft()) - - act(() => { - result.current.syncWorkflowDraftWhenPageClose() - }) - - expect(mockSendBeacon).not.toHaveBeenCalled() - }) - }) - - describe('workflow data not loaded', () => { - it('should not sync when workflow data is not loaded', async () => { - mockWorkflowStoreGetState.mockReturnValue( - createMockWorkflowStoreState({ isWorkflowDataLoaded: false }), - ) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSyncWorkflowDraft).not.toHaveBeenCalled() - }) - }) - - describe('no appId', () => { - it('should not sync when appId is not set', async () => { - mockWorkflowStoreGetState.mockReturnValue( - createMockWorkflowStoreState({ appId: null }), - ) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSyncWorkflowDraft).not.toHaveBeenCalled() - }) - }) - - describe('node filtering', () => { - it('should filter out temp nodes', async () => { - mockGetNodes.mockReturnValue([ - { id: 'node-1', type: 'start', data: { type: 'start' } }, - { id: 'node-temp', type: 'custom', data: { type: 'custom', _isTempNode: true } }, - { id: 'node-2', type: 'llm', data: { type: 'llm' } }, - ]) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSyncWorkflowDraft).toHaveBeenCalledWith( - expect.objectContaining({ - params: expect.objectContaining({ - graph: expect.objectContaining({ - nodes: expect.not.arrayContaining([ - expect.objectContaining({ id: 'node-temp' }), - ]), - }), - }), - }), - ) - }) - - it('should remove internal underscore properties from nodes', async () => { - mockGetNodes.mockReturnValue([ - { - id: 'node-1', - type: 'start', - data: { - type: 'start', - _internalProp: 'should be removed', - _anotherInternal: true, - publicProp: 'should remain', - }, - }, - ]) - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - const callArgs = mockSyncWorkflowDraft.mock.calls[0][0] - const sentNode = callArgs.params.graph.nodes[0] - - expect(sentNode.data).not.toHaveProperty('_internalProp') - expect(sentNode.data).not.toHaveProperty('_anotherInternal') - expect(sentNode.data).toHaveProperty('publicProp', 'should remain') - }) - }) - - describe('edge filtering', () => { - it('should filter out temp edges', async () => { - mockStoreState.edges = [ - { id: 'edge-1', source: 'node-1', target: 'node-2', data: {} }, - { id: 'edge-temp', source: 'node-1', target: 'node-3', data: { _isTemp: true } }, - ] - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - const callArgs = mockSyncWorkflowDraft.mock.calls[0][0] - const sentEdges = callArgs.params.graph.edges - - expect(sentEdges).toHaveLength(1) - expect(sentEdges[0].id).toBe('edge-1') - }) - - it('should remove internal underscore properties from edges', async () => { - mockStoreState.edges = [ - { - id: 'edge-1', - source: 'node-1', - target: 'node-2', - data: { - _internalEdgeProp: 'should be removed', - publicEdgeProp: 'should remain', - }, - }, - ] - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - const callArgs = mockSyncWorkflowDraft.mock.calls[0][0] - const sentEdge = callArgs.params.graph.edges[0] - - expect(sentEdge.data).not.toHaveProperty('_internalEdgeProp') - expect(sentEdge.data).toHaveProperty('publicEdgeProp', 'should remain') - }) - }) - - describe('viewport handling', () => { - it('should send current viewport from transform', async () => { - mockStoreState.transform = [100, 200, 1.5] - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - expect(mockSyncWorkflowDraft).toHaveBeenCalledWith( - expect.objectContaining({ - params: expect.objectContaining({ - graph: expect.objectContaining({ - viewport: { x: 100, y: 200, zoom: 1.5 }, - }), - }), - }), - ) - }) - }) - - describe('multi-tab concurrent editing scenario (END-TO-END TEST)', () => { - /** - * Simulates the complete multi-tab scenario to verify the fix works correctly. - * - * Scenario: - * 1. Tab A and Tab B both have the workflow open with hash 'hash-v1' - * 2. Tab A saves successfully, server returns 'hash-v2' - * 3. Tab B tries to save with 'hash-v1', gets 'draft_workflow_not_sync' error - * 4. Tab B should only update hash to 'hash-v2', not overwrite canvas - * 5. Tab B can now retry save with correct hash - */ - it('should preserve canvas data during hash conflict resolution', async () => { - // Initial state: both tabs have hash-v1 - mockWorkflowStoreGetState.mockReturnValue( - createMockWorkflowStoreState({ syncWorkflowDraftHash: 'hash-v1' }), - ) - - // Tab B tries to save with old hash, server returns error - const syncError = createMockErrorResponse('draft_workflow_not_sync') - mockSyncWorkflowDraft.mockRejectedValue(syncError) - - const { result } = renderHook(() => useNodesSyncDraft()) - - // Tab B attempts to sync - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - // Verify the sync was attempted with old hash - expect(mockSyncWorkflowDraft).toHaveBeenCalledWith( - expect.objectContaining({ - params: expect.objectContaining({ - hash: 'hash-v1', - }), - }), - ) - - // Verify handleRefreshWorkflowDraft was called with true (not overwrite canvas) - await waitFor(() => { - expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledWith(true) - }) - - // The key assertion: only one argument (true) was passed - expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledTimes(1) - expect(mockHandleRefreshWorkflowDraft.mock.calls[0]).toEqual([true]) - }) - - it('should handle multiple consecutive sync failures gracefully', async () => { - // Create fresh error for each call to avoid bodyUsed issue - mockSyncWorkflowDraft - .mockRejectedValueOnce(createMockErrorResponse('draft_workflow_not_sync')) - .mockRejectedValueOnce(createMockErrorResponse('draft_workflow_not_sync')) - - const { result } = renderHook(() => useNodesSyncDraft()) - - // First sync attempt - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - // Wait for first refresh call - await waitFor(() => { - expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledTimes(1) - }) - - // Second sync attempt - await act(async () => { - await result.current.doSyncWorkflowDraft() - }) - - // Both should call handleRefreshWorkflowDraft with true - await waitFor(() => { - expect(mockHandleRefreshWorkflowDraft).toHaveBeenCalledTimes(2) - }) - - mockHandleRefreshWorkflowDraft.mock.calls.forEach((call) => { - expect(call).toEqual([true]) - }) - }) - }) - - describe('callbacks behavior', () => { - it('should not call onSuccess when sync fails', async () => { - const syncError = createMockErrorResponse('draft_workflow_not_sync') - mockSyncWorkflowDraft.mockRejectedValue(syncError) - const onSuccess = vi.fn() - const onError = vi.fn() - - const { result } = renderHook(() => useNodesSyncDraft()) - - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onSuccess, onError }) - }) - - expect(onSuccess).not.toHaveBeenCalled() - expect(onError).toHaveBeenCalled() - }) - - it('should always call onSettled regardless of success or failure', async () => { - const onSettled = vi.fn() - - const { result } = renderHook(() => useNodesSyncDraft()) - - // Test success case - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onSettled }) - }) - expect(onSettled).toHaveBeenCalledTimes(1) - - // Reset - onSettled.mockClear() - - // Test failure case - const syncError = createMockErrorResponse('draft_workflow_not_sync') - mockSyncWorkflowDraft.mockRejectedValue(syncError) - - await act(async () => { - await result.current.doSyncWorkflowDraft(false, { onSettled }) - }) - expect(onSettled).toHaveBeenCalledTimes(1) - }) - }) -}) 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 5673d5edf1..5d394bab1e 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 @@ -115,7 +115,7 @@ export const useNodesSyncDraft = () => { if (error && error.json && !error.bodyUsed) { error.json().then((err: any) => { if (err.code === 'draft_workflow_not_sync' && !notRefreshWhenSyncError) - handleRefreshWorkflowDraft(true) + handleRefreshWorkflowDraft() }) } callback?.onError?.() diff --git a/web/app/components/workflow-app/hooks/use-workflow-refresh-draft.spec.ts b/web/app/components/workflow-app/hooks/use-workflow-refresh-draft.spec.ts deleted file mode 100644 index 702d8787f0..0000000000 --- a/web/app/components/workflow-app/hooks/use-workflow-refresh-draft.spec.ts +++ /dev/null @@ -1,556 +0,0 @@ -/** - * Test Suite for useWorkflowRefreshDraft Hook - * - * PURPOSE: - * This hook is responsible for refreshing workflow draft data from the server. - * The key fix being tested is the `notUpdateCanvas` parameter behavior. - * - * MULTI-TAB PROBLEM SCENARIO: - * 1. User opens the same workflow in Tab A and Tab B (both have hash: v1) - * 2. Tab A saves successfully, server returns new hash: v2 - * 3. Tab B tries to save with old hash: v1, server returns 400 error (draft_workflow_not_sync) - * 4. BEFORE FIX: handleRefreshWorkflowDraft() was called without args, which fetched - * draft AND overwrote canvas - user lost unsaved changes in Tab B - * 5. AFTER FIX: handleRefreshWorkflowDraft(true) is called, which fetches draft but - * only updates hash, preserving user's canvas changes - * - * TESTING STRATEGY: - * We don't simulate actual tab switching UI behavior. Instead, we test the hook's - * response to specific inputs: - * - When notUpdateCanvas=true: should NOT call handleUpdateWorkflowCanvas - * - When notUpdateCanvas=false/undefined: should call handleUpdateWorkflowCanvas - * - * This is behavior-driven testing - we verify "what the code does when given specific - * inputs" rather than simulating complete user interaction flows. - */ - -import { act, renderHook, waitFor } from '@testing-library/react' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useWorkflowRefreshDraft } from './use-workflow-refresh-draft' - -// Mock the workflow service -const mockFetchWorkflowDraft = vi.fn() -vi.mock('@/service/workflow', () => ({ - fetchWorkflowDraft: (...args: unknown[]) => mockFetchWorkflowDraft(...args), -})) - -// Mock the workflow update hook -const mockHandleUpdateWorkflowCanvas = vi.fn() -vi.mock('@/app/components/workflow/hooks', () => ({ - useWorkflowUpdate: () => ({ - handleUpdateWorkflowCanvas: mockHandleUpdateWorkflowCanvas, - }), -})) - -// Mock store state -const mockSetSyncWorkflowDraftHash = vi.fn() -const mockSetIsSyncingWorkflowDraft = vi.fn() -const mockSetEnvironmentVariables = vi.fn() -const mockSetEnvSecrets = vi.fn() -const mockSetConversationVariables = vi.fn() -const mockSetIsWorkflowDataLoaded = vi.fn() -const mockCancelDebouncedSync = vi.fn() - -const createMockStoreState = (overrides = {}) => ({ - appId: 'test-app-id', - setSyncWorkflowDraftHash: mockSetSyncWorkflowDraftHash, - setIsSyncingWorkflowDraft: mockSetIsSyncingWorkflowDraft, - setEnvironmentVariables: mockSetEnvironmentVariables, - setEnvSecrets: mockSetEnvSecrets, - setConversationVariables: mockSetConversationVariables, - setIsWorkflowDataLoaded: mockSetIsWorkflowDataLoaded, - isWorkflowDataLoaded: true, - debouncedSyncWorkflowDraft: { - cancel: mockCancelDebouncedSync, - }, - ...overrides, -}) - -const mockWorkflowStoreGetState = vi.fn() -vi.mock('@/app/components/workflow/store', () => ({ - useWorkflowStore: () => ({ - getState: mockWorkflowStoreGetState, - }), -})) - -// Default mock response from fetchWorkflowDraft -const createMockDraftResponse = (overrides = {}) => ({ - hash: 'new-hash-12345', - graph: { - nodes: [{ id: 'node-1', type: 'start', data: {} }], - edges: [{ id: 'edge-1', source: 'node-1', target: 'node-2' }], - viewport: { x: 100, y: 200, zoom: 1.5 }, - }, - environment_variables: [ - { id: 'env-1', name: 'API_KEY', value: 'secret-key', value_type: 'secret' }, - { id: 'env-2', name: 'BASE_URL', value: 'https://api.example.com', value_type: 'string' }, - ], - conversation_variables: [ - { id: 'conv-1', name: 'user_input', value: 'test' }, - ], - ...overrides, -}) - -describe('useWorkflowRefreshDraft', () => { - beforeEach(() => { - vi.clearAllMocks() - mockWorkflowStoreGetState.mockReturnValue(createMockStoreState()) - mockFetchWorkflowDraft.mockResolvedValue(createMockDraftResponse()) - }) - - afterEach(() => { - vi.resetAllMocks() - }) - - describe('handleRefreshWorkflowDraft function', () => { - it('should return handleRefreshWorkflowDraft function', () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - expect(result.current.handleRefreshWorkflowDraft).toBeDefined() - expect(typeof result.current.handleRefreshWorkflowDraft).toBe('function') - }) - }) - - describe('notUpdateCanvas parameter behavior (THE KEY FIX)', () => { - it('should NOT call handleUpdateWorkflowCanvas when notUpdateCanvas is true', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockFetchWorkflowDraft).toHaveBeenCalledWith('/apps/test-app-id/workflows/draft') - }) - - await waitFor(() => { - expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345') - }) - - // THE KEY ASSERTION: Canvas should NOT be updated when notUpdateCanvas is true - expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled() - }) - - it('should call handleUpdateWorkflowCanvas when notUpdateCanvas is false', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(false) - }) - - await waitFor(() => { - expect(mockFetchWorkflowDraft).toHaveBeenCalledWith('/apps/test-app-id/workflows/draft') - }) - - await waitFor(() => { - // Canvas SHOULD be updated when notUpdateCanvas is false - expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledWith({ - nodes: [{ id: 'node-1', type: 'start', data: {} }], - edges: [{ id: 'edge-1', source: 'node-1', target: 'node-2' }], - viewport: { x: 100, y: 200, zoom: 1.5 }, - }) - }) - - await waitFor(() => { - expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345') - }) - }) - - it('should call handleUpdateWorkflowCanvas when notUpdateCanvas is undefined (default)', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - await waitFor(() => { - expect(mockFetchWorkflowDraft).toHaveBeenCalled() - }) - - await waitFor(() => { - // Canvas SHOULD be updated when notUpdateCanvas is undefined - expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalled() - }) - }) - - it('should still update hash even when notUpdateCanvas is true', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345') - }) - - // Verify canvas was NOT updated - expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled() - }) - - it('should still update environment variables when notUpdateCanvas is true', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetEnvironmentVariables).toHaveBeenCalledWith([ - { id: 'env-1', name: 'API_KEY', value: '[__HIDDEN__]', value_type: 'secret' }, - { id: 'env-2', name: 'BASE_URL', value: 'https://api.example.com', value_type: 'string' }, - ]) - }) - - expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled() - }) - - it('should still update env secrets when notUpdateCanvas is true', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetEnvSecrets).toHaveBeenCalledWith({ - 'env-1': 'secret-key', - }) - }) - - expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled() - }) - - it('should still update conversation variables when notUpdateCanvas is true', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetConversationVariables).toHaveBeenCalledWith([ - { id: 'conv-1', name: 'user_input', value: 'test' }, - ]) - }) - - expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled() - }) - }) - - describe('syncing state management', () => { - it('should set isSyncingWorkflowDraft to true before fetch', () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - expect(mockSetIsSyncingWorkflowDraft).toHaveBeenCalledWith(true) - }) - - it('should set isSyncingWorkflowDraft to false after fetch completes', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - await waitFor(() => { - expect(mockSetIsSyncingWorkflowDraft).toHaveBeenCalledWith(false) - }) - }) - - it('should set isSyncingWorkflowDraft to false even when fetch fails', async () => { - mockFetchWorkflowDraft.mockRejectedValue(new Error('Network error')) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - await waitFor(() => { - expect(mockSetIsSyncingWorkflowDraft).toHaveBeenCalledWith(false) - }) - }) - }) - - describe('isWorkflowDataLoaded flag management', () => { - it('should set isWorkflowDataLoaded to false before fetch when it was true', () => { - mockWorkflowStoreGetState.mockReturnValue( - createMockStoreState({ isWorkflowDataLoaded: true }), - ) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - expect(mockSetIsWorkflowDataLoaded).toHaveBeenCalledWith(false) - }) - - it('should set isWorkflowDataLoaded to true after fetch succeeds', async () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - await waitFor(() => { - expect(mockSetIsWorkflowDataLoaded).toHaveBeenCalledWith(true) - }) - }) - - it('should restore isWorkflowDataLoaded when fetch fails and it was previously loaded', async () => { - mockWorkflowStoreGetState.mockReturnValue( - createMockStoreState({ isWorkflowDataLoaded: true }), - ) - mockFetchWorkflowDraft.mockRejectedValue(new Error('Network error')) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - await waitFor(() => { - // Should restore to true because wasLoaded was true - expect(mockSetIsWorkflowDataLoaded).toHaveBeenLastCalledWith(true) - }) - }) - }) - - describe('debounced sync cancellation', () => { - it('should cancel debounced sync before fetching draft', () => { - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - - expect(mockCancelDebouncedSync).toHaveBeenCalled() - }) - - it('should handle case when debouncedSyncWorkflowDraft has no cancel method', () => { - mockWorkflowStoreGetState.mockReturnValue( - createMockStoreState({ debouncedSyncWorkflowDraft: {} }), - ) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - // Should not throw - expect(() => { - act(() => { - result.current.handleRefreshWorkflowDraft() - }) - }).not.toThrow() - }) - }) - - describe('edge cases', () => { - it('should handle empty graph in response', async () => { - mockFetchWorkflowDraft.mockResolvedValue({ - hash: 'hash-empty', - graph: null, - environment_variables: [], - conversation_variables: [], - }) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(false) - }) - - await waitFor(() => { - expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledWith({ - nodes: [], - edges: [], - viewport: { x: 0, y: 0, zoom: 1 }, - }) - }) - }) - - it('should handle missing viewport in response', async () => { - mockFetchWorkflowDraft.mockResolvedValue({ - hash: 'hash-no-viewport', - graph: { - nodes: [{ id: 'node-1' }], - edges: [], - viewport: null, - }, - environment_variables: [], - conversation_variables: [], - }) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(false) - }) - - await waitFor(() => { - expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalledWith({ - nodes: [{ id: 'node-1' }], - edges: [], - viewport: { x: 0, y: 0, zoom: 1 }, - }) - }) - }) - - it('should handle missing environment_variables in response', async () => { - mockFetchWorkflowDraft.mockResolvedValue({ - hash: 'hash-no-env', - graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } }, - environment_variables: undefined, - conversation_variables: [], - }) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetEnvironmentVariables).toHaveBeenCalledWith([]) - expect(mockSetEnvSecrets).toHaveBeenCalledWith({}) - }) - }) - - it('should handle missing conversation_variables in response', async () => { - mockFetchWorkflowDraft.mockResolvedValue({ - hash: 'hash-no-conv', - graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } }, - environment_variables: [], - conversation_variables: undefined, - }) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetConversationVariables).toHaveBeenCalledWith([]) - }) - }) - - it('should filter only secret type for envSecrets', async () => { - mockFetchWorkflowDraft.mockResolvedValue({ - hash: 'hash-mixed-env', - graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } }, - environment_variables: [ - { id: 'env-1', name: 'SECRET_KEY', value: 'secret-value', value_type: 'secret' }, - { id: 'env-2', name: 'PUBLIC_URL', value: 'https://example.com', value_type: 'string' }, - { id: 'env-3', name: 'ANOTHER_SECRET', value: 'another-secret', value_type: 'secret' }, - ], - conversation_variables: [], - }) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetEnvSecrets).toHaveBeenCalledWith({ - 'env-1': 'secret-value', - 'env-3': 'another-secret', - }) - }) - }) - - it('should hide secret values in environment variables', async () => { - mockFetchWorkflowDraft.mockResolvedValue({ - hash: 'hash-secrets', - graph: { nodes: [], edges: [], viewport: { x: 0, y: 0, zoom: 1 } }, - environment_variables: [ - { id: 'env-1', name: 'SECRET_KEY', value: 'super-secret', value_type: 'secret' }, - { id: 'env-2', name: 'PUBLIC_URL', value: 'https://example.com', value_type: 'string' }, - ], - conversation_variables: [], - }) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockSetEnvironmentVariables).toHaveBeenCalledWith([ - { id: 'env-1', name: 'SECRET_KEY', value: '[__HIDDEN__]', value_type: 'secret' }, - { id: 'env-2', name: 'PUBLIC_URL', value: 'https://example.com', value_type: 'string' }, - ]) - }) - }) - }) - - describe('multi-tab scenario simulation (THE BUG FIX VERIFICATION)', () => { - /** - * This test verifies the fix for the multi-tab scenario: - * 1. User opens workflow in Tab A and Tab B - * 2. Tab A saves draft successfully - * 3. Tab B tries to save but gets 'draft_workflow_not_sync' error (hash mismatch) - * 4. BEFORE FIX: Tab B would fetch draft and overwrite canvas with old data - * 5. AFTER FIX: Tab B only updates hash, preserving user's canvas changes - */ - it('should only update hash when called with notUpdateCanvas=true (simulating sync error recovery)', async () => { - const mockResponse = createMockDraftResponse() - mockFetchWorkflowDraft.mockResolvedValue(mockResponse) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - // Simulate the sync error recovery scenario where notUpdateCanvas is true - act(() => { - result.current.handleRefreshWorkflowDraft(true) - }) - - await waitFor(() => { - expect(mockFetchWorkflowDraft).toHaveBeenCalled() - }) - - await waitFor(() => { - // Hash should be updated for next sync attempt - expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345') - }) - - // Canvas should NOT be updated - user's changes are preserved - expect(mockHandleUpdateWorkflowCanvas).not.toHaveBeenCalled() - - // Other states should still be updated - expect(mockSetEnvironmentVariables).toHaveBeenCalled() - expect(mockSetConversationVariables).toHaveBeenCalled() - }) - - it('should update canvas when called with notUpdateCanvas=false (normal refresh)', async () => { - const mockResponse = createMockDraftResponse() - mockFetchWorkflowDraft.mockResolvedValue(mockResponse) - - const { result } = renderHook(() => useWorkflowRefreshDraft()) - - // Simulate normal refresh scenario - act(() => { - result.current.handleRefreshWorkflowDraft(false) - }) - - await waitFor(() => { - expect(mockFetchWorkflowDraft).toHaveBeenCalled() - }) - - await waitFor(() => { - expect(mockSetSyncWorkflowDraftHash).toHaveBeenCalledWith('new-hash-12345') - }) - - // Canvas SHOULD be updated in normal refresh - await waitFor(() => { - expect(mockHandleUpdateWorkflowCanvas).toHaveBeenCalled() - }) - }) - }) -}) 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 a7283c0078..fa4a44d894 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((notUpdateCanvas?: boolean) => { + const handleRefreshWorkflowDraft = useCallback(() => { const { appId, setSyncWorkflowDraftHash, @@ -31,14 +31,12 @@ export const useWorkflowRefreshDraft = () => { fetchWorkflowDraft(`/apps/${appId}/workflows/draft`) .then((response) => { // Ensure we have a valid workflow structure with viewport - 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) + const workflowData: WorkflowDataUpdater = { + nodes: response.graph?.nodes || [], + edges: response.graph?.edges || [], + viewport: response.graph?.viewport || { x: 0, y: 0, zoom: 1 }, } + handleUpdateWorkflowCanvas(workflowData) setSyncWorkflowDraftHash(response.hash) setEnvSecrets((response.environment_variables || []).filter(env => env.value_type === 'secret').reduce((acc, env) => { acc[env.id] = env.value diff --git a/web/tsconfig.json b/web/tsconfig.json index 45d7fa7871..6df4f56bf1 100644 --- a/web/tsconfig.json +++ b/web/tsconfig.json @@ -2,7 +2,7 @@ "compilerOptions": { "incremental": true, "target": "es2022", - "jsx": "preserve", + "jsx": "react-jsx", "lib": [ "dom", "dom.iterable",