diff --git a/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.spec.tsx b/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.spec.tsx index 1c11091e38..4953b634ce 100644 --- a/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.spec.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.spec.tsx @@ -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, 'data' | 'reset' | 'submit'> +type MockNodeApi = Pick, 'data' | 'reset' | 'tree'> function createMockNode(overrides: Partial> = {}): MockNodeApi { const nodeType = overrides.node_type ?? 'file' @@ -18,7 +18,9 @@ function createMockNode(overrides: Partial['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((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', () => { diff --git a/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.tsx b/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.tsx index 898999fe17..27a2b53f63 100644 --- a/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.tsx +++ b/web/app/components/workflow/skill/file-tree/tree/tree-edit-input.tsx @@ -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(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) => { 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()} diff --git a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.spec.tsx b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.spec.tsx index 147a5c71f2..c10a40cd8d 100644 --- a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.spec.tsx +++ b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.spec.tsx @@ -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) => { return ({ children }: { children: ReactNode }) => ( - + {children} - + ) } @@ -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 | 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 | 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 | 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') + }) }) diff --git a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.ts b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.ts index 5c2fb1ea6a..81513a868a 100644 --- a/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.ts +++ b/web/app/components/workflow/skill/hooks/file-tree/interaction/use-inline-create-node.ts @@ -26,6 +26,15 @@ type RenamePayload = { name: string } +type MutationWithCallbacks = { + mutate: (variables: TVariables, options?: { + onSuccess?: (data: TData) => void + onError?: () => void + }) => void +} + +type MutationResult = { 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(( + mutation: MutationWithCallbacks, + variables: TVariables, + ) => { + return new Promise>((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,