From 06ea0f7ac2e5e61c5700d62fb6cce4a0d72be960 Mon Sep 17 00:00:00 2001 From: yyh <92089059+lyzno1@users.noreply.github.com> Date: Mon, 18 May 2026 16:03:08 +0800 Subject: [PATCH] fix: improve API extension dialog controls (#36323) --- .../apikey-info-panel.test-utils.tsx | 1 - .../settings/__tests__/index.spec.tsx | 1 - .../__tests__/index.spec.tsx | 65 +++--- .../__tests__/item.spec.tsx | 55 ++--- .../__tests__/modal.spec.tsx | 100 +++++++-- .../__tests__/selector.spec.tsx | 36 ++-- .../api-based-extension-page/index.tsx | 47 ++++- .../api-based-extension-page/item.tsx | 33 ++- .../api-based-extension-page/modal.tsx | 73 ++++--- .../api-based-extension-page/selector.tsx | 191 ++++++++++-------- .../__tests__/index.spec.tsx | 1 - web/context/modal-context-provider.tsx | 21 -- web/context/modal-context.ts | 3 - 13 files changed, 339 insertions(+), 288 deletions(-) diff --git a/web/app/components/app/overview/apikey-info-panel/apikey-info-panel.test-utils.tsx b/web/app/components/app/overview/apikey-info-panel/apikey-info-panel.test-utils.tsx index 1be3799480..e8d8266586 100644 --- a/web/app/components/app/overview/apikey-info-panel/apikey-info-panel.test-utils.tsx +++ b/web/app/components/app/overview/apikey-info-panel/apikey-info-panel.test-utils.tsx @@ -59,7 +59,6 @@ const defaultProviderContext = { const defaultModalContext: ModalContextState = { setShowAccountSettingModal: noop, - setShowApiBasedExtensionModal: noop, setShowModerationSettingModal: noop, setShowExternalDataToolModal: noop, setShowPricingModal: noop, diff --git a/web/app/components/app/overview/settings/__tests__/index.spec.tsx b/web/app/components/app/overview/settings/__tests__/index.spec.tsx index 8023155e5e..462227fbbe 100644 --- a/web/app/components/app/overview/settings/__tests__/index.spec.tsx +++ b/web/app/components/app/overview/settings/__tests__/index.spec.tsx @@ -55,7 +55,6 @@ const mockUseProviderContext = vi.fn<() => ProviderContextState>() const buildModalContext = (): ModalContextState => ({ setShowAccountSettingModal: mockSetShowAccountSettingModal, - setShowApiBasedExtensionModal: vi.fn(), setShowModerationSettingModal: vi.fn(), setShowExternalDataToolModal: vi.fn(), setShowPricingModal: mockSetShowPricingModal, diff --git a/web/app/components/header/account-setting/api-based-extension-page/__tests__/index.spec.tsx b/web/app/components/header/account-setting/api-based-extension-page/__tests__/index.spec.tsx index d70adc6b92..e45d6c2d16 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/__tests__/index.spec.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/__tests__/index.spec.tsx @@ -1,8 +1,6 @@ import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' -import type { SetStateAction } from 'react' -import type { ModalContextState, ModalState } from '@/context/modal-context' -import { fireEvent, render, screen } from '@testing-library/react' -import { useModalContext } from '@/context/modal-context' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { addApiBasedExtension, updateApiBasedExtension } from '@/service/common' import { useApiBasedExtensions } from '@/service/use-common' import ApiBasedExtensionPage from '../index' @@ -10,19 +8,16 @@ vi.mock('@/service/use-common', () => ({ useApiBasedExtensions: vi.fn(), })) -vi.mock('@/context/modal-context', () => ({ - useModalContext: vi.fn(), +vi.mock('@/service/common', () => ({ + addApiBasedExtension: vi.fn(), + updateApiBasedExtension: vi.fn(), })) describe('ApiBasedExtensionPage', () => { const mockRefetch = vi.fn<() => void>() - const mockSetShowApiBasedExtensionModal = vi.fn<(value: SetStateAction> | null>) => void>() beforeEach(() => { vi.clearAllMocks() - vi.mocked(useModalContext).mockReturnValue({ - setShowApiBasedExtensionModal: mockSetShowApiBasedExtensionModal, - } as unknown as ModalContextState) }) describe('Rendering', () => { @@ -128,13 +123,17 @@ describe('ApiBasedExtensionPage', () => { fireEvent.click(screen.getByText('common.apiBasedExtension.add')) // Assert - expect(mockSetShowApiBasedExtensionModal).toHaveBeenCalledWith(expect.objectContaining({ - payload: {}, - })) + expect(screen.getByRole('dialog', { name: 'common.apiBasedExtension.modal.title' })).toBeInTheDocument() }) - it('should call refetch when onSaveCallback is executed from the modal', () => { + it('should call refetch when add modal saves successfully', async () => { // Arrange + vi.mocked(addApiBasedExtension).mockResolvedValue({ + id: 'new-id', + name: 'New Ext', + api_endpoint: 'https://api.test', + api_key: 'secret-key', + }) vi.mocked(useApiBasedExtensions).mockReturnValue({ data: [], isPending: false, @@ -144,25 +143,23 @@ describe('ApiBasedExtensionPage', () => { // Act render() fireEvent.click(screen.getByText('common.apiBasedExtension.add')) + fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.name.placeholder'), { target: { value: 'New Ext' } }) + fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.apiEndpoint.placeholder'), { target: { value: 'https://api.test' } }) + fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.apiKey.placeholder'), { target: { value: 'secret-key' } }) + fireEvent.click(screen.getByText('common.operation.save')) - // Trigger callback manually from the mock call - const callArgs = mockSetShowApiBasedExtensionModal.mock.calls[0]![0] - if (typeof callArgs === 'object' && callArgs !== null && 'onSaveCallback' in callArgs) { - if (callArgs.onSaveCallback) { - callArgs.onSaveCallback() - // Assert - expect(mockRefetch).toHaveBeenCalled() - } - } + // Assert + await waitFor(() => { + expect(mockRefetch).toHaveBeenCalled() + }) }) - it('should call refetch when an item is updated', () => { + it('should call refetch when an item is updated', async () => { // Arrange - const mockData: ApiBasedExtensionResponse[] = [ - { id: '1', name: 'Extension 1', api_endpoint: 'url1', api_key: 'key1' }, - ] + const extension: ApiBasedExtensionResponse = { id: '1', name: 'Extension 1', api_endpoint: 'url1', api_key: 'long-api-key' } + vi.mocked(updateApiBasedExtension).mockResolvedValue({ ...extension, name: 'Updated' }) vi.mocked(useApiBasedExtensions).mockReturnValue({ - data: mockData, + data: [extension], isPending: false, refetch: mockRefetch, } as unknown as ReturnType) @@ -171,16 +168,12 @@ describe('ApiBasedExtensionPage', () => { // Act - Click edit on the rendered item fireEvent.click(screen.getByText('common.operation.edit')) - - // Retrieve the onSaveCallback from the modal call and execute it - const callArgs = mockSetShowApiBasedExtensionModal.mock.calls[0]![0] - if (typeof callArgs === 'object' && callArgs !== null && 'onSaveCallback' in callArgs) { - if (callArgs.onSaveCallback) - callArgs.onSaveCallback() - } + fireEvent.click(screen.getByText('common.operation.save')) // Assert - expect(mockRefetch).toHaveBeenCalled() + await waitFor(() => { + expect(mockRefetch).toHaveBeenCalled() + }) }) }) }) diff --git a/web/app/components/header/account-setting/api-based-extension-page/__tests__/item.spec.tsx b/web/app/components/header/account-setting/api-based-extension-page/__tests__/item.spec.tsx index 6f40b3d32a..2bbfc91c3e 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/__tests__/item.spec.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/__tests__/item.spec.tsx @@ -1,17 +1,10 @@ import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' import type { TFunction } from 'i18next' -import type { ModalContextState } from '@/context/modal-context' import { fireEvent, render, screen, waitFor, within } from '@testing-library/react' import * as reactI18next from 'react-i18next' -import { useModalContext } from '@/context/modal-context' import { deleteApiBasedExtension } from '@/service/common' import Item from '../item' -// Mock dependencies -vi.mock('@/context/modal-context', () => ({ - useModalContext: vi.fn(), -})) - vi.mock('@/service/common', () => ({ deleteApiBasedExtension: vi.fn(), })) @@ -24,19 +17,16 @@ describe('Item Component', () => { api_key: 'test-api-key', } const mockOnUpdate = vi.fn() - const mockSetShowApiBasedExtensionModal = vi.fn() + const mockOnEdit = vi.fn() beforeEach(() => { vi.clearAllMocks() - vi.mocked(useModalContext).mockReturnValue({ - setShowApiBasedExtensionModal: mockSetShowApiBasedExtensionModal, - } as unknown as ModalContextState) }) describe('Rendering', () => { it('should render extension data correctly', () => { // Act - render() + render() // Assert // Assert @@ -54,7 +44,7 @@ describe('Item Component', () => { } // Act - render() + render() // Assert // Assert @@ -64,41 +54,20 @@ describe('Item Component', () => { }) describe('Modal Interactions', () => { - it('should open edit modal with correct payload when clicking edit button', () => { + it('should request editing with the current extension when clicking edit button', () => { // Act - render() + render() fireEvent.click(screen.getByText('common.operation.edit')) // Assert - expect(mockSetShowApiBasedExtensionModal).toHaveBeenCalledWith(expect.objectContaining({ - payload: mockData, - })) - const lastCall = mockSetShowApiBasedExtensionModal.mock.calls[0]![0] - if (typeof lastCall === 'object' && lastCall !== null && 'onSaveCallback' in lastCall) - expect(lastCall.onSaveCallback).toBeInstanceOf(Function) - }) - - it('should execute onUpdate callback when edit modal save callback is invoked', () => { - // Act - render() - fireEvent.click(screen.getByText('common.operation.edit')) - - // Assert - const modalCallArg = mockSetShowApiBasedExtensionModal.mock.calls[0]![0] - if (typeof modalCallArg === 'object' && modalCallArg !== null && 'onSaveCallback' in modalCallArg) { - const onSaveCallback = modalCallArg.onSaveCallback - if (onSaveCallback) { - onSaveCallback() - expect(mockOnUpdate).toHaveBeenCalledTimes(1) - } - } + expect(mockOnEdit).toHaveBeenCalledWith(mockData) }) }) describe('Deletion', () => { it('should show delete confirmation dialog when clicking delete button', () => { // Act - render() + render() fireEvent.click(screen.getByText('common.operation.delete')) // Assert @@ -109,7 +78,7 @@ describe('Item Component', () => { it('should call delete API and triggers onUpdate when confirming deletion', async () => { // Arrange vi.mocked(deleteApiBasedExtension).mockResolvedValue({ result: 'success' }) - render() + render() // Act fireEvent.click(screen.getByText('common.operation.delete')) @@ -129,7 +98,7 @@ describe('Item Component', () => { it('should hide delete confirmation dialog after successful deletion', async () => { // Arrange vi.mocked(deleteApiBasedExtension).mockResolvedValue({ result: 'success' }) - render() + render() // Act fireEvent.click(screen.getByText('common.operation.delete')) @@ -147,7 +116,7 @@ describe('Item Component', () => { it('should close delete confirmation when clicking cancel button', async () => { // Act - render() + render() fireEvent.click(screen.getByText('common.operation.delete')) fireEvent.click(screen.getByText('common.operation.cancel')) @@ -159,7 +128,7 @@ describe('Item Component', () => { it('should not call delete API when canceling deletion', () => { // Act - render() + render() fireEvent.click(screen.getByText('common.operation.delete')) fireEvent.click(screen.getByText('common.operation.cancel')) @@ -188,7 +157,7 @@ describe('Item Component', () => { } as unknown as ReturnType) // Act - render() + render() const allButtons = screen.getAllByRole('button') const editBtn = screen.getByText('operation.edit') const deleteBtn = allButtons.find(btn => btn !== editBtn) diff --git a/web/app/components/header/account-setting/api-based-extension-page/__tests__/modal.spec.tsx b/web/app/components/header/account-setting/api-based-extension-page/__tests__/modal.spec.tsx index c6e28c9fa0..dcff720cdf 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/__tests__/modal.spec.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/__tests__/modal.spec.tsx @@ -1,6 +1,6 @@ import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' import type { TFunction } from 'i18next' -import type { ReactElement } from 'react' +import type { ComponentProps, ReactElement } from 'react' import { fireEvent, render as RTLRender, screen, waitFor } from '@testing-library/react' import * as reactI18next from 'react-i18next' import { useDocLink } from '@/context/i18n' @@ -34,7 +34,7 @@ vi.mock('@langgenius/dify-ui/toast', () => ({ })) describe('ApiBasedExtensionModal', () => { - const mockOnCancel = vi.fn() + const mockOnOpenChange = vi.fn() const mockOnSave = vi.fn() const mockDocLink = vi.fn((path?: string) => `https://docs.dify.ai${path || ''}`) const mockExtension = (overrides: Partial = {}): ApiBasedExtensionResponse => ({ @@ -46,6 +46,19 @@ describe('ApiBasedExtensionModal', () => { }) const render = (ui: ReactElement) => RTLRender(ui) + const renderModal = (props: Partial> = {}) => render( + , + ) + const expectCloseRequested = () => { + const calls = mockOnOpenChange.mock.calls + expect(calls[calls.length - 1]?.[0]).toBe(false) + } beforeEach(() => { vi.clearAllMocks() @@ -55,9 +68,10 @@ describe('ApiBasedExtensionModal', () => { describe('Rendering', () => { it('should render correctly for adding a new extension', () => { // Act - render() + renderModal() // Assert + expect(screen.getByRole('dialog', { name: 'common.apiBasedExtension.modal.title' })).toBeInTheDocument() expect(screen.getByText('common.apiBasedExtension.modal.title')).toBeInTheDocument() expect(screen.getByPlaceholderText('common.apiBasedExtension.modal.name.placeholder')).toBeInTheDocument() expect(screen.getByPlaceholderText('common.apiBasedExtension.modal.apiEndpoint.placeholder')).toBeInTheDocument() @@ -69,7 +83,7 @@ describe('ApiBasedExtensionModal', () => { const data = mockExtension() // Act - render() + renderModal({ extension: data }) // Assert expect(screen.getByText('common.apiBasedExtension.modal.editTitle')).toBeInTheDocument() @@ -77,6 +91,14 @@ describe('ApiBasedExtensionModal', () => { expect(screen.getByDisplayValue('url')).toBeInTheDocument() expect(screen.getByDisplayValue('key')).toBeInTheDocument() }) + + it('should not render dialog content when closed', () => { + // Act + renderModal({ open: false }) + + // Assert + expect(screen.queryByRole('dialog')).not.toBeInTheDocument() + }) }) describe('Form Submissions', () => { @@ -89,7 +111,7 @@ describe('ApiBasedExtensionModal', () => { api_key: 'secret-key', }) vi.mocked(addApiBasedExtension).mockResolvedValue(newExtension) - render() + renderModal() // Act fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.name.placeholder'), { target: { value: 'New Ext' } }) @@ -115,7 +137,7 @@ describe('ApiBasedExtensionModal', () => { // Arrange const data = mockExtension({ api_key: 'long-secret-key' }) vi.mocked(updateApiBasedExtension).mockResolvedValue({ ...data, name: 'Updated' }) - render() + renderModal({ extension: data }) // Act fireEvent.change(screen.getByDisplayValue('Existing'), { target: { value: 'Updated' } }) @@ -125,12 +147,11 @@ describe('ApiBasedExtensionModal', () => { await waitFor(() => { expect(updateApiBasedExtension).toHaveBeenCalledWith({ url: '/api-based-extension/1', - body: expect.objectContaining({ - id: '1', + body: { name: 'Updated', api_endpoint: 'url', api_key: '[__HIDDEN__]', - }), + }, }) expect(mockToast.success).toHaveBeenCalledWith('common.actionMsg.modifiedSuccessfully') expect(mockOnSave).toHaveBeenCalled() @@ -141,7 +162,7 @@ describe('ApiBasedExtensionModal', () => { // Arrange const data = mockExtension({ api_key: 'old-key' }) vi.mocked(updateApiBasedExtension).mockResolvedValue({ ...data, api_key: 'new-longer-key' }) - render() + renderModal({ extension: data }) // Act fireEvent.change(screen.getByDisplayValue('old-key'), { target: { value: 'new-longer-key' } }) @@ -151,9 +172,11 @@ describe('ApiBasedExtensionModal', () => { await waitFor(() => { expect(updateApiBasedExtension).toHaveBeenCalledWith({ url: '/api-based-extension/1', - body: expect.objectContaining({ + body: { + name: 'Existing', + api_endpoint: 'url', api_key: 'new-longer-key', - }), + }, }) }) }) @@ -162,7 +185,7 @@ describe('ApiBasedExtensionModal', () => { describe('Validation', () => { it('should show error if api key is too short', async () => { // Arrange - render() + renderModal() // Act fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.name.placeholder'), { target: { value: 'Ext' } }) @@ -180,7 +203,7 @@ describe('ApiBasedExtensionModal', () => { it('should work when onSave is not provided', async () => { // Arrange vi.mocked(addApiBasedExtension).mockResolvedValue(mockExtension({ id: 'new-id' })) - render() + renderModal({ onSave: undefined }) // Act fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.name.placeholder'), { target: { value: 'New Ext' } }) @@ -194,15 +217,56 @@ describe('ApiBasedExtensionModal', () => { }) }) - it('should call onCancel when clicking cancel button', () => { + it('should request closing when clicking cancel button', () => { // Arrange - render() + renderModal() // Act fireEvent.click(screen.getByText('common.operation.cancel')) // Assert - expect(mockOnCancel).toHaveBeenCalled() + expectCloseRequested() + }) + + it('should request closing when clicking close button', async () => { + // Arrange + renderModal() + + // Act + fireEvent.click(screen.getByRole('button', { name: 'Close' })) + + // Assert + await waitFor(() => { + expectCloseRequested() + }) + }) + + it('should request closing when pressing Escape', async () => { + // Arrange + renderModal() + + // Act + fireEvent.keyDown(document, { key: 'Escape' }) + + // Assert + await waitFor(() => { + expectCloseRequested() + }) + }) + + it('should keep open when clicking outside the dialog', () => { + // Arrange + renderModal() + + // Act + const backdrop = document.querySelector('.bg-background-overlay') + expect(backdrop).toBeInTheDocument() + fireEvent.pointerDown(backdrop!) + fireEvent.pointerUp(backdrop!) + fireEvent.click(backdrop!) + + // Assert + expect(mockOnOpenChange).not.toHaveBeenCalled() }) }) @@ -230,7 +294,7 @@ describe('ApiBasedExtensionModal', () => { } as unknown as ReturnType) // Act - const { container } = render() + const { container } = renderModal({ onSave: undefined }) // Assert const inputs = container.querySelectorAll('input') diff --git a/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx b/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx index c480b7a9f1..1eb4f9495f 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/__tests__/selector.spec.tsx @@ -1,9 +1,10 @@ import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' import type { UseQueryResult } from '@tanstack/react-query' import type { ModalContextState } from '@/context/modal-context' -import { fireEvent, render, screen } from '@testing-library/react' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { useModalContext } from '@/context/modal-context' +import { addApiBasedExtension } from '@/service/common' import { useApiBasedExtensions } from '@/service/use-common' import ApiBasedExtensionSelector from '../selector' @@ -15,12 +16,15 @@ vi.mock('@/service/use-common', () => ({ useApiBasedExtensions: vi.fn(), })) +vi.mock('@/service/common', () => ({ + addApiBasedExtension: vi.fn(), +})) + vi.mock('@langgenius/dify-ui/popover', async () => await import('@/__mocks__/base-ui-popover')) describe('ApiBasedExtensionSelector', () => { const mockOnChange = vi.fn() const mockSetShowAccountSettingModal = vi.fn() - const mockSetShowApiBasedExtensionModal = vi.fn() const mockRefetch = vi.fn() const mockData: ApiBasedExtensionResponse[] = [ @@ -32,7 +36,6 @@ describe('ApiBasedExtensionSelector', () => { vi.clearAllMocks() vi.mocked(useModalContext).mockReturnValue({ setShowAccountSettingModal: mockSetShowAccountSettingModal, - setShowApiBasedExtensionModal: mockSetShowApiBasedExtensionModal, } as unknown as ModalContextState) vi.mocked(useApiBasedExtensions).mockReturnValue({ data: mockData, @@ -103,26 +106,29 @@ describe('ApiBasedExtensionSelector', () => { }) it('should open add modal when clicking add button and refetches on save', async () => { + // Arrange + vi.mocked(addApiBasedExtension).mockResolvedValue({ + id: 'new-id', + name: 'New Ext', + api_endpoint: 'https://api.test', + api_key: 'secret-key', + }) + // Act render() fireEvent.click(screen.getByText('common.apiBasedExtension.selector.placeholder')) const addButton = await screen.findByText('common.operation.add') fireEvent.click(addButton) + fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.name.placeholder'), { target: { value: 'New Ext' } }) + fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.apiEndpoint.placeholder'), { target: { value: 'https://api.test' } }) + fireEvent.change(screen.getByPlaceholderText('common.apiBasedExtension.modal.apiKey.placeholder'), { target: { value: 'secret-key' } }) + fireEvent.click(screen.getByText('common.operation.save')) // Assert - expect(mockSetShowApiBasedExtensionModal).toHaveBeenCalledWith(expect.objectContaining({ - payload: {}, - })) - - // Trigger callback - const lastCall = mockSetShowApiBasedExtensionModal.mock.calls[0]![0] - if (typeof lastCall === 'object' && lastCall !== null && 'onSaveCallback' in lastCall) { - if (lastCall.onSaveCallback) { - lastCall.onSaveCallback() - expect(mockRefetch).toHaveBeenCalled() - } - } + await waitFor(() => { + expect(mockRefetch).toHaveBeenCalled() + }) }) }) }) diff --git a/web/app/components/header/account-setting/api-based-extension-page/index.tsx b/web/app/components/header/account-setting/api-based-extension-page/index.tsx index bc26f63838..ccef2022e7 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/index.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/index.tsx @@ -1,24 +1,42 @@ +import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' import { Button } from '@langgenius/dify-ui/button' -import { - RiAddLine, -} from '@remixicon/react' +import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { useModalContext } from '@/context/modal-context' import { useApiBasedExtensions } from '@/service/use-common' import Empty from './empty' import Item from './item' +import ApiBasedExtensionModal from './modal' + +type ApiBasedExtensionDialogState = { + extension: Partial + onSave: () => void +} | null const ApiBasedExtensionPage = () => { const { t } = useTranslation() - const { setShowApiBasedExtensionModal } = useModalContext() const { data, refetch: mutate, isPending: isLoading } = useApiBasedExtensions() + const [dialogState, setDialogState] = useState(null) const handleOpenApiBasedExtensionModal = () => { - setShowApiBasedExtensionModal({ - payload: {}, - onSaveCallback: () => mutate(), + setDialogState({ + extension: {}, + onSave: () => mutate(), }) } + const handleEditApiBasedExtension = (extension: ApiBasedExtensionResponse) => { + setDialogState({ + extension, + onSave: () => mutate(), + }) + } + const handleSaveApiBasedExtension = () => { + dialogState?.onSave() + setDialogState(null) + } + const handleApiBasedExtensionModalOpenChange = (open: boolean) => { + if (!open) + setDialogState(null) + } return (
@@ -33,6 +51,7 @@ const ApiBasedExtensionPage = () => { mutate()} /> )) @@ -43,9 +62,19 @@ const ApiBasedExtensionPage = () => { className="w-full" onClick={handleOpenApiBasedExtensionModal} > - +
) } diff --git a/web/app/components/header/account-setting/api-based-extension-page/item.tsx b/web/app/components/header/account-setting/api-based-extension-page/item.tsx index addc0e28d6..625e9ffaf3 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/item.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/item.tsx @@ -1,5 +1,4 @@ import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' -import type { FC } from 'react' import { AlertDialog, AlertDialogActions, @@ -9,32 +8,25 @@ import { AlertDialogTitle, } from '@langgenius/dify-ui/alert-dialog' import { Button } from '@langgenius/dify-ui/button' -import { - RiDeleteBinLine, - RiEditLine, -} from '@remixicon/react' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { useModalContext } from '@/context/modal-context' import { deleteApiBasedExtension } from '@/service/common' type ItemProps = { data: ApiBasedExtensionResponse + onEdit: (extension: ApiBasedExtensionResponse) => void onUpdate: () => void } -const Item: FC = ({ +const Item = ({ data, + onEdit, onUpdate, -}) => { +}: ItemProps) => { const { t } = useTranslation() - const { setShowApiBasedExtensionModal } = useModalContext() const [showDeleteConfirm, setShowDeleteConfirm] = useState(false) const handleOpenApiBasedExtensionModal = () => { - setShowApiBasedExtensionModal({ - payload: data, - onSaveCallback: () => onUpdate(), - }) + onEdit(data) } const handleDeleteApiBasedExtension = async () => { await deleteApiBasedExtension(`/api-based-extension/${data.id}`) @@ -44,28 +36,27 @@ const Item: FC = ({ } return ( -
-
+
+
{data.name}
-
{data.api_endpoint}
+
{data.api_endpoint}
-
+
!open && setShowDeleteConfirm(false)}> - +
{`${t('operation.delete', { ns: 'common' })} \u201C${data.name}\u201D?`} diff --git a/web/app/components/header/account-setting/api-based-extension-page/modal.tsx b/web/app/components/header/account-setting/api-based-extension-page/modal.tsx index 76b59183ec..ba7c6e4eb6 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/modal.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/modal.tsx @@ -2,51 +2,57 @@ import type { ApiBasedExtensionPayload, ApiBasedExtensionResponse, } from '@dify/contracts/api/console/api-based-extension/types.gen' -import type { FC } from 'react' import { Button } from '@langgenius/dify-ui/button' -import { Dialog, DialogContent } from '@langgenius/dify-ui/dialog' +import { Dialog, DialogCloseButton, DialogContent, DialogTitle } from '@langgenius/dify-ui/dialog' import { toast } from '@langgenius/dify-ui/toast' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { BookOpen01 } from '@/app/components/base/icons/src/vender/line/education' import { useDocLink } from '@/context/i18n' import { addApiBasedExtension, updateApiBasedExtension } from '@/service/common' +type ApiBasedExtensionField = 'name' | 'api_endpoint' | 'api_key' + type ApiBasedExtensionModalProps = { - data: Partial - onCancel: () => void + open: boolean + extension: Partial + onOpenChange: (open: boolean) => void onSave?: (newData: ApiBasedExtensionResponse) => void } -const ApiBasedExtensionModal: FC = ({ data, onCancel, onSave }) => { +const ApiBasedExtensionModal = ({ open, extension, onOpenChange, onSave }: ApiBasedExtensionModalProps) => { const { t } = useTranslation() const docLink = useDocLink() - const [localeData, setLocaleData] = useState(data) + const [localData, setLocalData] = useState(extension) const [loading, setLoading] = useState(false) - const handleDataChange = (type: string, value: string) => { - setLocaleData({ ...localeData, [type]: value }) + const handleDataChange = (field: ApiBasedExtensionField, value: string) => { + setLocalData({ ...localData, [field]: value }) } const handleSave = async () => { setLoading(true) - if (localeData && localeData.api_key && localeData.api_key?.length < 5) { + if (localData.api_key && localData.api_key.length < 5) { toast.error(t('apiBasedExtension.modal.apiKey.lengthError', { ns: 'common' })) setLoading(false) return } try { + const payload: ApiBasedExtensionPayload = { + name: localData.name || '', + api_endpoint: localData.api_endpoint || '', + api_key: localData.api_key || '', + } let res = {} as ApiBasedExtensionResponse - if (!data.id) { + if (!extension.id) { res = await addApiBasedExtension({ url: '/api-based-extension', - body: localeData as ApiBasedExtensionPayload, + body: payload, }) } else { res = await updateApiBasedExtension({ - url: `/api-based-extension/${data.id}`, + url: `/api-based-extension/${extension.id}`, body: { - ...localeData, - api_key: data.api_key === localeData.api_key ? '[__HIDDEN__]' : localeData.api_key, - } as ApiBasedExtensionPayload, + ...payload, + api_key: extension.api_key === localData.api_key ? '[__HIDDEN__]' : payload.api_key, + }, }) toast.success(t('actionMsg.modifiedSuccessfully', { ns: 'common' })) } @@ -57,44 +63,47 @@ const ApiBasedExtensionModal: FC = ({ data, onCance setLoading(false) } } - return ( - - -
- {data.name + return ( + + + + + + {extension.name ? t('apiBasedExtension.modal.editTitle', { ns: 'common' }) : t('apiBasedExtension.modal.title', { ns: 'common' })} -
+
{t('apiBasedExtension.modal.name.title', { ns: 'common' })}
- handleDataChange('name', e.target.value)} className="block h-9 w-full appearance-none rounded-lg bg-components-input-bg-normal px-3 text-sm text-text-primary outline-hidden" placeholder={t('apiBasedExtension.modal.name.placeholder', { ns: 'common' }) || ''} /> + handleDataChange('name', e.target.value)} className="block h-9 w-full appearance-none rounded-lg bg-components-input-bg-normal px-3 text-sm text-text-primary outline-hidden" placeholder={t('apiBasedExtension.modal.name.placeholder', { ns: 'common' }) || ''} />
{t('apiBasedExtension.modal.apiEndpoint.title', { ns: 'common' })} - - + +
- handleDataChange('api_endpoint', e.target.value)} className="block h-9 w-full appearance-none rounded-lg bg-components-input-bg-normal px-3 text-sm text-text-primary outline-hidden" placeholder={t('apiBasedExtension.modal.apiEndpoint.placeholder', { ns: 'common' }) || ''} /> + handleDataChange('api_endpoint', e.target.value)} className="block h-9 w-full appearance-none rounded-lg bg-components-input-bg-normal px-3 text-sm text-text-primary outline-hidden" placeholder={t('apiBasedExtension.modal.apiEndpoint.placeholder', { ns: 'common' }) || ''} />
{t('apiBasedExtension.modal.apiKey.title', { ns: 'common' })}
-
- handleDataChange('api_key', e.target.value)} className="mr-2 block h-9 grow appearance-none rounded-lg bg-components-input-bg-normal px-3 text-sm text-text-primary outline-hidden" placeholder={t('apiBasedExtension.modal.apiKey.placeholder', { ns: 'common' }) || ''} /> -
+ handleDataChange('api_key', e.target.value)} className="block h-9 w-full appearance-none rounded-lg bg-components-input-bg-normal px-3 text-sm text-text-primary outline-hidden" placeholder={t('apiBasedExtension.modal.apiKey.placeholder', { ns: 'common' }) || ''} />
-
- -
diff --git a/web/app/components/header/account-setting/api-based-extension-page/selector.tsx b/web/app/components/header/account-setting/api-based-extension-page/selector.tsx index c7640877ba..e98ff593cf 100644 --- a/web/app/components/header/account-setting/api-based-extension-page/selector.tsx +++ b/web/app/components/header/account-setting/api-based-extension-page/selector.tsx @@ -1,32 +1,25 @@ -import type { FC } from 'react' import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' -import { - RiAddLine, - RiArrowDownSLine, -} from '@remixicon/react' import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { - ArrowUpRight, -} from '@/app/components/base/icons/src/vender/line/arrows' import { ACCOUNT_SETTING_TAB } from '@/app/components/header/account-setting/constants' import { useModalContext } from '@/context/modal-context' import { useApiBasedExtensions } from '@/service/use-common' +import ApiBasedExtensionModal from './modal' type ApiBasedExtensionSelectorProps = { value: string onChange: (value: string) => void } -const ApiBasedExtensionSelector: FC = ({ +const ApiBasedExtensionSelector = ({ value, onChange, -}) => { +}: ApiBasedExtensionSelectorProps) => { const { t } = useTranslation() const [open, setOpen] = useState(false) + const [addModalOpen, setAddModalOpen] = useState(false) const { setShowAccountSettingModal, - setShowApiBasedExtensionModal, } = useModalContext() const { data, refetch: mutate } = useApiBasedExtensions() const handleSelect = (id: string) => { @@ -36,91 +29,115 @@ const ApiBasedExtensionSelector: FC = ({ const currentItem = data?.find(item => item.id === value) + const handleSaveApiBasedExtension = () => { + mutate() + setAddModalOpen(false) + } + const handleAddModalOpenChange = (nextOpen: boolean) => { + if (!nextOpen) + setAddModalOpen(false) + } + return ( - - - { - currentItem - ? ( -
-
{currentItem.name}
-
-
- {currentItem.api_endpoint} -
- -
-
- ) - : ( -
- {t('apiBasedExtension.selector.placeholder', { ns: 'common' })} - -
- ) - } - - )} - /> - + -
-
-
-
- {t('apiBasedExtension.selector.title', { ns: 'common' })} + + { + currentItem + ? ( +
+
{currentItem.name}
+
+
+ {currentItem.api_endpoint} +
+
+
+ ) + : ( +
+ {t('apiBasedExtension.selector.placeholder', { ns: 'common' })} +
+ ) + } + + )} + /> + +
+
+
+
+ {t('apiBasedExtension.selector.title', { ns: 'common' })} +
+
-
+ { + data?.map(item => ( + + )) + } +
+
+
+
+
-
-
- { - data?.map(item => ( -
handleSelect(item.id)} - > -
{item.name}
-
{item.api_endpoint}
-
- )) - } +
-
-
-
{ - setOpen(false) - setShowApiBasedExtensionModal({ payload: {}, onSaveCallback: () => mutate() }) - }} - > - - {t('operation.add', { ns: 'common' })} -
-
-
- - + + + { + addModalOpen && ( + + ) + } + ) } diff --git a/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx b/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx index 58ebab69b3..faa91a1cdf 100644 --- a/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx +++ b/web/app/components/tools/edit-custom-collection-modal/__tests__/index.spec.tsx @@ -26,7 +26,6 @@ const mockSetShowAccountSettingModal = vi.fn() vi.mock('@/context/modal-context', () => ({ useModalContext: (): ModalContextState => ({ setShowAccountSettingModal: mockSetShowAccountSettingModal, - setShowApiBasedExtensionModal: vi.fn(), setShowModerationSettingModal: vi.fn(), setShowExternalDataToolModal: vi.fn(), setShowPricingModal: mockSetShowPricingModal, diff --git a/web/context/modal-context-provider.tsx b/web/context/modal-context-provider.tsx index 54c3bb018d..b424a22bf4 100644 --- a/web/context/modal-context-provider.tsx +++ b/web/context/modal-context-provider.tsx @@ -1,6 +1,5 @@ 'use client' -import type { ApiBasedExtensionResponse } from '@dify/contracts/api/console/api-based-extension/types.gen' import type { ReactNode, SetStateAction } from 'react' import type { ModalState, ModelModalType } from './modal-context' import type { OpeningStatement } from '@/app/components/base/features/types' @@ -36,9 +35,6 @@ import { const AccountSetting = dynamic(() => import('@/app/components/header/account-setting'), { ssr: false, }) -const ApiBasedExtensionModal = dynamic(() => import('@/app/components/header/account-setting/api-based-extension-page/modal'), { - ssr: false, -}) const ModerationSettingModal = dynamic(() => import('@/app/components/base/features/new-feature-panel/moderation/moderation-setting-modal'), { ssr: false, }) @@ -90,7 +86,6 @@ export const ModalContextProvider = ({ ? urlAccountModalState.payload : DEFAULT_ACCOUNT_SETTING_TAB) : null - const [showApiBasedExtensionModal, setShowApiBasedExtensionModal] = useState> | null>(null) const [showModerationSettingModal, setShowModerationSettingModal] = useState | null>(null) const [showExternalDataToolModal, setShowExternalDataToolModal] = useState | null>(null) const [showModelModal, setShowModelModal] = useState | null>(null) @@ -206,12 +201,6 @@ export const ModalContextProvider = ({ showOpeningModal.onCancelCallback() }, [showOpeningModal]) - const handleSaveApiBasedExtension = (newApiBasedExtension: ApiBasedExtensionResponse) => { - if (showApiBasedExtensionModal?.onSaveCallback) - showApiBasedExtensionModal.onSaveCallback(newApiBasedExtension) - setShowApiBasedExtensionModal(null) - } - const handleSaveModeration = (newModerationConfig: ModerationConfig) => { if (showModerationSettingModal?.onSaveCallback) showModerationSettingModal.onSaveCallback(newModerationConfig) @@ -247,7 +236,6 @@ export const ModalContextProvider = ({ return ( setShowApiBasedExtensionModal(null)} - onSave={handleSaveApiBasedExtension} - /> - ) - } { !!showModerationSettingModal && ( | null>> - setShowApiBasedExtensionModal: Dispatch> | null>> setShowModerationSettingModal: Dispatch | null>> setShowExternalDataToolModal: Dispatch | null>> setShowPricingModal: () => void @@ -68,7 +66,6 @@ export type ModalContextState = { export const ModalContext = createContext({ setShowAccountSettingModal: noop, - setShowApiBasedExtensionModal: noop, setShowModerationSettingModal: noop, setShowExternalDataToolModal: noop, setShowPricingModal: noop,