diff --git a/frontend/src/utils/siteSettings.js b/frontend/src/utils/siteSettings.js index 87333a77..6be729ef 100644 --- a/frontend/src/utils/siteSettings.js +++ b/frontend/src/utils/siteSettings.js @@ -5,19 +5,38 @@ import { settingsApi } from '@/api' import { withBase } from '@/utils/basePath' let settingsCache = null +let inFlight = null +let retryAfter = 0 +// A failed load is cached briefly, not for the page session. A transient blip +// used to blank every settings-driven link until the operator reloaded, and +// they have no reason to know that would fix it. A short window still bounds a +// hard-down backend to two requests a minute per client. +const FAILURE_RETRY_MS = 30000 + +// The in-flight promise is cached alongside the result. Callers that arrive +// during the first load share one request instead of each firing their own; +// applyBranding() alone awaits five settings in sequence at bootstrap. async function loadSettings() { if (settingsCache) return settingsCache - try { - const response = await settingsApi.list() - const items = response.data?.data || response.data || [] - settingsCache = {} - for (const s of items) settingsCache[s.key] = s.value - } catch (err) { - console.error('Error loading site settings:', err) - settingsCache = {} - } - return settingsCache + if (inFlight) return inFlight + if (retryAfter && Date.now() < retryAfter) return {} + inFlight = settingsApi.list() + .then((response) => { + const items = response.data?.data || response.data || [] + const loaded = {} + for (const s of items) loaded[s.key] = s.value + settingsCache = loaded + retryAfter = 0 + return loaded + }) + .catch((error) => { + console.error('Error loading site settings:', error) + retryAfter = Date.now() + FAILURE_RETRY_MS + return {} + }) + .finally(() => { inFlight = null }) + return inFlight } export async function getSetting(key, fallback = '') { diff --git a/frontend/src/utils/siteSettings.spec.js b/frontend/src/utils/siteSettings.spec.js new file mode 100644 index 00000000..7174329d --- /dev/null +++ b/frontend/src/utils/siteSettings.spec.js @@ -0,0 +1,113 @@ +// Site settings load once, and a blip does not blank the page for good. +// +// Two bugs motivated these. The cache used to hold only the RESOLVED object, so +// every caller arriving before the first response fired its own request - +// applyBranding() alone awaits five settings in sequence at bootstrap. And a +// failed load cached {} for the page session, so one bad GET silently blanked +// every settings-driven link (the Intune/Entra buttons) until a manual reload. + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' + +const list = vi.fn() + +vi.mock('@/api', () => ({ settingsApi: { list: () => list() } })) + +// The module caches in module scope, so each test needs a fresh copy. +async function freshModule() { + vi.resetModules() + return import('./siteSettings') +} + +function respond(items) { + return { data: { data: items } } +} + +beforeEach(() => { + list.mockReset() + vi.useFakeTimers() +}) + +afterEach(() => { + vi.useRealTimers() +}) + +describe('loading', () => { + it('fetches once for concurrent callers rather than once per caller', async () => { + let release + list.mockReturnValue(new Promise((resolve) => { release = resolve })) + const settings = await freshModule() + + const waiting = Promise.all([ + settings.getSetting('entra_device_url'), + settings.getSetting('intune_device_url'), + settings.getFacilityName(), + ]) + release(respond([{ key: 'entra_device_url', value: 'https://entra/{id}' }])) + const [entra, intune, facility] = await waiting + + expect(list).toHaveBeenCalledTimes(1) + expect(entra).toBe('https://entra/{id}') + expect(intune).toBe('') + expect(facility).toBe('ShopDB') + }) + + it('serves later callers from cache without refetching', async () => { + list.mockResolvedValue(respond([{ key: 'facility_name', value: 'Wilmington' }])) + const settings = await freshModule() + + expect(await settings.getFacilityName()).toBe('Wilmington') + expect(await settings.getFacilityName()).toBe('Wilmington') + expect(list).toHaveBeenCalledTimes(1) + }) + + it('gives both portal templates to a single caller from one fetch', async () => { + list.mockResolvedValue(respond([ + { key: 'entra_device_url', value: 'https://entra/{id}' }, + { key: 'intune_device_url', value: 'https://intune/{id}' }, + ])) + const settings = await freshModule() + + expect(await settings.getEntraDeviceUrls()).toEqual({ + entraUrl: 'https://entra/{id}', + intuneUrl: 'https://intune/{id}', + }) + expect(list).toHaveBeenCalledTimes(1) + }) +}) + +describe('failure', () => { + it('falls back without throwing so a caller still renders', async () => { + list.mockRejectedValue(new Error('network down')) + vi.spyOn(console, 'error').mockImplementation(() => {}) + const settings = await freshModule() + + expect(await settings.getFacilityName()).toBe('ShopDB') + expect(await settings.getEntraDeviceUrls()).toEqual({ entraUrl: '', intuneUrl: '' }) + }) + + it('does not hammer a backend that is down', async () => { + list.mockRejectedValue(new Error('network down')) + vi.spyOn(console, 'error').mockImplementation(() => {}) + const settings = await freshModule() + + await settings.getFacilityName() + await settings.getFacilityName() + await settings.getFacilityName() + + expect(list).toHaveBeenCalledTimes(1) + }) + + it('recovers after the retry window instead of staying blank for the session', async () => { + list.mockRejectedValueOnce(new Error('network down')) + vi.spyOn(console, 'error').mockImplementation(() => {}) + const settings = await freshModule() + + expect(await settings.getSetting('entra_device_url')).toBe('') + + vi.setSystemTime(Date.now() + 31000) + list.mockResolvedValue(respond([{ key: 'entra_device_url', value: 'https://entra/{id}' }])) + + expect(await settings.getSetting('entra_device_url')).toBe('https://entra/{id}') + expect(list).toHaveBeenCalledTimes(2) + }) +})