diff --git a/docs/editor.md b/docs/editor.md index 8ed3fdb9b..9fef9be37 100644 --- a/docs/editor.md +++ b/docs/editor.md @@ -621,7 +621,7 @@ Site-specific controls that were previously sections of this modal (Pages roster **Data source — `useSiteSettingsController`** (`src/admin/modals/Settings/useSiteSettingsController.ts`): the General and Publishing sections edit fields of the persisted `SiteDocument` (`name`, `settings.*`, framework preferences), but where that document lives depends on the route. The modal is global, so a section cannot just read the editor store — that store is only hydrated on the Site editor (`AdminCanvasLayout`). The controller hides the split behind one uniform shape: - **Site editor** (editor store holds the live document): delegate to the editor-store mutations. Settings edits ride the collab write path alongside page-tree edits — streamed live to every peer and persisted by the server relay. -- **Every other admin page** (no in-memory draft): a standalone Zustand store loads the document once via `cmsAdapter`, edits a local copy, and persists immediately with a shell-only `saveSite` (empty dirty sets, so pages / components / layouts are left untouched). After each save it refreshes the `adminUi` site summary and fires `CMS_SITE_RELOAD_EVENT` so the toolbar brand and `useSiteSummary` re-sync. There is no Save button on those pages, so writes commit on blur / toggle. +- **Every other admin page** (no in-memory draft): a standalone Zustand store loads the document once via `cmsAdapter`, edits a local copy, and persists immediately with a shell-only `saveSite` (empty dirty sets, so pages / components / layouts are left untouched). After each save it refreshes the `adminUi` site summary and fires `CMS_SITE_RELOAD_EVENT` so the toolbar brand and `useSiteSummary` re-sync. There is no Save button on those pages, so writes commit on blur / toggle. The loaded copy is not kept forever: any **other** `CMS_SITE_RELOAD_EVENT` (site import, plugin install, data-workspace writes to site tables — also one arriving while the first load is in flight) marks it `stale`, and the next render of a section refetches it after pending saves have settled (immediately when the modal is open). A load whose fetch started before a save discards its response and fetches again, so it can never roll back that save or its shell conflict base (`saveGeneration`). Because the controller is imported only by the lazy section components, the editor-store import it carries stays inside the `SettingsModal` chunk and never enters the eager graph of the lightweight layouts. This is what makes the modal *actually* global — before it, General and Publishing rendered a permanent skeleton anywhere outside the Site editor. diff --git a/src/__tests__/fixtures/storeIsolation.ts b/src/__tests__/fixtures/storeIsolation.ts index cf7b22a8c..83eeb0407 100644 --- a/src/__tests__/fixtures/storeIsolation.ts +++ b/src/__tests__/fixtures/storeIsolation.ts @@ -24,6 +24,7 @@ import { useEditorStore } from '@site/store/store' import { useAdminUi } from '@admin/state/adminUi' import { useBranchStore } from '@admin/state/branchStore' import { useWorkspaceLayout } from '@admin/state/workspaceLayout' +import { useSettingsDraftStore } from '@admin/modals/Settings/useSiteSettingsController' interface ResettableStore { getState: () => T @@ -48,6 +49,7 @@ const RESETTERS = [ pristineResetter(useAdminUi), pristineResetter(useBranchStore), pristineResetter(useWorkspaceLayout), + pristineResetter(useSettingsDraftStore), ] /** Restore every app store to the state it had before the suite started. */ diff --git a/src/__tests__/settings/siteSettingsReload.test.tsx b/src/__tests__/settings/siteSettingsReload.test.tsx new file mode 100644 index 000000000..178407cf7 --- /dev/null +++ b/src/__tests__/settings/siteSettingsReload.test.tsx @@ -0,0 +1,150 @@ +/** + * The standalone branch of useSiteSettingsController (every admin page except + * the Site editor) caches the loaded site document. A site write made outside + * the Settings modal — an import, a plugin install — announces itself with + * `requestCmsSiteReload()`; the next Settings render must show that document, + * not the one cached before. + */ +import { describe, it, expect, beforeEach, afterEach } from 'bun:test' +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react' +import { useSiteSettingsController } from '@admin/modals/Settings/useSiteSettingsController' +import { requestCmsSiteReload } from '@admin/state/adminEvents' +import { cmsAdapter } from '@core/persistence/cms' +import type { SiteDocument } from '@core/page-tree' +import { useEditorStore } from '@site/store/store' +import { makeSite } from '../fixtures' + +/** Minimal Settings section: shows the site name and renames it on click. */ +function SiteNameProbe() { + const { site, updateSiteName } = useSiteSettingsController() + if (!site) return

loading

+ return ( +
+

{site.name}

+ +
+ ) +} + +const before: SiteDocument = makeSite({ name: 'Before import' }) +const after: SiteDocument = makeSite({ name: 'Imported site' }) + +let served: SiteDocument = before +let loads = 0 +const originalLoad = cmsAdapter.loadSite.bind(cmsAdapter) +const originalSave = cmsAdapter.saveSite.bind(cmsAdapter) + +beforeEach(() => { + served = before + loads = 0 + useEditorStore.setState({ site: null } as Parameters[0]) + cmsAdapter.loadSite = async () => { + loads++ + return { site: served, rowSeqs: {}, shellSeq: loads } + } + cmsAdapter.saveSite = async () => ({ seq: 100 + loads }) + // The standalone store is a module singleton: start every test from a + // stale cache so it loads `before`. + requestCmsSiteReload() +}) +afterEach(() => { + cleanup() + cmsAdapter.loadSite = originalLoad + cmsAdapter.saveSite = originalSave +}) + +async function shownName(): Promise { + return (await screen.findByTestId('site-name')).textContent ?? '' +} + +describe('Settings outside the Site editor after an out-of-modal site write', () => { + it('shows the imported document, not the one cached before the import', async () => { + const first = render() + expect(await shownName()).toBe('Before import') + first.unmount() + + // Site import (Replace) finished on the Data page. + served = after + requestCmsSiteReload() + + render() + await waitFor(async () => expect(await shownName()).toBe('Imported site')) + }) + + it('reloads while the modal stays open', async () => { + render() + expect(await shownName()).toBe('Before import') + served = after + requestCmsSiteReload() + await waitFor(async () => expect(await shownName()).toBe('Imported site')) + }) + + it('does not refetch after its own save', async () => { + render() + await shownName() + const loadsBefore = loads + fireEvent.click(screen.getByRole('button', { name: 'Rename' })) + await waitFor(async () => expect(await shownName()).toBe('Before import+')) + // Let the chained save settle (it dispatches the reload event itself). + await new Promise((r) => setTimeout(r, 20)) + expect(loads).toBe(loadsBefore) + }) + + it('an import announced while the first load is in flight still reloads', async () => { + // The beforeEach reload request ran before any render; start from a + // load that is still pending when the next import lands. + let release: (() => void) | undefined + const gate = new Promise((r) => { release = r }) + cmsAdapter.loadSite = async () => { + loads++ + const site = served + if (loads === 1) await gate + return { site, rowSeqs: {}, shellSeq: loads } + } + render() + await waitFor(() => expect(loads).toBe(1)) + served = after + requestCmsSiteReload() + release?.() + await waitFor(async () => expect(await shownName()).toBe('Imported site')) + }) + + it('a reload that started before a save never rolls the save back', async () => { + render() + await shownName() + // A reload starts; its response is held back and carries the old document. + let release: (() => void) | undefined + const gate = new Promise((r) => { release = r }) + const old = served + let serverSeq = 2 + cmsAdapter.loadSite = async () => { + loads++ + // The held-back response carries the document and seq from before the save. + const snapshot = loads === 2 ? { site: old, seq: 2 } : { site: served, seq: serverSeq } + if (loads === 2) await gate + return { site: snapshot.site, rowSeqs: {}, shellSeq: snapshot.seq } + } + const seqs: number[] = [] + cmsAdapter.saveSite = async (doc, opts) => { + seqs.push(opts?.shellBaseSeq ?? -1) + served = doc + serverSeq = 500 + seqs.length + return { seq: serverSeq } + } + requestCmsSiteReload() + await waitFor(() => expect(loads).toBe(2)) + // Meanwhile the user renames the site; it saves (seq 501). + fireEvent.click(screen.getByRole('button', { name: 'Rename' })) + await waitFor(() => expect(seqs.length).toBe(1)) + release?.() + await new Promise((r) => setTimeout(r, 30)) + // The old response did not replace the saved document … + await waitFor(async () => expect(await shownName()).toBe('Before import+')) + // … nor rewind the conflict base: the next save builds on seq 500 or newer. + fireEvent.click(screen.getByRole('button', { name: 'Rename' })) + await waitFor(() => expect(seqs.length).toBe(2)) + expect(seqs[1]).toBeGreaterThanOrEqual(500) + }) +}) diff --git a/src/admin/modals/Settings/useSiteSettingsController.ts b/src/admin/modals/Settings/useSiteSettingsController.ts index c5d7c8113..e8a1f7a77 100644 --- a/src/admin/modals/Settings/useSiteSettingsController.ts +++ b/src/admin/modals/Settings/useSiteSettingsController.ts @@ -45,7 +45,7 @@ import type { SiteDocument, SiteSettings } from '@core/page-tree' import type { FrameworkPreferencesSettings } from '@core/framework-schema' import { useEditorStore } from '@site/store/store' import { useAdminUi } from '@admin/state/adminUi' -import { requestCmsSiteReload } from '@admin/state/adminEvents' +import { CMS_SITE_RELOAD_EVENT, requestCmsSiteReload } from '@admin/state/adminEvents' import { getErrorMessage } from '@core/utils/errorMessage' const SITE_ID = 'default' @@ -76,7 +76,15 @@ interface SettingsDraftState { shellBaseSeq: number status: 'idle' | 'loading' | 'ready' | 'error' error: string | null - /** Load the document once (idempotent; shares a single in-flight fetch). */ + /** + * The site was written outside this store (import, plugin install, data + * workspace) since `doc` was loaded — the next `ensureLoaded` refetches. + */ + stale: boolean + /** + * Load the document (idempotent while fresh; shares a single in-flight + * fetch). A stale document is refetched once pending saves have settled. + */ ensureLoaded: () => Promise updateName: (name: string) => void updateSettings: (patch: Partial) => void @@ -87,14 +95,22 @@ let inFlightLoad: Promise | null = null // Serialise PUTs so two quick edits (e.g. toggle then blur) can't race to a // last-writer-wins on the server out of order. let saveChain: Promise = Promise.resolve() +// True while this store announces its own save — that reload request is for +// the editor and the site summary, not a reason to refetch our own write. +let announcingOwnSave = false +// Bumped by every save. A load whose fetch started before a save must not +// apply its (older) document and shell seq over that save. +let saveGeneration = 0 /** * Standalone draft store — used only when the editor store is NOT the source * of truth (any admin page other than the Site editor). Holds the full - * SiteDocument so shell saves can reuse it without re-fetching. + * SiteDocument so shell saves can reuse it without re-fetching. Exported only + * so the test preload can reset it between tests (`storeIsolation.ts`). */ -const useSettingsDraftStore = create((set, get) => { +export const useSettingsDraftStore = create((set, get) => { function persist(next: SiteDocument): void { + saveGeneration += 1 set({ doc: next }) saveChain = saveChain .then(() => @@ -113,7 +129,12 @@ const useSettingsDraftStore = create((set, get) => { name: next.name, faviconUrl: next.settings.faviconUrl ?? null, }) - requestCmsSiteReload() + announcingOwnSave = true + try { + requestCmsSiteReload() + } finally { + announcingOwnSave = false + } }) .catch((err: unknown) => { console.error('[useSiteSettingsController] failed to save site settings:', err) @@ -132,19 +153,44 @@ const useSettingsDraftStore = create((set, get) => { }) } + // Any other site write (Site import, plugin install, data workspace edits of + // site-backed tables) announces itself through `requestCmsSiteReload`. The + // cached document is stale from then on — without this, Settings opened + // before an import kept showing the pre-import site (its name, settings and + // classes) until a full page reload. + if (typeof window !== 'undefined') { + window.addEventListener(CMS_SITE_RELOAD_EVENT, () => { + if (announcingOwnSave) return + // Also while the first load is in flight: its response may predate the write. + if (get().doc || get().status === 'loading') set({ stale: true }) + }) + } + return { doc: null, shellBaseSeq: 0, status: 'idle', error: null, + stale: false, ensureLoaded: async () => { - if (get().status === 'ready' || get().status === 'loading') return + const { status, stale } = get() + if (status === 'loading' || (status === 'ready' && !stale)) return if (inFlightLoad) return inFlightLoad - set({ status: 'loading', error: null }) + set({ status: 'loading', error: null, stale: false }) inFlightLoad = (async () => { try { + // A save still in flight would otherwise land after the fetch and + // leave `doc` / `shellBaseSeq` behind the server. + await saveChain + const generation = saveGeneration const result = await cmsAdapter.loadSite(SITE_ID) + if (generation !== saveGeneration) { + // A save started meanwhile: this response may not contain it, and + // its shell seq would rewind the save's conflict base. Fetch again. + set({ status: 'ready', stale: true }) + return + } if (!result) { set({ status: 'error', error: 'No site found. Complete first-run setup first.' }) return @@ -156,6 +202,8 @@ const useSettingsDraftStore = create((set, get) => { } finally { inFlightLoad = null } + // Marked stale while this load ran (a write it may not include). + if (get().stale && get().status === 'ready') await get().ensureLoaded() })() return inFlightLoad }, @@ -220,6 +268,7 @@ export function useSiteSettingsController(): SiteSettingsController { // Standalone source (every other admin page). const draftDoc = useSettingsDraftStore((s) => s.doc) const draftError = useSettingsDraftStore((s) => s.error) + const draftStale = useSettingsDraftStore((s) => s.stale) const ensureLoaded = useSettingsDraftStore((s) => s.ensureLoaded) const standaloneUpdateName = useSettingsDraftStore((s) => s.updateName) const standaloneUpdateSettings = useSettingsDraftStore((s) => s.updateSettings) @@ -229,10 +278,12 @@ export function useSiteSettingsController(): SiteSettingsController { const editorActive = editorSite !== null + // `draftStale`: an out-of-modal site write while Settings is open reloads + // right away; otherwise the next open does. useEffect(() => { if (editorActive) return void ensureLoaded() - }, [editorActive, ensureLoaded]) + }, [editorActive, draftStale, ensureLoaded]) if (editorActive) { return {