diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c1835f888f..07504d2754 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -2417,7 +2417,7 @@ }, "web/app/components/plugins/install-plugin/install-from-github/index.tsx": { "ts/no-explicit-any": { - "count": 3 + "count": 2 } }, "web/app/components/plugins/install-plugin/install-from-github/steps/selectPackage.tsx": { diff --git a/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx b/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx index e923de5e38..105e1212b0 100644 --- a/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx +++ b/web/app/components/plugins/install-plugin/install-from-github/__tests__/index.spec.tsx @@ -101,25 +101,6 @@ vi.mock('../../hooks/use-hide-logic', () => ({ })) // Mock child components -vi.mock('../steps/setURL', () => ({ - default: ({ repoUrl, onChange, onNext, onCancel }: { - repoUrl: string - onChange: (value: string) => void - onNext: () => void - onCancel: () => void - }) => ( -
- onChange(e.target.value)} - /> - - -
- ), -})) - vi.mock('../steps/selectPackage', () => ({ default: ({ repoUrl, @@ -236,6 +217,10 @@ vi.mock('../../base/installed', () => ({ ), })) +const getRepoUrlInput = () => screen.getByLabelText('plugin.installFromGitHub.gitHubRepo') +const getNextButton = () => screen.getByRole('button', { name: 'plugin.installModal.next' }) +const getCancelButton = () => screen.getByRole('button', { name: 'plugin.installModal.cancel' }) + describe('InstallFromGitHub', () => { const defaultProps = { onClose: vi.fn(), @@ -261,8 +246,8 @@ describe('InstallFromGitHub', () => { it('should render modal with correct initial state for new installation', () => { render() - expect(screen.getByTestId('set-url-step')).toBeInTheDocument() - expect(screen.getByTestId('repo-url-input')).toHaveValue('') + expect(getRepoUrlInput()).toBeInTheDocument() + expect(getRepoUrlInput()).toHaveValue('') }) it('should render modal with selectPackage step when updatePayload is provided', () => { @@ -312,7 +297,7 @@ describe('InstallFromGitHub', () => { it('should update repoUrl when user types in input', () => { render() - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/test/repo' } }) expect(input).toHaveValue('https://github.com/test/repo') @@ -321,10 +306,10 @@ describe('InstallFromGitHub', () => { it('should transition from setUrl to selectPackage on successful URL submit', async () => { render() - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - const nextBtn = screen.getByTestId('next-btn') + const nextBtn = getNextButton() fireEvent.click(nextBtn) await waitFor(() => { @@ -443,10 +428,10 @@ describe('InstallFromGitHub', () => { it('should show error toast for invalid GitHub URL', async () => { render() - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'invalid-url' } }) - const nextBtn = screen.getByTestId('next-btn') + const nextBtn = getNextButton() fireEvent.click(nextBtn) await waitFor(() => { @@ -462,10 +447,10 @@ describe('InstallFromGitHub', () => { render() - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - const nextBtn = screen.getByTestId('next-btn') + const nextBtn = getNextButton() fireEvent.click(nextBtn) await waitFor(() => { @@ -481,10 +466,10 @@ describe('InstallFromGitHub', () => { render() - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - const nextBtn = screen.getByTestId('next-btn') + const nextBtn = getNextButton() fireEvent.click(nextBtn) await waitFor(() => { @@ -504,9 +489,9 @@ describe('InstallFromGitHub', () => { render() // Navigate to selectPackage - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - fireEvent.click(screen.getByTestId('next-btn')) + fireEvent.click(getNextButton()) await waitFor(() => { expect(screen.getByTestId('select-package-step')).toBeInTheDocument() @@ -516,7 +501,7 @@ describe('InstallFromGitHub', () => { fireEvent.click(screen.getByTestId('back-btn')) await waitFor(() => { - expect(screen.getByTestId('set-url-step')).toBeInTheDocument() + expect(getRepoUrlInput()).toBeInTheDocument() }) }) @@ -546,7 +531,7 @@ describe('InstallFromGitHub', () => { it('should call onClose when cancel button is clicked', () => { render() - fireEvent.click(screen.getByTestId('cancel-btn')) + fireEvent.click(getCancelButton()) expect(defaultProps.onClose).toHaveBeenCalledTimes(1) }) @@ -783,10 +768,10 @@ describe('InstallFromGitHub', () => { it('should handle URL without trailing slash', async () => { render() - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - fireEvent.click(screen.getByTestId('next-btn')) + fireEvent.click(getNextButton()) await waitFor(() => { expect(mockFetchReleases).toHaveBeenCalledWith('owner', 'repo') @@ -797,11 +782,11 @@ describe('InstallFromGitHub', () => { render() // Set URL - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/test/myrepo' } }) // Navigate to selectPackage - fireEvent.click(screen.getByTestId('next-btn')) + fireEvent.click(getNextButton()) await waitFor(() => { expect(screen.getByTestId('select-package-step')).toBeInTheDocument() @@ -980,12 +965,12 @@ describe('InstallFromGitHub', () => { render() // Start from setUrl step - expect(screen.getByTestId('set-url-step')).toBeInTheDocument() + expect(getRepoUrlInput()).toBeInTheDocument() // Enter URL - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - fireEvent.click(screen.getByTestId('next-btn')) + fireEvent.click(getNextButton()) await waitFor(() => { expect(screen.getByTestId('select-package-step')).toBeInTheDocument() @@ -1086,7 +1071,7 @@ describe('InstallFromGitHub', () => { render() // Verify we're on setUrl step - expect(screen.getByTestId('set-url-step')).toBeInTheDocument() + expect(getRepoUrlInput()).toBeInTheDocument() // The setUrl step doesn't expose onBack in the real component, // but our mock doesn't have it either - this is correct behavior @@ -1097,9 +1082,9 @@ describe('InstallFromGitHub', () => { render() // Navigate to selectPackage - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - fireEvent.click(screen.getByTestId('next-btn')) + fireEvent.click(getNextButton()) await waitFor(() => { expect(screen.getByTestId('select-package-step')).toBeInTheDocument() @@ -1123,11 +1108,11 @@ describe('InstallFromGitHub', () => { fireEvent.click(screen.getByTestId('back-btn')) await waitFor(() => { - expect(screen.getByTestId('set-url-step')).toBeInTheDocument() + expect(getRepoUrlInput()).toBeInTheDocument() }) // Verify URL is preserved after back navigation - expect(screen.getByTestId('repo-url-input')).toHaveValue('https://github.com/owner/repo') + expect(getRepoUrlInput()).toHaveValue('https://github.com/owner/repo') }) }) }) @@ -1368,114 +1353,6 @@ describe('Install Plugin Utils', () => { // Steps Components Tests // ================================ -// SetURL Component Tests -describe('SetURL Component', () => { - // Import the real component for testing - const SetURL = vi.fn() - - beforeEach(() => { - vi.clearAllMocks() - // Re-mock the SetURL component with a more testable version - vi.doMock('./steps/setURL', () => ({ - default: SetURL, - })) - }) - - describe('Rendering', () => { - it('should render label with correct text', () => { - render() - - // The mocked component should be rendered - expect(screen.getByTestId('set-url-step')).toBeInTheDocument() - }) - - it('should render input field with placeholder', () => { - render() - - const input = screen.getByTestId('repo-url-input') - expect(input).toBeInTheDocument() - }) - - it('should render cancel and next buttons', () => { - render() - - expect(screen.getByTestId('cancel-btn')).toBeInTheDocument() - expect(screen.getByTestId('next-btn')).toBeInTheDocument() - }) - }) - - describe('Props', () => { - it('should display repoUrl value in input', () => { - render() - - const input = screen.getByTestId('repo-url-input') - fireEvent.change(input, { target: { value: 'https://github.com/test/repo' } }) - - expect(input).toHaveValue('https://github.com/test/repo') - }) - - it('should call onChange when input value changes', () => { - render() - - const input = screen.getByTestId('repo-url-input') - fireEvent.change(input, { target: { value: 'new-value' } }) - - expect(input).toHaveValue('new-value') - }) - }) - - describe('User Interactions', () => { - it('should call onNext when next button is clicked', async () => { - mockFetchReleases.mockResolvedValue(createMockReleases()) - - render() - - const input = screen.getByTestId('repo-url-input') - fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - - fireEvent.click(screen.getByTestId('next-btn')) - - await waitFor(() => { - expect(mockFetchReleases).toHaveBeenCalled() - }) - }) - - it('should call onCancel when cancel button is clicked', () => { - const onClose = vi.fn() - render() - - fireEvent.click(screen.getByTestId('cancel-btn')) - - expect(onClose).toHaveBeenCalledTimes(1) - }) - }) - - describe('Edge Cases', () => { - it('should handle empty URL input', () => { - render() - - const input = screen.getByTestId('repo-url-input') - expect(input).toHaveValue('') - }) - - it('should handle URL with whitespace only', () => { - render() - - const input = screen.getByTestId('repo-url-input') - fireEvent.change(input, { target: { value: ' ' } }) - - // With whitespace only, next should still be submittable but validation will fail - fireEvent.click(screen.getByTestId('next-btn')) - - // Should show error for invalid URL - expect(mockNotify).toHaveBeenCalledWith({ - type: 'error', - message: 'plugin.error.inValidGitHubUrl', - }) - }) - }) -}) - // SelectPackage Component Tests describe('SelectPackage Component', () => { beforeEach(() => { @@ -1513,9 +1390,9 @@ describe('SelectPackage Component', () => { render() // Navigate to selectPackage step - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - fireEvent.click(screen.getByTestId('next-btn')) + fireEvent.click(getNextButton()) await waitFor(() => { expect(screen.getByTestId('back-btn')).toBeInTheDocument() @@ -1590,9 +1467,9 @@ describe('SelectPackage Component', () => { render() // Navigate to selectPackage - const input = screen.getByTestId('repo-url-input') + const input = getRepoUrlInput() fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - fireEvent.click(screen.getByTestId('next-btn')) + fireEvent.click(getNextButton()) await waitFor(() => { expect(screen.getByTestId('select-package-step')).toBeInTheDocument() @@ -1601,7 +1478,7 @@ describe('SelectPackage Component', () => { fireEvent.click(screen.getByTestId('back-btn')) await waitFor(() => { - expect(screen.getByTestId('set-url-step')).toBeInTheDocument() + expect(getRepoUrlInput()).toBeInTheDocument() }) }) diff --git a/web/app/components/plugins/install-plugin/install-from-github/index.tsx b/web/app/components/plugins/install-plugin/install-from-github/index.tsx index b0667a9ee0..71b0a431d3 100644 --- a/web/app/components/plugins/install-plugin/install-from-github/index.tsx +++ b/web/app/components/plugins/install-plugin/install-from-github/index.tsx @@ -2,6 +2,7 @@ import type { PluginDeclaration, UpdateFromGitHubPayload } from '../../types' import type { InstallState } from '@/app/components/plugins/types' +import { Button } from '@langgenius/dify-ui/button' import { cn } from '@langgenius/dify-ui/cn' import { Dialog, DialogCloseButton, DialogContent } from '@langgenius/dify-ui/dialog' import { toast } from '@langgenius/dify-ui/toast' @@ -17,7 +18,6 @@ import useRefreshPluginList from '../hooks/use-refresh-plugin-list' import { convertRepoToUrl, parseGitHubUrl } from '../utils' import Loaded from './steps/loaded' import SelectPackage from './steps/selectPackage' -import SetURL from './steps/setURL' const i18nPrefix = 'installFromGitHub' @@ -194,12 +194,41 @@ const InstallFromGitHub: React.FC = ({ updatePayload, on : (
{state.step === InstallStepFromGitHub.setUrl && ( - setState(prevState => ({ ...prevState, repoUrl: value }))} - onNext={handleUrlSubmit} - onCancel={onClose} - /> + <> + + setState(prevState => ({ ...prevState, repoUrl: e.target.value }))} + className="flex grow items-center gap-0.5 self-stretch overflow-hidden rounded-lg border border-components-input-border-active bg-components-input-bg-active p-2 system-sm-regular text-ellipsis text-components-input-text-filled outline-hidden" + placeholder="Please enter GitHub repo URL" + /> +
+ + +
+ )} {state.step === InstallStepFromGitHub.selectPackage && ( = ({ updatePayload, on onBack={handleBack} /> )} - {state.step === InstallStepFromGitHub.readyToInstall && ( + {state.step === InstallStepFromGitHub.readyToInstall && manifest && uniqueIdentifier && ( { - const defaultProps = { - repoUrl: '', - onChange: vi.fn(), - onNext: vi.fn(), - onCancel: vi.fn(), - } - - beforeEach(() => { - vi.clearAllMocks() - }) - - // ================================ - // Rendering Tests - // ================================ - describe('Rendering', () => { - it('should render label with GitHub repo text', () => { - render() - - expect(screen.getByText('plugin.installFromGitHub.gitHubRepo')).toBeInTheDocument() - }) - - it('should render input field with correct attributes', () => { - render() - - const input = screen.getByRole('textbox') - expect(input).toBeInTheDocument() - expect(input).toHaveAttribute('type', 'url') - expect(input).toHaveAttribute('id', 'repoUrl') - expect(input).toHaveAttribute('name', 'repoUrl') - expect(input).toHaveAttribute('placeholder', 'Please enter GitHub repo URL') - }) - - it('should render cancel button', () => { - render() - - expect(screen.getByRole('button', { name: 'plugin.installModal.cancel' })).toBeInTheDocument() - }) - - it('should render next button', () => { - render() - - expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).toBeInTheDocument() - }) - - it('should associate label with input field', () => { - render() - - const input = screen.getByLabelText('plugin.installFromGitHub.gitHubRepo') - expect(input).toBeInTheDocument() - }) - - it('should auto-focus the input on mount', async () => { - render() - - const input = screen.getByRole('textbox') - await waitFor(() => { - expect(input).toHaveFocus() - }) - }) - }) - - // ================================ - // Props Tests - // ================================ - describe('Props', () => { - it('should display repoUrl value in input', () => { - render() - - expect(screen.getByRole('textbox')).toHaveValue('https://github.com/test/repo') - }) - - it('should display empty string when repoUrl is empty', () => { - render() - - expect(screen.getByRole('textbox')).toHaveValue('') - }) - }) - - // ================================ - // User Interactions Tests - // ================================ - describe('User Interactions', () => { - it('should call onChange when input value changes', () => { - const onChange = vi.fn() - render() - - const input = screen.getByRole('textbox') - fireEvent.change(input, { target: { value: 'https://github.com/owner/repo' } }) - - expect(onChange).toHaveBeenCalledTimes(1) - expect(onChange).toHaveBeenCalledWith('https://github.com/owner/repo') - }) - - it('should call onCancel when cancel button is clicked', () => { - const onCancel = vi.fn() - render() - - fireEvent.click(screen.getByRole('button', { name: 'plugin.installModal.cancel' })) - - expect(onCancel).toHaveBeenCalledTimes(1) - }) - - it('should call onNext when next button is clicked', () => { - const onNext = vi.fn() - render() - - fireEvent.click(screen.getByRole('button', { name: 'plugin.installModal.next' })) - - expect(onNext).toHaveBeenCalledTimes(1) - }) - }) - - // ================================ - // Button State Tests - // ================================ - describe('Button State', () => { - it('should disable next button when repoUrl is empty', () => { - render() - - expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).toBeDisabled() - }) - - it('should disable next button when repoUrl is only whitespace', () => { - render() - - expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).toBeDisabled() - }) - - it('should enable next button when repoUrl has content', () => { - render() - - expect(screen.getByRole('button', { name: 'plugin.installModal.next' })).not.toBeDisabled() - }) - - it('should not disable cancel button regardless of repoUrl', () => { - render() - - expect(screen.getByRole('button', { name: 'plugin.installModal.cancel' })).not.toBeDisabled() - }) - }) - - // ================================ - // Edge Cases Tests - // ================================ - describe('Edge Cases', () => { - it('should handle URL with special characters', () => { - const onChange = vi.fn() - render() - - const input = screen.getByRole('textbox') - fireEvent.change(input, { target: { value: 'https://github.com/test-org/repo_name-123' } }) - - expect(onChange).toHaveBeenCalledWith('https://github.com/test-org/repo_name-123') - }) - - it('should handle very long URLs', () => { - const longUrl = `https://github.com/${'a'.repeat(100)}/${'b'.repeat(100)}` - render() - - expect(screen.getByRole('textbox')).toHaveValue(longUrl) - }) - - it('should handle onChange with empty string', () => { - const onChange = vi.fn() - render() - - const input = screen.getByRole('textbox') - fireEvent.change(input, { target: { value: '' } }) - - expect(onChange).toHaveBeenCalledWith('') - }) - - it('should preserve callback references on rerender', () => { - const onNext = vi.fn() - const { rerender } = render() - - rerender() - - fireEvent.click(screen.getByRole('button', { name: 'plugin.installModal.next' })) - - expect(onNext).toHaveBeenCalledTimes(1) - }) - }) -}) diff --git a/web/app/components/plugins/install-plugin/install-from-github/steps/setURL.tsx b/web/app/components/plugins/install-plugin/install-from-github/steps/setURL.tsx deleted file mode 100644 index 45ce5980ac..0000000000 --- a/web/app/components/plugins/install-plugin/install-from-github/steps/setURL.tsx +++ /dev/null @@ -1,69 +0,0 @@ -'use client' - -import { Button } from '@langgenius/dify-ui/button' -import * as React from 'react' -import { useTranslation } from 'react-i18next' - -type SetURLProps = { - repoUrl: string - onChange: (value: string) => void - onNext: () => void - onCancel: () => void -} - -const SetURL: React.FC = ({ repoUrl, onChange, onNext, onCancel }) => { - const { t } = useTranslation() - const inputRef = React.useRef(null) - - // Focus the input after the dropdown's focus-return animation settles. - // Using rAF avoids racing the DropdownMenu FloatingFocusManager that returns - // focus to the trigger on close. - React.useEffect(() => { - const frame = requestAnimationFrame(() => { - inputRef.current?.focus() - }) - return () => cancelAnimationFrame(frame) - }, []) - - return ( - <> - - onChange(e.target.value)} - className="shadows-shadow-xs flex grow items-center gap-[2px] self-stretch - overflow-hidden rounded-lg border border-components-input-border-active bg-components-input-bg-active p-2 - system-sm-regular text-ellipsis text-components-input-text-filled outline-hidden" - placeholder="Please enter GitHub repo URL" - /> -
- - -
- - ) -} - -export default SetURL diff --git a/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx b/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx index 9f883af1b2..bc39cfdbd4 100644 --- a/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx +++ b/web/app/components/plugins/plugin-page/__tests__/install-plugin-dropdown.spec.tsx @@ -60,16 +60,18 @@ vi.mock('@langgenius/dify-ui/dropdown-menu', async () => { DropdownMenu: ({ open, onOpenChange, + modal, children, }: { open: boolean onOpenChange?: (open: boolean) => void + modal?: boolean children: React.ReactNode }) => { portalOpen = open return ( -
{children}
+
{children}
) }, @@ -158,6 +160,7 @@ describe('InstallPluginDropdown', () => { fireEvent.click(screen.getByTestId('dropdown-trigger')) + expect(screen.getByTestId('dropdown-menu')).toHaveAttribute('data-modal', 'false') expect(screen.getByText('plugin.installFrom')).toBeInTheDocument() expect(screen.getByText('plugin.source.marketplace')).toBeInTheDocument() expect(screen.getByText('plugin.source.github')).toBeInTheDocument() diff --git a/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx b/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx index a972c9891c..8e996345af 100644 --- a/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx +++ b/web/app/components/plugins/plugin-page/install-plugin-dropdown.tsx @@ -111,7 +111,7 @@ const InstallPluginDropdown = ({ } return ( - +