diff --git a/app/api/site-features/route.test.ts b/app/api/site-features/route.test.ts index c3824a7..19aa893 100644 --- a/app/api/site-features/route.test.ts +++ b/app/api/site-features/route.test.ts @@ -1,40 +1,56 @@ -import { mkdtemp, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; - import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { getDefaultConfig, type SiteFeaturesConfig } from '@/lib/site-features'; -import { readSiteFeatures, writeSiteFeatures } from '@/lib/site-features-server'; +import { readSiteFeatures } from '@/lib/site-features-server'; vi.mock('next/cache', () => ({ revalidatePath: vi.fn() })); -vi.mock('@/lib/api-fetch', () => ({ - authFetch: vi.fn( - async () => - new Response(JSON.stringify({ is_staff: true }), { - status: 200, - headers: { 'content-type': 'application/json' }, - }) - ), +const { apiFetch, authFetch } = vi.hoisted(() => ({ + apiFetch: vi.fn(), + authFetch: vi.fn(), })); +vi.mock('@/lib/api-fetch', () => ({ apiFetch, authFetch })); + import type { NextRequest } from 'next/server'; import { PUT } from './route'; -let workDir: string; +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); +} -// The route persists through lib/json-config-file, which resolves -// `config/site-features.json` under process.cwd() — point it at a scratch dir -// so the repo's runtime config file is never touched. -beforeEach(async () => { - workDir = await mkdtemp(join(tmpdir(), 'site-features-route-')); - vi.spyOn(process, 'cwd').mockReturnValue(workDir); +/** + * A tiny in-memory stand-in for the backend's `AppSettings` row: `apiFetch` + * (the GET used by `readSiteFeatures`) reads it, `authFetch`'s PUT branch + * (the write used by `writeSiteFeatures`) replaces it. `isStaff` gates the + * PUT handler's own profile check the same way the real backend would. + */ +let stored: SiteFeaturesConfig; +let isStaff: boolean; + +beforeEach(() => { + stored = getDefaultConfig(); + isStaff = true; + + apiFetch.mockReset(); + authFetch.mockReset(); + + apiFetch.mockImplementation(async () => jsonResponse(stored)); + authFetch.mockImplementation(async (path: string, _token: string, init?: RequestInit) => { + if (path === '/api/v1/auth/profile') { + return jsonResponse({ is_staff: isStaff }); + } + // The site-features PUT: persist the body into the fake store. + stored = JSON.parse((init?.body as string) ?? '{}') as SiteFeaturesConfig; + return jsonResponse(stored); + }); }); -afterEach(async () => { +afterEach(() => { vi.restoreAllMocks(); - await rm(workDir, { recursive: true, force: true }); }); /** Minimal duck-typed request: the handler only reads the header and the body. */ @@ -63,7 +79,7 @@ describe('PUT /api/site-features — feature flags', () => { // toggles must not silently switch a feature the admin turned off back on. const disabled = getDefaultConfig(); disabled.features.manuscriptDescriptions = false; - await writeSiteFeatures(disabled); + stored = disabled; const response = await PUT(putRequest(payloadWithoutFeatures())); expect(response.status).toBe(200); @@ -82,7 +98,7 @@ describe('PUT /api/site-features — feature flags', () => { it('lets a current client re-enable a flag (merge is key-by-key, not one-way)', async () => { const disabled = getDefaultConfig(); disabled.features.manuscriptDescriptions = false; - await writeSiteFeatures(disabled); + stored = disabled; await PUT(putRequest(getDefaultConfig())); expect((await readSiteFeatures()).features.manuscriptDescriptions).toBe(true); @@ -120,13 +136,7 @@ describe('PUT /api/site-features — the staff gate protecting the flags', () => }); it('rejects a non-staff caller before touching the config', async () => { - const { authFetch } = await import('@/lib/api-fetch'); - vi.mocked(authFetch).mockResolvedValueOnce( - new Response(JSON.stringify({ is_staff: false }), { - status: 200, - headers: { 'content-type': 'application/json' }, - }) - ); + isStaff = false; const before = await readSiteFeatures(); const res = await PUT( requestWithAuth( @@ -138,3 +148,16 @@ describe('PUT /api/site-features — the staff gate protecting the flags', () => expect((await readSiteFeatures()).features).toEqual(before.features); }); }); + +describe('PUT /api/site-features — backend write failure', () => { + it('returns 502 when the backend PUT fails, without crashing the route', async () => { + authFetch.mockImplementation(async (path: string) => { + if (path === '/api/v1/auth/profile') return jsonResponse({ is_staff: true }); + return jsonResponse({ error: 'backend unavailable' }, 500); + }); + + const response = await PUT(putRequest(getDefaultConfig())); + expect(response.status).toBe(502); + expect(await response.json()).toEqual({ error: 'Failed to update site features' }); + }); +}); diff --git a/app/api/site-features/route.ts b/app/api/site-features/route.ts index 62a9b4e..255a01b 100644 --- a/app/api/site-features/route.ts +++ b/app/api/site-features/route.ts @@ -50,10 +50,18 @@ export async function PUT(request: NextRequest) { // the full map still wins key-by-key. const payload = body as SiteFeaturesConfig; const current = await readSiteFeatures(); - const normalized = await writeSiteFeatures({ - ...payload, - features: mergeFeatureFlags(current.features, (payload as { features?: unknown }).features), - }); + let normalized: SiteFeaturesConfig; + try { + normalized = await writeSiteFeatures( + { + ...payload, + features: mergeFeatureFlags(current.features, (payload as { features?: unknown }).features), + }, + token + ); + } catch { + return NextResponse.json({ error: 'Failed to update site features' }, { status: 502 }); + } revalidatePath('/', 'layout'); // Return the normalized config (with sectionOrder canonicalized) so the // client's cache reflects what's actually on disk. diff --git a/lib/site-features-server.test.ts b/lib/site-features-server.test.ts index 74fc6c7..9edcbc8 100644 --- a/lib/site-features-server.test.ts +++ b/lib/site-features-server.test.ts @@ -1,135 +1,153 @@ -import { mkdtemp, mkdir, readFile, rm, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; - import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { getDefaultConfig } from './site-features'; -import { readSiteFeatures, writeSiteFeatures } from './site-features-server'; - -// `json-config-file` resolves `config/.json` under `process.cwd()` at -// call time, so pointing cwd at a scratch dir exercises the real read/write -// path (real JSON on a real disk) without touching the repo's runtime -// config/site-features.json. -let workDir: string; - -async function writeRawConfig(contents: unknown): Promise { - await mkdir(join(workDir, 'config'), { recursive: true }); - await writeFile( - join(workDir, 'config', 'site-features.json'), - JSON.stringify(contents, null, 2), - 'utf-8' - ); -} -async function readRawConfig(): Promise> { - return JSON.parse( - await readFile(join(workDir, 'config', 'site-features.json'), 'utf-8') - ) as Record; +const { apiFetch, authFetch } = vi.hoisted(() => ({ + apiFetch: vi.fn(), + authFetch: vi.fn(), +})); + +vi.mock('./api-fetch', () => ({ apiFetch, authFetch })); + +// Imported after the mock so `readSiteFeatures`/`writeSiteFeatures` pick up +// the mocked `apiFetch`/`authFetch` rather than issuing real network calls. +const { readSiteFeatures, writeSiteFeatures } = await import('./site-features-server'); + +function jsonResponse(body: unknown, status = 200): Response { + return new Response(JSON.stringify(body), { + status, + headers: { 'content-type': 'application/json' }, + }); } -/** The on-disk shape as it exists today: no `features` key at all. */ -function legacyFileContents() { +/** The backend response shape as it exists today: no `features` key at all. */ +function legacyResponseBody() { const { sections, sectionOrder, searchCategories } = getDefaultConfig(); return { sections, sectionOrder, searchCategories }; } -beforeEach(async () => { - workDir = await mkdtemp(join(tmpdir(), 'site-features-')); - vi.spyOn(process, 'cwd').mockReturnValue(workDir); +beforeEach(() => { + apiFetch.mockReset(); + authFetch.mockReset(); }); -afterEach(async () => { +afterEach(() => { vi.restoreAllMocks(); - await rm(workDir, { recursive: true, force: true }); }); -describe('readSiteFeatures — feature flags', () => { - it('defaults every flag to enabled when there is no config file at all', async () => { +describe('readSiteFeatures', () => { + it('returns the backend config merged over defaults on a successful GET', async () => { + apiFetch.mockResolvedValueOnce( + jsonResponse({ ...legacyResponseBody(), features: { manuscriptDescriptions: false } }) + ); const config = await readSiteFeatures(); - expect(config.features.manuscriptDescriptions).toBe(true); + expect(config.features.manuscriptDescriptions).toBe(false); + expect(apiFetch).toHaveBeenCalledWith('/api/v1/site-features/'); }); - it('keeps a shipped feature enabled for a config file written before flags existed', async () => { - // This is the deploy case: the persisted file has sections + sectionOrder + - // searchCategories and nothing else. The feature must not vanish. - await writeRawConfig(legacyFileContents()); + it('defaults every flag to enabled when the backend response has no `features` key', async () => { + apiFetch.mockResolvedValueOnce(jsonResponse(legacyResponseBody())); const config = await readSiteFeatures(); expect(config.features.manuscriptDescriptions).toBe(true); }); - it('honours a persisted disabled flag', async () => { - await writeRawConfig({ ...legacyFileContents(), features: { manuscriptDescriptions: false } }); - expect((await readSiteFeatures()).features.manuscriptDescriptions).toBe(false); + it('merges a partial searchCategories override over the defaults', async () => { + apiFetch.mockResolvedValueOnce( + jsonResponse({ + ...legacyResponseBody(), + searchCategories: { images: { enabled: false } }, + }) + ); + const config = await readSiteFeatures(); + expect(config.searchCategories.images.enabled).toBe(false); + // Untouched keys of the same category, and other categories, keep defaults. + const defaults = getDefaultConfig(); + expect(config.searchCategories.images.visibleColumns).toEqual( + defaults.searchCategories.images.visibleColumns + ); + expect(config.searchCategories).not.toBe(defaults.searchCategories); + }); + + it('falls back to defaults when the response is not ok', async () => { + apiFetch.mockResolvedValueOnce(jsonResponse({ error: 'nope' }, 500)); + const config = await readSiteFeatures(); + expect(config).toEqual(getDefaultConfig()); + }); + + it('falls back to defaults when the fetch throws (network error)', async () => { + apiFetch.mockRejectedValueOnce(new Error('network down')); + const config = await readSiteFeatures(); + expect(config).toEqual(getDefaultConfig()); }); - it('falls back to defaults when `features` is not a plain object', async () => { - for (const junk of ['manuscriptDescriptions', ['manuscriptDescriptions'], 7, null]) { - await writeRawConfig({ ...legacyFileContents(), features: junk }); + it('falls back to defaults when the response body is not a plain object', async () => { + for (const junk of [null, [], 'oops', 7]) { + apiFetch.mockResolvedValueOnce(jsonResponse(junk)); const config = await readSiteFeatures(); - expect(config.features).toEqual(getDefaultConfig().features); + expect(config).toEqual(getDefaultConfig()); } }); - it('ignores unknown flag keys and non-boolean values', async () => { - await writeRawConfig({ - ...legacyFileContents(), - features: { manuscriptDescriptions: 'false', bogusFlag: true }, - }); + it('falls back to defaults when response.json() throws (malformed JSON)', async () => { + apiFetch.mockResolvedValueOnce({ + ok: true, + json: () => Promise.reject(new Error('bad json')), + } as unknown as Response); + const config = await readSiteFeatures(); + expect(config).toEqual(getDefaultConfig()); + }); + + it('ignores unknown flag keys and non-boolean values from the backend', async () => { + apiFetch.mockResolvedValueOnce( + jsonResponse({ + ...legacyResponseBody(), + features: { manuscriptDescriptions: 'false', bogusFlag: true }, + }) + ); const config = await readSiteFeatures(); expect(config.features).toEqual({ manuscriptDescriptions: true }); }); }); -describe('writeSiteFeatures → readSiteFeatures round trip', () => { - it('preserves a disabled flag (the strict-whitelist trap)', async () => { - // If `features` were left out of writeSiteFeatures' whitelist, this save - // would drop the key, the read would restore the default, and the admin's - // "off" would silently become "on" again on every save. +describe('writeSiteFeatures', () => { + it('PUTs the normalized config with the token and returns it on success', async () => { + authFetch.mockResolvedValueOnce(jsonResponse({ ok: true })); const config = getDefaultConfig(); config.features.manuscriptDescriptions = false; - const normalized = await writeSiteFeatures(config); - expect(normalized.features.manuscriptDescriptions).toBe(false); - - const persisted = await readRawConfig(); - expect(persisted.features).toEqual({ manuscriptDescriptions: false }); - - expect((await readSiteFeatures()).features.manuscriptDescriptions).toBe(false); - }); - - it('re-enabling a flag survives the round trip too (a flag never deletes data)', async () => { - const off = getDefaultConfig(); - off.features.manuscriptDescriptions = false; - await writeSiteFeatures(off); + const normalized = await writeSiteFeatures(config, 'staff-token'); - const on = getDefaultConfig(); - await writeSiteFeatures(on); - expect((await readSiteFeatures()).features.manuscriptDescriptions).toBe(true); + expect(normalized.features.manuscriptDescriptions).toBe(false); + expect(authFetch).toHaveBeenCalledTimes(1); + const [path, token, init] = authFetch.mock.calls[0]; + expect(path).toBe('/api/v1/site-features/'); + expect(token).toBe('staff-token'); + expect(init).toMatchObject({ + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + }); + expect(JSON.parse(init.body as string)).toEqual(normalized); }); it('persists a complete boolean map even if the caller hands over a partial one', async () => { + authFetch.mockResolvedValueOnce(jsonResponse({ ok: true })); const config = getDefaultConfig(); // @ts-expect-error — a hand-rolled/older payload without the flag map delete config.features; - await writeSiteFeatures(config); - expect(await readRawConfig()).toHaveProperty('features', { manuscriptDescriptions: true }); + const normalized = await writeSiteFeatures(config, 'staff-token'); + expect(normalized.features).toEqual({ manuscriptDescriptions: true }); }); - it('still writes the other whitelisted keys unchanged', async () => { - const config = getDefaultConfig(); - config.sections.lightbox = false; - config.searchCategories.images.enabled = false; - - const persisted = await writeSiteFeatures(config); - expect(persisted.sections.lightbox).toBe(false); - expect(persisted.searchCategories.images.enabled).toBe(false); - expect(Object.keys(await readRawConfig()).sort()).toEqual([ - 'features', - 'searchCategories', - 'sectionOrder', - 'sections', - ]); + it('throws when the backend responds with a non-ok status', async () => { + authFetch.mockResolvedValueOnce(jsonResponse({ error: 'bad request' }, 400)); + await expect(writeSiteFeatures(getDefaultConfig(), 'staff-token')).rejects.toThrow(); + }); + + it('propagates a network error', async () => { + authFetch.mockRejectedValueOnce(new Error('network down')); + await expect(writeSiteFeatures(getDefaultConfig(), 'staff-token')).rejects.toThrow( + 'network down' + ); }); }); diff --git a/lib/site-features-server.ts b/lib/site-features-server.ts index c4dd7f9..70e377c 100644 --- a/lib/site-features-server.ts +++ b/lib/site-features-server.ts @@ -1,4 +1,4 @@ -import { readJsonConfig, writeJsonConfig } from './json-config-file'; +import { apiFetch, authFetch } from './api-fetch'; import { getDefaultConfig, getDefaultFeatures, @@ -7,81 +7,97 @@ import { type SiteFeaturesConfig, } from './site-features'; -const CONFIG_FILE = 'site-features.json'; +const SITE_FEATURES_PATH = '/api/v1/site-features/'; function isPlainObject(value: unknown): value is Record { return value != null && typeof value === 'object' && !Array.isArray(value); } +/** + * Site features are backend-owned (`AppSettings`, superuser-editable via the + * backoffice). Any failure - network error, non-200, or an unexpected + * response shape - falls back to defaults so SSR never 500s over a backend + * hiccup (matches `readModelLabels`'s fallback behavior). + */ export async function readSiteFeatures(): Promise { const defaults = getDefaultConfig(); - return readJsonConfig( - CONFIG_FILE, - (raw) => { - // If the file was hand-edited to `null`, an array, or a primitive, we'd - // crash on `parsed.sections` reading below. Bail out to defaults so the - // SSR layout doesn't 500 the whole site over a broken config file. - if (!isPlainObject(raw)) return defaults; - const parsed = raw as Partial; - // Defensive: spreading a string or array into an object produces - // index-keyed entries (e.g. {"0": "l", "1": "o"}) and pollutes the - // runtime config. Treat anything non-plain-object as missing so - // malformed config rows fall back to defaults instead of leaking - // garbage keys to consumers. - const parsedSections = isPlainObject(parsed.sections) ? parsed.sections : {}; - const parsedCategories = isPlainObject(parsed.searchCategories) - ? parsed.searchCategories - : {}; - return { - sections: { ...defaults.sections, ...parsedSections }, - sectionOrder: normalizeSectionOrder(parsed.sectionOrder), - // Every config file written before feature flags existed has no - // `features` key at all, so this merge is the whole backward-compat - // story: unknown/absent flags fall back to the defaults (enabled) and - // an already-shipped feature survives the deploy that introduces its - // flag. `mergeFeatureFlags` applies the same non-plain-object defence - // used for `sections` above, and additionally drops non-boolean values. - features: mergeFeatureFlags(defaults.features, parsed.features), - searchCategories: { - ...defaults.searchCategories, - ...Object.fromEntries( - Object.entries(parsedCategories).map(([k, v]) => [ - k, - { - ...defaults.searchCategories[k as keyof typeof defaults.searchCategories], - ...(isPlainObject(v) ? v : {}), - }, - ]) - ), - }, - }; - }, - defaults - ); + try { + const res = await apiFetch(SITE_FEATURES_PATH); + if (!res.ok) return defaults; + const raw = await res.json(); + // If the response is `null`, an array, or a primitive, we'd crash on + // `parsed.sections` reading below. Bail out to defaults so the SSR + // layout doesn't 500 the whole site over a broken/unexpected response. + if (!isPlainObject(raw)) return defaults; + const parsed = raw as Partial; + // Defensive: spreading a string or array into an object produces + // index-keyed entries (e.g. {"0": "l", "1": "o"}) and pollutes the + // runtime config. Treat anything non-plain-object as missing so + // malformed responses fall back to defaults instead of leaking garbage + // keys to consumers. + const parsedSections = isPlainObject(parsed.sections) ? parsed.sections : {}; + const parsedCategories = isPlainObject(parsed.searchCategories) ? parsed.searchCategories : {}; + return { + sections: { ...defaults.sections, ...parsedSections }, + sectionOrder: normalizeSectionOrder(parsed.sectionOrder), + // Every config written before feature flags existed has no `features` + // key at all, so this merge is the whole backward-compat story: + // unknown/absent flags fall back to the defaults (enabled) and an + // already-shipped feature survives the deploy that introduces its + // flag. `mergeFeatureFlags` applies the same non-plain-object defence + // used for `sections` above, and additionally drops non-boolean values. + features: mergeFeatureFlags(defaults.features, parsed.features), + searchCategories: { + ...defaults.searchCategories, + ...Object.fromEntries( + Object.entries(parsedCategories).map(([k, v]) => [ + k, + { + ...defaults.searchCategories[k as keyof typeof defaults.searchCategories], + ...(isPlainObject(v) ? v : {}), + }, + ]) + ), + }, + }; + } catch { + return defaults; + } } -export async function writeSiteFeatures(config: SiteFeaturesConfig): Promise { +/** Upserts the given config via the backend's superuser-only PUT; throws on failure. */ +export async function writeSiteFeatures( + config: SiteFeaturesConfig, + token: string +): Promise { // Construct the normalized config from KNOWN keys only — `...config` would // also write any extra keys a malicious or buggy payload included, slowly - // bloating the on-disk JSON with garbage. readSiteFeatures already ignores - // unknown keys when loading, so a strict whitelist here keeps both ends - // symmetric and the file pristine. + // bloating the persisted config with garbage. readSiteFeatures already + // ignores unknown keys when loading, so a strict whitelist here keeps both + // ends symmetric and the stored config pristine. const normalized: SiteFeaturesConfig = { sections: config.sections, sectionOrder: normalizeSectionOrder(config.sectionOrder), // `features` MUST be listed here: the whitelist is exhaustive, so omitting - // it would silently drop every flag on each admin save — the file would - // lose the key, the next read would restore the defaults, and a disabled - // feature would reappear. Merging over the defaults (rather than trusting - // `config.features` verbatim) also keeps the persisted map complete and - // boolean-typed even if a caller hands us a partial object. + // it would silently drop every flag on each admin save — the stored config + // would lose the key, the next read would restore the defaults, and a + // disabled feature would reappear. Merging over the defaults (rather than + // trusting `config.features` verbatim) also keeps the persisted map + // complete and boolean-typed even if a caller hands us a partial object. features: mergeFeatureFlags(getDefaultFeatures(), config.features), searchCategories: config.searchCategories, }; - await writeJsonConfig(CONFIG_FILE, normalized); + const res = await authFetch(SITE_FEATURES_PATH, token, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(normalized), + }); + if (!res.ok) { + throw new Error(`Failed to write site features: ${res.status}`); + } // Return the normalized config so callers (e.g. the PUT route handler) can // echo back exactly what was persisted. Returning the input verbatim would - // let the client's TanStack Query cache diverge from disk on every save — - // sectionOrder would silently differ until the next refetch. + // let the client's TanStack Query cache diverge from the backend on every + // save — sectionOrder would silently differ until the next refetch. return normalized; }