diff --git a/eslint-suppressions.json b/eslint-suppressions.json index 0da7515286..1bff82ac17 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -124,11 +124,6 @@ "count": 1 } }, - "web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/provider-config-modal.tsx": { - "no-restricted-imports": { - "count": 1 - } - }, "web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/provider-panel.tsx": { "ts/no-explicit-any": { "count": 1 @@ -3248,14 +3243,6 @@ "count": 2 } }, - "web/app/components/plugins/plugin-auth/authorize/api-key-modal.tsx": { - "no-restricted-imports": { - "count": 1 - }, - "ts/no-explicit-any": { - "count": 2 - } - }, "web/app/components/plugins/plugin-auth/authorize/index.tsx": { "no-restricted-imports": { "count": 1 diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/__tests__/provider-config-modal.spec.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/__tests__/provider-config-modal.spec.tsx new file mode 100644 index 0000000000..f9e5ea28ee --- /dev/null +++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/__tests__/provider-config-modal.spec.tsx @@ -0,0 +1,346 @@ +import type { AliyunConfig, ArizeConfig, DatabricksConfig, LangFuseConfig, LangSmithConfig, MLflowConfig, OpikConfig, PhoenixConfig, TencentConfig, WeaveConfig } from '../type' +import { toast } from '@langgenius/dify-ui/toast' +import { render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { addTracingConfig, removeTracingConfig, updateTracingConfig } from '@/service/apps' +import ConfigBtn from '../config-button' +import ProviderConfigModal from '../provider-config-modal' +import { TracingProvider } from '../type' + +vi.mock('@/service/apps', () => ({ + addTracingConfig: vi.fn(), + removeTracingConfig: vi.fn(), + updateTracingConfig: vi.fn(), +})) + +vi.mock('@langgenius/dify-ui/toast', () => ({ + toast: vi.fn(), +})) + +type ProviderPayload = AliyunConfig | ArizeConfig | DatabricksConfig | LangFuseConfig | LangSmithConfig | MLflowConfig | OpikConfig | PhoenixConfig | TencentConfig | WeaveConfig + +const validConfigs = { + [TracingProvider.arize]: { + api_key: 'arize-api-key', + space_id: 'space-id', + project: 'arize-project', + endpoint: 'https://otlp.arize.com', + }, + [TracingProvider.phoenix]: { + api_key: 'phoenix-api-key', + project: 'phoenix-project', + endpoint: 'https://app.phoenix.arize.com', + }, + [TracingProvider.langSmith]: { + api_key: 'langsmith-api-key', + project: 'langsmith-project', + endpoint: 'https://api.smith.langchain.com', + }, + [TracingProvider.langfuse]: { + public_key: 'public-key', + secret_key: 'secret-key', + host: 'https://cloud.langfuse.com', + }, + [TracingProvider.opik]: { + api_key: 'opik-api-key', + project: 'opik-project', + workspace: 'default', + url: 'https://www.comet.com/opik/api/', + }, + [TracingProvider.weave]: { + api_key: 'weave-api-key', + entity: 'wandb-entity', + project: 'weave-project', + endpoint: 'https://trace.wandb.ai/', + host: 'https://api.wandb.ai', + }, + [TracingProvider.aliyun]: { + app_name: 'aliyun-app', + license_key: 'license-key', + endpoint: 'https://tracing.arms.aliyuncs.com', + }, + [TracingProvider.mlflow]: { + tracking_uri: 'http://localhost:5000', + experiment_id: 'experiment-id', + username: 'mlflow-user', + password: 'mlflow-password', + }, + [TracingProvider.databricks]: { + experiment_id: 'experiment-id', + host: 'https://workspace.cloud.databricks.com', + client_id: 'client-id', + client_secret: 'client-secret', + personal_access_token: 'personal-access-token', + }, + [TracingProvider.tencent]: { + token: 'tencent-token', + endpoint: 'https://your-region.cls.tencentcs.com', + service_name: 'dify_app', + }, +} satisfies Record + +const providerFieldLabels = [ + [TracingProvider.arize, ['API Key', 'Space ID', 'app.tracing.configProvider.project', 'Endpoint']], + [TracingProvider.phoenix, ['API Key', 'app.tracing.configProvider.project', 'Endpoint']], + [TracingProvider.langSmith, ['API Key', 'app.tracing.configProvider.project', 'Endpoint']], + [TracingProvider.langfuse, ['app.tracing.configProvider.secretKey', 'app.tracing.configProvider.publicKey', 'Host']], + [TracingProvider.opik, ['API Key', 'app.tracing.configProvider.project', 'Workspace', 'Url']], + [TracingProvider.weave, ['API Key', 'app.tracing.configProvider.project', 'Entity', 'Endpoint', 'Host']], + [TracingProvider.aliyun, ['License Key', 'Endpoint', 'App Name']], + [TracingProvider.mlflow, ['app.tracing.configProvider.trackingUri', 'app.tracing.configProvider.experimentId', 'app.tracing.configProvider.username', 'app.tracing.configProvider.password']], + [TracingProvider.databricks, ['app.tracing.configProvider.experimentId', 'app.tracing.configProvider.databricksHost', 'app.tracing.configProvider.clientId', 'app.tracing.configProvider.clientSecret', 'app.tracing.configProvider.personalAccessToken']], + [TracingProvider.tencent, ['Token', 'Endpoint', 'Service Name']], +] as const + +const invalidConfigCases: Array<{ + provider: TracingProvider + payload: ProviderPayload + missingField: string +}> = [ + { provider: TracingProvider.arize, payload: { ...validConfigs[TracingProvider.arize], api_key: '' }, missingField: 'API Key' }, + { provider: TracingProvider.arize, payload: { ...validConfigs[TracingProvider.arize], space_id: '' }, missingField: 'Space ID' }, + { provider: TracingProvider.arize, payload: { ...validConfigs[TracingProvider.arize], project: '' }, missingField: 'app.tracing.configProvider.project' }, + { provider: TracingProvider.phoenix, payload: { ...validConfigs[TracingProvider.phoenix], api_key: '' }, missingField: 'API Key' }, + { provider: TracingProvider.phoenix, payload: { ...validConfigs[TracingProvider.phoenix], project: '' }, missingField: 'app.tracing.configProvider.project' }, + { provider: TracingProvider.langSmith, payload: { ...validConfigs[TracingProvider.langSmith], api_key: '' }, missingField: 'API Key' }, + { provider: TracingProvider.langSmith, payload: { ...validConfigs[TracingProvider.langSmith], project: '' }, missingField: 'app.tracing.configProvider.project' }, + { provider: TracingProvider.langfuse, payload: { ...validConfigs[TracingProvider.langfuse], secret_key: '' }, missingField: 'app.tracing.configProvider.secretKey' }, + { provider: TracingProvider.langfuse, payload: { ...validConfigs[TracingProvider.langfuse], public_key: '' }, missingField: 'app.tracing.configProvider.publicKey' }, + { provider: TracingProvider.langfuse, payload: { ...validConfigs[TracingProvider.langfuse], host: '' }, missingField: 'Host' }, + { provider: TracingProvider.weave, payload: { ...validConfigs[TracingProvider.weave], api_key: '' }, missingField: 'API Key' }, + { provider: TracingProvider.weave, payload: { ...validConfigs[TracingProvider.weave], project: '' }, missingField: 'app.tracing.configProvider.project' }, + { provider: TracingProvider.aliyun, payload: { ...validConfigs[TracingProvider.aliyun], app_name: '' }, missingField: 'App Name' }, + { provider: TracingProvider.aliyun, payload: { ...validConfigs[TracingProvider.aliyun], license_key: '' }, missingField: 'License Key' }, + { provider: TracingProvider.aliyun, payload: { ...validConfigs[TracingProvider.aliyun], endpoint: '' }, missingField: 'Endpoint' }, + { provider: TracingProvider.mlflow, payload: { ...validConfigs[TracingProvider.mlflow], tracking_uri: '' }, missingField: 'Tracking URI' }, + { provider: TracingProvider.databricks, payload: { ...validConfigs[TracingProvider.databricks], experiment_id: '' }, missingField: 'Experiment ID' }, + { provider: TracingProvider.databricks, payload: { ...validConfigs[TracingProvider.databricks], host: '' }, missingField: 'Host' }, + { provider: TracingProvider.tencent, payload: { ...validConfigs[TracingProvider.tencent], token: '' }, missingField: 'Token' }, + { provider: TracingProvider.tencent, payload: { ...validConfigs[TracingProvider.tencent], endpoint: '' }, missingField: 'Endpoint' }, + { provider: TracingProvider.tencent, payload: { ...validConfigs[TracingProvider.tencent], service_name: '' }, missingField: 'Service Name' }, +] + +const renderConfigButton = () => { + return render( + + + , + ) +} + +const renderProviderConfigModal = ({ + type = TracingProvider.langfuse, + payload, +}: { + type?: TracingProvider + payload?: ProviderPayload | null +} = {}) => { + const callbacks = { + onCancel: vi.fn(), + onSaved: vi.fn(), + onChosen: vi.fn(), + onRemoved: vi.fn(), + } + + render( + , + ) + + return callbacks +} + +describe('ProviderConfigModal', () => { + beforeEach(() => { + vi.clearAllMocks() + vi.mocked(addTracingConfig).mockResolvedValue({ result: 'success' }) + vi.mocked(updateTracingConfig).mockResolvedValue({ result: 'success' }) + vi.mocked(removeTracingConfig).mockResolvedValue({ result: 'success' }) + }) + + describe('Nested Overlay Behavior', () => { + it('should keep the provider config modal open when clicking inside it', async () => { + const user = userEvent.setup() + renderConfigButton() + + await user.click(screen.getByRole('button', { name: 'Open tracing' })) + await waitFor(() => { + expect(screen.getByText('app.tracing.tracing')).toBeInTheDocument() + }) + + const configActions = screen.getAllByText('app.tracing.config') + expect(configActions.length).toBeGreaterThan(0) + await user.click(configActions[0]!) + await waitFor(() => { + expect(screen.getByText('app.tracing.configProvider.titleapp.tracing.langfuse.title')).toBeInTheDocument() + }) + expect(screen.getByRole('dialog')).toBeInTheDocument() + + await user.click(screen.getByPlaceholderText('https://cloud.langfuse.com')) + + expect(screen.getByText('app.tracing.tracing')).toBeInTheDocument() + expect(screen.getByText('app.tracing.configProvider.titleapp.tracing.langfuse.title')).toBeInTheDocument() + }) + }) + + describe('Rendering', () => { + it.each(providerFieldLabels)('should render %s fields when adding a provider', (provider, expectedLabels) => { + renderProviderConfigModal({ type: provider }) + + expect(screen.getByText(`app.tracing.configProvider.titleapp.tracing.${provider}.title`)).toBeInTheDocument() + expectedLabels.forEach((label) => { + expect(screen.getByText(label)).toBeInTheDocument() + }) + expect(screen.getByRole('button', { name: 'common.operation.saveAndEnable' })).toBeInTheDocument() + }) + }) + + describe('Saving', () => { + it('should add and choose the provider when saving a new config', async () => { + const user = userEvent.setup() + const callbacks = renderProviderConfigModal({ type: TracingProvider.langfuse }) + const textboxes = screen.getAllByRole('textbox') + + await user.type(textboxes[0]!, 'secret-key') + await user.type(textboxes[1]!, 'public-key') + await user.type(textboxes[2]!, 'https://cloud.langfuse.com') + await user.click(screen.getByRole('button', { name: 'common.operation.saveAndEnable' })) + + await waitFor(() => { + expect(addTracingConfig).toHaveBeenCalledWith({ + appId: 'app-id', + body: { + tracing_provider: TracingProvider.langfuse, + tracing_config: validConfigs[TracingProvider.langfuse], + }, + }) + }) + expect(callbacks.onSaved).toHaveBeenCalledWith(validConfigs[TracingProvider.langfuse]) + expect(callbacks.onChosen).toHaveBeenCalledWith(TracingProvider.langfuse) + expect(toast).toHaveBeenCalledWith('common.api.success', { type: 'success' }) + }) + + it.each(Object.values(TracingProvider))('should update valid %s config in edit mode', async (provider) => { + const user = userEvent.setup() + const callbacks = renderProviderConfigModal({ + type: provider, + payload: validConfigs[provider], + }) + + await user.click(screen.getByRole('button', { name: 'common.operation.save' })) + + await waitFor(() => { + expect(updateTracingConfig).toHaveBeenCalledWith({ + appId: 'app-id', + body: { + tracing_provider: provider, + tracing_config: validConfigs[provider], + }, + }) + }) + expect(callbacks.onSaved).toHaveBeenCalledWith(validConfigs[provider]) + expect(callbacks.onChosen).not.toHaveBeenCalled() + }) + + it.each(invalidConfigCases)('should reject $provider config when $missingField is missing', async ({ provider, payload, missingField }) => { + const user = userEvent.setup() + renderProviderConfigModal({ + type: provider, + payload, + }) + + await user.click(screen.getByRole('button', { name: 'common.operation.save' })) + + expect(updateTracingConfig).not.toHaveBeenCalled() + expect(toast).toHaveBeenCalledWith( + expect.stringContaining(missingField), + { type: 'error' }, + ) + }) + }) + + describe('Closing And Removing', () => { + it('should cancel when the cancel button is clicked', async () => { + const user = userEvent.setup() + const callbacks = renderProviderConfigModal() + + await user.click(screen.getByRole('button', { name: 'common.operation.cancel' })) + + expect(callbacks.onCancel).toHaveBeenCalledTimes(1) + }) + + it('should cancel when the dialog is closed with Escape', async () => { + const user = userEvent.setup() + const callbacks = renderProviderConfigModal() + + await user.keyboard('{Escape}') + + await waitFor(() => { + expect(callbacks.onCancel).toHaveBeenCalledTimes(1) + }) + }) + + it('should remove an existing provider after confirmation', async () => { + const user = userEvent.setup() + const callbacks = renderProviderConfigModal({ + type: TracingProvider.langfuse, + payload: validConfigs[TracingProvider.langfuse], + }) + + await user.click(screen.getByRole('button', { name: 'common.operation.remove' })) + expect(screen.getByText('app.tracing.configProvider.removeConfirmTitle:{"key":"app.tracing.langfuse.title"}')).toBeInTheDocument() + + await user.click(screen.getByRole('button', { name: 'common.operation.confirm' })) + + await waitFor(() => { + expect(removeTracingConfig).toHaveBeenCalledWith({ + appId: 'app-id', + provider: TracingProvider.langfuse, + }) + }) + expect(callbacks.onRemoved).toHaveBeenCalledTimes(1) + expect(toast).toHaveBeenCalledWith('common.api.remove', { type: 'success' }) + }) + + it('should return to the edit dialog when remove confirmation is canceled', async () => { + const user = userEvent.setup() + renderProviderConfigModal({ + type: TracingProvider.langfuse, + payload: validConfigs[TracingProvider.langfuse], + }) + + await user.click(screen.getByRole('button', { name: 'common.operation.remove' })) + await user.click(screen.getByRole('button', { name: 'common.operation.cancel' })) + + expect(removeTracingConfig).not.toHaveBeenCalled() + expect(screen.getByText('app.tracing.configProvider.titleapp.tracing.langfuse.title')).toBeInTheDocument() + }) + }) +}) diff --git a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/provider-config-modal.tsx b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/provider-config-modal.tsx index 4f2497ad71..734b39bd41 100644 --- a/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/provider-config-modal.tsx +++ b/web/app/(commonLayout)/app/(appDetailLayout)/[appId]/overview/tracing/provider-config-modal.tsx @@ -11,6 +11,10 @@ import { AlertDialogTitle, } from '@langgenius/dify-ui/alert-dialog' import { Button } from '@langgenius/dify-ui/button' +import { + Dialog, + DialogContent, +} from '@langgenius/dify-ui/dialog' import { toast } from '@langgenius/dify-ui/toast' import { useBoolean } from 'ahooks' import * as React from 'react' @@ -19,10 +23,6 @@ import { useTranslation } from 'react-i18next' import Divider from '@/app/components/base/divider' import { LinkExternal02 } from '@/app/components/base/icons/src/vender/line/general' import { Lock01 } from '@/app/components/base/icons/src/vender/solid/security' -import { - PortalToFollowElem, - PortalToFollowElemContent, -} from '@/app/components/base/portal-to-follow-elem' import { addTracingConfig, removeTracingConfig, updateTracingConfig } from '@/service/apps' import { docURL } from './config' import Field from './field' @@ -153,7 +153,11 @@ const ProviderConfigModal: FC = ({ return weaveConfigTemplate })()) - const [isShowRemoveConfirm, { + const [isConfigDialogOpen, { + set: setIsConfigDialogOpen, + }] = useBoolean(true) + const [isRemoveDialogOpen, { + set: setIsRemoveDialogOpen, setTrue: showRemoveConfirm, setFalse: hideRemoveConfirm, }] = useBoolean(false) @@ -291,13 +295,24 @@ const ProviderConfigModal: FC = ({ } }, [appId, checkValid, config, isAdd, isEdit, isSaving, onChosen, onSaved, t, type]) + // Defer onCancel to onOpenChangeComplete so the dialog's exit animation + // (scale/opacity transition) can finish before the parent unmounts this modal. + const handleConfigDialogOpenChangeComplete = useCallback((open: boolean) => { + if (!open) + onCancel() + }, [onCancel]) + return ( <> - {!isShowRemoveConfirm + {!isRemoveDialogOpen ? ( - - -
+ + +
@@ -650,7 +665,7 @@ const ProviderConfigModal: FC = ({ )} @@ -683,11 +698,11 @@ const ProviderConfigModal: FC = ({
- - + +
) : ( - !open && hideRemoveConfirm()}> +
diff --git a/web/app/components/plugins/plugin-auth/authorize/__tests__/add-api-key-button.spec.tsx b/web/app/components/plugins/plugin-auth/authorize/__tests__/add-api-key-button.spec.tsx index 794f847168..7caef50516 100644 --- a/web/app/components/plugins/plugin-auth/authorize/__tests__/add-api-key-button.spec.tsx +++ b/web/app/components/plugins/plugin-auth/authorize/__tests__/add-api-key-button.spec.tsx @@ -5,11 +5,29 @@ import AddApiKeyButton from '../add-api-key-button' let _mockModalOpen = false vi.mock('../api-key-modal', () => ({ - default: ({ onClose, onUpdate }: { onClose: () => void, onUpdate?: () => void }) => { - _mockModalOpen = true + default: ({ + open = true, + onClose, + onOpenChange, + onUpdate, + }: { + open?: boolean + onClose: () => void + onOpenChange?: (open: boolean) => void + onUpdate?: () => void + }) => { + _mockModalOpen = open + if (!open) + return null + + const handleClose = () => { + onOpenChange?.(false) + onClose() + } + return (
- +
) diff --git a/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx b/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx index 2bfa94d2ed..41f1aa3718 100644 --- a/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx +++ b/web/app/components/plugins/plugin-auth/authorize/__tests__/api-key-modal.spec.tsx @@ -1,5 +1,8 @@ import type { ApiKeyModalProps } from '../api-key-modal' +import type { FormSchema } from '@/app/components/base/form/types' +import { Popover, PopoverContent, PopoverTrigger } from '@langgenius/dify-ui/popover' import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import * as React from 'react' import { beforeEach, describe, expect, it, vi } from 'vitest' import { AuthCategory } from '../../types' @@ -20,17 +23,27 @@ vi.mock('@langgenius/dify-ui/toast', () => ({ })) const mockAddPluginCredential = vi.fn().mockResolvedValue({}) const mockUpdatePluginCredential = vi.fn().mockResolvedValue({}) -const mockFormValues = { isCheckValidated: true, values: { __name__: 'My Key', api_key: 'sk-123' } } +const defaultCredentialSchemas = [ + { name: 'api_key', label: 'API Key', type: 'secret-input', required: true }, +] +type MockFormValues = { + isCheckValidated: boolean + values: Record +} + +const defaultFormValues: MockFormValues = { isCheckValidated: true, values: { __name__: 'My Key', api_key: 'sk-123' } } +let mockCredentialSchemas = defaultCredentialSchemas +let mockIsSchemaLoading = false +let mockFormValues = defaultFormValues +const mockAuthFormProps = vi.fn() vi.mock('../../hooks/use-credential', () => ({ useAddPluginCredentialHook: () => ({ mutateAsync: mockAddPluginCredential, }), useGetPluginCredentialSchemaHook: () => ({ - data: [ - { name: 'api_key', label: 'API Key', type: 'secret-input', required: true }, - ], - isLoading: false, + data: mockCredentialSchemas, + isLoading: mockIsSchemaLoading, }), useUpdatePluginCredentialHook: () => ({ mutateAsync: mockUpdatePluginCredential, @@ -49,36 +62,19 @@ vi.mock('@/app/components/base/encrypted-bottom', () => ({ EncryptedBottom: () =>
, })) -vi.mock('@/app/components/base/modal/modal', () => ({ - default: ({ children, title, onClose, onConfirm, onExtraButtonClick, showExtraButton, disabled }: { - children: React.ReactNode - title: string - onClose?: () => void - onCancel?: () => void - onConfirm?: () => void - onExtraButtonClick?: () => void - showExtraButton?: boolean - disabled?: boolean - [key: string]: unknown - }) => ( -
-
{title}
- {children} - - - {showExtraButton && } -
- ), -})) - -vi.mock('@/app/components/base/form/form-scenarios/auth', () => ({ - default: React.forwardRef((_props: Record, ref: React.Ref) => { +vi.mock('@/app/components/base/form/form-scenarios/auth', () => { + const MockAuthForm = ({ ref, ...props }: { ref?: React.Ref } & Record) => { + mockAuthFormProps(props) React.useImperativeHandle(ref, () => ({ getFormValues: () => mockFormValues, })) return
- }), -})) + } + + return { + default: MockAuthForm, + } +}) vi.mock('@/app/components/base/form/types', () => ({ FormTypeEnum: { textInput: 'text-input' }, @@ -89,11 +85,73 @@ const basePayload = { provider: 'test-provider', } +const PopoverModalHarness = ({ + ApiKeyModal, + onClose, + onPopoverClose, +}: { + ApiKeyModal: React.FC + onClose: () => void + onPopoverClose: () => void +}) => { + const [open, setOpen] = React.useState(true) + + return ( + { + setOpen(nextOpen) + if (!nextOpen) + onPopoverClose() + }} + > + Credentials} /> + +
+ +
+ + + ) +} + +const ControlledModalHarness = ({ + ApiKeyModal, + onClose, +}: { + ApiKeyModal: React.FC + onClose: () => void +}) => { + const [open, setOpen] = React.useState(true) + + return ( + <> +
{String(open)}
+ + + ) +} + describe('ApiKeyModal', () => { let ApiKeyModal: React.FC beforeEach(async () => { vi.clearAllMocks() + mockCredentialSchemas = defaultCredentialSchemas + mockIsSchemaLoading = false + mockFormValues = defaultFormValues + mockAddPluginCredential.mockResolvedValue({}) + mockUpdatePluginCredential.mockResolvedValue({}) const mod = await import('../api-key-modal') ApiKeyModal = mod.default }) @@ -110,6 +168,56 @@ describe('ApiKeyModal', () => { expect(screen.getByTestId('auth-form')).toBeInTheDocument() }) + it('should prefer formSchemas prop and apply schema defaults', () => { + const customSchemas: FormSchema[] = [ + { + name: 'custom_api_key', + label: 'Custom API Key', + type: 'secret-input' as FormSchema['type'], + required: true, + default: 'default-key', + }, + ] + + render() + + expect(mockAuthFormProps).toHaveBeenCalledWith(expect.objectContaining({ + formSchemas: expect.arrayContaining([ + expect.objectContaining({ name: 'custom_api_key' }), + ]), + defaultValues: expect.objectContaining({ + custom_api_key: 'default-key', + }), + })) + }) + + it('should not render auth form when credential schema is empty', () => { + mockCredentialSchemas = [] + + render() + + expect(screen.queryByTestId('auth-form')).not.toBeInTheDocument() + }) + + it('should not submit when form ref is unavailable', () => { + mockCredentialSchemas = [] + + render() + + fireEvent.click(screen.getByTestId('modal-confirm')) + + expect(mockAddPluginCredential).not.toHaveBeenCalled() + }) + + it('should disable actions while loading credential schema', () => { + mockIsSchemaLoading = true + + render() + + expect(screen.queryByTestId('auth-form')).not.toBeInTheDocument() + expect(screen.getByTestId('modal-confirm')).toBeDisabled() + }) + it('should show remove button when editValues is provided', () => { render() @@ -130,6 +238,18 @@ describe('ApiKeyModal', () => { expect(mockOnClose).toHaveBeenCalled() }) + it('should close through controlled open state when cancel is clicked', async () => { + const mockOnClose = vi.fn() + render() + + fireEvent.click(screen.getByRole('button', { name: 'common.operation.cancel' })) + + await waitFor(() => { + expect(screen.getByTestId('modal-open-state')).toHaveTextContent('false') + }) + expect(mockOnClose).toHaveBeenCalled() + }) + it('should call addPluginCredential on confirm in add mode', async () => { const mockOnClose = vi.fn() const mockOnUpdate = vi.fn() @@ -145,6 +265,50 @@ describe('ApiKeyModal', () => { }) }) + it('should use empty credential name when authorization name is blank in add mode', async () => { + mockFormValues = { isCheckValidated: true, values: { api_key: 'sk-123' } } + + render() + + fireEvent.click(screen.getByTestId('modal-confirm')) + + await waitFor(() => { + expect(mockAddPluginCredential).toHaveBeenCalledWith(expect.objectContaining({ + name: '', + })) + }) + }) + + it('should not submit when form validation fails', () => { + mockFormValues = { isCheckValidated: false, values: {} } + + render() + + fireEvent.click(screen.getByTestId('modal-confirm')) + + expect(mockAddPluginCredential).not.toHaveBeenCalled() + expect(mockUpdatePluginCredential).not.toHaveBeenCalled() + }) + + it('should ignore repeated confirm while an action is in progress', async () => { + let repeatedClickTriggered = false + mockAddPluginCredential.mockImplementationOnce(async () => { + if (!repeatedClickTriggered) { + repeatedClickTriggered = true + fireEvent.click(screen.getByTestId('modal-confirm')) + } + return {} + }) + + render() + + fireEvent.click(screen.getByTestId('modal-confirm')) + + await waitFor(() => { + expect(mockAddPluginCredential).toHaveBeenCalledTimes(1) + }) + }) + it('should call updatePluginCredential on confirm in edit mode', async () => { render() @@ -155,6 +319,20 @@ describe('ApiKeyModal', () => { }) }) + it('should use empty credential name when authorization name is blank in edit mode', async () => { + mockFormValues = { isCheckValidated: true, values: { api_key: 'updated', __credential_id__: 'cred-1' } } + + render() + + fireEvent.click(screen.getByTestId('modal-confirm')) + + await waitFor(() => { + expect(mockUpdatePluginCredential).toHaveBeenCalledWith(expect.objectContaining({ + name: '', + })) + }) + }) + it('should call onRemove when remove button clicked', () => { const mockOnRemove = vi.fn() render() @@ -163,6 +341,49 @@ describe('ApiKeyModal', () => { expect(mockOnRemove).toHaveBeenCalled() }) + it('should stay open when clicking inside the modal from a popover', async () => { + // Use userEvent instead of fireEvent to avoid CI flakiness: userEvent + // awaits React act() between pointer/mouse/click so base-ui's dialog + // popup ref is guaranteed committed before outside-click detection runs. + const user = userEvent.setup() + const mockOnClose = vi.fn() + const mockOnPopoverClose = vi.fn() + + render( + , + ) + + const form = await screen.findByTestId('auth-form') + + await user.click(form) + + expect(mockOnClose).not.toHaveBeenCalled() + expect(mockOnPopoverClose).not.toHaveBeenCalled() + expect(screen.getByTestId('modal')).toBeInTheDocument() + }) + + it('should close on backdrop click through controlled open state', async () => { + const mockOnClose = vi.fn() + render() + + const backdrop = document.querySelector('.bg-background-overlay') + if (!backdrop) + throw new Error('Expected dialog backdrop to render') + + fireEvent.pointerDown(backdrop) + fireEvent.mouseDown(backdrop) + fireEvent.click(backdrop) + + await waitFor(() => { + expect(screen.getByTestId('modal-open-state')).toHaveTextContent('false') + }) + expect(mockOnClose).toHaveBeenCalled() + }) + it('should render readme entrance when detail is provided', () => { const payload = { ...basePayload, detail: { name: 'Test' } as never } render() diff --git a/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx b/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx index 648a87dabc..38f3f85643 100644 --- a/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx +++ b/web/app/components/plugins/plugin-auth/authorize/add-api-key-button.tsx @@ -25,20 +25,26 @@ const AddApiKeyButton = ({ formSchemas = [], }: AddApiKeyButtonProps) => { const [isApiKeyModalOpen, setIsApiKeyModalOpen] = useState(false) + const [isApiKeyModalMounted, setIsApiKeyModalMounted] = useState(false) return ( <> { - isApiKeyModalOpen && ( + isApiKeyModalMounted && ( setIsApiKeyModalOpen(false)} onUpdate={onUpdate} diff --git a/web/app/components/plugins/plugin-auth/authorize/api-key-modal.tsx b/web/app/components/plugins/plugin-auth/authorize/api-key-modal.tsx index db513ecb6f..290621141c 100644 --- a/web/app/components/plugins/plugin-auth/authorize/api-key-modal.tsx +++ b/web/app/components/plugins/plugin-auth/authorize/api-key-modal.tsx @@ -3,6 +3,8 @@ import type { FormRefObject, FormSchema, } from '@/app/components/base/form/types' +import { Button } from '@langgenius/dify-ui/button' +import { Dialog, DialogCloseButton, DialogContent, DialogTitle } from '@langgenius/dify-ui/dialog' import { toast } from '@langgenius/dify-ui/toast' import { memo, @@ -16,7 +18,6 @@ import { EncryptedBottom } from '@/app/components/base/encrypted-bottom' import AuthForm from '@/app/components/base/form/form-scenarios/auth' import { FormTypeEnum } from '@/app/components/base/form/types' import Loading from '@/app/components/base/loading' -import Modal from '@/app/components/base/modal/modal' import { ReadmeEntrance } from '../../readme-panel/entrance' import { ReadmeShowType } from '../../readme-panel/store' import { @@ -28,8 +29,10 @@ import { CredentialTypeEnum } from '../types' export type ApiKeyModalProps = { pluginPayload: PluginPayload + open?: boolean + onOpenChange?: (open: boolean) => void onClose?: () => void - editValues?: Record + editValues?: Record onRemove?: () => void disabled?: boolean onUpdate?: () => void @@ -37,6 +40,8 @@ export type ApiKeyModalProps = { } const ApiKeyModal = ({ pluginPayload, + open = true, + onOpenChange, onClose, editValues, onRemove, @@ -73,7 +78,7 @@ const ApiKeyModal = ({ if (schema.default) acc[schema.name] = schema.default return acc - }, {} as Record) + }, {} as Record) const { mutateAsync: addPluginCredential } = useAddPluginCredentialHook(pluginPayload) const { mutateAsync: updatePluginCredential } = useUpdatePluginCredentialHook(pluginPayload) const formRef = useRef(null) @@ -114,53 +119,102 @@ const ApiKeyModal = ({ } toast.success(t('api.actionSuccess', { ns: 'common' })) + onOpenChange?.(false) onClose?.() onUpdate?.() } finally { handleSetDoingAction(false) } - }, [addPluginCredential, onClose, onUpdate, updatePluginCredential, t, editValues, handleSetDoingAction]) + }, [addPluginCredential, onClose, onOpenChange, onUpdate, updatePluginCredential, t, editValues, handleSetDoingAction]) + + const isDisabled = disabled || isLoading || doingAction + const handleOpenChange = useCallback((nextOpen: boolean) => { + onOpenChange?.(nextOpen) + if (!nextOpen) + onClose?.() + }, [onClose, onOpenChange]) return ( -
) - } - bottomSlot={} - onConfirm={handleConfirm} - showExtraButton={!!editValues} - onExtraButtonClick={onRemove} - disabled={disabled || isLoading || doingAction} - clickOutsideNotClose={true} - wrapperClassName="z-1002!" + - {pluginPayload.detail && ( - - )} - { - isLoading && ( -
- + +
+
+ + {t('auth.useApiAuth', { ns: 'plugin' })} + +
+ {t('auth.useApiAuthDesc', { ns: 'plugin' })} +
+
- ) - } - { - !isLoading && !!mergedData.length && ( - - ) - } - +
+ {pluginPayload.detail && ( + + )} + { + isLoading && ( +
+ +
+ ) + } + { + !isLoading && !!mergedData.length && ( + + ) + } +
+
+
+
+ {editValues && ( + <> + +
+ + )} + + +
+
+
+ +
+
+ +
) } diff --git a/web/app/components/plugins/plugin-auth/authorized/index.tsx b/web/app/components/plugins/plugin-auth/authorized/index.tsx index b8b34e33e0..774821b0c8 100644 --- a/web/app/components/plugins/plugin-auth/authorized/index.tsx +++ b/web/app/components/plugins/plugin-auth/authorized/index.tsx @@ -19,9 +19,6 @@ import { PopoverTrigger, } from '@langgenius/dify-ui/popover' import { toast } from '@langgenius/dify-ui/toast' -import { - RiArrowDownSLine, -} from '@remixicon/react' import { memo, useCallback, @@ -93,19 +90,19 @@ const Authorized = ({ }, [onOpenChange]) const oAuthCredentials = credentials.filter(credential => credential.credential_type === CredentialTypeEnum.OAUTH2) const apiKeyCredentials = credentials.filter(credential => credential.credential_type === CredentialTypeEnum.API_KEY) - const pendingOperationCredentialId = useRef(null) + const pendingOperationCredentialIdRef = useRef(null) const [deleteCredentialId, setDeleteCredentialId] = useState(null) const { mutateAsync: deletePluginCredential } = useDeletePluginCredentialHook(pluginPayload) const openConfirm = useCallback((credentialId?: string) => { setMergedIsOpen(false) if (credentialId) - pendingOperationCredentialId.current = credentialId + pendingOperationCredentialIdRef.current = credentialId - setDeleteCredentialId(pendingOperationCredentialId.current) + setDeleteCredentialId(pendingOperationCredentialIdRef.current) }, [setMergedIsOpen]) const closeConfirm = useCallback(() => { setDeleteCredentialId(null) - pendingOperationCredentialId.current = null + pendingOperationCredentialIdRef.current = null }, []) const [doingAction, setDoingAction] = useState(false) const doingActionRef = useRef(doingAction) @@ -116,30 +113,37 @@ const Authorized = ({ const handleConfirm = useCallback(async () => { if (doingActionRef.current) return - if (!pendingOperationCredentialId.current) { + if (!pendingOperationCredentialIdRef.current) { setDeleteCredentialId(null) return } try { handleSetDoingAction(true) - await deletePluginCredential({ credential_id: pendingOperationCredentialId.current }) + await deletePluginCredential({ credential_id: pendingOperationCredentialIdRef.current }) toast.success(t('api.actionSuccess', { ns: 'common' })) onUpdate?.() setDeleteCredentialId(null) - pendingOperationCredentialId.current = null + pendingOperationCredentialIdRef.current = null } finally { handleSetDoingAction(false) } }, [deletePluginCredential, onUpdate, t, handleSetDoingAction]) const [editValues, setEditValues] = useState | null>(null) + const [isApiKeyModalOpen, setIsApiKeyModalOpen] = useState(false) const handleEdit = useCallback((id: string, values: Record) => { setMergedIsOpen(false) - pendingOperationCredentialId.current = id + pendingOperationCredentialIdRef.current = id setEditValues(values) + setIsApiKeyModalOpen(true) }, [setMergedIsOpen]) + const handleApiKeyModalOpenChange = useCallback((open: boolean) => { + setIsApiKeyModalOpen(open) + if (!open) + pendingOperationCredentialIdRef.current = null + }, []) const handleRemove = useCallback(() => { - setDeleteCredentialId(pendingOperationCredentialId.current) + setDeleteCredentialId(pendingOperationCredentialIdRef.current) }, []) const { mutateAsync: setPluginDefaultCredential } = useSetPluginDefaultCredentialHook(pluginPayload) const handleSetDefault = useCallback(async (id: string) => { @@ -213,7 +217,7 @@ const Authorized = ({ ` (${unavailableCredentials.length} ${t('auth.unavailable', { ns: 'plugin' })})` ) } - + ) } @@ -356,12 +360,11 @@ const Authorized = ({ { !!editValues && ( { - setEditValues(null) - pendingOperationCredentialId.current = null - }} + onClose={() => handleApiKeyModalOpenChange(false)} onRemove={handleRemove} disabled={disabled || doingAction} onUpdate={onUpdate}