diff --git a/web/__tests__/explore/explore-app-list-flow.test.tsx b/web/__tests__/explore/explore-app-list-flow.test.tsx index 1a54135420..344543c094 100644 --- a/web/__tests__/explore/explore-app-list-flow.test.tsx +++ b/web/__tests__/explore/explore-app-list-flow.test.tsx @@ -127,13 +127,7 @@ const createApp = (overrides: Partial = {}): App => ({ }) const createContextValue = (hasEditPermission = true) => ({ - controlUpdateInstalledApps: 0, - setControlUpdateInstalledApps: vi.fn(), hasEditPermission, - installedApps: [] as never[], - setInstalledApps: vi.fn(), - isFetchingInstalledApps: false, - setIsFetchingInstalledApps: vi.fn(), isShowTryAppPanel: false, setShowTryAppPanel: vi.fn(), }) diff --git a/web/__tests__/explore/installed-app-flow.test.tsx b/web/__tests__/explore/installed-app-flow.test.tsx index 69dcb116aa..6570a260e3 100644 --- a/web/__tests__/explore/installed-app-flow.test.tsx +++ b/web/__tests__/explore/installed-app-flow.test.tsx @@ -8,20 +8,13 @@ import type { Mock } from 'vitest' import type { InstalledApp as InstalledAppModel } from '@/models/explore' import { render, screen, waitFor } from '@testing-library/react' -import { useContext } from 'use-context-selector' import InstalledApp from '@/app/components/explore/installed-app' import { useWebAppStore } from '@/context/web-app-context' import { AccessMode } from '@/models/access-control' import { useGetUserCanAccessApp } from '@/service/access-control' -import { useGetInstalledAppAccessModeByAppId, useGetInstalledAppMeta, useGetInstalledAppParams } from '@/service/use-explore' +import { useGetInstalledAppAccessModeByAppId, useGetInstalledAppMeta, useGetInstalledAppParams, useGetInstalledApps } from '@/service/use-explore' import { AppModeEnum } from '@/types/app' -// Mock external dependencies -vi.mock('use-context-selector', () => ({ - useContext: vi.fn(), - createContext: vi.fn(() => ({})), -})) - vi.mock('@/context/web-app-context', () => ({ useWebAppStore: vi.fn(), })) @@ -34,6 +27,7 @@ vi.mock('@/service/use-explore', () => ({ useGetInstalledAppAccessModeByAppId: vi.fn(), useGetInstalledAppParams: vi.fn(), useGetInstalledAppMeta: vi.fn(), + useGetInstalledApps: vi.fn(), })) vi.mock('@/app/components/share/text-generation', () => ({ @@ -86,18 +80,18 @@ describe('Installed App Flow', () => { } type MockOverrides = { - context?: { installedApps?: InstalledAppModel[], isFetchingInstalledApps?: boolean } - accessMode?: { isFetching?: boolean, data?: unknown, error?: unknown } - params?: { isFetching?: boolean, data?: unknown, error?: unknown } - meta?: { isFetching?: boolean, data?: unknown, error?: unknown } + installedApps?: { apps?: InstalledAppModel[], isPending?: boolean } + accessMode?: { isLoading?: boolean, data?: unknown, error?: unknown } + params?: { isLoading?: boolean, data?: unknown, error?: unknown } + meta?: { isLoading?: boolean, data?: unknown, error?: unknown } userAccess?: { data?: unknown, error?: unknown } } const setupDefaultMocks = (app?: InstalledAppModel, overrides: MockOverrides = {}) => { - ;(useContext as Mock).mockReturnValue({ - installedApps: app ? [app] : [], - isFetchingInstalledApps: false, - ...overrides.context, + ;(useGetInstalledApps as Mock).mockReturnValue({ + data: { installed_apps: app ? [app] : [] }, + isPending: false, + ...overrides.installedApps, }) ;(useWebAppStore as unknown as Mock).mockImplementation((selector: (state: Record) => unknown) => { @@ -111,21 +105,21 @@ describe('Installed App Flow', () => { }) ;(useGetInstalledAppAccessModeByAppId as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: { accessMode: AccessMode.PUBLIC }, error: null, ...overrides.accessMode, }) ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: mockAppParams, error: null, ...overrides.params, }) ;(useGetInstalledAppMeta as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: { tool_icons: {} }, error: null, ...overrides.meta, @@ -182,7 +176,7 @@ describe('Installed App Flow', () => { describe('Data Loading Flow', () => { it('should show loading spinner when params are being fetched', () => { const app = createInstalledApp() - setupDefaultMocks(app, { params: { isFetching: true, data: null } }) + setupDefaultMocks(app, { params: { isLoading: true, data: null } }) const { container } = render() diff --git a/web/__tests__/explore/sidebar-lifecycle-flow.test.tsx b/web/__tests__/explore/sidebar-lifecycle-flow.test.tsx index bf4821ced4..e2c18bcc4f 100644 --- a/web/__tests__/explore/sidebar-lifecycle-flow.test.tsx +++ b/web/__tests__/explore/sidebar-lifecycle-flow.test.tsx @@ -1,4 +1,3 @@ -import type { IExplore } from '@/context/explore-context' /** * Integration test: Sidebar Lifecycle Flow * @@ -10,14 +9,12 @@ import type { InstalledApp } from '@/models/explore' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import Toast from '@/app/components/base/toast' import SideBar from '@/app/components/explore/sidebar' -import ExploreContext from '@/context/explore-context' import { MediaType } from '@/hooks/use-breakpoints' import { AppModeEnum } from '@/types/app' let mockMediaType: string = MediaType.pc const mockSegments = ['apps'] const mockPush = vi.fn() -const mockRefetch = vi.fn() const mockUninstall = vi.fn() const mockUpdatePinStatus = vi.fn() let mockInstalledApps: InstalledApp[] = [] @@ -40,9 +37,8 @@ vi.mock('@/hooks/use-breakpoints', () => ({ vi.mock('@/service/use-explore', () => ({ useGetInstalledApps: () => ({ - isFetching: false, + isPending: false, data: { installed_apps: mockInstalledApps }, - refetch: mockRefetch, }), useUninstallApp: () => ({ mutateAsync: mockUninstall, @@ -69,24 +65,8 @@ const createInstalledApp = (overrides: Partial = {}): InstalledApp }, }) -const createContextValue = (installedApps: InstalledApp[] = []): IExplore => ({ - controlUpdateInstalledApps: 0, - setControlUpdateInstalledApps: vi.fn(), - hasEditPermission: true, - installedApps, - setInstalledApps: vi.fn(), - isFetchingInstalledApps: false, - setIsFetchingInstalledApps: vi.fn(), - isShowTryAppPanel: false, - setShowTryAppPanel: vi.fn(), -}) - -const renderSidebar = (installedApps: InstalledApp[] = []) => { - return render( - - - , - ) +const renderSidebar = () => { + return render() } describe('Sidebar Lifecycle Flow', () => { @@ -104,7 +84,7 @@ describe('Sidebar Lifecycle Flow', () => { // Step 1: Start with an unpinned app and pin it const unpinnedApp = createInstalledApp({ is_pinned: false }) mockInstalledApps = [unpinnedApp] - const { unmount } = renderSidebar(mockInstalledApps) + const { unmount } = renderSidebar() fireEvent.click(screen.getByTestId('item-operation-trigger')) fireEvent.click(await screen.findByText('explore.sidebar.action.pin')) @@ -123,7 +103,7 @@ describe('Sidebar Lifecycle Flow', () => { const pinnedApp = createInstalledApp({ is_pinned: true }) mockInstalledApps = [pinnedApp] - renderSidebar(mockInstalledApps) + renderSidebar() fireEvent.click(screen.getByTestId('item-operation-trigger')) fireEvent.click(await screen.findByText('explore.sidebar.action.unpin')) @@ -141,7 +121,7 @@ describe('Sidebar Lifecycle Flow', () => { mockInstalledApps = [app] mockUninstall.mockResolvedValue(undefined) - renderSidebar(mockInstalledApps) + renderSidebar() // Step 1: Open operation menu and click delete fireEvent.click(screen.getByTestId('item-operation-trigger')) @@ -167,7 +147,7 @@ describe('Sidebar Lifecycle Flow', () => { const app = createInstalledApp() mockInstalledApps = [app] - renderSidebar(mockInstalledApps) + renderSidebar() // Open delete flow fireEvent.click(screen.getByTestId('item-operation-trigger')) @@ -188,7 +168,7 @@ describe('Sidebar Lifecycle Flow', () => { createInstalledApp({ id: 'unpinned-1', is_pinned: false, app: { ...createInstalledApp().app, name: 'Regular App' } }), ] - const { container } = renderSidebar(mockInstalledApps) + const { container } = renderSidebar() // Both apps are rendered const pinnedApp = screen.getByText('Pinned App') @@ -210,14 +190,14 @@ describe('Sidebar Lifecycle Flow', () => { describe('Empty State', () => { it('should show NoApps component when no apps are installed on desktop', () => { mockMediaType = MediaType.pc - renderSidebar([]) + renderSidebar() expect(screen.getByText('explore.sidebar.noApps.title')).toBeInTheDocument() }) it('should hide NoApps on mobile', () => { mockMediaType = MediaType.mobile - renderSidebar([]) + renderSidebar() expect(screen.queryByText('explore.sidebar.noApps.title')).not.toBeInTheDocument() }) diff --git a/web/app/components/explore/__tests__/index.spec.tsx b/web/app/components/explore/__tests__/index.spec.tsx index b84b168333..ff1161769a 100644 --- a/web/app/components/explore/__tests__/index.spec.tsx +++ b/web/app/components/explore/__tests__/index.spec.tsx @@ -32,9 +32,8 @@ vi.mock('@/hooks/use-breakpoints', () => ({ vi.mock('@/service/use-explore', () => ({ useGetInstalledApps: () => ({ - isFetching: false, + isPending: false, data: mockInstalledAppsData, - refetch: vi.fn(), }), useUninstallApp: () => ({ mutateAsync: vi.fn(), diff --git a/web/app/components/explore/app-card/__tests__/index.spec.tsx b/web/app/components/explore/app-card/__tests__/index.spec.tsx index 8bc0fa99d2..a1d7cd0998 100644 --- a/web/app/components/explore/app-card/__tests__/index.spec.tsx +++ b/web/app/components/explore/app-card/__tests__/index.spec.tsx @@ -145,13 +145,7 @@ describe('AppCard', () => { render( void, se { { { = ({ +const Explore = ({ children, +}: { + children: React.ReactNode }) => { const router = useRouter() - const [controlUpdateInstalledApps, setControlUpdateInstalledApps] = useState(0) const { userProfile, isCurrentWorkspaceDatasetOperator } = useAppContext() - const [hasEditPermission, setHasEditPermission] = useState(false) - const [installedApps, setInstalledApps] = useState([]) - const [isFetchingInstalledApps, setIsFetchingInstalledApps] = useState(false) const { t } = useTranslation() const { data: membersData } = useMembers() useDocumentTitle(t('menus.explore', { ns: 'common' })) - useEffect(() => { - if (!membersData?.accounts) - return - const currUser = membersData.accounts.find(account => account.id === userProfile.id) - setHasEditPermission(currUser?.role !== 'normal') - }, [membersData, userProfile.id]) + const userAccount = membersData?.accounts?.find(account => account.id === userProfile.id) + const hasEditPermission = !!userAccount && userAccount.role !== 'normal' useEffect(() => { if (isCurrentWorkspaceDatasetOperator) @@ -57,20 +45,14 @@ const Explore: FC = ({ - +
{children}
diff --git a/web/app/components/explore/installed-app/__tests__/index.spec.tsx b/web/app/components/explore/installed-app/__tests__/index.spec.tsx index eca7b3139d..10c784f92a 100644 --- a/web/app/components/explore/installed-app/__tests__/index.spec.tsx +++ b/web/app/components/explore/installed-app/__tests__/index.spec.tsx @@ -1,19 +1,14 @@ import type { Mock } from 'vitest' import type { InstalledApp as InstalledAppType } from '@/models/explore' import { render, screen, waitFor } from '@testing-library/react' -import { useContext } from 'use-context-selector' import { useWebAppStore } from '@/context/web-app-context' import { AccessMode } from '@/models/access-control' import { useGetUserCanAccessApp } from '@/service/access-control' -import { useGetInstalledAppAccessModeByAppId, useGetInstalledAppMeta, useGetInstalledAppParams } from '@/service/use-explore' +import { useGetInstalledAppAccessModeByAppId, useGetInstalledAppMeta, useGetInstalledAppParams, useGetInstalledApps } from '@/service/use-explore' import { AppModeEnum } from '@/types/app' import InstalledApp from '../index' -vi.mock('use-context-selector', () => ({ - useContext: vi.fn(), - createContext: vi.fn(() => ({})), -})) vi.mock('@/context/web-app-context', () => ({ useWebAppStore: vi.fn(), })) @@ -24,28 +19,9 @@ vi.mock('@/service/use-explore', () => ({ useGetInstalledAppAccessModeByAppId: vi.fn(), useGetInstalledAppParams: vi.fn(), useGetInstalledAppMeta: vi.fn(), + useGetInstalledApps: vi.fn(), })) -/** - * Mock child components for unit testing - * - * RATIONALE FOR MOCKING: - * - TextGenerationApp: 648 lines, complex batch processing, task management, file uploads - * - ChatWithHistory: 576-line custom hook, complex conversation/history management, 30+ context values - * - * These components are too complex to test as real components. Using real components would: - * 1. Require mocking dozens of their dependencies (services, contexts, hooks) - * 2. Make tests fragile and coupled to child component implementation details - * 3. Violate the principle of testing one component in isolation - * - * For a container component like InstalledApp, its responsibility is to: - * - Correctly route to the appropriate child component based on app mode - * - Pass the correct props to child components - * - Handle loading/error states before rendering children - * - * The internal logic of ChatWithHistory and TextGenerationApp should be tested - * in their own dedicated test files. - */ vi.mock('@/app/components/share/text-generation', () => ({ default: ({ isInstalledApp, installedAppInfo, isWorkflow }: { isInstalledApp?: boolean @@ -115,13 +91,17 @@ describe('InstalledApp', () => { result: true, } + const setupMocks = (installedApps: InstalledAppType[] = [mockInstalledApp], isPending = false) => { + ;(useGetInstalledApps as Mock).mockReturnValue({ + data: { installed_apps: installedApps }, + isPending, + }) + } + beforeEach(() => { vi.clearAllMocks() - ;(useContext as Mock).mockReturnValue({ - installedApps: [mockInstalledApp], - isFetchingInstalledApps: false, - }) + setupMocks() ;(useWebAppStore as unknown as Mock).mockImplementation(( selector: (state: { @@ -143,19 +123,19 @@ describe('InstalledApp', () => { }) ;(useGetInstalledAppAccessModeByAppId as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: mockWebAppAccessMode, error: null, }) ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: mockAppParams, error: null, }) ;(useGetInstalledAppMeta as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: mockAppMeta, error: null, }) @@ -174,7 +154,7 @@ describe('InstalledApp', () => { it('should render loading state when fetching app params', () => { ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: true, + isLoading: true, data: null, error: null, }) @@ -186,7 +166,7 @@ describe('InstalledApp', () => { it('should render loading state when fetching app meta', () => { ;(useGetInstalledAppMeta as Mock).mockReturnValue({ - isFetching: true, + isLoading: true, data: null, error: null, }) @@ -198,7 +178,7 @@ describe('InstalledApp', () => { it('should render loading state when fetching web app access mode', () => { ;(useGetInstalledAppAccessModeByAppId as Mock).mockReturnValue({ - isFetching: true, + isLoading: true, data: null, error: null, }) @@ -209,10 +189,7 @@ describe('InstalledApp', () => { }) it('should render loading state when fetching installed apps', () => { - ;(useContext as Mock).mockReturnValue({ - installedApps: [mockInstalledApp], - isFetchingInstalledApps: true, - }) + setupMocks([mockInstalledApp], true) const { container } = render() const svg = container.querySelector('svg.spin-animation') @@ -220,10 +197,7 @@ describe('InstalledApp', () => { }) it('should render app not found (404) when installedApp does not exist', () => { - ;(useContext as Mock).mockReturnValue({ - installedApps: [], - isFetchingInstalledApps: false, - }) + setupMocks([]) render() expect(screen.getByText(/404/)).toBeInTheDocument() @@ -234,7 +208,7 @@ describe('InstalledApp', () => { it('should render error when app params fails to load', () => { const error = new Error('Failed to load app params') ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: null, error, }) @@ -246,7 +220,7 @@ describe('InstalledApp', () => { it('should render error when app meta fails to load', () => { const error = new Error('Failed to load app meta') ;(useGetInstalledAppMeta as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: null, error, }) @@ -258,7 +232,7 @@ describe('InstalledApp', () => { it('should render error when web app access mode fails to load', () => { const error = new Error('Failed to load access mode') ;(useGetInstalledAppAccessModeByAppId as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: null, error, }) @@ -305,10 +279,7 @@ describe('InstalledApp', () => { mode: AppModeEnum.ADVANCED_CHAT, }, } - ;(useContext as Mock).mockReturnValue({ - installedApps: [advancedChatApp], - isFetchingInstalledApps: false, - }) + setupMocks([advancedChatApp]) render() expect(screen.getByText(/Chat With History/i)).toBeInTheDocument() @@ -323,10 +294,7 @@ describe('InstalledApp', () => { mode: AppModeEnum.AGENT_CHAT, }, } - ;(useContext as Mock).mockReturnValue({ - installedApps: [agentChatApp], - isFetchingInstalledApps: false, - }) + setupMocks([agentChatApp]) render() expect(screen.getByText(/Chat With History/i)).toBeInTheDocument() @@ -341,10 +309,7 @@ describe('InstalledApp', () => { mode: AppModeEnum.COMPLETION, }, } - ;(useContext as Mock).mockReturnValue({ - installedApps: [completionApp], - isFetchingInstalledApps: false, - }) + setupMocks([completionApp]) render() expect(screen.getByText(/Text Generation App/i)).toBeInTheDocument() @@ -359,10 +324,7 @@ describe('InstalledApp', () => { mode: AppModeEnum.WORKFLOW, }, } - ;(useContext as Mock).mockReturnValue({ - installedApps: [workflowApp], - isFetchingInstalledApps: false, - }) + setupMocks([workflowApp]) render() expect(screen.getByText(/Text Generation App/i)).toBeInTheDocument() @@ -374,10 +336,7 @@ describe('InstalledApp', () => { it('should use id prop to find installed app', () => { const app1 = { ...mockInstalledApp, id: 'app-1' } const app2 = { ...mockInstalledApp, id: 'app-2' } - ;(useContext as Mock).mockReturnValue({ - installedApps: [app1, app2], - isFetchingInstalledApps: false, - }) + setupMocks([app1, app2]) render() expect(screen.getByText(/app-2/)).toBeInTheDocument() @@ -416,10 +375,7 @@ describe('InstalledApp', () => { }) it('should update app info to null when installedApp is not found', async () => { - ;(useContext as Mock).mockReturnValue({ - installedApps: [], - isFetchingInstalledApps: false, - }) + setupMocks([]) render() @@ -488,7 +444,7 @@ describe('InstalledApp', () => { it('should not update app params when data is null', async () => { ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: null, error: null, }) @@ -504,7 +460,7 @@ describe('InstalledApp', () => { it('should not update app meta when data is null', async () => { ;(useGetInstalledAppMeta as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: null, error: null, }) @@ -520,7 +476,7 @@ describe('InstalledApp', () => { it('should not update access mode when data is null', async () => { ;(useGetInstalledAppAccessModeByAppId as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: null, error: null, }) @@ -537,10 +493,7 @@ describe('InstalledApp', () => { describe('Edge Cases', () => { it('should handle empty installedApps array', () => { - ;(useContext as Mock).mockReturnValue({ - installedApps: [], - isFetchingInstalledApps: false, - }) + setupMocks([]) render() expect(screen.getByText(/404/)).toBeInTheDocument() @@ -555,10 +508,7 @@ describe('InstalledApp', () => { name: 'Other App', }, } - ;(useContext as Mock).mockReturnValue({ - installedApps: [otherApp, mockInstalledApp], - isFetchingInstalledApps: false, - }) + setupMocks([otherApp, mockInstalledApp]) render() expect(screen.getByText(/Chat With History/i)).toBeInTheDocument() @@ -568,10 +518,7 @@ describe('InstalledApp', () => { it('should handle rapid id prop changes', async () => { const app1 = { ...mockInstalledApp, id: 'app-1' } const app2 = { ...mockInstalledApp, id: 'app-2' } - ;(useContext as Mock).mockReturnValue({ - installedApps: [app1, app2], - isFetchingInstalledApps: false, - }) + setupMocks([app1, app2]) const { rerender } = render() expect(screen.getByText(/app-1/)).toBeInTheDocument() @@ -593,10 +540,7 @@ describe('InstalledApp', () => { }) it('should call service hooks with null when installedApp is not found', () => { - ;(useContext as Mock).mockReturnValue({ - installedApps: [], - isFetchingInstalledApps: false, - }) + setupMocks([]) render() @@ -613,7 +557,7 @@ describe('InstalledApp', () => { describe('Render Priority', () => { it('should show error before loading state', () => { ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: true, + isLoading: true, data: null, error: new Error('Some error'), }) @@ -624,7 +568,7 @@ describe('InstalledApp', () => { it('should show error before permission check', () => { ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: false, + isLoading: false, data: null, error: new Error('Params error'), }) @@ -639,10 +583,7 @@ describe('InstalledApp', () => { }) it('should show permission error before 404', () => { - ;(useContext as Mock).mockReturnValue({ - installedApps: [], - isFetchingInstalledApps: false, - }) + setupMocks([]) ;(useGetUserCanAccessApp as Mock).mockReturnValue({ data: { result: false }, error: null, @@ -654,12 +595,9 @@ describe('InstalledApp', () => { }) it('should show loading before 404', () => { - ;(useContext as Mock).mockReturnValue({ - installedApps: [], - isFetchingInstalledApps: false, - }) + setupMocks([]) ;(useGetInstalledAppParams as Mock).mockReturnValue({ - isFetching: true, + isLoading: true, data: null, error: null, }) diff --git a/web/app/components/explore/installed-app/index.tsx b/web/app/components/explore/installed-app/index.tsx index 7366057445..e5c45266b0 100644 --- a/web/app/components/explore/installed-app/index.tsx +++ b/web/app/components/explore/installed-app/index.tsx @@ -1,37 +1,32 @@ 'use client' -import type { FC } from 'react' import type { AccessMode } from '@/models/access-control' import type { AppData } from '@/models/share' import * as React from 'react' import { useEffect } from 'react' -import { useContext } from 'use-context-selector' import ChatWithHistory from '@/app/components/base/chat/chat-with-history' import Loading from '@/app/components/base/loading' import TextGenerationApp from '@/app/components/share/text-generation' -import ExploreContext from '@/context/explore-context' import { useWebAppStore } from '@/context/web-app-context' import { useGetUserCanAccessApp } from '@/service/access-control' -import { useGetInstalledAppAccessModeByAppId, useGetInstalledAppMeta, useGetInstalledAppParams } from '@/service/use-explore' +import { useGetInstalledAppAccessModeByAppId, useGetInstalledAppMeta, useGetInstalledAppParams, useGetInstalledApps } from '@/service/use-explore' import { AppModeEnum } from '@/types/app' import AppUnavailable from '../../base/app-unavailable' -export type IInstalledAppProps = { - id: string -} - -const InstalledApp: FC = ({ +const InstalledApp = ({ id, +}: { + id: string }) => { - const { installedApps, isFetchingInstalledApps } = useContext(ExploreContext) + const { data, isPending: isPendingInstalledApps } = useGetInstalledApps() + const installedApp = data?.installed_apps?.find(item => item.id === id) const updateAppInfo = useWebAppStore(s => s.updateAppInfo) - const installedApp = installedApps.find(item => item.id === id) const updateWebAppAccessMode = useWebAppStore(s => s.updateWebAppAccessMode) const updateAppParams = useWebAppStore(s => s.updateAppParams) const updateWebAppMeta = useWebAppStore(s => s.updateWebAppMeta) const updateUserCanAccessApp = useWebAppStore(s => s.updateUserCanAccessApp) - const { isFetching: isFetchingWebAppAccessMode, data: webAppAccessMode, error: webAppAccessModeError } = useGetInstalledAppAccessModeByAppId(installedApp?.id ?? null) - const { isFetching: isFetchingAppParams, data: appParams, error: appParamsError } = useGetInstalledAppParams(installedApp?.id ?? null) - const { isFetching: isFetchingAppMeta, data: appMeta, error: appMetaError } = useGetInstalledAppMeta(installedApp?.id ?? null) + const { isLoading: isLoadingWebAppAccessMode, data: webAppAccessMode, error: webAppAccessModeError } = useGetInstalledAppAccessModeByAppId(installedApp?.id ?? null) + const { isLoading: isLoadingAppParams, data: appParams, error: appParamsError } = useGetInstalledAppParams(installedApp?.id ?? null) + const { isLoading: isLoadingAppMeta, data: appMeta, error: appMetaError } = useGetInstalledAppMeta(installedApp?.id ?? null) const { data: userCanAccessApp, error: useCanAccessAppError } = useGetUserCanAccessApp({ appId: installedApp?.app.id, isInstalledApp: true }) useEffect(() => { @@ -102,7 +97,7 @@ const InstalledApp: FC = ({ ) } - if (isFetchingAppParams || isFetchingAppMeta || isFetchingWebAppAccessMode || isFetchingInstalledApps) { + if (isLoadingAppParams || isLoadingAppMeta || isLoadingWebAppAccessMode || isPendingInstalledApps) { return (
diff --git a/web/app/components/explore/sidebar/__tests__/index.spec.tsx b/web/app/components/explore/sidebar/__tests__/index.spec.tsx index 2fcc48fc56..36e6ab217c 100644 --- a/web/app/components/explore/sidebar/__tests__/index.spec.tsx +++ b/web/app/components/explore/sidebar/__tests__/index.spec.tsx @@ -1,18 +1,15 @@ -import type { IExplore } from '@/context/explore-context' import type { InstalledApp } from '@/models/explore' import { fireEvent, render, screen, waitFor } from '@testing-library/react' import Toast from '@/app/components/base/toast' -import ExploreContext from '@/context/explore-context' import { MediaType } from '@/hooks/use-breakpoints' import { AppModeEnum } from '@/types/app' import SideBar from '../index' const mockSegments = ['apps'] const mockPush = vi.fn() -const mockRefetch = vi.fn() const mockUninstall = vi.fn() const mockUpdatePinStatus = vi.fn() -let mockIsFetching = false +let mockIsPending = false let mockInstalledApps: InstalledApp[] = [] let mockMediaType: string = MediaType.pc @@ -34,9 +31,8 @@ vi.mock('@/hooks/use-breakpoints', () => ({ vi.mock('@/service/use-explore', () => ({ useGetInstalledApps: () => ({ - isFetching: mockIsFetching, + isPending: mockIsPending, data: { installed_apps: mockInstalledApps }, - refetch: mockRefetch, }), useUninstallApp: () => ({ mutateAsync: mockUninstall, @@ -63,28 +59,14 @@ const createInstalledApp = (overrides: Partial = {}): InstalledApp }, }) -const renderWithContext = (installedApps: InstalledApp[] = []) => { - return render( - - - , - ) +const renderSideBar = () => { + return render() } describe('SideBar', () => { beforeEach(() => { vi.clearAllMocks() - mockIsFetching = false + mockIsPending = false mockInstalledApps = [] mockMediaType = MediaType.pc vi.spyOn(Toast, 'notify').mockImplementation(() => ({ clear: vi.fn() })) @@ -92,31 +74,38 @@ describe('SideBar', () => { describe('Rendering', () => { it('should render discovery link', () => { - renderWithContext() + renderSideBar() expect(screen.getByText('explore.sidebar.title')).toBeInTheDocument() }) it('should render workspace items when installed apps exist', () => { mockInstalledApps = [createInstalledApp()] - renderWithContext(mockInstalledApps) + renderSideBar() expect(screen.getByText('explore.sidebar.webApps')).toBeInTheDocument() expect(screen.getByText('My App')).toBeInTheDocument() }) it('should render NoApps component when no installed apps on desktop', () => { - renderWithContext([]) + renderSideBar() expect(screen.getByText('explore.sidebar.noApps.title')).toBeInTheDocument() }) + it('should not render NoApps while loading', () => { + mockIsPending = true + renderSideBar() + + expect(screen.queryByText('explore.sidebar.noApps.title')).not.toBeInTheDocument() + }) + it('should render multiple installed apps', () => { mockInstalledApps = [ createInstalledApp({ id: 'app-1', app: { ...createInstalledApp().app, name: 'Alpha' } }), createInstalledApp({ id: 'app-2', app: { ...createInstalledApp().app, name: 'Beta' } }), ] - renderWithContext(mockInstalledApps) + renderSideBar() expect(screen.getByText('Alpha')).toBeInTheDocument() expect(screen.getByText('Beta')).toBeInTheDocument() @@ -127,27 +116,18 @@ describe('SideBar', () => { createInstalledApp({ id: 'app-1', is_pinned: true, app: { ...createInstalledApp().app, name: 'Pinned' } }), createInstalledApp({ id: 'app-2', is_pinned: false, app: { ...createInstalledApp().app, name: 'Unpinned' } }), ] - const { container } = renderWithContext(mockInstalledApps) + const { container } = renderSideBar() const dividers = container.querySelectorAll('[class*="divider"], hr') expect(dividers.length).toBeGreaterThan(0) }) }) - describe('Effects', () => { - it('should refetch installed apps on mount', () => { - mockInstalledApps = [createInstalledApp()] - renderWithContext(mockInstalledApps) - - expect(mockRefetch).toHaveBeenCalledTimes(1) - }) - }) - describe('User Interactions', () => { it('should uninstall app and show toast when delete is confirmed', async () => { mockInstalledApps = [createInstalledApp()] mockUninstall.mockResolvedValue(undefined) - renderWithContext(mockInstalledApps) + renderSideBar() fireEvent.click(screen.getByTestId('item-operation-trigger')) fireEvent.click(await screen.findByText('explore.sidebar.action.delete')) @@ -165,7 +145,7 @@ describe('SideBar', () => { it('should update pin status and show toast when pin is clicked', async () => { mockInstalledApps = [createInstalledApp({ is_pinned: false })] mockUpdatePinStatus.mockResolvedValue(undefined) - renderWithContext(mockInstalledApps) + renderSideBar() fireEvent.click(screen.getByTestId('item-operation-trigger')) fireEvent.click(await screen.findByText('explore.sidebar.action.pin')) @@ -182,7 +162,7 @@ describe('SideBar', () => { it('should unpin an already pinned app', async () => { mockInstalledApps = [createInstalledApp({ is_pinned: true })] mockUpdatePinStatus.mockResolvedValue(undefined) - renderWithContext(mockInstalledApps) + renderSideBar() fireEvent.click(screen.getByTestId('item-operation-trigger')) fireEvent.click(await screen.findByText('explore.sidebar.action.unpin')) @@ -194,7 +174,7 @@ describe('SideBar', () => { it('should open and close confirm dialog for delete', async () => { mockInstalledApps = [createInstalledApp()] - renderWithContext(mockInstalledApps) + renderSideBar() fireEvent.click(screen.getByTestId('item-operation-trigger')) fireEvent.click(await screen.findByText('explore.sidebar.action.delete')) @@ -212,7 +192,7 @@ describe('SideBar', () => { describe('Edge Cases', () => { it('should hide NoApps and app names on mobile', () => { mockMediaType = MediaType.mobile - renderWithContext([]) + renderSideBar() expect(screen.queryByText('explore.sidebar.noApps.title')).not.toBeInTheDocument() expect(screen.queryByText('explore.sidebar.webApps')).not.toBeInTheDocument() diff --git a/web/app/components/explore/sidebar/index.tsx b/web/app/components/explore/sidebar/index.tsx index 3e9b664580..bafc745b01 100644 --- a/web/app/components/explore/sidebar/index.tsx +++ b/web/app/components/explore/sidebar/index.tsx @@ -1,16 +1,12 @@ 'use client' -import type { FC } from 'react' -import { RiAppsFill, RiExpandRightLine, RiLayoutLeft2Line } from '@remixicon/react' import { useBoolean } from 'ahooks' import Link from 'next/link' import { useSelectedLayoutSegments } from 'next/navigation' import * as React from 'react' -import { useEffect, useState } from 'react' +import { useState } from 'react' import { useTranslation } from 'react-i18next' -import { useContext } from 'use-context-selector' import Confirm from '@/app/components/base/confirm' import Divider from '@/app/components/base/divider' -import ExploreContext from '@/context/explore-context' import useBreakpoints, { MediaType } from '@/hooks/use-breakpoints' import { useGetInstalledApps, useUninstallApp, useUpdateAppPinStatus } from '@/service/use-explore' import { cn } from '@/utils/classnames' @@ -18,19 +14,13 @@ import Toast from '../../base/toast' import Item from './app-nav-item' import NoApps from './no-apps' -export type IExploreSideBarProps = { - controlUpdateInstalledApps: number -} - -const SideBar: FC = ({ - controlUpdateInstalledApps, -}) => { +const SideBar = () => { const { t } = useTranslation() const segments = useSelectedLayoutSegments() const lastSegment = segments.slice(-1)[0] const isDiscoverySelected = lastSegment === 'apps' - const { installedApps, setInstalledApps, setIsFetchingInstalledApps } = useContext(ExploreContext) - const { isFetching: isFetchingInstalledApps, data: ret, refetch: fetchInstalledAppList } = useGetInstalledApps() + const { data, isPending } = useGetInstalledApps() + const installedApps = data?.installed_apps ?? [] const { mutateAsync: uninstallApp } = useUninstallApp() const { mutateAsync: updatePinStatus } = useUpdateAppPinStatus() @@ -60,22 +50,6 @@ const SideBar: FC = ({ }) } - useEffect(() => { - const installed_apps = (ret as any)?.installed_apps - if (installed_apps && installed_apps.length > 0) - setInstalledApps(installed_apps) - else - setInstalledApps([]) - }, [ret, setInstalledApps]) - - useEffect(() => { - setIsFetchingInstalledApps(isFetchingInstalledApps) - }, [isFetchingInstalledApps, setIsFetchingInstalledApps]) - - useEffect(() => { - fetchInstalledAppList() - }, [controlUpdateInstalledApps, fetchInstalledAppList]) - const pinnedAppsCount = installedApps.filter(({ is_pinned }) => is_pinned).length return (
@@ -85,13 +59,13 @@ const SideBar: FC = ({ className={cn(isDiscoverySelected ? 'bg-state-base-active' : 'hover:bg-state-base-hover', 'flex h-8 items-center gap-2 rounded-lg px-1 mobile:w-fit mobile:justify-center pc:w-full pc:justify-start')} >
- +
- {!isMobile && !isFold &&
{t('sidebar.title', { ns: 'explore' })}
} + {!isMobile && !isFold &&
{t('sidebar.title', { ns: 'explore' })}
}
- {installedApps.length === 0 && !isMobile && !isFold + {!isPending && installedApps.length === 0 && !isMobile && !isFold && (
@@ -100,7 +74,7 @@ const SideBar: FC = ({ {installedApps.length > 0 && (
- {!isMobile && !isFold &&

{t('sidebar.webApps', { ns: 'explore' })}

} + {!isMobile && !isFold &&

{t('sidebar.webApps', { ns: 'explore' })}

}
= ({ {!isMobile && (
{isFold - ? + ? : ( - + )}
)} diff --git a/web/context/explore-context.ts b/web/context/explore-context.ts index 8ecaa7af19..15ee5bee9e 100644 --- a/web/context/explore-context.ts +++ b/web/context/explore-context.ts @@ -1,4 +1,4 @@ -import type { App, InstalledApp } from '@/models/explore' +import type { App } from '@/models/explore' import { noop } from 'es-toolkit/function' import { createContext } from 'use-context-selector' @@ -8,26 +8,14 @@ export type CurrentTryAppParams = { } export type IExplore = { - controlUpdateInstalledApps: number - setControlUpdateInstalledApps: (controlUpdateInstalledApps: number) => void hasEditPermission: boolean - installedApps: InstalledApp[] - setInstalledApps: (installedApps: InstalledApp[]) => void - isFetchingInstalledApps: boolean - setIsFetchingInstalledApps: (isFetchingInstalledApps: boolean) => void currentApp?: CurrentTryAppParams isShowTryAppPanel: boolean setShowTryAppPanel: (showTryAppPanel: boolean, params?: CurrentTryAppParams) => void } const ExploreContext = createContext({ - controlUpdateInstalledApps: 0, - setControlUpdateInstalledApps: noop, hasEditPermission: false, - installedApps: [], - setInstalledApps: noop, - isFetchingInstalledApps: false, - setIsFetchingInstalledApps: noop, isShowTryAppPanel: false, setShowTryAppPanel: noop, currentApp: undefined, diff --git a/web/eslint-suppressions.json b/web/eslint-suppressions.json index 09575c28d7..a20ea7b6dd 100644 --- a/web/eslint-suppressions.json +++ b/web/eslint-suppressions.json @@ -4100,11 +4100,6 @@ "count": 1 } }, - "app/components/explore/index.tsx": { - "react-hooks-extra/no-direct-set-state-in-use-effect": { - "count": 1 - } - }, "app/components/explore/item-operation/index.tsx": { "react-hooks-extra/no-direct-set-state-in-use-effect": { "count": 1 @@ -4115,14 +4110,6 @@ "count": 2 } }, - "app/components/explore/sidebar/index.tsx": { - "tailwindcss/enforce-consistent-class-order": { - "count": 3 - }, - "ts/no-explicit-any": { - "count": 1 - } - }, "app/components/explore/sidebar/no-apps/index.tsx": { "tailwindcss/enforce-consistent-class-order": { "count": 3 diff --git a/web/service/explore.ts b/web/service/explore.ts index 3d43dc2bbe..7da8937303 100644 --- a/web/service/explore.ts +++ b/web/service/explore.ts @@ -1,6 +1,6 @@ import type { AccessMode } from '@/models/access-control' import type { Banner } from '@/models/app' -import type { App, AppCategory } from '@/models/explore' +import type { App, AppCategory, InstalledApp } from '@/models/explore' import { del, get, patch } from './base' export const fetchAppList = () => { @@ -16,7 +16,7 @@ export const fetchAppDetail = (id: string): Promise => { } export const fetchInstalledAppList = (app_id?: string | null) => { - return get(`/installed-apps${app_id ? `?app_id=${app_id}` : ''}`) + return get<{ installed_apps: InstalledApp[] }>(`/installed-apps${app_id ? `?app_id=${app_id}` : ''}`) } export const uninstallApp = (id: string) => {