mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(client): stop the startup redirect racing the settings request
RootRedirect waited for whoever else might call loadSettings, which on a cold start meant waiting for a request queued behind everything else a launch fires off. Traced it in the browser: the settings came back as the 388th request, long after the redirect had given up and gone to the dashboard. That is what "it still just opens the dashboard" was. It now asks for the settings itself - the store de-dupes, so this costs nothing when a load is already in flight - and login spends its 2.6s takeoff animation fetching them instead of leaving that time idle, so by the time it hands over they are already there. Also folds the five copies of setShowTakeoff + navigate-after-2600ms into one takeOff(), since all of them needed the same fix.
This commit is contained in:
@@ -421,6 +421,20 @@ describe('RootRedirect — preference not mirrored on this device', () => {
|
||||
await waitFor(() => expect(screen.getByText('TripPlanner')).toBeInTheDocument())
|
||||
})
|
||||
|
||||
// Waiting for someone else's effect to fetch the settings made the decision
|
||||
// ride on where that request landed in the queue — behind ~380 others on a
|
||||
// cold start, which is how the redirect silently lost the race and fell
|
||||
// through to the dashboard. It now asks for them itself.
|
||||
it('FE-COMP-APP-033: asks for the settings itself instead of waiting to be told', async () => {
|
||||
const loadSettings = vi.fn().mockResolvedValue(undefined)
|
||||
seedAuth({ isAuthenticated: true, user: buildUser() })
|
||||
useSettingsStore.setState({ isLoaded: false, loadSettings })
|
||||
|
||||
renderApp('/')
|
||||
|
||||
await waitFor(() => expect(loadSettings).toHaveBeenCalled())
|
||||
})
|
||||
|
||||
// loadSettings leaves isLoaded false on a failed request so it can retry, so
|
||||
// the wait above needs a floor or a launch with no backend never resolves.
|
||||
it('FE-COMP-APP-032: gives up on the settings after the timeout and opens the dashboard', async () => {
|
||||
|
||||
+9
-1
@@ -190,6 +190,7 @@ function RootRedirect() {
|
||||
const { isAuthenticated, isLoading } = useAuthStore()
|
||||
const settingsLoaded = useSettingsStore((s) => s.isLoaded)
|
||||
const settings = useSettingsStore((s) => s.settings)
|
||||
const loadSettings = useSettingsStore((s) => s.loadSettings)
|
||||
const [target, setTarget] = useState<string | null>(null)
|
||||
// Bounds the wait below: loadSettings deliberately leaves isLoaded false on a
|
||||
// failed request so it can retry, which would otherwise strand a launch that
|
||||
@@ -205,7 +206,14 @@ function RootRedirect() {
|
||||
useEffect(() => {
|
||||
if (isLoading || !isAuthenticated || target) return
|
||||
const mirrored = readStartDestination()
|
||||
if (!mirrored && !settingsLoaded && !settingsGaveUp) return
|
||||
if (!mirrored && !settingsLoaded && !settingsGaveUp) {
|
||||
// Ask for the settings instead of waiting for whoever else might. The
|
||||
// store de-dupes concurrent loads, so this is free when one is already in
|
||||
// flight — and it stops the decision from riding on where that request
|
||||
// happens to land in the queue behind everything else a launch fires off.
|
||||
loadSettings()
|
||||
return
|
||||
}
|
||||
// Loaded settings are the truth; the mirror is what we knew last time.
|
||||
const { page, tab } = settingsLoaded
|
||||
? { page: settings.start_page ?? DEFAULT_START_PAGE, tab: settings.start_trip_tab ?? DEFAULT_START_TRIP_TAB }
|
||||
|
||||
@@ -62,7 +62,7 @@ export function useLogin() {
|
||||
const [confirmPassword, setConfirmPassword] = useState('')
|
||||
|
||||
const { login, register, demoLogin, completeMfaLogin, loadUser } = useAuthStore()
|
||||
const { setLanguageLocal, setLanguageTransient } = useSettingsStore()
|
||||
const { setLanguageLocal, setLanguageTransient, loadSettings } = useSettingsStore()
|
||||
const navigate = useNavigate()
|
||||
const location = useLocation()
|
||||
const noRedirect = !!(location.state as { noRedirect?: boolean } | null)?.noRedirect
|
||||
@@ -85,6 +85,18 @@ export function useLogin() {
|
||||
}
|
||||
}, [redirectTarget])
|
||||
|
||||
/**
|
||||
* Start the takeoff animation, and spend those 2.6 seconds fetching the
|
||||
* settings instead of letting them sit idle. Whoever we hand over to then
|
||||
* already has them — which matters for the startup destination, decided the
|
||||
* moment we navigate, and otherwise racing this exact request.
|
||||
*/
|
||||
const takeOff = (): void => {
|
||||
setShowTakeoff(true)
|
||||
loadSettings()
|
||||
setTimeout(() => navigate(redirectTarget), 2600)
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
const params = new URLSearchParams(window.location.search)
|
||||
|
||||
@@ -197,8 +209,7 @@ export function useLogin() {
|
||||
setIsLoading(true)
|
||||
try {
|
||||
await demoLogin()
|
||||
setShowTakeoff(true)
|
||||
setTimeout(() => navigate(redirectTarget), 2600)
|
||||
takeOff()
|
||||
} catch (err: unknown) {
|
||||
setError(err instanceof Error ? err.message : t('login.demoFailed'))
|
||||
} finally {
|
||||
@@ -214,8 +225,7 @@ export function useLogin() {
|
||||
const assertion = await startAuthentication({ optionsJSON: options })
|
||||
await authApi.passkey.loginVerify(assertion)
|
||||
await loadUser({ silent: true })
|
||||
setShowTakeoff(true)
|
||||
setTimeout(() => navigate(redirectTarget), 2600)
|
||||
takeOff()
|
||||
} catch (err: unknown) {
|
||||
// The user dismissing the native prompt isn't an error worth surfacing.
|
||||
const name = (err as { name?: string })?.name
|
||||
@@ -240,8 +250,7 @@ export function useLogin() {
|
||||
if (newPassword !== confirmPassword) { setError(t('settings.passwordMismatch')); setIsLoading(false); return }
|
||||
await authApi.changePassword({ current_password: savedLoginPassword, new_password: newPassword })
|
||||
await loadUser({ silent: true })
|
||||
setShowTakeoff(true)
|
||||
setTimeout(() => navigate(redirectTarget), 2600)
|
||||
takeOff()
|
||||
return
|
||||
}
|
||||
if (mode === 'login' && mfaStep) {
|
||||
@@ -257,8 +266,7 @@ export function useLogin() {
|
||||
setIsLoading(false)
|
||||
return
|
||||
}
|
||||
setShowTakeoff(true)
|
||||
setTimeout(() => navigate(redirectTarget), 2600)
|
||||
takeOff()
|
||||
return
|
||||
}
|
||||
if (mode === 'register') {
|
||||
@@ -288,8 +296,7 @@ export function useLogin() {
|
||||
return
|
||||
}
|
||||
}
|
||||
setShowTakeoff(true)
|
||||
setTimeout(() => navigate(redirectTarget), 2600)
|
||||
takeOff()
|
||||
} catch (err: unknown) {
|
||||
setError(getApiErrorMessage(err, t('login.error')))
|
||||
setIsLoading(false)
|
||||
|
||||
Reference in New Issue
Block a user