diff --git a/web/app/components/workflow/hooks/use-inspect-vars-crud-common.ts b/web/app/components/workflow/hooks/use-inspect-vars-crud-common.ts index 3297dd3397..14dac2e09b 100644 --- a/web/app/components/workflow/hooks/use-inspect-vars-crud-common.ts +++ b/web/app/components/workflow/hooks/use-inspect-vars-crud-common.ts @@ -224,7 +224,7 @@ export const useInspectVarsCrudCommon = ({ await Promise.all([ invalidateConversationVarValues(), invalidateSysVarValues(), - invalidateSandboxFiles(), + invalidateSandboxFiles({ refetchDownloadFile: false }), ]) deleteAllInspectVars() handleEdgeCancelRunningStatus() diff --git a/web/app/components/workflow/variable-inspect/artifacts-tab.spec.tsx b/web/app/components/workflow/variable-inspect/artifacts-tab.spec.tsx new file mode 100644 index 0000000000..9ea8a0dc00 --- /dev/null +++ b/web/app/components/workflow/variable-inspect/artifacts-tab.spec.tsx @@ -0,0 +1,141 @@ +import type { SandboxFileNode, SandboxFileTreeNode } from '@/types/sandbox-file' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import ArtifactsTab from './artifacts-tab' +import { InspectTab } from './types' + +type MockStoreState = { + appId: string | undefined + workflowRunningData?: { + result?: { + status?: string + } + } + isResponding: boolean + bottomPanelWidth: number +} + +const mocks = vi.hoisted(() => ({ + storeState: { + appId: 'app-1', + workflowRunningData: undefined, + isResponding: false, + bottomPanelWidth: 640, + } as MockStoreState, + treeData: undefined as SandboxFileTreeNode[] | undefined, + flatData: [] as SandboxFileNode[], + hasFiles: false, + isLoading: false, + fetchDownloadUrl: vi.fn(), + useSandboxFileDownloadUrl: vi.fn(), +})) + +vi.mock('../store', () => ({ + useStore: (selector: (state: MockStoreState) => unknown) => selector(mocks.storeState), +})) + +vi.mock('@/service/use-sandbox-file', () => ({ + useSandboxFilesTree: () => ({ + data: mocks.treeData, + flatData: mocks.flatData, + hasFiles: mocks.hasFiles, + isLoading: mocks.isLoading, + }), + useDownloadSandboxFile: () => ({ + mutateAsync: mocks.fetchDownloadUrl, + isPending: false, + }), + useSandboxFileDownloadUrl: (...args: unknown[]) => mocks.useSandboxFileDownloadUrl(...args), +})) + +vi.mock('@/context/i18n', () => ({ + useDocLink: () => (path: string) => path, +})) + +vi.mock('@/app/components/base/features/hooks', () => ({ + useFeatures: (selector: (state: { features: { sandbox: { enabled: boolean } } }) => unknown) => selector({ + features: { + sandbox: { + enabled: true, + }, + }, + }), +})) + +vi.mock('@/utils/download', () => ({ + downloadUrl: vi.fn(), +})) + +const createTreeFileNode = (overrides: Partial = {}): SandboxFileTreeNode => ({ + id: 'a.txt', + name: 'a.txt', + path: 'a.txt', + node_type: 'file', + size: 128, + mtime: 1700000000, + extension: 'txt', + children: [], + ...overrides, +}) + +const createFlatFileNode = (overrides: Partial = {}): SandboxFileNode => ({ + path: 'a.txt', + is_dir: false, + size: 128, + mtime: 1700000000, + extension: 'txt', + ...overrides, +}) + +describe('ArtifactsTab', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.storeState.appId = 'app-1' + mocks.storeState.workflowRunningData = undefined + mocks.storeState.isResponding = false + mocks.storeState.bottomPanelWidth = 640 + + mocks.treeData = [createTreeFileNode()] + mocks.flatData = [createFlatFileNode()] + mocks.hasFiles = true + mocks.isLoading = false + mocks.useSandboxFileDownloadUrl.mockReturnValue({ + data: undefined, + isLoading: false, + }) + }) + + it('should stop using stale file path for download url query after files are cleared', async () => { + const headerProps = { + activeTab: InspectTab.Artifacts, + onTabChange: vi.fn(), + onClose: vi.fn(), + } + + const { rerender } = render() + + fireEvent.click(screen.getByRole('button', { name: 'a.txt' })) + + await waitFor(() => { + expect(mocks.useSandboxFileDownloadUrl).toHaveBeenCalledWith( + 'app-1', + 'a.txt', + { retry: false }, + ) + }) + + mocks.treeData = undefined + mocks.flatData = [] + mocks.hasFiles = false + + rerender() + + await waitFor(() => { + const lastCall = mocks.useSandboxFileDownloadUrl.mock.calls.at(-1) + expect(lastCall).toEqual([ + 'app-1', + undefined, + { retry: false }, + ]) + }) + }) +}) diff --git a/web/app/components/workflow/variable-inspect/artifacts-tab.tsx b/web/app/components/workflow/variable-inspect/artifacts-tab.tsx index 6d4f28b5fb..7e745c436a 100644 --- a/web/app/components/workflow/variable-inspect/artifacts-tab.tsx +++ b/web/app/components/workflow/variable-inspect/artifacts-tab.tsx @@ -4,7 +4,7 @@ import { RiCloseLine, RiMenuLine, } from '@remixicon/react' -import { useCallback, useState } from 'react' +import { useCallback, useMemo, useState } from 'react' import { useTranslation } from 'react-i18next' import ActionButton from '@/app/components/base/action-button' import SearchLinesSparkle from '@/app/components/base/icons/src/vender/knowledge/SearchLinesSparkle' @@ -62,15 +62,27 @@ const ArtifactsTab = (headerProps: InspectHeaderProps) => { ) const isResponding = useStore(s => s.isResponding) - const { data: treeData, hasFiles, isLoading } = useSandboxFilesTree(appId, { + const { data: treeData, flatData, hasFiles, isLoading } = useSandboxFilesTree(appId, { enabled: !!appId, refetchInterval: (isWorkflowRunning || isResponding) ? 5000 : false, }) const { mutateAsync: fetchDownloadUrl, isPending: isDownloading } = useDownloadSandboxFile(appId) const [selectedFile, setSelectedFile] = useState(null) + const selectedFilePath = useMemo(() => { + if (!selectedFile) + return undefined + + const selectedExists = flatData?.some( + node => !node.is_dir && node.path === selectedFile.path, + ) ?? false + + return selectedExists ? selectedFile.path : undefined + }, [flatData, selectedFile]) + const { data: downloadUrlData, isLoading: isDownloadUrlLoading } = useSandboxFileDownloadUrl( appId, - selectedFile?.path, + selectedFilePath, + { retry: false }, ) const handleFileSelect = useCallback((node: SandboxFileTreeNode) => { @@ -113,7 +125,7 @@ const ArtifactsTab = (headerProps: InspectHeaderProps) => { ) } - const file = selectedFile + const file = selectedFilePath ? selectedFile : null const parts = file?.path.split('/') ?? [] let cumPath = '' const pathSegments = parts.map((part, i) => { @@ -130,7 +142,7 @@ const ArtifactsTab = (headerProps: InspectHeaderProps) => { data={treeData} onDownload={handleTreeDownload} onSelect={handleFileSelect} - selectedPath={selectedFile?.path} + selectedPath={selectedFilePath} isDownloading={isDownloading} /> diff --git a/web/service/use-sandbox-file.spec.tsx b/web/service/use-sandbox-file.spec.tsx new file mode 100644 index 0000000000..a28f531110 --- /dev/null +++ b/web/service/use-sandbox-file.spec.tsx @@ -0,0 +1,60 @@ +import type { ReactNode } from 'react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { act, renderHook } from '@testing-library/react' +import { consoleQuery } from './client' +import { useInvalidateSandboxFiles } from './use-sandbox-file' + +const createQueryClient = () => new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, +}) + +const createWrapper = (queryClient: QueryClient) => { + return ({ children }: { children: ReactNode }) => ( + {children} + ) +} + +describe('useInvalidateSandboxFiles', () => { + it('should keep download query refetch enabled by default', async () => { + const queryClient = createQueryClient() + const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries').mockResolvedValue(undefined) + const { result } = renderHook(() => useInvalidateSandboxFiles(), { + wrapper: createWrapper(queryClient), + }) + + await act(async () => { + await result.current() + }) + + expect(invalidateSpy).toHaveBeenNthCalledWith(1, { + queryKey: consoleQuery.sandboxFile.listFiles.key(), + }) + expect(invalidateSpy).toHaveBeenNthCalledWith(2, { + queryKey: consoleQuery.sandboxFile.downloadFile.key(), + }) + }) + + it('should disable download query refetch when requested', async () => { + const queryClient = createQueryClient() + const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries').mockResolvedValue(undefined) + const { result } = renderHook(() => useInvalidateSandboxFiles(), { + wrapper: createWrapper(queryClient), + }) + + await act(async () => { + await result.current({ refetchDownloadFile: false }) + }) + + expect(invalidateSpy).toHaveBeenNthCalledWith(1, { + queryKey: consoleQuery.sandboxFile.listFiles.key(), + }) + expect(invalidateSpy).toHaveBeenNthCalledWith(2, { + queryKey: consoleQuery.sandboxFile.downloadFile.key(), + refetchType: 'none', + }) + }) +}) diff --git a/web/service/use-sandbox-file.ts b/web/service/use-sandbox-file.ts index 2f046ddb0f..aa7b91705d 100644 --- a/web/service/use-sandbox-file.ts +++ b/web/service/use-sandbox-file.ts @@ -14,6 +14,15 @@ type UseGetSandboxFilesOptions = { refetchInterval?: number | false } +type UseSandboxFileDownloadUrlOptions = { + enabled?: boolean + retry?: boolean | number +} + +type InvalidateSandboxFilesOptions = { + refetchDownloadFile?: boolean +} + export function useGetSandboxFiles( appId: string | undefined, options?: UseGetSandboxFilesOptions, @@ -39,6 +48,7 @@ export function useGetSandboxFiles( export function useSandboxFileDownloadUrl( appId: string | undefined, path: string | undefined, + options?: UseSandboxFileDownloadUrlOptions, ) { return useQuery({ queryKey: consoleQuery.sandboxFile.downloadFile.queryKey({ @@ -48,19 +58,22 @@ export function useSandboxFileDownloadUrl( params: { appId: appId! }, body: { path: path! }, }), - enabled: !!appId && !!path, + enabled: !!appId && !!path && (options?.enabled ?? true), + retry: options?.retry, }) } export function useInvalidateSandboxFiles() { const queryClient = useQueryClient() - return useCallback(() => { + return useCallback((options?: InvalidateSandboxFilesOptions) => { + const shouldRefetchDownloadFile = options?.refetchDownloadFile ?? true return Promise.all([ queryClient.invalidateQueries({ queryKey: consoleQuery.sandboxFile.listFiles.key(), }), queryClient.invalidateQueries({ queryKey: consoleQuery.sandboxFile.downloadFile.key(), + ...(shouldRefetchDownloadFile ? {} : { refetchType: 'none' as const }), }), ]) }, [queryClient])