diff --git a/craft/src/editor/LoadGate.conflict.test.tsx b/craft/src/editor/LoadGate.conflict.test.tsx index bb4760d..405edfb 100644 --- a/craft/src/editor/LoadGate.conflict.test.tsx +++ b/craft/src/editor/LoadGate.conflict.test.tsx @@ -4,10 +4,10 @@ import { createRoot, Root } from 'react-dom/client'; import { act } from 'react-dom/test-utils'; /** - * Data-safety: a failed initial load must never let autosave overwrite the - * saved site with a blank canvas. Drives the real LoadGate + TopBar + - * SiteDesignPanel against a mocked `fetch` (the backend contract) and fake - * timers; only `@craftjs/core`'s useEditor and sibling chrome are stubbed. + * Two-tab conflict handling: rev tracking, base_rev on every save, and the + * 409 conflict modal (Reload latest / Keep mine). Drives the real LoadGate + + * TopBar + SiteDesignPanel against a mocked `fetch` and fake timers; only + * `@craftjs/core`'s useEditor and sibling chrome are stubbed. */ const undoMock = vi.fn(); const deserializeMock = vi.fn(); @@ -84,6 +84,7 @@ function mount() { beforeEach(() => { vi.useFakeTimers(); fetchMock = vi.fn(); + reloadMock.mockReset(); vi.stubGlobal('fetch', fetchMock); vi.spyOn(console, 'error').mockImplementation(() => {}); undoMock.mockClear(); @@ -99,8 +100,6 @@ afterEach(() => { }); -beforeEach(() => { reloadMock.mockReset(); }); - const loadedThen = (rev: string | null, saveResponses: unknown[]) => { const saves = [...saveResponses]; fetchMock.mockImplementation(async (url: string) => @@ -189,14 +188,12 @@ describe('conflict modal', () => { mount(); await flush(); await clickSave(); - const prevented: boolean[] = []; act(() => { guard.markDirty(); }); act(() => { q('[data-testid="conflict-reload"]')!.click(); }); const ev = new Event('beforeunload', { cancelable: true }); window.dispatchEvent(ev); - prevented.push(ev.defaultPrevented); expect(reloadMock).toHaveBeenCalledTimes(1); - expect(prevented).toEqual([false]); + expect(ev.defaultPrevented).toBe(false); expect(saveCalls()).toHaveLength(1); }); @@ -230,3 +227,36 @@ describe('conflict modal', () => { expect(saveBody(2).force_empty).toBe(true); }); }); + +describe('revision and overwrite edge cases', () => { + test('a brand-new site (rev null) saves, then the next save sends the returned rev', async () => { + loadedThen(null, [jsonResp({ success: true, rev: 'rX' })]); + mount(); + await flush(); + await clickSave(); + await clickSave(); + expect(saveBody(0).base_rev).toBeNull(); + expect(saveBody(1).base_rev).toBe('rX'); + }); + + test('Keep mine -> empty refusal -> Undo disarms force_overwrite: the next autosave is conflict-checked', async () => { + loadedThen('r1', [ + conflict409, + jsonResp({ success: false, error: 'empty_overwrite_refused', message: 'e' }, 409), + jsonResp({ success: true, rev: 'r2' }), + ]); + mount(); + await flush(); + await clickSave(); + await act(async () => { q('[data-testid="conflict-keep-mine"]')!.click(); }); + await flush(); + expect(saveBody(1).force_overwrite).toBe(true); + act(() => { q('[data-testid="empty-save-undo"]')!.click(); }); + act(() => { guard.markDirty(); }); + await act(async () => { vi.advanceTimersByTime(30000); }); + await flush(); + expect(saveCalls()).toHaveLength(3); + expect(saveBody(2).force_overwrite).toBeUndefined(); + expect(saveBody(2).base_rev).toBe('r1'); + }); +}); diff --git a/craft/src/panels/topbar/ConflictModal.tsx b/craft/src/panels/topbar/ConflictModal.tsx index cace451..c298d0c 100644 --- a/craft/src/panels/topbar/ConflictModal.tsx +++ b/craft/src/panels/topbar/ConflictModal.tsx @@ -1,8 +1,6 @@ import React from 'react'; export interface ConflictModalProps { - /** Why the save was refused (the server's message); a fixed fallback when empty. */ - message?: string | null; /** True while "Keep mine" is re-saving. */ busy: boolean; /** Set when "Keep mine" failed, so the customer sees why the modal is still up. */ diff --git a/craft/src/panels/topbar/TopBar.tsx b/craft/src/panels/topbar/TopBar.tsx index f110586..73e355d 100644 --- a/craft/src/panels/topbar/TopBar.tsx +++ b/craft/src/panels/topbar/TopBar.tsx @@ -37,7 +37,7 @@ export const TopBar: React.FC = ({ device, onDeviceChange, showGuid const { save, publish } = useWhpApi(); const { loadState, emptyRefusal, setEmptyRefusal, dirty, isDirty, getVersion, - conflict, setConflict, armForceOverwriteNextSave, reloadLatest, + conflict, setConflict, armForceOverwriteNextSave, disarmForceOverwriteNextSave, reloadLatest, } = useSaveGuard(); const canSave = loadState === 'loaded'; const { headerPage, footerPage, pages, activePageId } = usePages(); @@ -59,6 +59,9 @@ export const TopBar: React.FC = ({ device, onDeviceChange, showGuid const { open: openSitesmith } = useSitesmithModal(); const savingRef = useRef(false); const publishingRef = useRef(false); + // Read at tick time by autosaveDue, never a stale closure. + const conflictRef = useRef(null); + conflictRef.current = conflict; // An edit after a publish means the live site no longer matches the editor. useEffect(() => { @@ -115,7 +118,9 @@ export const TopBar: React.FC = ({ device, onDeviceChange, showGuid if (!isWHP || !canSave || emptyRefusal || conflict) return; // A running publish does its own save; a second one would race it. // Read through refs/isDirty() at tick time, never a stale closure. - const autosaveDue = () => isDirty() && !publishingRef.current; + // Also paused while the conflict modal is up: it is the authority, the + // effect guard above only avoids a pointless timer. + const autosaveDue = () => isDirty() && !publishingRef.current && !conflictRef.current; const interval = setInterval(() => { if (autosaveDue()) void runSave(); }, 30000); return () => clearInterval(interval); }, [isWHP, canSave, emptyRefusal, conflict, runSave, isDirty]); @@ -172,7 +177,6 @@ export const TopBar: React.FC = ({ device, onDeviceChange, showGuid const conflictModal = conflict ? ( = ({ device, onDeviceChange, showGuid const emptyBanner = emptyRefusal ? ( { void handleSave({ forceEmpty: true }); }} - onUndo={() => { actions.history.undo(); setEmptyRefusal(null); }} + onUndo={() => { + actions.history.undo(); + setEmptyRefusal(null); + // Backing out ends the Keep mine decision; a later autosave must be conflict-checked again. + disarmForceOverwriteNextSave(); + }} /> ) : null; diff --git a/craft/src/state/SaveGuardContext.tsx b/craft/src/state/SaveGuardContext.tsx index 0135de7..ebd5d9e 100644 --- a/craft/src/state/SaveGuardContext.tsx +++ b/craft/src/state/SaveGuardContext.tsx @@ -42,6 +42,8 @@ interface SaveGuardContextValue { /** One-shot: the next save sends force_overwrite (armed by "Keep mine", spent by a save that goes through). */ forceOverwriteNextSave: React.MutableRefObject; armForceOverwriteNextSave: () => void; + /** Cancel an armed overwrite (the customer backed out of the follow-up empty-save prompt). */ + disarmForceOverwriteNextSave: () => void; /** "Reload latest": skip the unsaved-changes prompt, then reload the whole editor. */ reloadLatest: () => void; } @@ -74,6 +76,7 @@ const defaultValue: SaveGuardContextValue = { setConflict: () => {}, forceOverwriteNextSave: { current: false }, armForceOverwriteNextSave: () => {}, + disarmForceOverwriteNextSave: () => {}, reloadLatest: () => {}, }; @@ -170,6 +173,7 @@ export const SaveGuardProvider: React.FC<{ children: ReactNode; /** Injectable f setConflict, forceOverwriteNextSave, armForceOverwriteNextSave: () => { forceOverwriteNextSave.current = true; }, + disarmForceOverwriteNextSave: () => { forceOverwriteNextSave.current = false; }, reloadLatest, }), [loadState, emptyRefusal, dirty, saving, isDirty, getVersion, markDirty, beginSave, endSave, suppressBeforeUnload, getRev, setRev, conflict, reloadLatest]);