diff --git a/web/app/components/workflow/skill/hooks/file-tree/dnd/use-file-drop.spec.tsx b/web/app/components/workflow/skill/hooks/file-tree/dnd/__tests__/use-file-drop.spec.tsx similarity index 80% rename from web/app/components/workflow/skill/hooks/file-tree/dnd/use-file-drop.spec.tsx rename to web/app/components/workflow/skill/hooks/file-tree/dnd/__tests__/use-file-drop.spec.tsx index 65f88a07357..d90f8b6547e 100644 --- a/web/app/components/workflow/skill/hooks/file-tree/dnd/use-file-drop.spec.tsx +++ b/web/app/components/workflow/skill/hooks/file-tree/dnd/__tests__/use-file-drop.spec.tsx @@ -4,8 +4,8 @@ import { act, renderHook } from '@testing-library/react' import { useStore as useAppStore } from '@/app/components/app/store' import { WorkflowContext } from '@/app/components/workflow/context' import { createWorkflowStore } from '@/app/components/workflow/store' -import { ROOT_ID } from '../../../constants' -import { useFileDrop } from './use-file-drop' +import { ROOT_ID } from '../../../../constants' +import { useFileDrop } from '../use-file-drop' const { mockUploadMutateAsync, @@ -28,11 +28,11 @@ vi.mock('@/service/use-app-asset', () => ({ }), })) -vi.mock('../../../utils/skill-upload-utils', () => ({ +vi.mock('../../../../utils/skill-upload-utils', () => ({ prepareSkillUploadFile: mockPrepareSkillUploadFile, })) -vi.mock('../data/use-skill-tree-collaboration', () => ({ +vi.mock('../../data/use-skill-tree-collaboration', () => ({ useSkillTreeUpdateEmitter: () => mockEmitTreeUpdate, })) @@ -61,9 +61,9 @@ type MockDragEvent = { const createWrapper = (store: ReturnType) => { return ({ children }: { children: ReactNode }) => ( - + {children} - + ) } @@ -116,7 +116,6 @@ describe('useFileDrop', () => { mockUploadMutateAsync.mockResolvedValue(undefined) }) - // Scenario: drag-over updates upload drag state for valid external file drags. describe('Drag Over', () => { it('should set upload drag state when file drag enters root target', () => { const store = createWorkflowStore({}) @@ -155,7 +154,6 @@ describe('useFileDrop', () => { }) }) - // Scenario: directory drops are rejected and do not trigger upload mutations. describe('Folder Drop Rejection', () => { it('should reject dropped folders and show an error toast', async () => { const store = createWorkflowStore({}) @@ -175,6 +173,7 @@ describe('useFileDrop', () => { expect(mockToastError).toHaveBeenCalledWith('workflow.skillSidebar.menu.folderDropNotSupported') expect(store.getState().currentDragType).toBeNull() expect(store.getState().dragOverFolderId).toBeNull() + expect(store.getState().uploadStatus).toBe('idle') }) it('should upload valid files while rejecting directories in a mixed drop payload', async () => { @@ -206,9 +205,8 @@ describe('useFileDrop', () => { }) }) - // Scenario: successful drops upload prepared files and emit collaboration updates. describe('Upload Success', () => { - it('should upload dropped files and show success toast when upload succeeds', async () => { + it('should write upload success state when dropped files upload succeeds', async () => { const store = createWorkflowStore({}) const { result } = renderHook(() => useFileDrop(), { wrapper: createWrapper(store) }) const firstFile = new File(['alpha'], 'alpha.md', { type: 'text/markdown' }) @@ -240,12 +238,40 @@ describe('useFileDrop', () => { }) expect(mockEmitTreeUpdate).toHaveBeenCalledTimes(1) expect(mockToastSuccess).toHaveBeenCalledWith('workflow.skillSidebar.menu.filesUploaded:{"count":2}') + expect(store.getState().uploadStatus).toBe('success') + expect(store.getState().uploadProgress).toEqual({ uploaded: 2, total: 2, failed: 0 }) }) }) - // Scenario: failed uploads surface an error toast and skip collaboration updates. describe('Upload Error', () => { - it('should show error toast when upload fails', async () => { + it('should write partial error state when only part of the dropped files fail', async () => { + const store = createWorkflowStore({}) + const { result } = renderHook(() => useFileDrop(), { wrapper: createWrapper(store) }) + const firstFile = new File(['ok'], 'ok.md', { type: 'text/markdown' }) + const secondFile = new File(['failed'], 'failed.md', { type: 'text/markdown' }) + const event = createDragEvent({ + items: [ + createDataTransferItem({ file: firstFile }), + createDataTransferItem({ file: secondFile }), + ], + }) + mockUploadMutateAsync + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(new Error('upload failed')) + + await act(async () => { + await result.current.handleDrop(event as unknown as React.DragEvent, 'folder-err') + }) + + expect(mockUploadMutateAsync).toHaveBeenCalledTimes(2) + expect(mockEmitTreeUpdate).toHaveBeenCalledTimes(1) + expect(mockToastSuccess).not.toHaveBeenCalled() + expect(mockToastError).not.toHaveBeenCalled() + expect(store.getState().uploadStatus).toBe('partial_error') + expect(store.getState().uploadProgress).toEqual({ uploaded: 1, total: 2, failed: 1 }) + }) + + it('should write partial error state and show error toast when all dropped files fail', async () => { const store = createWorkflowStore({}) const { result } = renderHook(() => useFileDrop(), { wrapper: createWrapper(store) }) const file = new File(['content'], 'failed.md', { type: 'text/markdown' }) @@ -261,6 +287,8 @@ describe('useFileDrop', () => { expect(mockUploadMutateAsync).toHaveBeenCalledTimes(1) expect(mockEmitTreeUpdate).not.toHaveBeenCalled() expect(mockToastError).toHaveBeenCalledWith('workflow.skillSidebar.menu.uploadError') + expect(store.getState().uploadStatus).toBe('partial_error') + expect(store.getState().uploadProgress).toEqual({ uploaded: 0, total: 1, failed: 1 }) }) }) }) diff --git a/web/app/components/workflow/skill/hooks/file-tree/dnd/use-file-drop.ts b/web/app/components/workflow/skill/hooks/file-tree/dnd/use-file-drop.ts index f6ec93e29fc..c0b80351f95 100644 --- a/web/app/components/workflow/skill/hooks/file-tree/dnd/use-file-drop.ts +++ b/web/app/components/workflow/skill/hooks/file-tree/dnd/use-file-drop.ts @@ -10,8 +10,8 @@ import { toast } from '@/app/components/base/ui/toast' import { useWorkflowStore } from '@/app/components/workflow/store' import { useUploadFileWithPresignedUrl } from '@/service/use-app-asset' import { ROOT_ID } from '../../../constants' -import { prepareSkillUploadFile } from '../../../utils/skill-upload-utils' import { useSkillTreeUpdateEmitter } from '../data/use-skill-tree-collaboration' +import { uploadFilesWithStatus } from '../operations/upload-files-with-status' type FileDropTarget = { folderId: string | null @@ -78,21 +78,24 @@ export function useFileDrop() { return try { - const uploadFiles = await Promise.all(files.map(file => prepareSkillUploadFile(file))) - await Promise.all( - uploadFiles.map(file => - uploadFile.mutateAsync({ - appId, - file, - parentId: targetFolderId, - }), - ), - ) + const result = await uploadFilesWithStatus({ + files, + appId, + parentId: targetFolderId, + storeApi, + uploadFile: uploadFile.mutateAsync, + }) - emitTreeUpdate() - toast.success(t('skillSidebar.menu.filesUploaded', { count: files.length })) + if (result.uploaded > 0) + emitTreeUpdate() + + if (result.status === 'success') + toast.success(t('skillSidebar.menu.filesUploaded', { count: result.uploaded })) + else if (result.uploaded === 0) + toast.error(t('skillSidebar.menu.uploadError')) } catch { + storeApi.getState().setUploadStatus('partial_error') toast.error(t('skillSidebar.menu.uploadError')) } }, [appId, uploadFile, t, storeApi, emitTreeUpdate]) diff --git a/web/app/components/workflow/skill/hooks/file-tree/operations/__tests__/upload-files-with-status.spec.ts b/web/app/components/workflow/skill/hooks/file-tree/operations/__tests__/upload-files-with-status.spec.ts new file mode 100644 index 00000000000..7d3967f66e3 --- /dev/null +++ b/web/app/components/workflow/skill/hooks/file-tree/operations/__tests__/upload-files-with-status.spec.ts @@ -0,0 +1,122 @@ +import type { StoreApi } from 'zustand' +import type { SkillEditorSliceShape, UploadProgress, UploadStatus } from '@/app/components/workflow/store/workflow/skill-editor/types' +import type { AppAssetNode } from '@/types/app-asset' +import { uploadFilesWithStatus } from '../upload-files-with-status' + +const mocks = vi.hoisted(() => ({ + prepareSkillUploadFile: vi.fn<(file: File) => Promise>(), +})) + +vi.mock('../../../../utils/skill-upload-utils', () => ({ + prepareSkillUploadFile: mocks.prepareSkillUploadFile, +})) + +type MockStoreState = { + uploadStatus: UploadStatus + uploadProgress: UploadProgress + setUploadStatus: (status: UploadStatus) => void + setUploadProgress: (progress: UploadProgress) => void +} + +const createStoreApi = () => { + const state: MockStoreState = { + uploadStatus: 'idle', + uploadProgress: { uploaded: 0, total: 0, failed: 0 }, + setUploadStatus: vi.fn((status: UploadStatus) => { + state.uploadStatus = status + }), + setUploadProgress: vi.fn((progress: UploadProgress) => { + state.uploadProgress = progress + }), + } + + return { + state, + storeApi: { + getState: () => state, + } as unknown as StoreApi, + } +} + +const createNode = (id: string, name: string): AppAssetNode => ({ + id, + name, + node_type: 'file', + parent_id: null, + order: 0, + size: 1, + extension: name.split('.').pop() || '', +}) + +describe('uploadFilesWithStatus', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + describe('success flow', () => { + it('should write uploading and success states when all files upload successfully', async () => { + const { state, storeApi } = createStoreApi() + const files = [ + new File(['a'], 'guide.md', { type: 'text/markdown' }), + new File(['b'], 'notes.md', { type: 'text/markdown' }), + ] + const uploadFile = vi.fn(async ({ file }: { file: File }) => createNode(file.name, file.name)) + mocks.prepareSkillUploadFile.mockImplementation(async file => file) + + const result = await uploadFilesWithStatus({ + files, + appId: 'app-id', + parentId: 'folder-id', + storeApi, + uploadFile, + }) + + expect(result.status).toBe('success') + expect(result.uploaded).toBe(2) + expect(result.failed).toBe(0) + expect(result.uploadedNodes).toHaveLength(2) + expect(state.uploadStatus).toBe('success') + expect(state.uploadProgress).toEqual({ uploaded: 2, total: 2, failed: 0 }) + expect(state.setUploadStatus).toHaveBeenNthCalledWith(1, 'uploading') + expect(state.setUploadStatus).toHaveBeenLastCalledWith('success') + }) + }) + + describe('partial failure flow', () => { + it('should keep progress and mark partial_error when preparation or upload fails', async () => { + const { state, storeApi } = createStoreApi() + const files = [ + new File(['ok'], 'ok.md', { type: 'text/markdown' }), + new File(['bad-prepare'], 'prepare-fail.md', { type: 'text/markdown' }), + new File(['bad-upload'], 'upload-fail.md', { type: 'text/markdown' }), + ] + const uploadFile = vi.fn(async ({ file }: { file: File }) => { + if (file.name === 'upload-fail.md') + throw new Error('upload failed') + return createNode(file.name, file.name) + }) + mocks.prepareSkillUploadFile.mockImplementation(async (file) => { + if (file.name === 'prepare-fail.md') + throw new Error('prepare failed') + return file + }) + + const result = await uploadFilesWithStatus({ + files, + appId: 'app-id', + parentId: null, + storeApi, + uploadFile, + }) + + expect(result.status).toBe('partial_error') + expect(result.uploaded).toBe(1) + expect(result.failed).toBe(2) + expect(result.uploadedNodes).toEqual([createNode('ok.md', 'ok.md')]) + expect(state.uploadStatus).toBe('partial_error') + expect(state.uploadProgress).toEqual({ uploaded: 1, total: 3, failed: 2 }) + expect(state.setUploadStatus).toHaveBeenNthCalledWith(1, 'uploading') + expect(state.setUploadStatus).toHaveBeenLastCalledWith('partial_error') + }) + }) +}) diff --git a/web/app/components/workflow/skill/hooks/file-tree/operations/upload-files-with-status.ts b/web/app/components/workflow/skill/hooks/file-tree/operations/upload-files-with-status.ts new file mode 100644 index 00000000000..084a9703770 --- /dev/null +++ b/web/app/components/workflow/skill/hooks/file-tree/operations/upload-files-with-status.ts @@ -0,0 +1,80 @@ +import type { StoreApi } from 'zustand' +import type { SkillEditorSliceShape } from '@/app/components/workflow/store/workflow/skill-editor/types' +import type { AppAssetNode } from '@/types/app-asset' +import { prepareSkillUploadFile } from '../../../utils/skill-upload-utils' + +type UploadFilesWithStatusOptions = { + files: File[] + appId: string + parentId: string | null + storeApi: StoreApi + uploadFile: (payload: { + appId: string + file: File + parentId: string | null + }) => Promise +} + +export type UploadFilesWithStatusResult = { + uploadedNodes: AppAssetNode[] + uploaded: number + total: number + failed: number + status: 'success' | 'partial_error' +} + +export const uploadFilesWithStatus = async ({ + files, + appId, + parentId, + storeApi, + uploadFile, +}: UploadFilesWithStatusOptions): Promise => { + const total = files.length + const progress = { uploaded: 0, failed: 0 } + + storeApi.getState().setUploadStatus('uploading') + storeApi.getState().setUploadProgress({ uploaded: 0, total, failed: 0 }) + + const preparedFiles = await Promise.all(files.map(async (file) => { + try { + return await prepareSkillUploadFile(file) + } + catch { + progress.failed++ + storeApi.getState().setUploadProgress({ uploaded: progress.uploaded, total, failed: progress.failed }) + return null + } + })) + + const uploadedNodes = (await Promise.all( + preparedFiles + .filter((file): file is File => !!file) + .map(async (file) => { + try { + const node = await uploadFile({ appId, file, parentId }) + progress.uploaded++ + return node + } + catch { + progress.failed++ + return null + } + finally { + storeApi.getState().setUploadProgress({ uploaded: progress.uploaded, total, failed: progress.failed }) + } + }), + )).filter((node): node is AppAssetNode => !!node) + + const status = progress.failed > 0 ? 'partial_error' : 'success' + storeApi.getState().setUploadStatus(status) + storeApi.getState().setUploadProgress({ uploaded: progress.uploaded, total, failed: progress.failed }) + + return { + uploadedNodes, + uploaded: progress.uploaded, + total, + failed: progress.failed, + status, + } +} diff --git a/web/app/components/workflow/skill/hooks/file-tree/operations/use-create-operations.ts b/web/app/components/workflow/skill/hooks/file-tree/operations/use-create-operations.ts index b78bcd4fbe9..fe86aee95f7 100644 --- a/web/app/components/workflow/skill/hooks/file-tree/operations/use-create-operations.ts +++ b/web/app/components/workflow/skill/hooks/file-tree/operations/use-create-operations.ts @@ -11,6 +11,7 @@ import { } from '@/service/use-app-asset' import { prepareSkillUploadFile } from '../../../utils/skill-upload-utils' import { useSkillTreeUpdateEmitter } from '../data/use-skill-tree-collaboration' +import { uploadFilesWithStatus } from './upload-files-with-status' type UseCreateOperationsOptions = { parentId: string | null @@ -55,42 +56,25 @@ export function useCreateOperations({ onClose() return } - - const total = files.length - const progress = { uploaded: 0, failed: 0 } - - storeApi.getState().setUploadStatus('uploading') - storeApi.getState().setUploadProgress({ uploaded: 0, total, failed: 0 }) + let uploadedCount = 0 try { - const uploadFiles = await Promise.all(files.map(file => prepareSkillUploadFile(file))) - const uploadedNodes = (await Promise.all( - uploadFiles.map(async (file) => { - try { - const node = await uploadFileAsync({ appId, file, parentId }) - progress.uploaded++ - return node - } - catch { - progress.failed++ - return null - } - finally { - storeApi.getState().setUploadProgress({ uploaded: progress.uploaded, total, failed: progress.failed }) - } - }), - )).filter((node): node is AppAssetNode => !!node) - - storeApi.getState().setUploadStatus(progress.failed > 0 ? 'partial_error' : 'success') - storeApi.getState().setUploadProgress({ uploaded: progress.uploaded, total, failed: progress.failed }) - if (uploadedNodes.length > 0) - onFilesUploaded?.(uploadedNodes) + const result = await uploadFilesWithStatus({ + files, + appId, + parentId, + storeApi, + uploadFile: uploadFileAsync, + }) + uploadedCount = result.uploaded + if (result.uploadedNodes.length > 0) + onFilesUploaded?.(result.uploadedNodes) } catch { storeApi.getState().setUploadStatus('partial_error') } finally { - if (progress.uploaded > 0) + if (uploadedCount > 0) emitTreeUpdate() e.target.value = '' onClose()