Merge remote-tracking branch 'origin/main' into refactor/setup-status-cache

Add useIsSystemFeaturesPending to mock exports in:
- embedded-user-id-store.test.tsx
- installForm.spec.tsx

web/__tests__/embedded-user-id-store.test.tsx

@@ -53,6 +53,7 @@ vi.mock('@/context/global-public-context', () => {
   )
   return {
     useGlobalPublicStore,
+    useIsSystemFeaturesPending: () => false,
   }
 })
 

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)

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,
   })
 }
 

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<typeof import('@/context/global-public-context')>()
+  return {
+    ...actual,
+    useIsSystemFeaturesPending: () => false,
+  }
+})
+
 const mockFetchSetupStatus = vi.mocked(fetchSetupStatus)
 const mockFetchInitValidateStatus = vi.mocked(fetchInitValidateStatus)
 const mockSetup = vi.mocked(setup)

web/context/global-public-context.tsx

@@ -1,45 +1,62 @@
 'use client'
 import type { FC, PropsWithChildren } from 'react'
 import type { SystemFeatures } from '@/types/feature'
-import { useQuery } from '@tanstack/react-query'
-import { useEffect } from 'react'
+import { useQueries, useQuery } from '@tanstack/react-query'
 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<GlobalPublicStore>(set => ({
-  isGlobalPending: true,
-  setIsGlobalPending: (isPending: boolean) => set(() => ({ isGlobalPending: isPending })),
   systemFeatures: defaultSystemFeatures,
   setSystemFeatures: (systemFeatures: SystemFeatures) => set(() => ({ systemFeatures })),
 }))
 
+const systemFeaturesQueryKey = ['systemFeatures'] as const
+
+async function fetchSystemFeatures() {
+  const data = await getSystemFeatures()
+  const { setSystemFeatures } = useGlobalPublicStore.getState()
+  setSystemFeatures({ ...defaultSystemFeatures, ...data })
+  return data
+}
+
+export function useIsSystemFeaturesPending() {
+  const { isPending } = useQuery({
+    queryKey: systemFeaturesQueryKey,
+    queryFn: fetchSystemFeatures,
+  })
+  return isPending
+}
+
 const GlobalPublicStoreProvider: FC<PropsWithChildren> = ({
   children,
 }) => {
-  const { isPending, data } = useQuery({
-    queryKey: ['systemFeatures'],
-    queryFn: getSystemFeatures,
+  // Fetch systemFeatures and setupStatus in parallel to reduce waterfall.
+  // setupStatus is prefetched here and cached in localStorage for AppInitializer.
+  // We only destructure featuresQuery since setupStatus result is not used directly.
+  const [featuresQuery] = useQueries({
+    queries: [
+      {
+        queryKey: systemFeaturesQueryKey,
+        queryFn: fetchSystemFeatures,
+      },
+      {
+        queryKey: ['setupStatus'],
+        queryFn: fetchSetupStatusWithCache,
+        staleTime: Infinity, // Once fetched, no need to refetch
+      },
+    ],
   })
-  const { setSystemFeatures, setIsGlobalPending: setIsPending } = useGlobalPublicStore()
-  useEffect(() => {
-    if (data)
-      setSystemFeatures({ ...defaultSystemFeatures, ...data })
-  }, [data, setSystemFeatures])
 
-  useEffect(() => {
-    setIsPending(isPending)
-  }, [isPending, setIsPending])
-
-  if (isPending)
+  // Only block on systemFeatures, setupStatus is prefetched for AppInitializer
+  if (featuresQuery.isPending)
     return <div className="flex h-screen w-screen items-center justify-center"><Loading /></div>
   return <>{children}</>
 }

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<PropsWithChildren> = ({ 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)

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<typeof import('@/context/global-public-context')>()
+  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' } },
       })
     })

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 = ''

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()
+      })
+    })
+  })
+})

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<SetupStatusResponse> => {
+  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
+}