test(workflow): improve dataset item tests with edit and remove functionality (#33937)

web/app/components/base/markdown-blocks/__tests__/code-block.spec.tsx

@@ -21,6 +21,8 @@ let clientWidthSpy: { mockRestore: () => void } | null = null
 let clientHeightSpy: { mockRestore: () => void } | null = null
 let offsetWidthSpy: { mockRestore: () => void } | null = null
 let offsetHeightSpy: { mockRestore: () => void } | null = null
+let consoleErrorSpy: ReturnType<typeof vi.spyOn> | null = null
+let consoleWarnSpy: ReturnType<typeof vi.spyOn> | null = null
 
 type AudioContextCtor = new () => unknown
 type WindowWithLegacyAudio = Window & {
@@ -83,6 +85,8 @@ describe('CodeBlock', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     mockUseTheme.mockReturnValue({ theme: Theme.light })
+    consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
+    consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
     clientWidthSpy = vi.spyOn(HTMLElement.prototype, 'clientWidth', 'get').mockReturnValue(900)
     clientHeightSpy = vi.spyOn(HTMLElement.prototype, 'clientHeight', 'get').mockReturnValue(400)
     offsetWidthSpy = vi.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockReturnValue(900)
@@ -98,6 +102,10 @@ describe('CodeBlock', () => {
 
   afterEach(() => {
     vi.useRealTimers()
+    consoleErrorSpy?.mockRestore()
+    consoleWarnSpy?.mockRestore()
+    consoleErrorSpy = null
+    consoleWarnSpy = null
     clientWidthSpy?.mockRestore()
     clientHeightSpy?.mockRestore()
     offsetWidthSpy?.mockRestore()

web/app/components/base/markdown-blocks/code-block.tsx

@@ -85,13 +85,30 @@ const CodeBlock: any = memo(({ inline, className, children = '', ...props }: any
   const processedRef = useRef<boolean>(false) // Track if content was successfully processed
   const isInitialRenderRef = useRef<boolean>(true) // Track if this is initial render
   const chartInstanceRef = useRef<any>(null) // Direct reference to ECharts instance
-  const resizeTimerRef = useRef<NodeJS.Timeout | null>(null) // For debounce handling
+  const resizeTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null) // For debounce handling
+  const chartReadyTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
   const finishedEventCountRef = useRef<number>(0) // Track finished event trigger count
   const match = /language-(\w+)/.exec(className || '')
   const language = match?.[1]
   const languageShowName = getCorrectCapitalizationLanguageName(language || '')
   const isDarkMode = theme === Theme.dark
 
+  const clearResizeTimer = useCallback(() => {
+    if (!resizeTimerRef.current)
+      return
+
+    clearTimeout(resizeTimerRef.current)
+    resizeTimerRef.current = null
+  }, [])
+
+  const clearChartReadyTimer = useCallback(() => {
+    if (!chartReadyTimerRef.current)
+      return
+
+    clearTimeout(chartReadyTimerRef.current)
+    chartReadyTimerRef.current = null
+  }, [])
+
   const echartsStyle = useMemo(() => ({
     height: '350px',
     width: '100%',
@@ -104,26 +121,27 @@ const CodeBlock: any = memo(({ inline, className, children = '', ...props }: any
 
   // Debounce resize operations
   const debouncedResize = useCallback(() => {
-    if (resizeTimerRef.current)
-      clearTimeout(resizeTimerRef.current)
+    clearResizeTimer()
 
     resizeTimerRef.current = setTimeout(() => {
       if (chartInstanceRef.current)
         chartInstanceRef.current.resize()
       resizeTimerRef.current = null
     }, 200)
-  }, [])
+  }, [clearResizeTimer])
 
   // Handle ECharts instance initialization
   const handleChartReady = useCallback((instance: any) => {
     chartInstanceRef.current = instance
 
     // Force resize to ensure timeline displays correctly
-    setTimeout(() => {
+    clearChartReadyTimer()
+    chartReadyTimerRef.current = setTimeout(() => {
       if (chartInstanceRef.current)
         chartInstanceRef.current.resize()
+      chartReadyTimerRef.current = null
     }, 200)
-  }, [])
+  }, [clearChartReadyTimer])
 
   // Store event handlers in useMemo to avoid recreating them
   const echartsEvents = useMemo(() => ({
@@ -157,10 +175,20 @@ const CodeBlock: any = memo(({ inline, className, children = '', ...props }: any
 
     return () => {
       window.removeEventListener('resize', handleResize)
-      if (resizeTimerRef.current)
-        clearTimeout(resizeTimerRef.current)
+      clearResizeTimer()
+      clearChartReadyTimer()
+      chartInstanceRef.current = null
     }
-  }, [language, debouncedResize])
+  }, [language, debouncedResize, clearResizeTimer, clearChartReadyTimer])
+
+  useEffect(() => {
+    return () => {
+      clearResizeTimer()
+      clearChartReadyTimer()
+      chartInstanceRef.current = null
+      echartsRef.current = null
+    }
+  }, [clearResizeTimer, clearChartReadyTimer])
   // Process chart data when content changes
   useEffect(() => {
     // Only process echarts content

web/app/components/workflow/nodes/knowledge-retrieval/__tests__/integration.spec.tsx

@@ -4,8 +4,9 @@ import type {
   MetadataShape,
 } from '../types'
 import type { DataSet, MetadataInDoc } from '@/models/datasets'
-import { fireEvent, render, screen } from '@testing-library/react'
+import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
+import { useEffect, useRef } from 'react'
 import {
   ChunkingMode,
   DatasetPermission,
@@ -173,17 +174,26 @@ vi.mock('@/app/components/app/configuration/dataset-config/select-dataset', () =
 
 vi.mock('@/app/components/app/configuration/dataset-config/settings-modal', () => ({
   __esModule: true,
-  default: ({ currentDataset, onSave, onCancel }: { currentDataset: DataSet, onSave: (dataset: DataSet) => void, onCancel: () => void }) => (
-    <div>
-      <div>{currentDataset.name}</div>
-      <button type="button" onClick={() => onSave(createDataset({ ...currentDataset, name: 'Updated Dataset' }))}>
-        save-settings
-      </button>
-      <button type="button" onClick={onCancel}>
-        cancel-settings
-      </button>
-    </div>
-  ),
+  default: function MockSettingsModal({ currentDataset, onSave, onCancel }: { currentDataset: DataSet, onSave: (dataset: DataSet) => void, onCancel: () => void }) {
+    const hasSavedRef = useRef(false)
+
+    useEffect(() => {
+      if (hasSavedRef.current)
+        return
+
+      hasSavedRef.current = true
+      onSave(createDataset({ ...currentDataset, name: 'Updated Dataset' }))
+    }, [currentDataset, onSave])
+
+    return (
+      <div>
+        <div>{currentDataset.name}</div>
+        <button type="button" onClick={onCancel}>
+          cancel-settings
+        </button>
+      </div>
+    )
+  },
 }))
 
 vi.mock('@/app/components/app/configuration/dataset-config/params-config/config-content', () => ({
@@ -265,6 +275,13 @@ vi.mock('../components/metadata/metadata-panel', () => ({
 }))
 
 describe('knowledge-retrieval path', () => {
+  const getDatasetItem = () => {
+    const datasetItem = screen.getByText('Dataset Name').closest('.group\\/dataset-item')
+    if (!(datasetItem instanceof HTMLElement))
+      throw new Error('Dataset item container not found')
+    return datasetItem
+  }
+
   beforeEach(() => {
     vi.clearAllMocks()
     mockHasEditPermissionForDataset.mockReturnValue(true)
@@ -293,33 +310,43 @@ describe('knowledge-retrieval path', () => {
       ])
     })
 
-    it('should support editing and removing a dataset item', async () => {
-      const user = userEvent.setup()
+    it('should support editing a dataset item', async () => {
       const onChange = vi.fn()
-      const onRemove = vi.fn()
 
       render(
         <DatasetItem
           payload={createDataset({ is_multimodal: true })}
           onChange={onChange}
-          onRemove={onRemove}
+          onRemove={vi.fn()}
         />,
       )
 
       expect(screen.getByText('Dataset Name')).toBeInTheDocument()
-      fireEvent.mouseOver(screen.getByText('Dataset Name').closest('.group\\/dataset-item')!)
+      const datasetItem = getDatasetItem()
+      fireEvent.click(within(datasetItem).getByRole('button', { name: 'common.operation.edit' }))
 
-      const buttons = screen.getAllByRole('button')
-      await user.click(buttons[0]!)
-      await user.click(screen.getByText('save-settings'))
-      await user.click(buttons[1]!)
+      await waitFor(() => {
+        expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ name: 'Updated Dataset' }))
+      })
+    })
 
-      expect(onChange).toHaveBeenCalledWith(expect.objectContaining({ name: 'Updated Dataset' }))
+    it('should support removing a dataset item', () => {
+      const onRemove = vi.fn()
+
+      render(
+        <DatasetItem
+          payload={createDataset({ is_multimodal: true })}
+          onChange={vi.fn()}
+          onRemove={onRemove}
+        />,
+      )
+
+      const datasetItem = getDatasetItem()
+      fireEvent.click(within(datasetItem).getByRole('button', { name: 'common.operation.remove' }))
       expect(onRemove).toHaveBeenCalled()
     })
 
-    it('should render empty and populated dataset lists', async () => {
-      const user = userEvent.setup()
+    it('should render empty and populated dataset lists', () => {
       const onChange = vi.fn()
 
       const { rerender } = render(
@@ -338,8 +365,8 @@ describe('knowledge-retrieval path', () => {
         />,
       )
 
-      fireEvent.mouseOver(screen.getByText('Dataset Name').closest('.group\\/dataset-item')!)
-      await user.click(screen.getAllByRole('button')[1]!)
+      const datasetItem = getDatasetItem()
+      fireEvent.click(within(datasetItem).getByRole('button', { name: 'common.operation.remove' }))
 
       expect(onChange).toHaveBeenCalledWith([])
     })

web/app/components/workflow/nodes/knowledge-retrieval/components/dataset-item.tsx

@@ -85,6 +85,8 @@ const DatasetItem: FC<Props> = ({
           {
             editable && (
               <ActionButton
+                aria-label={t('operation.edit', { ns: 'common' })}
+                data-testid="dataset-item-edit-button"
                 onClick={(e) => {
                   e.stopPropagation()
                   showSettingsModal()
@@ -95,6 +97,8 @@ const DatasetItem: FC<Props> = ({
             )
           }
           <ActionButton
+            aria-label={t('operation.remove', { ns: 'common' })}
+            data-testid="dataset-item-remove-button"
             onClick={handleRemove}
             state={isDeleteHovered ? ActionButtonState.Destructive : ActionButtonState.Default}
             onMouseEnter={() => setIsDeleteHovered(true)}