Load site settings once and recover from a failed fetch

The cache held only the resolved settings object, so every caller arriving
before the first response fired its own request: nothing populated the cache
until that response landed. applyBranding awaits five settings in sequence at
bootstrap, getServicenowUrls four and getEntraDeviceUrls two, all of which
became separate GETs on a cold load.

That race also hid the Intune and Entra buttons on PC Details. The page loads
the URL templates without awaiting them so a settings failure cannot block
rendering, and on a cold cache the response could arrive after the record had
rendered, leaving the templates empty and the links hidden until a refresh
warmed the cache.

Cache the in-flight promise so concurrent callers share one request, and hold a
failed load for 30s instead of the page session. A transient blip used to blank
every settings-driven link until the operator reloaded, which they have no
reason to know would help; the window still bounds a hard-down backend to two
requests a minute per client.

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 = '') {

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)
+  })
+})