(BSR) refactor(useMustUpdateApp): use isLoading to not return a value…

… before getting data (#7644)

src/features/errors/pages/ScreenErrorProvider.tsx

@@ -1,6 +1,6 @@
 import React from 'react'
 
-import { useMustUpdateApp } from 'features/forceUpdate/helpers/useMustUpdateApp'
+import { MustUpdateAppState, useMustUpdateApp } from 'features/forceUpdate/helpers/useMustUpdateApp'
 import { ForceUpdateWithResetErrorBoundary } from 'features/forceUpdate/pages/ForceUpdateWithResetErrorBoundary'
 import { useMaintenance } from 'features/maintenance/helpers/useMaintenance/useMaintenance'
 import { MaintenanceErrorPage } from 'features/maintenance/pages/MaintenanceErrorPage'
@@ -17,7 +17,7 @@ export const ScreenErrorProvider = ({
   const mustUpdateApp = useMustUpdateApp()
   const { logType } = useLogTypeFromRemoteConfig()
 
-  if (mustUpdateApp) {
+  if (mustUpdateApp === MustUpdateAppState.SHOULD_UPDATE) {
     throw new ScreenError('Must update app', {
       Screen: ForceUpdateWithResetErrorBoundary,
       logType,

src/features/forceUpdate/helpers/useMinimalBuildNumber.native.test.ts

@@ -19,7 +19,7 @@ describe('useMinimalBuildNumber', () => {
     const { result } = renderUseMinimalBuildNumber()
 
     await waitFor(() => {
-      expect(result.current).toEqual(10306000)
+      expect(result.current.minimalBuildNumber).toEqual(10306000)
     })
   })
 
@@ -28,7 +28,7 @@ describe('useMinimalBuildNumber', () => {
 
     const { result } = renderUseMinimalBuildNumber()
 
-    expect(result.current).toEqual(undefined)
+    expect(result.current.minimalBuildNumber).toEqual(undefined)
   })
 })
 

src/features/forceUpdate/helpers/useMinimalBuildNumber.ts

@@ -4,7 +4,7 @@ import { getMinimalBuildNumber } from 'libs/firebase/firestore/getMinimalBuildNu
 import { QueryKeys } from 'libs/queryKeys'
 
 export const useMinimalBuildNumber = () => {
-  const { data: minimalBuildNumber } = useQuery(
+  const { data: minimalBuildNumber, isLoading } = useQuery(
     QueryKeys.MINIMAL_BUILD_NUMBER,
     getMinimalBuildNumber,
     {
@@ -14,5 +14,5 @@ export const useMinimalBuildNumber = () => {
     }
   )
 
-  return minimalBuildNumber
+  return { minimalBuildNumber, isLoading }
 }

src/features/forceUpdate/helpers/useMustUpdateApp.native.test.ts

@@ -0,0 +1,58 @@
+import * as useMinimalBuildNumber from 'features/forceUpdate/helpers/useMinimalBuildNumber'
+import { MustUpdateAppState, useMustUpdateApp } from 'features/forceUpdate/helpers/useMustUpdateApp'
+import * as PackageJson from 'libs/packageJson'
+import { reactQueryProviderHOC } from 'tests/reactQueryProviderHOC'
+import { act, renderHook } from 'tests/utils'
+
+jest.mock('@react-native-firebase/firestore')
+
+const useGetMinimalBuildNumberSpy = jest.spyOn(useMinimalBuildNumber, 'useMinimalBuildNumber')
+
+const buildVersion = 10_304_000
+jest.spyOn(PackageJson, 'getAppBuildVersion').mockReturnValue(buildVersion)
+
+describe('useMustUpdateApp', () => {
+  it('should not update when the minimal build number is lower than local version', async () => {
+    useGetMinimalBuildNumberSpy.mockReturnValueOnce({
+      minimalBuildNumber: 10_303_999,
+      isLoading: false,
+    })
+
+    const { result } = renderUseMustUpdateApp()
+
+    await act(() => {})
+
+    expect(result.current).toEqual(MustUpdateAppState.SHOULD_NOT_UPDATE)
+  })
+
+  it('should update when the minimal build number is higher than local version', async () => {
+    useGetMinimalBuildNumberSpy.mockReturnValueOnce({
+      minimalBuildNumber: 10_304_001,
+      isLoading: false,
+    })
+
+    const { result } = renderUseMustUpdateApp()
+
+    await act(() => {})
+
+    expect(result.current).toEqual(MustUpdateAppState.SHOULD_UPDATE)
+  })
+
+  it('should not update when the minimal build number is equal to the local version', async () => {
+    useGetMinimalBuildNumberSpy.mockReturnValueOnce({
+      minimalBuildNumber: buildVersion,
+      isLoading: false,
+    })
+
+    const { result } = renderUseMustUpdateApp()
+
+    await act(() => {})
+
+    expect(result.current).toEqual(MustUpdateAppState.SHOULD_NOT_UPDATE)
+  })
+})
+
+const renderUseMustUpdateApp = () =>
+  renderHook(useMustUpdateApp, {
+    wrapper: ({ children }) => reactQueryProviderHOC(children),
+  })

src/features/forceUpdate/helpers/useMustUpdateApp.ts

@@ -1,33 +1,45 @@
-import { useEffect, useRef } from 'react'
+import { useEffect } from 'react'
 
 import { useMinimalBuildNumber } from 'features/forceUpdate/helpers/useMinimalBuildNumber'
 import { eventMonitoring } from 'libs/monitoring/services'
 import { getAppBuildVersion } from 'libs/packageJson'
 
 const DELAY_BEFORE_VALUE_SHOULD_BE_SET_IN_MS = 15000
 
-export const useMustUpdateApp = () => {
-  const minimalBuildNumber = useRef<number | undefined>(undefined)
-  minimalBuildNumber.current = useMinimalBuildNumber()
-  const mustUpdateApp =
-    !!minimalBuildNumber.current && getAppBuildVersion() < minimalBuildNumber.current
+export enum MustUpdateAppState {
+  PENDING = 'pending',
+  SHOULD_UPDATE = 'shouldUpdate',
+  SHOULD_NOT_UPDATE = 'shouldNotUpdate',
+}
+
+export const useMustUpdateApp: () => MustUpdateAppState = () => {
+  const { minimalBuildNumber, isLoading } = useMinimalBuildNumber()
+  const appBuildVersion = getAppBuildVersion()
 
   useEffect(() => {
     const timer = globalThis.setTimeout(() => {
       if (!minimalBuildNumber) {
         eventMonitoring.captureException(new Error('MustUpdateNoMinimalBuildNumberError'), {
           extra: {
-            mustUpdateApp,
             minimalBuildNumber,
-            build: getAppBuildVersion(),
+            build: appBuildVersion,
           },
         })
       }
     }, DELAY_BEFORE_VALUE_SHOULD_BE_SET_IN_MS)
     return () => {
       clearInterval(timer)
     }
-  }, [mustUpdateApp])
+  }, [appBuildVersion, minimalBuildNumber])
+
+  if (isLoading) return MustUpdateAppState.PENDING
+
+  const isLocalBuildSmallerThanMinimalBuild =
+    !!minimalBuildNumber && appBuildVersion < minimalBuildNumber
+
+  const mustUpdateApp = isLocalBuildSmallerThanMinimalBuild
+    ? MustUpdateAppState.SHOULD_UPDATE
+    : MustUpdateAppState.SHOULD_NOT_UPDATE
 
   return mustUpdateApp
 }

src/features/forceUpdate/helpers/useResetOnMinimalBuild.native.test.ts

@@ -32,7 +32,10 @@ describe('useResetOnMinimalBuild', () => {
   })
 
   it('should not call resetErrorBoundary if minimalBuildNumber is null', () => {
-    jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce(undefined)
+    jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce({
+      minimalBuildNumber: undefined,
+      isLoading: false,
+    })
     jest.spyOn(packageJson, 'getAppBuildVersion').mockReturnValueOnce(123)
 
     renderHookWithProvider(() => useResetOnMinimalBuild(mockResetErrorBoundary))
@@ -41,7 +44,10 @@ describe('useResetOnMinimalBuild', () => {
   })
 
   it('should not call resetErrorBoundary if minimalBuildNumber is greater than app build version', () => {
-    jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce(200)
+    jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce({
+      minimalBuildNumber: 200,
+      isLoading: false,
+    })
     jest.spyOn(packageJson, 'getAppBuildVersion').mockReturnValueOnce(123)
 
     renderHookWithProvider(() => useResetOnMinimalBuild(mockResetErrorBoundary))
@@ -50,7 +56,10 @@ describe('useResetOnMinimalBuild', () => {
   })
 
   it('should call resetErrorBoundary if minimalBuildNumber is less than or equal to app build version', () => {
-    jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce(100)
+    jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce({
+      minimalBuildNumber: 100,
+      isLoading: false,
+    })
     jest.spyOn(packageJson, 'getAppBuildVersion').mockReturnValueOnce(123)
 
     renderHookWithProvider(() => useResetOnMinimalBuild(mockResetErrorBoundary))

src/features/forceUpdate/helpers/useResetOnMinimalBuild.ts

@@ -4,18 +4,16 @@ import { useMinimalBuildNumber } from 'features/forceUpdate/helpers/useMinimalBu
 import { getAppBuildVersion } from 'libs/packageJson'
 
 export function useResetOnMinimalBuild(resetErrorBoundary: () => void) {
-  const minimalBuildNumber = useMinimalBuildNumber()
+  const { minimalBuildNumber, isLoading } = useMinimalBuildNumber()
 
   // This first hook will be like a componentWillUnmount
-  // eslint-disable-next-line react-hooks/exhaustive-deps
-  useEffect(() => resetErrorBoundary, [])
+  useEffect(() => resetErrorBoundary, [resetErrorBoundary])
 
   // This one is for when minimalBuildNumber gets back to an older value
   useEffect(() => {
     // it must be false and not null (which means not fetched)
-    if (!!minimalBuildNumber && getAppBuildVersion() >= minimalBuildNumber) {
+    if (!isLoading && minimalBuildNumber && getAppBuildVersion() >= minimalBuildNumber) {
       resetErrorBoundary()
     }
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [minimalBuildNumber])
+  }, [isLoading, minimalBuildNumber, resetErrorBoundary])
 }

src/features/forceUpdate/pages/ForceUpdateWithResetErrorBoundary.native.test.tsx

@@ -1,5 +1,6 @@
 import React from 'react'
 
+import * as useMinimalBuildNumberModule from 'features/forceUpdate/helpers/useMinimalBuildNumber'
 import { render, screen } from 'tests/utils'
 
 import { ForceUpdateWithResetErrorBoundary } from './ForceUpdateWithResetErrorBoundary'
@@ -14,6 +15,11 @@ jest.mock('react-native/Libraries/Animated/createAnimatedComponent', () => {
 
 describe('<ForceUpdateWithResetErrorBoundary/>', () => {
   it('should match snapshot', async () => {
+    jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce({
+      minimalBuildNumber: 10_304_000,
+      isLoading: false,
+    })
+
     await render(<ForceUpdateWithResetErrorBoundary resetErrorBoundary={() => null} />)
 
     expect(screen).toMatchSnapshot()

src/features/forceUpdate/pages/ForceUpdateWithResetErrorBoundary.web.test.tsx

@@ -1,5 +1,6 @@
 import React from 'react'
 
+import * as useMinimalBuildNumberModule from 'features/forceUpdate/helpers/useMinimalBuildNumber'
 import { checkAccessibilityFor, render } from 'tests/utils/web'
 
 import { ForceUpdateWithResetErrorBoundary } from './ForceUpdateWithResetErrorBoundary'
@@ -11,6 +12,11 @@ jest.mock('features/forceUpdate/helpers/useMinimalBuildNumber')
 describe('<ForceUpdateWithResetErrorBoundary/>', () => {
   describe('Accessibility', () => {
     it('should not have basic accessibility issues', async () => {
+      jest.spyOn(useMinimalBuildNumberModule, 'useMinimalBuildNumber').mockReturnValueOnce({
+        minimalBuildNumber: 10_304_000,
+        isLoading: false,
+      })
+
       const { container } = render(
         <ForceUpdateWithResetErrorBoundary resetErrorBoundary={() => null} />
       )

src/features/navigation/RootNavigator/RootNavigator.native.test.tsx

@@ -1,7 +1,7 @@
 import { NavigationContainer } from '@react-navigation/native'
 import React from 'react'
 
-import { useMustUpdateApp } from 'features/forceUpdate/helpers/useMustUpdateApp'
+import { MustUpdateAppState, useMustUpdateApp } from 'features/forceUpdate/helpers/useMustUpdateApp'
 import { setFeatureFlags } from 'libs/firebase/firestore/featureFlags/__tests__/setFeatureFlags'
 import { useSplashScreenContext } from 'libs/splashscreen'
 import { storage } from 'libs/storage'
@@ -50,12 +50,12 @@ describe('<RootNavigator />', () => {
   })
 
   beforeEach(() => {
-    mockedUseMustUpdateApp.mockReturnValue(true)
+    mockedUseMustUpdateApp.mockReturnValue(MustUpdateAppState.SHOULD_NOT_UPDATE)
     storage.clear('logged_in_session_count')
   })
 
   it('should NOT display PrivacyPolicy if splash screen is not yet hidden', async () => {
-    mockedUseMustUpdateApp.mockReturnValueOnce(false)
+    mockedUseMustUpdateApp.mockReturnValueOnce(MustUpdateAppState.SHOULD_NOT_UPDATE)
     mockUseSplashScreenContext.mockReturnValueOnce({ isSplashScreenHidden: false })
     renderRootNavigator()
 
@@ -65,7 +65,7 @@ describe('<RootNavigator />', () => {
   })
 
   it('should display PrivacyPolicy if splash screen is hidden', async () => {
-    mockedUseMustUpdateApp.mockReturnValueOnce(false)
+    mockedUseMustUpdateApp.mockReturnValueOnce(MustUpdateAppState.SHOULD_NOT_UPDATE)
     mockUseSplashScreenContext.mockReturnValueOnce({ isSplashScreenHidden: true })
 
     renderRootNavigator()