mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(settings): retry settings load on reconnect so region prefs stop resetting (#1626)
This commit is contained in:
committed by
GitHub
parent
b6a3cced43
commit
a3cb783df7
@@ -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<void> | null = null
|
||||
|
||||
export const useSettingsStore = create<SettingsState>((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]) => {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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<void>) | 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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user