diff --git a/client/src/store/settingsStore.ts b/client/src/store/settingsStore.ts index e58499106..085a9ae00 100644 --- a/client/src/store/settingsStore.ts +++ b/client/src/store/settingsStore.ts @@ -52,21 +52,34 @@ export const DEFAULT_SETTINGS: Settings = { // chosen" (fall back to home + defaults) from an explicitly emptied list. } +// De-dupe concurrent loads: the reconnection triggers (online / visibility / +// periodic) can all fire a retry at once — collapse them into one in-flight GET. +let _loadInFlight: Promise | null = null + export const useSettingsStore = create((set, get) => ({ settings: { ...DEFAULT_SETTINGS }, isLoaded: false, loadSettings: async () => { - try { - const data = await settingsApi.get() - set((state) => ({ - settings: { ...state.settings, ...data.settings }, - isLoaded: true, - })) - } catch (err: unknown) { - set({ isLoaded: true }) - console.error('Failed to load settings:', err) - } + if (_loadInFlight) return _loadInFlight + _loadInFlight = (async () => { + try { + const data = await settingsApi.get() + set((state) => ({ + settings: { ...state.settings, ...data.settings }, + isLoaded: true, + })) + } catch (err: unknown) { + // Leave isLoaded false so a transient failure — offline at launch, or a + // (docker) server cold-start racing the first request — is retried on + // reconnect instead of stranding the user on built-in defaults (wrong + // currency/units) for the whole session (#1618). + console.error('Failed to load settings:', err) + } finally { + _loadInFlight = null + } + })() + return _loadInFlight }, updateSetting: async (key: keyof Settings, value: Settings[keyof Settings]) => { diff --git a/client/src/sync/syncTriggers.ts b/client/src/sync/syncTriggers.ts index a94aaf1fd..60867ee0d 100644 --- a/client/src/sync/syncTriggers.ts +++ b/client/src/sync/syncTriggers.ts @@ -17,6 +17,8 @@ import { tripSyncManager } from './tripSyncManager' import { isEffectivelyOnline, onNetworkModeChange } from './networkMode' import { setPreReconnectHook, setRefetchCallback, getActiveTrips } from '../api/websocket' import { useTripStore } from '../store/tripStore' +import { useSettingsStore } from '../store/settingsStore' +import { useAuthStore } from '../store/authStore' const PERIODIC_MS = 30_000 @@ -25,6 +27,20 @@ let _unsubscribeNetworkMode: (() => void) | null = null let _wasEffectivelyOnline = isEffectivelyOnline() let _registered = false +/** + * User settings are loaded once on the auth transition (App.tsx), with no retry. + * If that single GET fails — offline at launch, or a server cold-start racing the + * first request — the store stays on built-in DEFAULT_SETTINGS and the user sees + * their currency/units/time-format "reset" for the whole session (#1618). Heal it + * on the same connectivity triggers that re-hydrate trips: retry until it loads. + */ +function ensureSettingsLoaded() { + if (!useAuthStore.getState().isAuthenticated) return + if (!isEffectivelyOnline()) return + if (useSettingsStore.getState().isLoaded) return + useSettingsStore.getState().loadSettings().catch(console.error) +} + /** Pull the latest server state for every open trip into the Zustand store. */ function rehydrateActiveTrips() { const store = useTripStore.getState() @@ -44,6 +60,7 @@ function onOnline() { // edits from the cache/UI. Stay put until the user lifts the switch (which // routes through onNetworkMode → here with the force flag already cleared). if (!isEffectivelyOnline()) return + ensureSettingsLoaded() mutationQueue.flush() .catch(console.error) .finally(() => { @@ -55,6 +72,7 @@ function onOnline() { /** Tab became visible — flush only; don't trigger a potentially expensive syncAll. */ function onVisibility() { if (!document.hidden && isEffectivelyOnline()) { + ensureSettingsLoaded() mutationQueue.flush().catch(console.error) } } @@ -62,6 +80,7 @@ function onVisibility() { /** Periodic heartbeat — drain any lingering pending mutations. */ function onPeriodic() { if (isEffectivelyOnline()) { + ensureSettingsLoaded() mutationQueue.flush().catch(console.error) } } diff --git a/client/tests/unit/stores/settingsStore.test.ts b/client/tests/unit/stores/settingsStore.test.ts index eb26319a4..8f88f02da 100644 --- a/client/tests/unit/stores/settingsStore.test.ts +++ b/client/tests/unit/stores/settingsStore.test.ts @@ -66,7 +66,7 @@ describe('settingsStore', () => { }); describe('FE-SETTINGS-005: loadSettings failure', () => { - it('sets isLoaded: true even on API failure (graceful)', async () => { + it('leaves isLoaded false on API failure so the load is retried (#1618)', async () => { server.use( http.get('/api/settings', () => HttpResponse.json({ error: 'Server error' }, { status: 500 }) @@ -74,9 +74,31 @@ describe('settingsStore', () => { ); await useSettingsStore.getState().loadSettings(); - const state = useSettingsStore.getState(); + // A transient failure must NOT be treated as "loaded" — otherwise the store + // stays on built-in DEFAULT_SETTINGS (wrong currency/units) for the whole + // session with no retry. isLoaded stays false so the reconnect triggers retry. + expect(useSettingsStore.getState().isLoaded).toBe(false); + }); + + it('recovers on a later retry after an initial failure (#1618)', async () => { + server.use( + http.get('/api/settings', () => + HttpResponse.json({ error: 'Server error' }, { status: 500 }) + ) + ); + await useSettingsStore.getState().loadSettings(); + expect(useSettingsStore.getState().isLoaded).toBe(false); + + const settings = buildSettings({ default_currency: 'EUR' }); + server.use( + http.get('/api/settings', () => HttpResponse.json({ settings })) + ); + await useSettingsStore.getState().loadSettings(); + + const state = useSettingsStore.getState(); expect(state.isLoaded).toBe(true); + expect(state.settings.default_currency).toBe('EUR'); }); }); diff --git a/client/tests/unit/sync/syncTriggers.test.ts b/client/tests/unit/sync/syncTriggers.test.ts index ad3bc2656..fb1aa6eb3 100644 --- a/client/tests/unit/sync/syncTriggers.test.ts +++ b/client/tests/unit/sync/syncTriggers.test.ts @@ -9,9 +9,12 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; const flush = vi.fn(() => Promise.resolve()); const syncAll = vi.fn(() => Promise.resolve()); const hydrate = vi.fn(() => Promise.resolve()); +const loadSettings = vi.fn(() => Promise.resolve()); let refetchCb: ((tripId: string) => void) | null = null; let preReconnect: (() => Promise) | null = null; +let settingsLoaded = false; +let authenticated = true; vi.mock('../../../src/sync/mutationQueue', () => ({ mutationQueue: { flush: () => flush() }, @@ -27,6 +30,12 @@ vi.mock('../../../src/api/websocket', () => ({ vi.mock('../../../src/store/tripStore', () => ({ useTripStore: { getState: () => ({ hydrateActiveTrip: hydrate }) }, })); +vi.mock('../../../src/store/settingsStore', () => ({ + useSettingsStore: { getState: () => ({ isLoaded: settingsLoaded, loadSettings }) }, +})); +vi.mock('../../../src/store/authStore', () => ({ + useAuthStore: { getState: () => ({ isAuthenticated: authenticated }) }, +})); import { registerSyncTriggers, unregisterSyncTriggers } from '../../../src/sync/syncTriggers'; @@ -35,8 +44,9 @@ const flushMicrotasks = async () => { }; beforeEach(() => { - flush.mockClear(); syncAll.mockClear(); hydrate.mockClear(); + flush.mockClear(); syncAll.mockClear(); hydrate.mockClear(); loadSettings.mockClear(); refetchCb = null; preReconnect = null; + settingsLoaded = false; authenticated = true; Object.defineProperty(navigator, 'onLine', { value: true, writable: true, configurable: true }); }); @@ -73,4 +83,30 @@ describe('syncTriggers', () => { expect(syncAll).toHaveBeenCalled(); expect(hydrate).toHaveBeenCalledWith('7'); }); + + it('online event retries the settings load when it has not yet succeeded (#1618)', async () => { + registerSyncTriggers(); + window.dispatchEvent(new Event('online')); + await flushMicrotasks(); + + expect(loadSettings).toHaveBeenCalled(); + }); + + it('does not retry the settings load once it has loaded', async () => { + settingsLoaded = true; + registerSyncTriggers(); + window.dispatchEvent(new Event('online')); + await flushMicrotasks(); + + expect(loadSettings).not.toHaveBeenCalled(); + }); + + it('does not retry the settings load while unauthenticated', async () => { + authenticated = false; + registerSyncTriggers(); + window.dispatchEvent(new Event('online')); + await flushMicrotasks(); + + expect(loadSettings).not.toHaveBeenCalled(); + }); });