Merge 576eb663c3 into 9a6b4147bc

web/app/components/app/configuration/config-var/index.spec.tsx

@@ -0,0 +1,372 @@
+import type { ReactNode } from 'react'
+import type { IConfigVarProps } from './index'
+import type { ExternalDataTool } from '@/models/common'
+import type { PromptVariable } from '@/models/debug'
+import { act, fireEvent, render, screen } from '@testing-library/react'
+import * as React from 'react'
+import { vi } from 'vitest'
+import Toast from '@/app/components/base/toast'
+import DebugConfigurationContext from '@/context/debug-configuration'
+import { AppModeEnum } from '@/types/app'
+
+import ConfigVar, { ADD_EXTERNAL_DATA_TOOL } from './index'
+
+const notifySpy = vi.spyOn(Toast, 'notify').mockImplementation(vi.fn())
+
+const setShowExternalDataToolModal = vi.fn()
+
+type SubscriptionEvent = {
+  type: string
+  payload: ExternalDataTool
+}
+
+let subscriptionCallback: ((event: SubscriptionEvent) => void) | null = null
+
+vi.mock('@/context/event-emitter', () => ({
+  useEventEmitterContextContext: () => ({
+    eventEmitter: {
+      useSubscription: (callback: (event: SubscriptionEvent) => void) => {
+        subscriptionCallback = callback
+      },
+    },
+  }),
+}))
+
+vi.mock('@/context/modal-context', () => ({
+  useModalContext: () => ({
+    setShowExternalDataToolModal,
+  }),
+}))
+
+type SortableItem = {
+  id: string
+  variable: PromptVariable
+}
+
+type SortableProps = {
+  list: SortableItem[]
+  setList: (list: SortableItem[]) => void
+  children: ReactNode
+}
+
+let latestSortableProps: SortableProps | null = null
+
+vi.mock('react-sortablejs', () => ({
+  ReactSortable: (props: SortableProps) => {
+    latestSortableProps = props
+    return <div data-testid="sortable">{props.children}</div>
+  },
+}))
+
+type DebugConfigurationState = any
+
+const defaultDebugConfigValue: DebugConfigurationState = {
+  mode: AppModeEnum.CHAT,
+  dataSets: [],
+  modelConfig: {
+    model_id: 'test-model',
+  },
+}
+
+const createDebugConfigValue = (overrides: Partial<DebugConfigurationState> = {}): DebugConfigurationState => ({
+  ...defaultDebugConfigValue,
+  ...overrides,
+})
+
+let variableIndex = 0
+const createPromptVariable = (overrides: Partial<PromptVariable> = {}): PromptVariable => {
+  variableIndex += 1
+  return {
+    key: `var_${variableIndex}`,
+    name: `Variable ${variableIndex}`,
+    type: 'string',
+    required: false,
+    ...overrides,
+  }
+}
+
+const renderConfigVar = (props: Partial<IConfigVarProps> = {}, debugOverrides: Partial<DebugConfigurationState> = {}) => {
+  const defaultProps: IConfigVarProps = {
+    promptVariables: [],
+    readonly: false,
+    onPromptVariablesChange: vi.fn(),
+  }
+
+  const mergedProps = {
+    ...defaultProps,
+    ...props,
+  }
+
+  return render(
+    <DebugConfigurationContext.Provider value={createDebugConfigValue(debugOverrides)}>
+      <ConfigVar {...mergedProps} />
+    </DebugConfigurationContext.Provider>,
+  )
+}
+
+describe('ConfigVar', () => {
+  // Rendering behavior for empty and populated states.
+  describe('ConfigVar Rendering', () => {
+    beforeEach(() => {
+      vi.clearAllMocks()
+      latestSortableProps = null
+      subscriptionCallback = null
+      variableIndex = 0
+      notifySpy.mockClear()
+    })
+
+    it('should show empty state when no variables exist', () => {
+      renderConfigVar({ promptVariables: [] })
+
+      expect(screen.getByText('appDebug.notSetVar')).toBeInTheDocument()
+    })
+
+    it('should render variable items and allow reordering via sortable list', () => {
+      const onPromptVariablesChange = vi.fn()
+      const firstVar = createPromptVariable({ key: 'first', name: 'First' })
+      const secondVar = createPromptVariable({ key: 'second', name: 'Second' })
+
+      renderConfigVar({
+        promptVariables: [firstVar, secondVar],
+        onPromptVariablesChange,
+      })
+
+      expect(screen.getByText('first')).toBeInTheDocument()
+      expect(screen.getByText('second')).toBeInTheDocument()
+
+      act(() => {
+        latestSortableProps?.setList([
+          { id: 'second', variable: secondVar },
+          { id: 'first', variable: firstVar },
+        ])
+      })
+
+      expect(onPromptVariablesChange).toHaveBeenCalledWith([secondVar, firstVar])
+    })
+  })
+
+  // Variable creation flows using the add menu.
+  describe('ConfigVar Add Variable', () => {
+    beforeEach(() => {
+      vi.clearAllMocks()
+      latestSortableProps = null
+      subscriptionCallback = null
+      variableIndex = 0
+      notifySpy.mockClear()
+    })
+
+    it('should add a text variable when selecting the string option', async () => {
+      const onPromptVariablesChange = vi.fn()
+      renderConfigVar({ promptVariables: [], onPromptVariablesChange })
+
+      fireEvent.click(screen.getByText('common.operation.add'))
+      fireEvent.click(await screen.findByText('appDebug.variableConfig.string'))
+
+      expect(onPromptVariablesChange).toHaveBeenCalledTimes(1)
+      const [nextVariables] = onPromptVariablesChange.mock.calls[0]
+      expect(nextVariables).toHaveLength(1)
+      expect(nextVariables[0].type).toBe('string')
+    })
+
+    it('should open the external data tool modal when adding an api variable', async () => {
+      const onPromptVariablesChange = vi.fn()
+      renderConfigVar({ promptVariables: [], onPromptVariablesChange })
+
+      fireEvent.click(screen.getByText('common.operation.add'))
+      fireEvent.click(await screen.findByText('appDebug.variableConfig.apiBasedVar'))
+
+      expect(onPromptVariablesChange).toHaveBeenCalledTimes(1)
+      expect(setShowExternalDataToolModal).toHaveBeenCalledTimes(1)
+
+      const modalState = setShowExternalDataToolModal.mock.calls[0][0]
+      expect(modalState.payload.type).toBe('api')
+
+      act(() => {
+        modalState.onCancelCallback?.()
+      })
+
+      expect(onPromptVariablesChange).toHaveBeenLastCalledWith([])
+    })
+  })
+
+  // Editing flows for variables through the modal.
+  describe('ConfigVar Edit Variable', () => {
+    beforeEach(() => {
+      vi.clearAllMocks()
+      latestSortableProps = null
+      subscriptionCallback = null
+      variableIndex = 0
+      notifySpy.mockClear()
+    })
+
+    it('should save updates when editing a basic variable', async () => {
+      const onPromptVariablesChange = vi.fn()
+      const variable = createPromptVariable({ key: 'name', name: 'Name' })
+
+      renderConfigVar({
+        promptVariables: [variable],
+        onPromptVariablesChange,
+      })
+
+      const item = screen.getByTitle('name · Name')
+      const itemContainer = item.closest('div.group')
+      expect(itemContainer).not.toBeNull()
+      const actionButtons = itemContainer!.querySelectorAll('div.h-6.w-6')
+      expect(actionButtons).toHaveLength(2)
+      fireEvent.click(actionButtons[0])
+
+      const saveButton = await screen.findByRole('button', { name: 'common.operation.save' })
+      fireEvent.click(saveButton)
+
+      expect(onPromptVariablesChange).toHaveBeenCalledTimes(1)
+    })
+
+    it('should show error when variable key is duplicated', async () => {
+      const onPromptVariablesChange = vi.fn()
+      const firstVar = createPromptVariable({ key: 'first', name: 'First' })
+      const secondVar = createPromptVariable({ key: 'second', name: 'Second' })
+
+      renderConfigVar({
+        promptVariables: [firstVar, secondVar],
+        onPromptVariablesChange,
+      })
+
+      const item = screen.getByTitle('first · First')
+      const itemContainer = item.closest('div.group')
+      expect(itemContainer).not.toBeNull()
+      const actionButtons = itemContainer!.querySelectorAll('div.h-6.w-6')
+      expect(actionButtons).toHaveLength(2)
+      fireEvent.click(actionButtons[0])
+
+      const inputs = await screen.findAllByPlaceholderText('appDebug.variableConfig.inputPlaceholder')
+      fireEvent.change(inputs[0], { target: { value: 'second' } })
+
+      fireEvent.click(screen.getByRole('button', { name: 'common.operation.save' }))
+
+      expect(Toast.notify).toHaveBeenCalled()
+      expect(onPromptVariablesChange).not.toHaveBeenCalled()
+    })
+
+    it('should show error when variable label is duplicated', async () => {
+      const onPromptVariablesChange = vi.fn()
+      const firstVar = createPromptVariable({ key: 'first', name: 'First' })
+      const secondVar = createPromptVariable({ key: 'second', name: 'Second' })
+
+      renderConfigVar({
+        promptVariables: [firstVar, secondVar],
+        onPromptVariablesChange,
+      })
+
+      const item = screen.getByTitle('first · First')
+      const itemContainer = item.closest('div.group')
+      expect(itemContainer).not.toBeNull()
+      const actionButtons = itemContainer!.querySelectorAll('div.h-6.w-6')
+      expect(actionButtons).toHaveLength(2)
+      fireEvent.click(actionButtons[0])
+
+      const inputs = await screen.findAllByPlaceholderText('appDebug.variableConfig.inputPlaceholder')
+      fireEvent.change(inputs[1], { target: { value: 'Second' } })
+
+      fireEvent.click(screen.getByRole('button', { name: 'common.operation.save' }))
+
+      expect(Toast.notify).toHaveBeenCalled()
+      expect(onPromptVariablesChange).not.toHaveBeenCalled()
+    })
+  })
+
+  // Removal behavior including confirm modal branch.
+  describe('ConfigVar Remove Variable', () => {
+    beforeEach(() => {
+      vi.clearAllMocks()
+      latestSortableProps = null
+      subscriptionCallback = null
+      variableIndex = 0
+      notifySpy.mockClear()
+    })
+
+    it('should remove variable directly when context confirmation is not required', () => {
+      const onPromptVariablesChange = vi.fn()
+      const variable = createPromptVariable({ key: 'name', name: 'Name' })
+
+      renderConfigVar({
+        promptVariables: [variable],
+        onPromptVariablesChange,
+      })
+
+      const removeBtn = screen.getByTestId('var-item-delete-btn')
+      fireEvent.click(removeBtn)
+
+      expect(onPromptVariablesChange).toHaveBeenCalledWith([])
+    })
+
+    it('should require confirmation when removing context variable with datasets in completion mode', () => {
+      const onPromptVariablesChange = vi.fn()
+      const variable = createPromptVariable({
+        key: 'context',
+        name: 'Context',
+        is_context_var: true,
+      })
+
+      renderConfigVar(
+        {
+          promptVariables: [variable],
+          onPromptVariablesChange,
+        },
+        {
+          mode: AppModeEnum.COMPLETION,
+          dataSets: [{ id: 'dataset-1' } as DebugConfigurationState['dataSets'][number]],
+        },
+      )
+
+      const deleteBtn = screen.getByTestId('var-item-delete-btn')
+      fireEvent.click(deleteBtn)
+      // confirmation modal should show up
+      fireEvent.click(screen.getByRole('button', { name: 'common.operation.confirm' }))
+
+      expect(onPromptVariablesChange).toHaveBeenCalledWith([])
+    })
+  })
+
+  // Event subscription support for external data tools.
+  describe('ConfigVar External Data Tool Events', () => {
+    beforeEach(() => {
+      vi.clearAllMocks()
+      latestSortableProps = null
+      subscriptionCallback = null
+      variableIndex = 0
+      notifySpy.mockClear()
+    })
+
+    it('should append external data tool variables from event emitter', () => {
+      const onPromptVariablesChange = vi.fn()
+      renderConfigVar({
+        promptVariables: [],
+        onPromptVariablesChange,
+      })
+
+      act(() => {
+        subscriptionCallback?.({
+          type: ADD_EXTERNAL_DATA_TOOL,
+          payload: {
+            variable: 'api_var',
+            label: 'API Var',
+            enabled: true,
+            type: 'api',
+            config: {},
+            icon: 'icon',
+            icon_background: 'bg',
+          },
+        })
+      })
+
+      expect(onPromptVariablesChange).toHaveBeenCalledWith([
+        expect.objectContaining({
+          key: 'api_var',
+          name: 'API Var',
+          required: true,
+          type: 'api',
+        }),
+      ])
+    })
+  })
+})

web/app/components/app/configuration/config-var/index.tsx

@@ -3,10 +3,11 @@ import type { FC } from 'react'
 import type { InputVar } from '@/app/components/workflow/types'
 import type { ExternalDataTool } from '@/models/common'
 import type { PromptVariable } from '@/models/debug'
+import type { I18nKeysByPrefix } from '@/types/i18n'
 import { useBoolean } from 'ahooks'
 import { produce } from 'immer'
 import * as React from 'react'
-import { useMemo, useState } from 'react'
+import { useCallback, useMemo, useState } from 'react'
 import { useTranslation } from 'react-i18next'
 import { ReactSortable } from 'react-sortablejs'
 import { useContext } from 'use-context-selector'
@@ -33,11 +34,55 @@ type ExternalDataToolParams = {
   type: string
   index: number
   name: string
-  config?: Record<string, any>
+  config?: PromptVariable['config']
   icon?: string
   icon_background?: string
 }
 
+const BASIC_INPUT_TYPES = new Set(['string', 'paragraph', 'select', 'number', 'checkbox'])
+
+const toInputVar = (item: PromptVariable): InputVar => ({
+  ...item,
+  label: item.name,
+  variable: item.key,
+  type: (item.type === 'string' ? InputVarType.textInput : item.type) as InputVarType,
+  required: item.required ?? false,
+})
+
+const buildPromptVariableFromInput = (payload: InputVar): PromptVariable => {
+  const { variable, label, type, ...rest } = payload
+  const nextType = type === InputVarType.textInput ? 'string' : type
+  const nextItem: PromptVariable = {
+    ...rest,
+    type: nextType,
+    key: variable,
+    name: label as string,
+  }
+  if (payload.type === InputVarType.textInput)
+    nextItem.max_length = nextItem.max_length || DEFAULT_VALUE_MAX_LEN
+
+  if (payload.type !== InputVarType.select)
+    delete nextItem.options
+
+  return nextItem
+}
+
+const getDuplicateError = (list: PromptVariable[]) => {
+  if (hasDuplicateStr(list.map(item => item.key))) {
+    return {
+      errorMsgKey: 'varKeyError.keyAlreadyExists',
+      typeName: 'variableConfig.varName',
+    }
+  }
+  if (hasDuplicateStr(list.map(item => item.name as string))) {
+    return {
+      errorMsgKey: 'varKeyError.keyAlreadyExists',
+      typeName: 'variableConfig.labelName',
+    }
+  }
+  return null
+}
+
 export type IConfigVarProps = {
   promptVariables: PromptVariable[]
   readonly?: boolean
@@ -55,61 +100,31 @@ const ConfigVar: FC<IConfigVarProps> = ({ promptVariables, readonly, onPromptVar
   const hasVar = promptVariables.length > 0
   const [currIndex, setCurrIndex] = useState<number>(-1)
   const currItem = currIndex !== -1 ? promptVariables[currIndex] : null
-  const currItemToEdit: InputVar | null = (() => {
+  const currItemToEdit = useMemo(() => {
     if (!currItem)
       return null
-
+    return toInputVar(currItem)
-    return {
+  }, [currItem])
-      ...currItem,
+  const updatePromptVariableItem = useCallback((payload: InputVar) => {
-      label: currItem.name,
-      variable: currItem.key,
-      type: currItem.type === 'string' ? InputVarType.textInput : currItem.type,
-    } as InputVar
-  })()
-  const updatePromptVariableItem = (payload: InputVar) => {
     const newPromptVariables = produce(promptVariables, (draft) => {
-      const { variable, label, type, ...rest } = payload
+      draft[currIndex] = buildPromptVariableFromInput(payload)
-      draft[currIndex] = {
-        ...rest,
-        type: type === InputVarType.textInput ? 'string' : type,
-        key: variable,
-        name: label as string,
-      }
-
-      if (payload.type === InputVarType.textInput)
-        draft[currIndex].max_length = draft[currIndex].max_length || DEFAULT_VALUE_MAX_LEN
-
-      if (payload.type !== InputVarType.select)
-        delete draft[currIndex].options
     })
-
+    const duplicateError = getDuplicateError(newPromptVariables)
-    const newList = newPromptVariables
+    if (duplicateError) {
-    let errorMsgKey: 'varKeyError.keyAlreadyExists' | '' = ''
-    let typeName: 'variableConfig.varName' | 'variableConfig.labelName' | '' = ''
-    if (hasDuplicateStr(newList.map(item => item.key))) {
-      errorMsgKey = 'varKeyError.keyAlreadyExists'
-      typeName = 'variableConfig.varName'
-    }
-    else if (hasDuplicateStr(newList.map(item => item.name as string))) {
-      errorMsgKey = 'varKeyError.keyAlreadyExists'
-      typeName = 'variableConfig.labelName'
-    }
-
-    if (errorMsgKey && typeName) {
       Toast.notify({
         type: 'error',
-        message: t(errorMsgKey, { ns: 'appDebug', key: t(typeName, { ns: 'appDebug' }) }),
+        message: t(duplicateError.errorMsgKey as I18nKeysByPrefix<'appDebug', 'duplicateError.'>, { ns: 'appDebug', key: t(duplicateError.typeName as I18nKeysByPrefix<'appDebug', 'duplicateError.'>, { ns: 'appDebug' }) }) as string,
       })
       return false
     }
 
     onPromptVariablesChange?.(newPromptVariables)
     return true
-  }
+  }, [currIndex, onPromptVariablesChange, promptVariables, t])
 
   const { setShowExternalDataToolModal } = useModalContext()
 
-  const handleOpenExternalDataToolModal = (
+  const handleOpenExternalDataToolModal = useCallback((
     { key, type, index, name, config, icon, icon_background }: ExternalDataToolParams,
     oldPromptVariables: PromptVariable[],
   ) => {
@@ -157,9 +172,9 @@ const ConfigVar: FC<IConfigVarProps> = ({ promptVariables, readonly, onPromptVar
         return true
       },
     })
-  }
+  }, [onPromptVariablesChange, promptVariables, setShowExternalDataToolModal, t])
 
-  const handleAddVar = (type: string) => {
+  const handleAddVar = useCallback((type: string) => {
     const newVar = getNewVar('', type)
     const newPromptVariables = [...promptVariables, newVar]
     onPromptVariablesChange?.(newPromptVariables)
@@ -172,8 +187,9 @@ const ConfigVar: FC<IConfigVarProps> = ({ promptVariables, readonly, onPromptVar
         index: promptVariables.length,
       }, newPromptVariables)
     }
-  }
+  }, [handleOpenExternalDataToolModal, onPromptVariablesChange, promptVariables])
 
+  // eslint-disable-next-line ts/no-explicit-any
   eventEmitter?.useSubscription((v: any) => {
     if (v.type === ADD_EXTERNAL_DATA_TOOL) {
       const payload = v.payload
@@ -195,11 +211,11 @@ const ConfigVar: FC<IConfigVarProps> = ({ promptVariables, readonly, onPromptVar
 
   const [isShowDeleteContextVarModal, { setTrue: showDeleteContextVarModal, setFalse: hideDeleteContextVarModal }] = useBoolean(false)
   const [removeIndex, setRemoveIndex] = useState<number | null>(null)
-  const didRemoveVar = (index: number) => {
+  const didRemoveVar = useCallback((index: number) => {
     onPromptVariablesChange?.(promptVariables.filter((_, i) => i !== index))
-  }
+  }, [onPromptVariablesChange, promptVariables])
 
-  const handleRemoveVar = (index: number) => {
+  const handleRemoveVar = useCallback((index: number) => {
     const removeVar = promptVariables[index]
 
     if (mode === AppModeEnum.COMPLETION && dataSets.length > 0 && removeVar.is_context_var) {
@@ -208,21 +224,20 @@ const ConfigVar: FC<IConfigVarProps> = ({ promptVariables, readonly, onPromptVar
       return
     }
     didRemoveVar(index)
-  }
+  }, [dataSets.length, didRemoveVar, mode, promptVariables, showDeleteContextVarModal])
 
-  // const [currKey, setCurrKey] = useState<string | null>(null)
   const [isShowEditModal, { setTrue: showEditModal, setFalse: hideEditModal }] = useBoolean(false)
 
-  const handleConfig = ({ key, type, index, name, config, icon, icon_background }: ExternalDataToolParams) => {
+  const handleConfig = useCallback(({ key, type, index, name, config, icon, icon_background }: ExternalDataToolParams) => {
     // setCurrKey(key)
     setCurrIndex(index)
-    if (type !== 'string' && type !== 'paragraph' && type !== 'select' && type !== 'number' && type !== 'checkbox') {
+    if (!BASIC_INPUT_TYPES.has(type)) {
       handleOpenExternalDataToolModal({ key, type, index, name, config, icon, icon_background }, promptVariables)
       return
     }
 
     showEditModal()
-  }
+  }, [handleOpenExternalDataToolModal, promptVariables, showEditModal])
 
   const promptVariablesWithIds = useMemo(() => promptVariables.map((item) => {
     return {

web/app/components/app/configuration/config-var/var-item.tsx

@@ -65,6 +65,7 @@ const VarItem: FC<ItemProps> = ({
             <RiEditLine className="h-4 w-4 text-text-tertiary" />
           </div>
           <div
+            data-testid="var-item-delete-btn"
             className="flex h-6 w-6 cursor-pointer items-center  justify-center text-text-tertiary hover:text-text-destructive"
             onClick={onRemove}
             onMouseOver={() => setIsDeleting(true)}