diff --git a/web/__tests__/embedded-user-id-store.test.tsx b/web/__tests__/embedded-user-id-store.test.tsx index 276b22bcd7..901218e76b 100644 --- a/web/__tests__/embedded-user-id-store.test.tsx +++ b/web/__tests__/embedded-user-id-store.test.tsx @@ -53,6 +53,7 @@ vi.mock('@/context/global-public-context', () => { ) return { useGlobalPublicStore, + useIsSystemFeaturesPending: () => false, } }) diff --git a/web/app/components/app-initializer.tsx b/web/app/components/app-initializer.tsx index e30646eb3f..3410ecbe9a 100644 --- a/web/app/components/app-initializer.tsx +++ b/web/app/components/app-initializer.tsx @@ -9,8 +9,8 @@ import { EDUCATION_VERIFY_URL_SEARCHPARAMS_ACTION, EDUCATION_VERIFYING_LOCALSTORAGE_ITEM, } from '@/app/education-apply/constants' -import { fetchSetupStatus } from '@/service/common' import { sendGAEvent } from '@/utils/gtag' +import { fetchSetupStatusWithCache } from '@/utils/setup-status' import { resolvePostLoginRedirect } from '../signin/utils/post-login-redirect' import { trackEvent } from './base/amplitude' @@ -33,15 +33,8 @@ export const AppInitializer = ({ const isSetupFinished = useCallback(async () => { try { - if (localStorage.getItem('setup_status') === 'finished') - return true - const setUpStatus = await fetchSetupStatus() - if (setUpStatus.step !== 'finished') { - localStorage.removeItem('setup_status') - return false - } - localStorage.setItem('setup_status', 'finished') - return true + const setUpStatus = await fetchSetupStatusWithCache() + return setUpStatus.step === 'finished' } catch (error) { console.error(error) diff --git a/web/app/components/app/app-access-control/access-control.spec.tsx b/web/app/components/app/app-access-control/access-control.spec.tsx index dd9acd3479..0624cb316b 100644 --- a/web/app/components/app/app-access-control/access-control.spec.tsx +++ b/web/app/components/app/app-access-control/access-control.spec.tsx @@ -125,7 +125,6 @@ const resetAccessControlStore = () => { const resetGlobalStore = () => { useGlobalPublicStore.setState({ systemFeatures: defaultSystemFeatures, - isGlobalPending: false, }) } diff --git a/web/app/install/installForm.spec.tsx b/web/app/install/installForm.spec.tsx index 74602f916a..5efd5cebb6 100644 --- a/web/app/install/installForm.spec.tsx +++ b/web/app/install/installForm.spec.tsx @@ -19,6 +19,14 @@ vi.mock('@/service/common', () => ({ getSystemFeatures: vi.fn(), })) +vi.mock('@/context/global-public-context', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useIsSystemFeaturesPending: () => false, + } +}) + const mockFetchSetupStatus = vi.mocked(fetchSetupStatus) const mockFetchInitValidateStatus = vi.mocked(fetchInitValidateStatus) const mockSetup = vi.mocked(setup) diff --git a/web/context/global-public-context.tsx b/web/context/global-public-context.tsx index c2742bb7a9..9b2b0834e2 100644 --- a/web/context/global-public-context.tsx +++ b/web/context/global-public-context.tsx @@ -2,42 +2,61 @@ import type { FC, PropsWithChildren } from 'react' import type { SystemFeatures } from '@/types/feature' import { useQuery } from '@tanstack/react-query' -import { useEffect } from 'react' import { create } from 'zustand' import Loading from '@/app/components/base/loading' import { getSystemFeatures } from '@/service/common' import { defaultSystemFeatures } from '@/types/feature' +import { fetchSetupStatusWithCache } from '@/utils/setup-status' type GlobalPublicStore = { - isGlobalPending: boolean - setIsGlobalPending: (isPending: boolean) => void systemFeatures: SystemFeatures setSystemFeatures: (systemFeatures: SystemFeatures) => void } export const useGlobalPublicStore = create(set => ({ - isGlobalPending: true, - setIsGlobalPending: (isPending: boolean) => set(() => ({ isGlobalPending: isPending })), systemFeatures: defaultSystemFeatures, setSystemFeatures: (systemFeatures: SystemFeatures) => set(() => ({ systemFeatures })), })) +const systemFeaturesQueryKey = ['systemFeatures'] as const +const setupStatusQueryKey = ['setupStatus'] as const + +async function fetchSystemFeatures() { + const data = await getSystemFeatures() + const { setSystemFeatures } = useGlobalPublicStore.getState() + setSystemFeatures({ ...defaultSystemFeatures, ...data }) + return data +} + +export function useSystemFeaturesQuery() { + return useQuery({ + queryKey: systemFeaturesQueryKey, + queryFn: fetchSystemFeatures, + }) +} + +export function useIsSystemFeaturesPending() { + const { isPending } = useSystemFeaturesQuery() + return isPending +} + +export function useSetupStatusQuery() { + return useQuery({ + queryKey: setupStatusQueryKey, + queryFn: fetchSetupStatusWithCache, + staleTime: Infinity, + }) +} + const GlobalPublicStoreProvider: FC = ({ children, }) => { - const { isPending, data } = useQuery({ - queryKey: ['systemFeatures'], - queryFn: getSystemFeatures, - }) - const { setSystemFeatures, setIsGlobalPending: setIsPending } = useGlobalPublicStore() - useEffect(() => { - if (data) - setSystemFeatures({ ...defaultSystemFeatures, ...data }) - }, [data, setSystemFeatures]) + // Fetch systemFeatures and setupStatus in parallel to reduce waterfall. + // setupStatus is prefetched here and cached in localStorage for AppInitializer. + const { isPending } = useSystemFeaturesQuery() - useEffect(() => { - setIsPending(isPending) - }, [isPending, setIsPending]) + // Prefetch setupStatus for AppInitializer (result not needed here) + useSetupStatusQuery() if (isPending) return
diff --git a/web/context/web-app-context.tsx b/web/context/web-app-context.tsx index e6680c95a5..c5488a565c 100644 --- a/web/context/web-app-context.tsx +++ b/web/context/web-app-context.tsx @@ -10,7 +10,7 @@ import { getProcessedSystemVariablesFromUrlParams } from '@/app/components/base/ import Loading from '@/app/components/base/loading' import { AccessMode } from '@/models/access-control' import { useGetWebAppAccessModeByCode } from '@/service/use-share' -import { useGlobalPublicStore } from './global-public-context' +import { useIsSystemFeaturesPending } from './global-public-context' type WebAppStore = { shareCode: string | null @@ -65,7 +65,7 @@ const getShareCodeFromPathname = (pathname: string): string | null => { } const WebAppStoreProvider: FC = ({ children }) => { - const isGlobalPending = useGlobalPublicStore(s => s.isGlobalPending) + const isGlobalPending = useIsSystemFeaturesPending() const updateWebAppAccessMode = useWebAppStore(state => state.updateWebAppAccessMode) const updateShareCode = useWebAppStore(state => state.updateShareCode) const updateEmbeddedUserId = useWebAppStore(state => state.updateEmbeddedUserId) diff --git a/web/hooks/use-document-title.spec.ts b/web/hooks/use-document-title.spec.ts index 3909978591..efa72cac5c 100644 --- a/web/hooks/use-document-title.spec.ts +++ b/web/hooks/use-document-title.spec.ts @@ -1,5 +1,5 @@ import { act, renderHook } from '@testing-library/react' -import { useGlobalPublicStore } from '@/context/global-public-context' +import { useGlobalPublicStore, useIsSystemFeaturesPending } from '@/context/global-public-context' /** * Test suite for useDocumentTitle hook * @@ -15,6 +15,14 @@ import { useGlobalPublicStore } from '@/context/global-public-context' import { defaultSystemFeatures } from '@/types/feature' import useDocumentTitle from './use-document-title' +vi.mock('@/context/global-public-context', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useIsSystemFeaturesPending: vi.fn(() => false), + } +}) + vi.mock('@/service/common', () => ({ getSystemFeatures: vi.fn(() => ({ ...defaultSystemFeatures })), })) @@ -24,10 +32,12 @@ vi.mock('@/service/common', () => ({ * Title should remain empty to prevent flicker */ describe('title should be empty if systemFeatures is pending', () => { - act(() => { - useGlobalPublicStore.setState({ - systemFeatures: { ...defaultSystemFeatures, branding: { ...defaultSystemFeatures.branding, enabled: false } }, - isGlobalPending: true, + beforeEach(() => { + vi.mocked(useIsSystemFeaturesPending).mockReturnValue(true) + act(() => { + useGlobalPublicStore.setState({ + systemFeatures: { ...defaultSystemFeatures, branding: { ...defaultSystemFeatures.branding, enabled: false } }, + }) }) }) /** @@ -52,9 +62,9 @@ describe('title should be empty if systemFeatures is pending', () => { */ describe('use default branding', () => { beforeEach(() => { + vi.mocked(useIsSystemFeaturesPending).mockReturnValue(false) act(() => { useGlobalPublicStore.setState({ - isGlobalPending: false, systemFeatures: { ...defaultSystemFeatures, branding: { ...defaultSystemFeatures.branding, enabled: false } }, }) }) @@ -84,9 +94,9 @@ describe('use default branding', () => { */ describe('use specific branding', () => { beforeEach(() => { + vi.mocked(useIsSystemFeaturesPending).mockReturnValue(false) act(() => { useGlobalPublicStore.setState({ - isGlobalPending: false, systemFeatures: { ...defaultSystemFeatures, branding: { ...defaultSystemFeatures.branding, enabled: true, application_title: 'Test' } }, }) }) diff --git a/web/hooks/use-document-title.ts b/web/hooks/use-document-title.ts index bb69aeb20f..37b31a7dea 100644 --- a/web/hooks/use-document-title.ts +++ b/web/hooks/use-document-title.ts @@ -1,11 +1,11 @@ 'use client' import { useFavicon, useTitle } from 'ahooks' import { useEffect } from 'react' -import { useGlobalPublicStore } from '@/context/global-public-context' +import { useGlobalPublicStore, useIsSystemFeaturesPending } from '@/context/global-public-context' import { basePath } from '@/utils/var' export default function useDocumentTitle(title: string) { - const isPending = useGlobalPublicStore(s => s.isGlobalPending) + const isPending = useIsSystemFeaturesPending() const systemFeatures = useGlobalPublicStore(s => s.systemFeatures) const prefix = title ? `${title} - ` : '' let titleStr = '' diff --git a/web/utils/setup-status.spec.ts b/web/utils/setup-status.spec.ts new file mode 100644 index 0000000000..be96b43eba --- /dev/null +++ b/web/utils/setup-status.spec.ts @@ -0,0 +1,139 @@ +import type { SetupStatusResponse } from '@/models/common' + +import { fetchSetupStatus } from '@/service/common' + +import { fetchSetupStatusWithCache } from './setup-status' + +vi.mock('@/service/common', () => ({ + fetchSetupStatus: vi.fn(), +})) + +const mockFetchSetupStatus = vi.mocked(fetchSetupStatus) + +describe('setup-status utilities', () => { + beforeEach(() => { + vi.clearAllMocks() + localStorage.clear() + }) + + describe('fetchSetupStatusWithCache', () => { + describe('when cache exists', () => { + it('should return cached finished status without API call', async () => { + localStorage.setItem('setup_status', 'finished') + + const result = await fetchSetupStatusWithCache() + + expect(result).toEqual({ step: 'finished' }) + expect(mockFetchSetupStatus).not.toHaveBeenCalled() + }) + + it('should not modify localStorage when returning cached value', async () => { + localStorage.setItem('setup_status', 'finished') + + await fetchSetupStatusWithCache() + + expect(localStorage.getItem('setup_status')).toBe('finished') + }) + }) + + describe('when cache does not exist', () => { + it('should call API and cache finished status', async () => { + const apiResponse: SetupStatusResponse = { step: 'finished' } + mockFetchSetupStatus.mockResolvedValue(apiResponse) + + const result = await fetchSetupStatusWithCache() + + expect(mockFetchSetupStatus).toHaveBeenCalledTimes(1) + expect(result).toEqual(apiResponse) + expect(localStorage.getItem('setup_status')).toBe('finished') + }) + + it('should call API and remove cache when not finished', async () => { + const apiResponse: SetupStatusResponse = { step: 'not_started' } + mockFetchSetupStatus.mockResolvedValue(apiResponse) + + const result = await fetchSetupStatusWithCache() + + expect(mockFetchSetupStatus).toHaveBeenCalledTimes(1) + expect(result).toEqual(apiResponse) + expect(localStorage.getItem('setup_status')).toBeNull() + }) + + it('should clear stale cache when API returns not_started', async () => { + localStorage.setItem('setup_status', 'some_invalid_value') + const apiResponse: SetupStatusResponse = { step: 'not_started' } + mockFetchSetupStatus.mockResolvedValue(apiResponse) + + const result = await fetchSetupStatusWithCache() + + expect(result).toEqual(apiResponse) + expect(localStorage.getItem('setup_status')).toBeNull() + }) + }) + + describe('cache edge cases', () => { + it('should call API when cache value is empty string', async () => { + localStorage.setItem('setup_status', '') + const apiResponse: SetupStatusResponse = { step: 'finished' } + mockFetchSetupStatus.mockResolvedValue(apiResponse) + + const result = await fetchSetupStatusWithCache() + + expect(mockFetchSetupStatus).toHaveBeenCalledTimes(1) + expect(result).toEqual(apiResponse) + }) + + it('should call API when cache value is not "finished"', async () => { + localStorage.setItem('setup_status', 'not_started') + const apiResponse: SetupStatusResponse = { step: 'finished' } + mockFetchSetupStatus.mockResolvedValue(apiResponse) + + const result = await fetchSetupStatusWithCache() + + expect(mockFetchSetupStatus).toHaveBeenCalledTimes(1) + expect(result).toEqual(apiResponse) + }) + + it('should call API when localStorage key does not exist', async () => { + const apiResponse: SetupStatusResponse = { step: 'finished' } + mockFetchSetupStatus.mockResolvedValue(apiResponse) + + const result = await fetchSetupStatusWithCache() + + expect(mockFetchSetupStatus).toHaveBeenCalledTimes(1) + expect(result).toEqual(apiResponse) + }) + }) + + describe('API response handling', () => { + it('should preserve setup_at from API response', async () => { + const setupDate = new Date('2024-01-01') + const apiResponse: SetupStatusResponse = { + step: 'finished', + setup_at: setupDate, + } + mockFetchSetupStatus.mockResolvedValue(apiResponse) + + const result = await fetchSetupStatusWithCache() + + expect(result).toEqual(apiResponse) + expect(result.setup_at).toEqual(setupDate) + }) + + it('should propagate API errors', async () => { + const apiError = new Error('Network error') + mockFetchSetupStatus.mockRejectedValue(apiError) + + await expect(fetchSetupStatusWithCache()).rejects.toThrow('Network error') + }) + + it('should not update cache when API call fails', async () => { + mockFetchSetupStatus.mockRejectedValue(new Error('API error')) + + await expect(fetchSetupStatusWithCache()).rejects.toThrow() + + expect(localStorage.getItem('setup_status')).toBeNull() + }) + }) + }) +}) diff --git a/web/utils/setup-status.ts b/web/utils/setup-status.ts new file mode 100644 index 0000000000..7a2810bffd --- /dev/null +++ b/web/utils/setup-status.ts @@ -0,0 +1,21 @@ +import type { SetupStatusResponse } from '@/models/common' +import { fetchSetupStatus } from '@/service/common' + +const SETUP_STATUS_KEY = 'setup_status' + +const isSetupStatusCached = (): boolean => + localStorage.getItem(SETUP_STATUS_KEY) === 'finished' + +export const fetchSetupStatusWithCache = async (): Promise => { + if (isSetupStatusCached()) + return { step: 'finished' } + + const status = await fetchSetupStatus() + + if (status.step === 'finished') + localStorage.setItem(SETUP_STATUS_KEY, 'finished') + else + localStorage.removeItem(SETUP_STATUS_KEY) + + return status +}