mirror of
https://github.com/langgenius/dify.git
synced 2026-04-05 16:03:14 +08:00
fix: unify skill file upload status handling
This commit is contained in:
@@ -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<typeof createWorkflowStore>) => {
|
||||
return ({ children }: { children: ReactNode }) => (
|
||||
<WorkflowContext.Provider value={store}>
|
||||
<WorkflowContext value={store}>
|
||||
{children}
|
||||
</WorkflowContext.Provider>
|
||||
</WorkflowContext>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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 })
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -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])
|
||||
|
||||
@@ -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<File>>(),
|
||||
}))
|
||||
|
||||
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<SkillEditorSliceShape>,
|
||||
}
|
||||
}
|
||||
|
||||
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')
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -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<SkillEditorSliceShape>
|
||||
uploadFile: (payload: {
|
||||
appId: string
|
||||
file: File
|
||||
parentId: string | null
|
||||
}) => Promise<AppAssetNode>
|
||||
}
|
||||
|
||||
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<UploadFilesWithStatusResult> => {
|
||||
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,
|
||||
}
|
||||
}
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user