diff --git a/web/app/components/app/create-from-dsl-modal/__tests__/index.spec.tsx b/web/app/components/app/create-from-dsl-modal/__tests__/index.spec.tsx index b3d007c2e7..65b5c47ae7 100644 --- a/web/app/components/app/create-from-dsl-modal/__tests__/index.spec.tsx +++ b/web/app/components/app/create-from-dsl-modal/__tests__/index.spec.tsx @@ -397,6 +397,7 @@ describe('CreateFromDSLModal', () => { mockImportDSL.mockResolvedValueOnce({ id: 'import-failed', status: DSLImportStatus.FAILED, + error: 'Invalid YAML format', }) mockImportDSL.mockRejectedValueOnce(new Error('boom')) @@ -412,7 +413,7 @@ describe('CreateFromDSLModal', () => { await act(async () => { fireEvent.click(getCreateButton()) }) - expect(toastMocks.error).toHaveBeenCalledWith('newApp.appCreateFailed') + expect(toastMocks.error).toHaveBeenCalledWith('Invalid YAML format') rerender( { fireEvent.click(getCreateButton()) }) expect(toastMocks.error).toHaveBeenCalledTimes(2) + expect(toastMocks.error).toHaveBeenLastCalledWith('newApp.appCreateFailed') }) it('should handle pending import confirmation failures and cancellation', async () => { @@ -438,7 +440,7 @@ describe('CreateFromDSLModal', () => { current_dsl_version: '2.0.0', }) mockImportDSLConfirm - .mockResolvedValueOnce({ status: DSLImportStatus.FAILED }) + .mockResolvedValueOnce({ status: DSLImportStatus.FAILED, error: 'Confirm failed' }) .mockRejectedValueOnce(new Error('boom')) render( @@ -465,11 +467,12 @@ describe('CreateFromDSLModal', () => { await act(async () => { fireEvent.click(screen.getAllByRole('button', { name: 'newApp.Confirm' })[0]!) }) - expect(toastMocks.error).toHaveBeenCalledWith('newApp.appCreateFailed') + expect(toastMocks.error).toHaveBeenCalledWith('Confirm failed') await act(async () => { fireEvent.click(screen.getAllByRole('button', { name: 'newApp.Confirm' })[0]!) }) expect(toastMocks.error).toHaveBeenCalledTimes(2) + expect(toastMocks.error).toHaveBeenLastCalledWith('newApp.appCreateFailed') }) }) diff --git a/web/app/components/app/create-from-dsl-modal/index.tsx b/web/app/components/app/create-from-dsl-modal/index.tsx index 028290ec19..5b8a4e7469 100644 --- a/web/app/components/app/create-from-dsl-modal/index.tsx +++ b/web/app/components/app/create-from-dsl-modal/index.tsx @@ -5,9 +5,8 @@ import { Button } from '@langgenius/dify-ui/button' import { cn } from '@langgenius/dify-ui/cn' import { Dialog, DialogContent } from '@langgenius/dify-ui/dialog' import { toast } from '@langgenius/dify-ui/toast' -import { RiCloseLine } from '@remixicon/react' import { useDebounceFn, useKeyPress } from 'ahooks' -import { useEffect, useMemo, useRef, useState } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useTranslation } from 'react-i18next' import Input from '@/app/components/base/input' import AppsFull from '@/app/components/billing/apps-full-in-dialog' @@ -46,7 +45,7 @@ export enum CreateFromDSLModalTab { const CreateFromDSLModal = ({ show, onSuccess, onClose, activeTab = CreateFromDSLModalTab.FROM_FILE, dslUrl = '', droppedFile }: CreateFromDSLModalProps) => { const { push } = useRouter() const { t } = useTranslation() - const [currentFile, setDSLFile] = useState(droppedFile) + const [currentFile, setCurrentFile] = useState(droppedFile) const [fileContent, setFileContent] = useState() const [currentTab, setCurrentTab] = useState(activeTab) const [dslUrlValue, setDslUrlValue] = useState(dslUrl) @@ -55,22 +54,22 @@ const CreateFromDSLModal = ({ show, onSuccess, onClose, activeTab = CreateFromDS const [importId, setImportId] = useState() const { handleCheckPluginDependencies } = usePluginDependencies() - const readFile = (file: File) => { + const readFile = useCallback((file: File) => { const reader = new FileReader() reader.onload = function (event) { const content = event.target?.result setFileContent(content as string) } reader.readAsText(file) - } + }, []) - const handleFile = (file?: File) => { - setDSLFile(file) + const handleFile = useCallback((file?: File) => { + setCurrentFile(file) if (file) readFile(file) if (!file) setFileContent('') - } + }, [readFile]) const { isCurrentWorkspaceEditor } = useAppContext() const { plan, enableBilling } = useProviderContext() @@ -81,7 +80,7 @@ const CreateFromDSLModal = ({ show, onSuccess, onClose, activeTab = CreateFromDS useEffect(() => { if (droppedFile) handleFile(droppedFile) - }, [droppedFile]) + }, [droppedFile, handleFile]) const onCreate = async (_e?: React.MouseEvent) => { if (currentTab === CreateFromDSLModalTab.FROM_FILE && !currentFile) @@ -140,11 +139,10 @@ const CreateFromDSLModal = ({ show, onSuccess, onClose, activeTab = CreateFromDS setImportId(id) } else { - toast.error(t('newApp.appCreateFailed', { ns: 'app' })) + toast.error(response.error || t('newApp.appCreateFailed', { ns: 'app' })) } } - // eslint-disable-next-line unused-imports/no-unused-vars - catch (e) { + catch { toast.error(t('newApp.appCreateFailed', { ns: 'app' })) } isCreatingRef.current = false @@ -186,11 +184,10 @@ const CreateFromDSLModal = ({ show, onSuccess, onClose, activeTab = CreateFromDS getRedirection(isCurrentWorkspaceEditor, { id: app_id!, mode: app_mode }, push) } else if (status === DSLImportStatus.FAILED) { - toast.error(t('newApp.appCreateFailed', { ns: 'app' })) + toast.error(response.error || t('newApp.appCreateFailed', { ns: 'app' })) } } - // eslint-disable-next-line unused-imports/no-unused-vars - catch (e) { + catch { toast.error(t('newApp.appCreateFailed', { ns: 'app' })) } } @@ -227,7 +224,7 @@ const CreateFromDSLModal = ({ show, onSuccess, onClose, activeTab = CreateFromDS className="flex h-8 w-8 cursor-pointer items-center" onClick={() => onClose()} > - +
diff --git a/web/app/components/base/prompt-editor/plugins/hitl-input-block/input-field.tsx b/web/app/components/base/prompt-editor/plugins/hitl-input-block/input-field.tsx index 7511406718..e0fcd3bdba 100644 --- a/web/app/components/base/prompt-editor/plugins/hitl-input-block/input-field.tsx +++ b/web/app/components/base/prompt-editor/plugins/hitl-input-block/input-field.tsx @@ -82,7 +82,9 @@ const InputField: React.FC = ({ }, [handleSave]) return ( -
+
{t(`${i18nPrefix}.title`, { ns: 'workflow' })}
diff --git a/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/dsl-confirm-modal.tsx b/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/dsl-confirm-modal.tsx index 726b3f1742..849d36807f 100644 --- a/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/dsl-confirm-modal.tsx +++ b/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/dsl-confirm-modal.tsx @@ -34,8 +34,8 @@ const DSLConfirmModal = ({ onCancel() }} > - -
+ +
{t('newApp.appCreateDSLErrorTitle', { ns: 'app' })} } className="flex grow flex-col system-md-regular text-text-secondary">
{t('newApp.appCreateDSLErrorPart1', { ns: 'app' })}
@@ -51,7 +51,7 @@ const DSLConfirmModal = ({
- + {t('newApp.Cancel', { ns: 'app' })} {t('newApp.Confirm', { ns: 'app' })} diff --git a/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/__tests__/use-dsl-import.spec.tsx b/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/__tests__/use-dsl-import.spec.tsx index b95fa4d5db..a00cdf6098 100644 --- a/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/__tests__/use-dsl-import.spec.tsx +++ b/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/__tests__/use-dsl-import.spec.tsx @@ -418,7 +418,10 @@ describe('useDSLImport', () => { it('should handle FAILED status', async () => { vi.useFakeTimers({ shouldAdvanceTime: true }) - mockImportDSL.mockResolvedValue(createImportDSLResponse({ status: 'failed' })) + mockImportDSL.mockResolvedValue(createImportDSLResponse({ + status: 'failed', + error: 'Missing rag_pipeline data in YAML content', + })) const { result } = renderHook( () => useDSLImport({ @@ -434,9 +437,42 @@ describe('useDSLImport', () => { }) await waitFor(() => { - expect(toastMocks.record).toHaveBeenCalledWith(expect.objectContaining({ + expect(toastMocks.record).toHaveBeenCalledWith({ type: 'error', - })) + message: 'Missing rag_pipeline data in YAML content', + }) + }) + + vi.useRealTimers() + }) + + it('should show response error when import request rejects with a response body', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + mockImportDSL.mockRejectedValue(new Response(JSON.stringify({ + error: 'Missing rag_pipeline data in YAML content', + }), { + status: 400, + headers: { 'Content-Type': 'application/json' }, + })) + + const { result } = renderHook( + () => useDSLImport({ + activeTab: CreateFromDSLModalTab.FROM_URL, + dslUrl: 'https://example.com/test.pipeline', + }), + { wrapper: createWrapper() }, + ) + + await act(async () => { + result.current.handleCreateApp() + vi.advanceTimersByTime(400) + }) + + await waitFor(() => { + expect(toastMocks.record).toHaveBeenCalledWith({ + type: 'error', + message: 'Missing rag_pipeline data in YAML content', + }) }) vi.useRealTimers() @@ -692,6 +728,7 @@ describe('useDSLImport', () => { status: 'failed', pipeline_id: 'pipeline-456', dataset_id: 'dataset-789', + error: 'Import information expired or does not exist', }) const { result } = renderHook( @@ -718,9 +755,56 @@ describe('useDSLImport', () => { }) await waitFor(() => { - expect(toastMocks.record).toHaveBeenCalledWith(expect.objectContaining({ + expect(toastMocks.record).toHaveBeenCalledWith({ type: 'error', - })) + message: 'Import information expired or does not exist', + }) + }) + + vi.useRealTimers() + }) + + it('should show response error when confirm request rejects with a response body', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }) + + mockImportDSL.mockResolvedValue(createImportDSLResponse({ + id: 'import-123', + status: 'pending', + })) + + mockImportDSLConfirm.mockRejectedValue(new Response(JSON.stringify({ + error: 'Import information expired or does not exist', + }), { + status: 400, + headers: { 'Content-Type': 'application/json' }, + })) + + const { result } = renderHook( + () => useDSLImport({ + activeTab: CreateFromDSLModalTab.FROM_URL, + dslUrl: 'https://example.com/test.pipeline', + }), + { wrapper: createWrapper() }, + ) + + await act(async () => { + result.current.handleCreateApp() + vi.advanceTimersByTime(400) + }) + + await act(async () => { + vi.advanceTimersByTime(400) + }) + + await act(async () => { + result.current.onDSLConfirm() + }) + + await waitFor(() => { + expect(toastMocks.record).toHaveBeenCalledWith({ + type: 'error', + message: 'Import information expired or does not exist', + }) }) vi.useRealTimers() diff --git a/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/use-dsl-import.ts b/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/use-dsl-import.ts index e085ffe1bc..0dbfdf2156 100644 --- a/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/use-dsl-import.ts +++ b/web/app/components/datasets/create-from-pipeline/create-options/create-from-dsl-modal/hooks/use-dsl-import.ts @@ -22,6 +22,31 @@ type DSLVersions = { importedVersion: string systemVersion: string } +type ImportErrorResponse = { + message?: unknown + error?: unknown +} +const getNonEmptyString = (value: unknown): string | undefined => { + if (typeof value !== 'string') + return undefined + + const trimmedValue = value.trim() + return trimmedValue || undefined +} +const getImportErrorMessage = async (error: unknown): Promise => { + if (error instanceof Response && !error.bodyUsed) { + try { + const errorData = await error.clone().json() as ImportErrorResponse + return getNonEmptyString(errorData.message) ?? getNonEmptyString(errorData.error) + } + catch {} + } + + if (error instanceof Error) + return getNonEmptyString(error.message) + + return undefined +} export const useDSLImport = ({ activeTab = CreateFromDSLModalTab.FROM_FILE, dslUrl = '', onSuccess, onClose }: UseDSLImportOptions) => { const { push } = useRouter() const { t } = useTranslation() @@ -37,6 +62,9 @@ export const useDSLImport = ({ activeTab = CreateFromDSLModalTab.FROM_FILE, dslU const isCreatingRef = useRef(false) const { mutateAsync: importDSL } = useImportPipelineDSL() const { mutateAsync: importDSLConfirm } = useImportPipelineDSLConfirm() + const notifyError = useCallback((message?: string) => { + toast.error(message || t('creation.errorTip', { ns: 'datasetPipeline' })) + }, [t]) const readFile = useCallback((file: File) => { const reader = new FileReader() reader.onload = (event) => { @@ -60,51 +88,55 @@ export const useDSLImport = ({ activeTab = CreateFromDSLModalTab.FROM_FILE, dslU if (isCreatingRef.current) return isCreatingRef.current = true - let response - if (currentTab === CreateFromDSLModalTab.FROM_FILE) { - response = await importDSL({ - mode: DSLImportMode.YAML_CONTENT, - yaml_content: fileContent || '', - }) + try { + let response + if (currentTab === CreateFromDSLModalTab.FROM_FILE) { + response = await importDSL({ + mode: DSLImportMode.YAML_CONTENT, + yaml_content: fileContent || '', + }) + } + if (currentTab === CreateFromDSLModalTab.FROM_URL) { + response = await importDSL({ + mode: DSLImportMode.YAML_URL, + yaml_url: dslUrlValue || '', + }) + } + if (!response) { + notifyError() + return + } + const { id, status, pipeline_id, dataset_id, imported_dsl_version, current_dsl_version } = response + if (status === DSLImportStatus.COMPLETED || status === DSLImportStatus.COMPLETED_WITH_WARNINGS) { + onSuccess?.() + onClose?.() + toast(t(status === DSLImportStatus.COMPLETED ? 'creation.successTip' : 'creation.caution', { ns: 'datasetPipeline' }), { + type: status === DSLImportStatus.COMPLETED ? 'success' : 'warning', + description: status === DSLImportStatus.COMPLETED_WITH_WARNINGS && t('newApp.appCreateDSLWarning', { ns: 'app' }), + }) + if (pipeline_id) + await handleCheckPluginDependencies(pipeline_id, true) + push(`/datasets/${dataset_id}/pipeline`) + } + else if (status === DSLImportStatus.PENDING) { + setVersions({ + importedVersion: imported_dsl_version ?? '', + systemVersion: current_dsl_version ?? '', + }) + onClose?.() + setTimeout(() => { + setShowConfirmModal(true) + }, 300) + setImportId(id) + } + else { + notifyError(response.error) + } } - if (currentTab === CreateFromDSLModalTab.FROM_URL) { - response = await importDSL({ - mode: DSLImportMode.YAML_URL, - yaml_url: dslUrlValue || '', - }) + catch (error) { + notifyError(await getImportErrorMessage(error)) } - if (!response) { - toast.error(t('creation.errorTip', { ns: 'datasetPipeline' })) - isCreatingRef.current = false - return - } - const { id, status, pipeline_id, dataset_id, imported_dsl_version, current_dsl_version } = response - if (status === DSLImportStatus.COMPLETED || status === DSLImportStatus.COMPLETED_WITH_WARNINGS) { - onSuccess?.() - onClose?.() - toast(t(status === DSLImportStatus.COMPLETED ? 'creation.successTip' : 'creation.caution', { ns: 'datasetPipeline' }), { - type: status === DSLImportStatus.COMPLETED ? 'success' : 'warning', - description: status === DSLImportStatus.COMPLETED_WITH_WARNINGS && t('newApp.appCreateDSLWarning', { ns: 'app' }), - }) - if (pipeline_id) - await handleCheckPluginDependencies(pipeline_id, true) - push(`/datasets/${dataset_id}/pipeline`) - isCreatingRef.current = false - } - else if (status === DSLImportStatus.PENDING) { - setVersions({ - importedVersion: imported_dsl_version ?? '', - systemVersion: current_dsl_version ?? '', - }) - onClose?.() - setTimeout(() => { - setShowConfirmModal(true) - }, 300) - setImportId(id) - isCreatingRef.current = false - } - else { - toast.error(t('creation.errorTip', { ns: 'datasetPipeline' })) + finally { isCreatingRef.current = false } }, [ @@ -114,6 +146,7 @@ export const useDSLImport = ({ activeTab = CreateFromDSLModalTab.FROM_FILE, dslU fileContent, importDSL, t, + notifyError, onSuccess, onClose, handleCheckPluginDependencies, @@ -124,25 +157,32 @@ export const useDSLImport = ({ activeTab = CreateFromDSLModalTab.FROM_FILE, dslU if (!importId) return setIsConfirming(true) - const response = await importDSLConfirm(importId) - setIsConfirming(false) - if (!response) { - toast.error(t('creation.errorTip', { ns: 'datasetPipeline' })) - return + try { + const response = await importDSLConfirm(importId) + if (!response) { + notifyError() + return + } + const { status, pipeline_id, dataset_id, error } = response + if (status === DSLImportStatus.COMPLETED) { + onSuccess?.() + setShowConfirmModal(false) + toast.success(t('creation.successTip', { ns: 'datasetPipeline' })) + if (pipeline_id) + await handleCheckPluginDependencies(pipeline_id, true) + push(`/datasets/${dataset_id}/pipeline`) + } + else if (status === DSLImportStatus.FAILED) { + notifyError(error) + } } - const { status, pipeline_id, dataset_id } = response - if (status === DSLImportStatus.COMPLETED) { - onSuccess?.() - setShowConfirmModal(false) - toast.success(t('creation.successTip', { ns: 'datasetPipeline' })) - if (pipeline_id) - await handleCheckPluginDependencies(pipeline_id, true) - push(`/datasets/${dataset_id}/pipeline`) + catch (error) { + notifyError(await getImportErrorMessage(error)) } - else if (status === DSLImportStatus.FAILED) { - toast.error(t('creation.errorTip', { ns: 'datasetPipeline' })) + finally { + setIsConfirming(false) } - }, [importId, importDSLConfirm, t, onSuccess, handleCheckPluginDependencies, push]) + }, [importId, importDSLConfirm, notifyError, t, onSuccess, handleCheckPluginDependencies, push]) const handleCancelConfirm = useCallback(() => { setShowConfirmModal(false) }, []) diff --git a/web/app/components/datasets/create-from-pipeline/list/template-card/index.tsx b/web/app/components/datasets/create-from-pipeline/list/template-card/index.tsx index 938a67e50d..4663c55e38 100644 --- a/web/app/components/datasets/create-from-pipeline/list/template-card/index.tsx +++ b/web/app/components/datasets/create-from-pipeline/list/template-card/index.tsx @@ -160,7 +160,7 @@ const TemplateCard = ({ closeEditModal() }} > - + - +
= React.memo(({ return ( <> -
+
{t('segment.regenerationConfirmTitle', { ns: 'datasetDocuments' })} {t('segment.regenerationConfirmMessage', { ns: 'datasetDocuments' })}
- - +
+ + +
) }) @@ -52,11 +49,11 @@ const RegeneratingContent: FC = React.memo(() => { return ( <> -
+
{t('segment.regeneratingTitle', { ns: 'datasetDocuments' })}

{t('segment.regeneratingMessage', { ns: 'datasetDocuments' })}

-
+
@@ -131,7 +128,7 @@ const RegenerationModal: FC = ({ return ( - + {!loading && !updateSucceeded && } {loading && !updateSucceeded && } {!loading && updateSucceeded && } diff --git a/web/app/components/datasets/hit-testing/components/__tests__/chunk-detail-modal.spec.tsx b/web/app/components/datasets/hit-testing/components/__tests__/chunk-detail-modal.spec.tsx index cd73bdcee1..28f32561c4 100644 --- a/web/app/components/datasets/hit-testing/components/__tests__/chunk-detail-modal.spec.tsx +++ b/web/app/components/datasets/hit-testing/components/__tests__/chunk-detail-modal.spec.tsx @@ -1,5 +1,5 @@ import type { HitTesting } from '@/models/datasets' -import { render, screen } from '@testing-library/react' +import { fireEvent, render, screen } from '@testing-library/react' import { beforeEach, describe, expect, it, vi } from 'vitest' import ChunkDetailModal from '../chunk-detail-modal' @@ -20,7 +20,7 @@ vi.mock('@langgenius/dify-ui/dialog', () => ({
), DialogTitle: ({ children }: { children: React.ReactNode }) =>
{children}
, - DialogCloseButton: () => , + DialogCloseButton: (props: React.ButtonHTMLAttributes) => , })) vi.mock('../../../common/image-list', () => ({ @@ -136,4 +136,10 @@ describe('ChunkDetailModal', () => { render() expect(screen.getByTestId('mask')).toBeInTheDocument() }) + + it('should call onHide when close button is clicked', () => { + render() + fireEvent.click(screen.getByTestId('modal-close-button')) + expect(onHide).toHaveBeenCalledTimes(1) + }) }) diff --git a/web/app/components/datasets/hit-testing/components/chunk-detail-modal.tsx b/web/app/components/datasets/hit-testing/components/chunk-detail-modal.tsx index 01d8780a4f..6f789a81ce 100644 --- a/web/app/components/datasets/hit-testing/components/chunk-detail-modal.tsx +++ b/web/app/components/datasets/hit-testing/components/chunk-detail-modal.tsx @@ -59,8 +59,14 @@ const ChunkDetailModal = ({ onHide() }} > - - + + { + e.stopPropagation() + onHide() + }} + /> {t(`${i18nPrefix}chunkDetail`, { ns: 'datasetHitTesting' })} diff --git a/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx b/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx index 333ef45162..d52df9f0a4 100644 --- a/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx +++ b/web/app/components/develop/secret-key/__tests__/secret-key-modal.spec.tsx @@ -290,6 +290,36 @@ describe('SecretKeyModal', () => { }) }) + it('should place the generated key backdrop above the API keys modal', async () => { + const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) + mockAppApiKeysData.mockReturnValue({ + data: [ + { id: 'key-1', token: 'sk-abc123def456ghi789', created_at: 1700000000, last_used_at: null }, + ], + }) + await renderModal() + + const createButton = screen.getByText('appApi.apiKeyModal.createNewSecretKey') + await act(async () => { + await user.click(createButton) + }) + + await waitFor(() => { + expect(screen.getByText('appApi.apiKeyModal.generateTips')).toBeInTheDocument() + }) + + const parentDialog = screen.getByText('appApi.apiKeyModal.apiSecretKeyTips').closest('[role="dialog"]') + const generatedKeyDialog = screen.getByText('appApi.apiKeyModal.generateTips').closest('[role="dialog"]') + const backdrops = document.body.querySelectorAll('.bg-background-overlay') + const generatedKeyBackdrop = backdrops[1] + + expect(parentDialog).toBeInTheDocument() + expect(generatedKeyDialog).toBeInTheDocument() + expect(backdrops).toHaveLength(2) + expect(parentDialog!.compareDocumentPosition(generatedKeyBackdrop!) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy() + expect(generatedKeyBackdrop!.compareDocumentPosition(generatedKeyDialog!) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy() + }) + it('should invalidate app API keys after creating', async () => { const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime }) await renderModal() diff --git a/web/app/components/develop/secret-key/secret-key-modal.tsx b/web/app/components/develop/secret-key/secret-key-modal.tsx index b2d2c907b4..71cb87019c 100644 --- a/web/app/components/develop/secret-key/secret-key-modal.tsx +++ b/web/app/components/develop/secret-key/secret-key-modal.tsx @@ -104,90 +104,99 @@ const SecretKeyModal = ({ setShowConfirmDelete(false) } - return ( - { - if (!open) - onClose() - }} - > - - - {`${t('apiKeyModal.apiSecretKey', { ns: 'appApi' })}`} - + const handleClose = () => { + setVisible(false) + onClose() + } -
- -
-

{t('apiKeyModal.apiSecretKeyTips', { ns: 'appApi' })}

- {isApiKeysLoading &&
} - { - !!apiKeysList?.data?.length && ( -
-
-
{t('apiKeyModal.secretKey', { ns: 'appApi' })}
-
{t('apiKeyModal.created', { ns: 'appApi' })}
-
{t('apiKeyModal.lastUsed', { ns: 'appApi' })}
-
-
-
- {apiKeysList.data.map(api => ( -
-
{generateToken(api.token)}
-
{formatTime(Number(api.created_at), t('dateTimeFormat', { ns: 'appLog' }) as string)}
-
{api.last_used_at ? formatTime(Number(api.last_used_at), t('dateTimeFormat', { ns: 'appLog' }) as string) : t('never', { ns: 'appApi' })}
-
- - {isCurrentWorkspaceManager && ( - { - setDelKeyId(api.id) - setShowConfirmDelete(true) - }} - > - - - )} + return ( + <> + { + if (!open) + handleClose() + }} + > + + + {`${t('apiKeyModal.apiSecretKey', { ns: 'appApi' })}`} + + +
+ +
+

{t('apiKeyModal.apiSecretKeyTips', { ns: 'appApi' })}

+ {isApiKeysLoading &&
} + { + !!apiKeysList?.data?.length && ( +
+
+
{t('apiKeyModal.secretKey', { ns: 'appApi' })}
+
{t('apiKeyModal.created', { ns: 'appApi' })}
+
{t('apiKeyModal.lastUsed', { ns: 'appApi' })}
+
+
+
+ {apiKeysList.data.map(api => ( +
+
{generateToken(api.token)}
+
{formatTime(Number(api.created_at), t('dateTimeFormat', { ns: 'appLog' }) as string)}
+
{api.last_used_at ? formatTime(Number(api.last_used_at), t('dateTimeFormat', { ns: 'appLog' }) as string) : t('never', { ns: 'appApi' })}
+
+ + {isCurrentWorkspaceManager && ( + { + setDelKeyId(api.id) + setShowConfirmDelete(true) + }} + > + + + )} +
-
- ))} + ))} +
-
- ) - } -
- -
+ ) + } +
+ +
+ + +
+ + {t('actionMsg.deleteConfirmTitle', { ns: 'appApi' })} + + + {t('actionMsg.deleteConfirmTips', { ns: 'appApi' })} + +
+ + + {t('operation.cancel', { ns: 'common' })} + + + {t('operation.confirm', { ns: 'common' })} + + + + + +
+ {isShow && ( setVisible(false)} newKey={newKey} /> - - -
- - {t('actionMsg.deleteConfirmTitle', { ns: 'appApi' })} - - - {t('actionMsg.deleteConfirmTips', { ns: 'appApi' })} - -
- - - {t('operation.cancel', { ns: 'common' })} - - - {t('operation.confirm', { ns: 'common' })} - - - - - - + )} + ) } diff --git a/web/app/components/plugins/install-plugin/install-from-local-package/steps/__tests__/uploading.spec.tsx b/web/app/components/plugins/install-plugin/install-from-local-package/steps/__tests__/uploading.spec.tsx index aace5dcbe9..674d6a451c 100644 --- a/web/app/components/plugins/install-plugin/install-from-local-package/steps/__tests__/uploading.spec.tsx +++ b/web/app/components/plugins/install-plugin/install-from-local-package/steps/__tests__/uploading.spec.tsx @@ -161,11 +161,9 @@ describe('Uploading', () => { }) }) - // NOTE: The uploadFile API has an unconventional contract where it always rejects. - // Success vs failure is determined by whether response.message exists: - // - If response.message exists → treated as failure (calls onFailed) - // - If response.message is absent → treated as success (calls onPackageUploaded/onBundleUploaded) - // This explains why we use mockRejectedValue for "success" scenarios below. + // NOTE: Some upload endpoints have historically returned successful plugin upload + // payloads through rejected XHR objects, so the component accepts both resolved + // responses and rejected responses without an error message. it('should call onPackageUploaded when upload rejects without error message (success case)', async () => { const mockResult = { @@ -193,6 +191,30 @@ describe('Uploading', () => { }) }) + it('should call onPackageUploaded when upload resolves with package response', async () => { + const mockResult = { + unique_identifier: 'test-uid', + manifest: createMockManifest(), + } + mockUploadFile.mockResolvedValue(mockResult) + + const onPackageUploaded = vi.fn() + render( + , + ) + + await waitFor(() => { + expect(onPackageUploaded).toHaveBeenCalledWith({ + uniqueIdentifier: mockResult.unique_identifier, + manifest: mockResult.manifest, + }) + }) + }) + it('should call onBundleUploaded when upload rejects without error message (success case)', async () => { const mockDependencies = createMockDependencies() mockUploadFile.mockRejectedValue({ @@ -212,6 +234,24 @@ describe('Uploading', () => { expect(onBundleUploaded).toHaveBeenCalledWith(mockDependencies) }) }) + + it('should call onBundleUploaded when upload resolves with bundle response', async () => { + const mockDependencies = createMockDependencies() + mockUploadFile.mockResolvedValue(mockDependencies) + + const onBundleUploaded = vi.fn() + render( + , + ) + + await waitFor(() => { + expect(onBundleUploaded).toHaveBeenCalledWith(mockDependencies) + }) + }) }) // ================================ @@ -260,35 +300,48 @@ describe('Uploading', () => { // Edge Cases Tests // ================================ describe('Edge Cases', () => { - it('should handle empty response gracefully', async () => { + it('should fail gracefully when upload response is empty', async () => { mockUploadFile.mockRejectedValue({ response: {}, }) const onPackageUploaded = vi.fn() - render() + const onFailed = vi.fn() + render() await waitFor(() => { - expect(onPackageUploaded).toHaveBeenCalledWith({ - uniqueIdentifier: undefined, - manifest: undefined, - }) + expect(onPackageUploaded).not.toHaveBeenCalled() + expect(onFailed).toHaveBeenCalledWith('plugin.installModal.uploadFailed') }) }) - it('should handle response with only unique_identifier', async () => { + it('should fail gracefully when upload response has no manifest', async () => { mockUploadFile.mockRejectedValue({ response: { unique_identifier: 'only-uid' }, }) const onPackageUploaded = vi.fn() - render() + const onFailed = vi.fn() + render() await waitFor(() => { - expect(onPackageUploaded).toHaveBeenCalledWith({ - uniqueIdentifier: 'only-uid', - manifest: undefined, - }) + expect(onPackageUploaded).not.toHaveBeenCalled() + expect(onFailed).toHaveBeenCalledWith('plugin.installModal.uploadFailed') + }) + }) + + it('should fail gracefully when upload response is null', async () => { + mockUploadFile.mockRejectedValue({ + response: null, + }) + + const onPackageUploaded = vi.fn() + const onFailed = vi.fn() + render() + + await waitFor(() => { + expect(onPackageUploaded).not.toHaveBeenCalled() + expect(onFailed).toHaveBeenCalledWith('plugin.installModal.uploadFailed') }) }) diff --git a/web/app/components/plugins/install-plugin/install-from-local-package/steps/uploading.tsx b/web/app/components/plugins/install-plugin/install-from-local-package/steps/uploading.tsx index e7c3b4cd0f..ea90975ec5 100644 --- a/web/app/components/plugins/install-plugin/install-from-local-package/steps/uploading.tsx +++ b/web/app/components/plugins/install-plugin/install-from-local-package/steps/uploading.tsx @@ -1,8 +1,7 @@ 'use client' import type { FC } from 'react' -import type { Dependency, PluginDeclaration } from '../../../types' +import type { Dependency, Plugin, PluginDeclaration } from '../../../types' import { Button } from '@langgenius/dify-ui/button' -import { RiLoader2Line } from '@remixicon/react' import * as React from 'react' import { useTranslation } from 'react-i18next' import { uploadFile } from '@/service/plugins' @@ -10,6 +9,40 @@ import Card from '../../../card' const i18nPrefix = 'installModal' +type PackageUploadResponse = { + unique_identifier: string + manifest: PluginDeclaration +} + +type UploadFailureResponse = { + message?: string +} + +function isObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null +} + +function isPackageUploadResponse(value: unknown): value is PackageUploadResponse { + if (!isObject(value)) + return false + + return typeof value.unique_identifier === 'string' && isObject(value.manifest) +} + +function getRejectedResponse(error: unknown): unknown { + if (!isObject(error) || !('response' in error)) + return undefined + + return error.response +} + +function getUploadFailureMessage(response: unknown): string | undefined { + if (!isObject(response)) + return undefined + + return (response as UploadFailureResponse).message +} + type Props = { isBundle: boolean file: File @@ -32,36 +65,50 @@ const Uploading: FC = ({ }) => { const { t } = useTranslation() const fileName = file.name - const handleUpload = async () => { + const handleUploadedResponse = React.useCallback((response: unknown) => { + if (isBundle) { + if (Array.isArray(response)) { + onBundleUploaded(response as Dependency[]) + return + } + onFailed(t(`${i18nPrefix}.uploadFailed`, { ns: 'plugin' })) + return + } + + if (!isPackageUploadResponse(response)) { + onFailed(t(`${i18nPrefix}.uploadFailed`, { ns: 'plugin' })) + return + } + + onPackageUploaded({ + uniqueIdentifier: response.unique_identifier, + manifest: response.manifest, + }) + }, [isBundle, onBundleUploaded, onFailed, onPackageUploaded, t]) + + const handleUpload = React.useCallback(async () => { try { - await uploadFile(file, isBundle) + handleUploadedResponse(await uploadFile(file, isBundle)) } - catch (e: any) { - if (e.response?.message) { - onFailed(e.response?.message) - } - else { // Why it would into this branch? - const res = e.response - if (isBundle) { - onBundleUploaded(res) - return - } - onPackageUploaded({ - uniqueIdentifier: res.unique_identifier, - manifest: res.manifest, - }) + catch (error: unknown) { + const response = getRejectedResponse(error) + const message = getUploadFailureMessage(response) + if (message) { + onFailed(message) + return } + handleUploadedResponse(response) } - } + }, [file, handleUploadedResponse, isBundle, onFailed]) React.useEffect(() => { handleUpload() - }, []) + }, [handleUpload]) return ( <>
- +
{t(`${i18nPrefix}.uploadingPackage`, { ns: 'plugin', @@ -72,7 +119,7 @@ const Uploading: FC = ({
- -
+ +
{t('newApp.appCreateDSLErrorTitle', { ns: 'app' })} } className="flex grow flex-col system-md-regular text-text-secondary">
{t('newApp.appCreateDSLErrorPart1', { ns: 'app' })}
@@ -53,7 +53,7 @@ const VersionMismatchModal = ({
- + {t('newApp.Cancel', { ns: 'app' })} {t('newApp.Confirm', { ns: 'app' })} diff --git a/web/app/components/rag-pipeline/hooks/__tests__/use-update-dsl-modal.spec.ts b/web/app/components/rag-pipeline/hooks/__tests__/use-update-dsl-modal.spec.ts index 2449bcc605..6876df9014 100644 --- a/web/app/components/rag-pipeline/hooks/__tests__/use-update-dsl-modal.spec.ts +++ b/web/app/components/rag-pipeline/hooks/__tests__/use-update-dsl-modal.spec.ts @@ -340,6 +340,7 @@ describe('useUpdateDSLModal', () => { id: 'import-id', status: DSLImportStatus.FAILED, pipeline_id: 'test-pipeline-id', + error: 'Missing rag_pipeline data in YAML content', }) const { result } = renderUpdateDSLModal() @@ -351,7 +352,10 @@ describe('useUpdateDSLModal', () => { await (result.current.handleImport as unknown as AsyncFn)() }) - expect(toastMocks.call).toHaveBeenCalledWith(expect.objectContaining({ type: 'error' })) + expect(toastMocks.call).toHaveBeenCalledWith({ + type: 'error', + message: 'Missing rag_pipeline data in YAML content', + }) }) it('should notify error when importDSL throws', async () => { @@ -369,6 +373,29 @@ describe('useUpdateDSLModal', () => { expect(toastMocks.call).toHaveBeenCalledWith(expect.objectContaining({ type: 'error' })) }) + it('should notify response error when importDSL rejects with a response body', async () => { + mockImportDSL.mockRejectedValue(new Response(JSON.stringify({ + error: 'Missing rag_pipeline data in YAML content', + }), { + status: 400, + headers: { 'Content-Type': 'application/json' }, + })) + + const { result } = renderUpdateDSLModal() + act(() => { + result.current.handleFile(createFile()) + }) + + await act(async () => { + await (result.current.handleImport as unknown as AsyncFn)() + }) + + expect(toastMocks.call).toHaveBeenCalledWith({ + type: 'error', + message: 'Missing rag_pipeline data in YAML content', + }) + }) + it('should notify error when pipeline_id is missing on success', async () => { mockImportDSL.mockResolvedValue({ id: 'import-id', @@ -468,6 +495,7 @@ describe('useUpdateDSLModal', () => { mockImportDSLConfirm.mockResolvedValue({ status: DSLImportStatus.FAILED, pipeline_id: 'test-pipeline-id', + error: 'Import information expired or does not exist', }) const { result } = renderUpdateDSLModal() @@ -477,7 +505,10 @@ describe('useUpdateDSLModal', () => { await (result.current.onUpdateDSLConfirm as unknown as AsyncFn)() }) - expect(toastMocks.call).toHaveBeenCalledWith(expect.objectContaining({ type: 'error' })) + expect(toastMocks.call).toHaveBeenCalledWith({ + type: 'error', + message: 'Import information expired or does not exist', + }) }) it('should notify error when confirm throws exception', async () => { @@ -493,6 +524,27 @@ describe('useUpdateDSLModal', () => { expect(toastMocks.call).toHaveBeenCalledWith(expect.objectContaining({ type: 'error' })) }) + it('should notify response error when confirm rejects with a response body', async () => { + mockImportDSLConfirm.mockRejectedValue(new Response(JSON.stringify({ + error: 'Import information expired or does not exist', + }), { + status: 400, + headers: { 'Content-Type': 'application/json' }, + })) + + const { result } = renderUpdateDSLModal() + await setupPendingState(result) + + await act(async () => { + await (result.current.onUpdateDSLConfirm as unknown as AsyncFn)() + }) + + expect(toastMocks.call).toHaveBeenCalledWith({ + type: 'error', + message: 'Import information expired or does not exist', + }) + }) + it('should notify error when confirm succeeds but pipeline_id is missing', async () => { mockImportDSLConfirm.mockResolvedValue({ status: DSLImportStatus.COMPLETED, diff --git a/web/app/components/rag-pipeline/hooks/use-update-dsl-modal.ts b/web/app/components/rag-pipeline/hooks/use-update-dsl-modal.ts index 7271279996..0d12a0a881 100644 --- a/web/app/components/rag-pipeline/hooks/use-update-dsl-modal.ts +++ b/web/app/components/rag-pipeline/hooks/use-update-dsl-modal.ts @@ -15,11 +15,36 @@ type VersionInfo = { importedVersion: string systemVersion: string } +type ImportErrorResponse = { + message?: unknown + error?: unknown +} type UseUpdateDSLModalParams = { onCancel: () => void onImport?: () => void } const isCompletedStatus = (status: DSLImportStatus): boolean => status === DSLImportStatus.COMPLETED || status === DSLImportStatus.COMPLETED_WITH_WARNINGS +const getNonEmptyString = (value: unknown): string | undefined => { + if (typeof value !== 'string') + return undefined + + const trimmedValue = value.trim() + return trimmedValue || undefined +} +const getImportErrorMessage = async (error: unknown): Promise => { + if (error instanceof Response && !error.bodyUsed) { + try { + const errorData = await error.clone().json() as ImportErrorResponse + return getNonEmptyString(errorData.message) ?? getNonEmptyString(errorData.error) + } + catch {} + } + + if (error instanceof Error) + return getNonEmptyString(error.message) + + return undefined +} export const useUpdateDSLModal = ({ onCancel, onImport }: UseUpdateDSLModalParams) => { const { t } = useTranslation() const { eventEmitter } = useEventEmitterContextContext() @@ -52,9 +77,9 @@ export const useUpdateDSLModal = ({ onCancel, onImport }: UseUpdateDSLModalParam if (!file) setFileContent('') } - const notifyError = useCallback(() => { + const notifyError = useCallback((message?: string) => { setLoading(false) - toast.error(t('common.importFailure', { ns: 'workflow' })) + toast.error(message || t('common.importFailure', { ns: 'workflow' })) }, [t]) const updateWorkflow = useCallback(async (pipelineId: string) => { const { graph, hash, rag_pipeline_variables } = await fetchWorkflowDraft(`/rag/pipelines/${pipelineId}/workflows/draft`) @@ -117,10 +142,10 @@ export const useUpdateDSLModal = ({ onCancel, onImport }: UseUpdateDSLModalParam else if (status === DSLImportStatus.PENDING) showVersionMismatch(id, imported_dsl_version, current_dsl_version) else - notifyError() + notifyError(response.error) } - catch { - notifyError() + catch (error) { + notifyError(await getImportErrorMessage(error)) } isCreatingRef.current = false }, [currentFile, fileContent, workflowStore, importDSL, completeImport, showVersionMismatch, notifyError]) @@ -128,16 +153,16 @@ export const useUpdateDSLModal = ({ onCancel, onImport }: UseUpdateDSLModalParam if (!importId) return try { - const { status, pipeline_id } = await importDSLConfirm(importId) + const { status, pipeline_id, error } = await importDSLConfirm(importId) if (status === DSLImportStatus.COMPLETED) { await completeImport(pipeline_id) return } if (status === DSLImportStatus.FAILED) - notifyError() + notifyError(error) } - catch { - notifyError() + catch (error) { + notifyError(await getImportErrorMessage(error)) } }, [importId, importDSLConfirm, completeImport, notifyError]) return { diff --git a/web/models/pipeline.ts b/web/models/pipeline.ts index e75ee7569a..30389ab5e0 100644 --- a/web/models/pipeline.ts +++ b/web/models/pipeline.ts @@ -89,6 +89,7 @@ export type ImportPipelineDSLResponse = { dataset_id: string current_dsl_version: string imported_dsl_version: string + error?: string } export type ImportPipelineDSLConfirmResponse = { diff --git a/web/service/__tests__/use-pipeline.spec.tsx b/web/service/__tests__/use-pipeline.spec.tsx new file mode 100644 index 0000000000..411d29ac6a --- /dev/null +++ b/web/service/__tests__/use-pipeline.spec.tsx @@ -0,0 +1,81 @@ +import type { ReactNode } from 'react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { act, renderHook } from '@testing-library/react' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { DSLImportMode, DSLImportStatus } from '@/models/app' +import { post } from '../base' +import { useImportPipelineDSL, useImportPipelineDSLConfirm } from '../use-pipeline' + +vi.mock('../base', () => ({ + post: vi.fn(), + get: vi.fn(), + patch: vi.fn(), + del: vi.fn(), +})) + +const createWrapper = () => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }) + + return ({ children }: { children: ReactNode }) => ( + + {children} + + ) +} + +describe('use-pipeline imports', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(post).mockResolvedValue({ + id: 'import-id', + status: DSLImportStatus.COMPLETED, + pipeline_id: 'pipeline-id', + dataset_id: 'dataset-id', + current_dsl_version: '0.1.0', + imported_dsl_version: '0.1.0', + }) + }) + + it('should import pipeline DSL silently so callers can own error toasts', async () => { + const { result } = renderHook( + () => useImportPipelineDSL(), + { wrapper: createWrapper() }, + ) + const request = { + mode: DSLImportMode.YAML_CONTENT, + yaml_content: 'rag_pipeline: {}', + } + + await act(async () => { + await result.current.mutateAsync(request) + }) + + expect(post).toHaveBeenCalledWith( + '/rag/pipelines/imports', + { body: request }, + { silent: true }, + ) + }) + + it('should confirm pipeline DSL import silently so callers can own error toasts', async () => { + const { result } = renderHook( + () => useImportPipelineDSLConfirm(), + { wrapper: createWrapper() }, + ) + + await act(async () => { + await result.current.mutateAsync('import-id') + }) + + expect(post).toHaveBeenCalledWith( + '/rag/pipelines/imports/import-id/confirm', + {}, + { silent: true }, + ) + }) +}) diff --git a/web/service/use-pipeline.ts b/web/service/use-pipeline.ts index 6efb5bb8c6..7e1bd9711f 100644 --- a/web/service/use-pipeline.ts +++ b/web/service/use-pipeline.ts @@ -113,7 +113,7 @@ export const useImportPipelineDSL = ( return useMutation({ mutationKey: [NAME_SPACE, 'dsl-import'], mutationFn: (request: ImportPipelineDSLRequest) => { - return post('/rag/pipelines/imports', { body: request }) + return post('/rag/pipelines/imports', { body: request }, { silent: true }) }, ...mutationOptions, }) @@ -125,7 +125,7 @@ export const useImportPipelineDSLConfirm = ( return useMutation({ mutationKey: [NAME_SPACE, 'dsl-import-confirm'], mutationFn: (importId: string) => { - return post(`/rag/pipelines/imports/${importId}/confirm`) + return post(`/rag/pipelines/imports/${importId}/confirm`, {}, { silent: true }) }, ...mutationOptions, })