mirror of https://github.com/langgenius/dify.git
fix: prevent duplicate skill file creation submits
This commit is contained in:
parent
27f32ce383
commit
29469a8600
|
|
@ -1,9 +1,9 @@
|
|||
import type { NodeApi } from 'react-arborist'
|
||||
import type { TreeNodeData } from '../../type'
|
||||
import { fireEvent, render, screen } from '@testing-library/react'
|
||||
import { fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import TreeEditInput from './tree-edit-input'
|
||||
|
||||
type MockNodeApi = Pick<NodeApi<TreeNodeData>, 'data' | 'reset' | 'submit'>
|
||||
type MockNodeApi = Pick<NodeApi<TreeNodeData>, 'data' | 'reset' | 'tree'>
|
||||
|
||||
function createMockNode(overrides: Partial<Pick<TreeNodeData, 'name' | 'node_type'>> = {}): MockNodeApi {
|
||||
const nodeType = overrides.node_type ?? 'file'
|
||||
|
|
@ -18,7 +18,9 @@ function createMockNode(overrides: Partial<Pick<TreeNodeData, 'name' | 'node_typ
|
|||
children: [],
|
||||
},
|
||||
reset: vi.fn(),
|
||||
submit: vi.fn(),
|
||||
tree: {
|
||||
submit: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as NodeApi<TreeNodeData>['tree'],
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -125,7 +127,7 @@ describe('TreeEditInput', () => {
|
|||
})
|
||||
|
||||
describe('Keyboard interactions', () => {
|
||||
it('should call node.submit with input value on Enter', () => {
|
||||
it('should call tree.submit with input value on Enter', async () => {
|
||||
const node = createMockNode({ name: 'old.txt' })
|
||||
renderInput(node)
|
||||
|
||||
|
|
@ -133,10 +135,12 @@ describe('TreeEditInput', () => {
|
|||
fireEvent.change(input, { target: { value: 'new.txt' } })
|
||||
fireEvent.keyDown(input, { key: 'Enter' })
|
||||
|
||||
expect(node.submit).toHaveBeenCalledWith('new.txt')
|
||||
await waitFor(() => {
|
||||
expect(node.tree.submit).toHaveBeenCalledWith(expect.objectContaining({ data: expect.objectContaining({ id: 'node-1' }) }), 'new.txt')
|
||||
})
|
||||
})
|
||||
|
||||
it('should call node.submit with empty string when input is cleared', () => {
|
||||
it('should call tree.submit with empty string when input is cleared', async () => {
|
||||
const node = createMockNode({ name: 'old.txt' })
|
||||
renderInput(node)
|
||||
|
||||
|
|
@ -144,7 +148,32 @@ describe('TreeEditInput', () => {
|
|||
fireEvent.change(input, { target: { value: '' } })
|
||||
fireEvent.keyDown(input, { key: 'Enter' })
|
||||
|
||||
expect(node.submit).toHaveBeenCalledWith('')
|
||||
await waitFor(() => {
|
||||
expect(node.tree.submit).toHaveBeenCalledWith(expect.objectContaining({ data: expect.objectContaining({ id: 'node-1' }) }), '')
|
||||
})
|
||||
})
|
||||
|
||||
it('should submit only once when Enter is pressed repeatedly during submission', async () => {
|
||||
let resolveSubmit: (() => void) | undefined
|
||||
const node = createMockNode({ name: 'old.txt' })
|
||||
node.tree.submit = vi.fn().mockImplementation(() => new Promise<void>((resolve) => {
|
||||
resolveSubmit = resolve
|
||||
}))
|
||||
renderInput(node)
|
||||
|
||||
const input = screen.getByRole('textbox')
|
||||
fireEvent.change(input, { target: { value: 'new.txt' } })
|
||||
fireEvent.keyDown(input, { key: 'Enter' })
|
||||
fireEvent.keyDown(input, { key: 'Enter' })
|
||||
|
||||
expect(node.tree.submit).toHaveBeenCalledTimes(1)
|
||||
expect(input).toBeDisabled()
|
||||
|
||||
resolveSubmit?.()
|
||||
|
||||
await waitFor(() => {
|
||||
expect(input).not.toBeDisabled()
|
||||
})
|
||||
})
|
||||
|
||||
it('should call node.reset on Escape', () => {
|
||||
|
|
@ -154,7 +183,7 @@ describe('TreeEditInput', () => {
|
|||
fireEvent.keyDown(screen.getByRole('textbox'), { key: 'Escape' })
|
||||
|
||||
expect(node.reset).toHaveBeenCalledTimes(1)
|
||||
expect(node.submit).not.toHaveBeenCalled()
|
||||
expect(node.tree.submit).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should stop propagation for all key events', () => {
|
||||
|
|
@ -181,6 +210,18 @@ describe('TreeEditInput', () => {
|
|||
|
||||
expect(node.reset).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('should ignore blur while submission is in progress', () => {
|
||||
const node = createMockNode()
|
||||
node.tree.submit = vi.fn().mockImplementation(() => new Promise(() => {}))
|
||||
renderInput(node)
|
||||
|
||||
const input = screen.getByRole('textbox')
|
||||
fireEvent.keyDown(input, { key: 'Enter' })
|
||||
fireEvent.blur(input)
|
||||
|
||||
expect(node.reset).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('Click', () => {
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
import type { NodeApi } from 'react-arborist'
|
||||
import type { TreeNodeData } from '../../type'
|
||||
import * as React from 'react'
|
||||
import { useEffect, useRef } from 'react'
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { useTranslation } from 'react-i18next'
|
||||
|
||||
type TreeEditInputProps = {
|
||||
|
|
@ -13,6 +13,9 @@ type TreeEditInputProps = {
|
|||
const TreeEditInput = ({ node }: TreeEditInputProps) => {
|
||||
const { t } = useTranslation('workflow')
|
||||
const inputRef = useRef<HTMLInputElement>(null)
|
||||
const isMountedRef = useRef(true)
|
||||
const submitLockRef = useRef(false)
|
||||
const [isSubmitting, setIsSubmitting] = useState(false)
|
||||
const isFolder = node.data.node_type === 'folder'
|
||||
const placeholder = isFolder
|
||||
? t('skillSidebar.folderNamePlaceholder')
|
||||
|
|
@ -29,20 +32,47 @@ const TreeEditInput = ({ node }: TreeEditInputProps) => {
|
|||
input.setSelectionRange(0, dotIndex)
|
||||
else
|
||||
input.select()
|
||||
}, [isFolder])
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
isMountedRef.current = false
|
||||
}
|
||||
}, [])
|
||||
|
||||
const handleSubmit = useCallback(async () => {
|
||||
if (submitLockRef.current)
|
||||
return
|
||||
|
||||
submitLockRef.current = true
|
||||
setIsSubmitting(true)
|
||||
|
||||
try {
|
||||
await node.tree.submit(node, inputRef.current?.value || '')
|
||||
}
|
||||
finally {
|
||||
submitLockRef.current = false
|
||||
if (isMountedRef.current)
|
||||
setIsSubmitting(false)
|
||||
}
|
||||
}, [node])
|
||||
|
||||
const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
|
||||
e.stopPropagation()
|
||||
if (e.key === 'Escape') {
|
||||
if (submitLockRef.current)
|
||||
return
|
||||
node.reset()
|
||||
}
|
||||
else if (e.key === 'Enter') {
|
||||
e.preventDefault()
|
||||
node.submit(inputRef.current?.value || '')
|
||||
void handleSubmit()
|
||||
}
|
||||
}
|
||||
|
||||
const handleBlur = () => {
|
||||
if (submitLockRef.current)
|
||||
return
|
||||
node.reset()
|
||||
}
|
||||
|
||||
|
|
@ -57,6 +87,7 @@ const TreeEditInput = ({ node }: TreeEditInputProps) => {
|
|||
defaultValue={node.data.name}
|
||||
placeholder={placeholder}
|
||||
aria-label={ariaLabel}
|
||||
disabled={isSubmitting}
|
||||
onKeyDown={handleKeyDown}
|
||||
onBlur={handleBlur}
|
||||
onClick={e => e.stopPropagation()}
|
||||
|
|
|
|||
|
|
@ -10,28 +10,30 @@ import { START_TAB_ID } from '../../../constants'
|
|||
import { useInlineCreateNode } from './use-inline-create-node'
|
||||
|
||||
const {
|
||||
mockUploadMutateAsync,
|
||||
mockCreateFolderMutateAsync,
|
||||
mockRenameMutateAsync,
|
||||
mockUploadMutate,
|
||||
mockCreateFolderMutate,
|
||||
mockRenameMutate,
|
||||
mockEmitTreeUpdate,
|
||||
mockToastNotify,
|
||||
mockToastSuccess,
|
||||
mockToastError,
|
||||
} = vi.hoisted(() => ({
|
||||
mockUploadMutateAsync: vi.fn(),
|
||||
mockCreateFolderMutateAsync: vi.fn(),
|
||||
mockRenameMutateAsync: vi.fn(),
|
||||
mockUploadMutate: vi.fn(),
|
||||
mockCreateFolderMutate: vi.fn(),
|
||||
mockRenameMutate: vi.fn(),
|
||||
mockEmitTreeUpdate: vi.fn(),
|
||||
mockToastNotify: vi.fn(),
|
||||
mockToastSuccess: vi.fn(),
|
||||
mockToastError: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/service/use-app-asset', () => ({
|
||||
useUploadFileWithPresignedUrl: () => ({
|
||||
mutateAsync: mockUploadMutateAsync,
|
||||
mutate: mockUploadMutate,
|
||||
}),
|
||||
useCreateAppAssetFolder: () => ({
|
||||
mutateAsync: mockCreateFolderMutateAsync,
|
||||
mutate: mockCreateFolderMutate,
|
||||
}),
|
||||
useRenameAppAssetNode: () => ({
|
||||
mutateAsync: mockRenameMutateAsync,
|
||||
mutate: mockRenameMutate,
|
||||
}),
|
||||
}))
|
||||
|
||||
|
|
@ -39,17 +41,18 @@ vi.mock('../data/use-skill-tree-collaboration', () => ({
|
|||
useSkillTreeUpdateEmitter: () => mockEmitTreeUpdate,
|
||||
}))
|
||||
|
||||
vi.mock('@/app/components/base/toast', () => ({
|
||||
default: {
|
||||
notify: mockToastNotify,
|
||||
vi.mock('@/app/components/base/ui/toast', () => ({
|
||||
toast: {
|
||||
success: mockToastSuccess,
|
||||
error: mockToastError,
|
||||
},
|
||||
}))
|
||||
|
||||
const createWrapper = (store: ReturnType<typeof createWorkflowStore>) => {
|
||||
return ({ children }: { children: ReactNode }) => (
|
||||
<WorkflowContext.Provider value={store}>
|
||||
<WorkflowContext value={store}>
|
||||
{children}
|
||||
</WorkflowContext.Provider>
|
||||
</WorkflowContext>
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -64,9 +67,11 @@ describe('useInlineCreateNode', () => {
|
|||
it('should open created text file tab with editor auto focus intent', async () => {
|
||||
const store = createWorkflowStore({})
|
||||
const treeRef = { current: null } as React.RefObject<TreeApi<TreeNodeData> | null>
|
||||
mockUploadMutateAsync.mockResolvedValue({
|
||||
id: 'file-1',
|
||||
extension: 'md',
|
||||
mockUploadMutate.mockImplementation((_, options) => {
|
||||
options?.onSuccess?.({
|
||||
id: 'file-1',
|
||||
extension: 'md',
|
||||
})
|
||||
})
|
||||
|
||||
store.getState().startCreateNode('file', null)
|
||||
|
|
@ -84,19 +89,22 @@ describe('useInlineCreateNode', () => {
|
|||
})
|
||||
})
|
||||
|
||||
expect(mockUploadMutateAsync).toHaveBeenCalledTimes(1)
|
||||
expect(mockUploadMutate).toHaveBeenCalledTimes(1)
|
||||
expect(store.getState().activeTabId).toBe('file-1')
|
||||
expect(store.getState().editorAutoFocusFileId).toBe('file-1')
|
||||
expect(store.getState().openTabIds).toEqual(['file-1'])
|
||||
expect(store.getState().pendingCreateNode).toBeNull()
|
||||
expect(mockToastSuccess).toHaveBeenCalledWith('workflow.skillSidebar.menu.fileCreated')
|
||||
})
|
||||
|
||||
it('should not open tab for non-text-like created files', async () => {
|
||||
const store = createWorkflowStore({})
|
||||
const treeRef = { current: null } as React.RefObject<TreeApi<TreeNodeData> | null>
|
||||
mockUploadMutateAsync.mockResolvedValue({
|
||||
id: 'file-2',
|
||||
extension: 'png',
|
||||
mockUploadMutate.mockImplementation((_, options) => {
|
||||
options?.onSuccess?.({
|
||||
id: 'file-2',
|
||||
extension: 'png',
|
||||
})
|
||||
})
|
||||
|
||||
store.getState().startCreateNode('file', null)
|
||||
|
|
@ -114,10 +122,42 @@ describe('useInlineCreateNode', () => {
|
|||
})
|
||||
})
|
||||
|
||||
expect(mockUploadMutateAsync).toHaveBeenCalledTimes(1)
|
||||
expect(mockUploadMutate).toHaveBeenCalledTimes(1)
|
||||
expect(store.getState().activeTabId).toBe(START_TAB_ID)
|
||||
expect(store.getState().editorAutoFocusFileId).toBeNull()
|
||||
expect(store.getState().openTabIds).toEqual([])
|
||||
expect(store.getState().pendingCreateNode).toBeNull()
|
||||
})
|
||||
|
||||
it('should wait for rename mutation callbacks before resolving existing node rename', async () => {
|
||||
const store = createWorkflowStore({})
|
||||
const treeRef = { current: null } as React.RefObject<TreeApi<TreeNodeData> | null>
|
||||
let onSuccess: (() => void) | undefined
|
||||
mockRenameMutate.mockImplementation((_, options) => {
|
||||
onSuccess = () => options?.onSuccess?.({})
|
||||
})
|
||||
|
||||
const { result } = renderHook(() => useInlineCreateNode({
|
||||
treeRef,
|
||||
treeChildren: [],
|
||||
}), { wrapper: createWrapper(store) })
|
||||
|
||||
let resolved = false
|
||||
const renamePromise = act(async () => {
|
||||
await result.current.handleRename({
|
||||
id: 'file-1',
|
||||
name: 'renamed.ts',
|
||||
})
|
||||
resolved = true
|
||||
})
|
||||
|
||||
expect(resolved).toBe(false)
|
||||
|
||||
onSuccess?.()
|
||||
await renamePromise
|
||||
|
||||
expect(mockRenameMutate).toHaveBeenCalledTimes(1)
|
||||
expect(mockEmitTreeUpdate).toHaveBeenCalled()
|
||||
expect(mockToastSuccess).toHaveBeenCalledWith('workflow.skillSidebar.menu.renamed')
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -26,6 +26,15 @@ type RenamePayload = {
|
|||
name: string
|
||||
}
|
||||
|
||||
type MutationWithCallbacks<TData, TVariables> = {
|
||||
mutate: (variables: TVariables, options?: {
|
||||
onSuccess?: (data: TData) => void
|
||||
onError?: () => void
|
||||
}) => void
|
||||
}
|
||||
|
||||
type MutationResult<TData> = { ok: true, data: TData } | { ok: false }
|
||||
|
||||
export function useInlineCreateNode({
|
||||
treeRef,
|
||||
treeChildren,
|
||||
|
|
@ -56,6 +65,18 @@ export function useInlineCreateNode({
|
|||
return insertDraftTreeNode(treeChildren, pendingCreateNode.parentId, draftNode)
|
||||
}, [pendingCreateNode, treeChildren])
|
||||
|
||||
const runMutation = useCallback(<TData, TVariables>(
|
||||
mutation: MutationWithCallbacks<TData, TVariables>,
|
||||
variables: TVariables,
|
||||
) => {
|
||||
return new Promise<MutationResult<TData>>((resolve) => {
|
||||
mutation.mutate(variables, {
|
||||
onSuccess: data => resolve({ ok: true, data }),
|
||||
onError: () => resolve({ ok: false }),
|
||||
})
|
||||
})
|
||||
}, [])
|
||||
|
||||
const handleRename = useCallback(async ({ id, name }: RenamePayload) => {
|
||||
if (pendingCreateId && id === pendingCreateId) {
|
||||
const trimmedName = name.trim()
|
||||
|
|
@ -66,50 +87,58 @@ export function useInlineCreateNode({
|
|||
|
||||
try {
|
||||
if (pendingCreateType === 'folder') {
|
||||
await createFolder.mutateAsync({
|
||||
const createFolderResult = await runMutation(createFolder, {
|
||||
appId,
|
||||
payload: {
|
||||
name: trimmedName,
|
||||
parent_id: pendingCreateParentId,
|
||||
},
|
||||
})
|
||||
if (!createFolderResult.ok) {
|
||||
toast.error(t('skillSidebar.menu.createError'))
|
||||
return
|
||||
}
|
||||
emitTreeUpdate()
|
||||
toast.success(t('skillSidebar.menu.folderCreated'))
|
||||
}
|
||||
else {
|
||||
const emptyBlob = new Blob([''], { type: 'text/plain' })
|
||||
const file = new File([emptyBlob], trimmedName)
|
||||
const createdFile = await uploadFile.mutateAsync({
|
||||
const createFileResult = await runMutation(uploadFile, {
|
||||
appId,
|
||||
file,
|
||||
parentId: pendingCreateParentId,
|
||||
})
|
||||
if (!createFileResult.ok) {
|
||||
toast.error(t('skillSidebar.menu.createError'))
|
||||
return
|
||||
}
|
||||
emitTreeUpdate()
|
||||
const extension = getFileExtension(trimmedName, createdFile.extension)
|
||||
const extension = getFileExtension(trimmedName, createFileResult.data.extension)
|
||||
if (isTextLikeFile(extension))
|
||||
storeApi.getState().openTab(createdFile.id, { pinned: true, autoFocusEditor: true })
|
||||
storeApi.getState().openTab(createFileResult.data.id, { pinned: true, autoFocusEditor: true })
|
||||
toast.success(t('skillSidebar.menu.fileCreated'))
|
||||
}
|
||||
}
|
||||
catch {
|
||||
toast.error(t('skillSidebar.menu.createError'))
|
||||
}
|
||||
finally {
|
||||
storeApi.getState().clearCreateNode()
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
renameNode.mutateAsync({
|
||||
const renameResult = await runMutation(renameNode, {
|
||||
appId,
|
||||
nodeId: id,
|
||||
payload: { name },
|
||||
}).then(() => {
|
||||
})
|
||||
|
||||
if (renameResult.ok) {
|
||||
emitTreeUpdate()
|
||||
toast.success(t('skillSidebar.menu.renamed'))
|
||||
}).catch(() => {
|
||||
}
|
||||
else {
|
||||
toast.error(t('skillSidebar.menu.renameError'))
|
||||
})
|
||||
}
|
||||
}, [
|
||||
appId,
|
||||
uploadFile,
|
||||
|
|
@ -118,6 +147,7 @@ export function useInlineCreateNode({
|
|||
pendingCreateParentId,
|
||||
pendingCreateType,
|
||||
renameNode,
|
||||
runMutation,
|
||||
storeApi,
|
||||
t,
|
||||
emitTreeUpdate,
|
||||
|
|
|
|||
Loading…
Reference in New Issue