fix(site-builder): fail load on unreadable page state; disable editing controls until loaded
Refs #28
This commit is contained in:
1 parent
66e669fa01
commit
a406ff8548
10 files changed
+110
-48
No files matched your search
+10
-10
@@ -25,16 +25,16 @@ export const App: React.FC<AppProps> = ({ whpConfig }) => {
|
||||
return (
|
||||
<EditorConfigProvider config={whpConfig}>
|
||||
<SaveGuardProvider>
|
||||
<SiteDesignProvider>
|
||||
<Editor resolver={componentResolver} enabled={true} onRender={RenderNode}>
|
||||
<PageProvider>
|
||||
<SitesmithProvider>
|
||||
<EditorShell />
|
||||
<SitesmithModalMount />
|
||||
</SitesmithProvider>
|
||||
</PageProvider>
|
||||
</Editor>
|
||||
</SiteDesignProvider>
|
||||
<SiteDesignProvider>
|
||||
<Editor resolver={componentResolver} enabled={true} onRender={RenderNode}>
|
||||
<PageProvider>
|
||||
<SitesmithProvider>
|
||||
<EditorShell />
|
||||
<SitesmithModalMount />
|
||||
</SitesmithProvider>
|
||||
</PageProvider>
|
||||
</Editor>
|
||||
</SiteDesignProvider>
|
||||
</SaveGuardProvider>
|
||||
</EditorConfigProvider>
|
||||
);
|
||||
|
||||
@@ -82,6 +82,7 @@ beforeEach(() => {
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
undoMock.mockClear();
|
||||
deserializeMock.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -124,6 +125,33 @@ describe('load gate', () => {
|
||||
mount(); // so afterEach has something to unmount
|
||||
});
|
||||
|
||||
test('a page state that fails to deserialize fails the load, names the problem, and never saves', async () => {
|
||||
fetchMock.mockResolvedValue(jsonResp({
|
||||
success: true,
|
||||
rev: 'r1',
|
||||
project: {
|
||||
pages_craft_state: [{ id: 'home', name: 'Home', slug: 'index', craftState: '{"ROOT":{}}' }],
|
||||
},
|
||||
}));
|
||||
deserializeMock.mockImplementationOnce(() => { throw new Error('malformed'); });
|
||||
mount();
|
||||
await flush();
|
||||
|
||||
expect(q('[data-testid="load-gate"]')!.textContent).toContain("We couldn't read part of your saved site.");
|
||||
expect(q('[data-testid="load-retry"]')).not.toBeNull();
|
||||
await act(async () => { vi.advanceTimersByTime(30000); });
|
||||
expect(saveCalls()).toHaveLength(0);
|
||||
});
|
||||
|
||||
test('canvas-changing top-bar controls are disabled until the site has loaded', async () => {
|
||||
fetchMock.mockRejectedValue(new Error('down'));
|
||||
mount();
|
||||
await flush();
|
||||
for (const label of ['Undo', 'Redo', 'Templates', 'Custom Head Code']) {
|
||||
expect((q(`button[aria-label="${label}"]`) as HTMLButtonElement).disabled).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
test('shows a loading overlay while the load is in flight', async () => {
|
||||
fetchMock.mockReturnValue(new Promise(() => {}));
|
||||
mount();
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import React, { useCallback, useEffect, useRef } from 'react';
|
||||
import React, { useCallback, useEffect, useRef, useState } from 'react';
|
||||
import { useEditorConfig } from '../state/EditorConfigContext';
|
||||
import { useSaveGuard } from '../state/SaveGuardContext';
|
||||
import { useWhpApi } from '../hooks/useWhpApi';
|
||||
import { useWhpApi, UnreadableSiteError } from '../hooks/useWhpApi';
|
||||
|
||||
/**
|
||||
* Owns the initial project load (WHP mode only) and blocks the whole editor
|
||||
@@ -13,13 +13,16 @@ export const LoadGate: React.FC = () => {
|
||||
const { loadState, setLoadState } = useSaveGuard();
|
||||
const { load } = useWhpApi();
|
||||
const startedRef = useRef(false);
|
||||
const [unreadable, setUnreadable] = useState(false);
|
||||
|
||||
const runLoad = useCallback(() => {
|
||||
setUnreadable(false);
|
||||
setLoadState('loading');
|
||||
load()
|
||||
.then(() => setLoadState('loaded'))
|
||||
.catch((e) => {
|
||||
console.error('Failed to load project from WHP API:', e);
|
||||
setUnreadable(e instanceof UnreadableSiteError);
|
||||
setLoadState('failed');
|
||||
});
|
||||
}, [load, setLoadState]);
|
||||
@@ -38,7 +41,11 @@ export const LoadGate: React.FC = () => {
|
||||
<p><i className="fa fa-spinner fa-spin" aria-hidden="true" /> Loading your site…</p>
|
||||
) : (
|
||||
<>
|
||||
<p>We couldn't load your site. Your saved work has not been changed.</p>
|
||||
<p>
|
||||
{unreadable
|
||||
? "We couldn't read part of your saved site. Your saved work has not been changed. Contact support if this keeps happening."
|
||||
: "We couldn't load your site. Your saved work has not been changed."}
|
||||
</p>
|
||||
<button type="button" className="topbar-btn primary" onClick={runLoad} data-testid="load-retry">
|
||||
Retry
|
||||
</button>
|
||||
|
||||
@@ -5,7 +5,7 @@ import { act } from 'react-dom/test-utils';
|
||||
import { EditorConfigProvider } from '../state/EditorConfigContext';
|
||||
import { EMPTY_CANVAS, PageProvider, usePages } from '../state/PageContext';
|
||||
import { SiteDesignProvider } from '../state/SiteDesignContext';
|
||||
import { useWhpApi } from './useWhpApi';
|
||||
import { useWhpApi, UnreadableSiteError } from './useWhpApi';
|
||||
import { WhpConfig } from '../types';
|
||||
|
||||
/**
|
||||
@@ -169,9 +169,10 @@ describe('useWhpApi load() repairs an orphaned node on the FIRST page before des
|
||||
* load, which decides whether the user sees a working editor at all, both
|
||||
* failed harder (silently leaving the Frame undeserialized) and reported
|
||||
* quieter than every subsequent page switch. This pins the aligned
|
||||
* behaviour.
|
||||
* behaviour, except that the initial load now FAILS instead of falling back
|
||||
* to EMPTY_CANVAS (a blank canvas could be autosaved over the real page).
|
||||
*/
|
||||
describe('useWhpApi load() falls back to EMPTY_CANVAS when the first page state cannot be deserialized', () => {
|
||||
describe('useWhpApi load() fails (no EMPTY_CANVAS fallback) when the first page state cannot be deserialized', () => {
|
||||
beforeEach(() => {
|
||||
deserializeMock.mockClear();
|
||||
});
|
||||
@@ -180,7 +181,7 @@ describe('useWhpApi load() falls back to EMPTY_CANVAS when the first page state
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
test('a deserialize failure on the first page logs console.error and retries with EMPTY_CANVAS', async () => {
|
||||
test('a deserialize failure on the first page logs console.error and fails the load (no EMPTY_CANVAS fallback)', async () => {
|
||||
const fetchMock = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({
|
||||
@@ -204,13 +205,16 @@ describe('useWhpApi load() falls back to EMPTY_CANVAS when the first page state
|
||||
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
const harness = render();
|
||||
|
||||
let caught: unknown;
|
||||
await act(async () => {
|
||||
await harness.get().load();
|
||||
await harness.get().load().catch((e: unknown) => { caught = e; });
|
||||
});
|
||||
|
||||
// First call (the broken state) threw; the second call is the fallback.
|
||||
expect(deserializeMock).toHaveBeenCalledTimes(2);
|
||||
expect(deserializeMock.mock.calls[1][0]).toBe(EMPTY_CANVAS);
|
||||
// The broken state threw and the load FAILS: no blank-canvas fallback
|
||||
// that autosave could later write over the customer's real page.
|
||||
expect(caught).toBeInstanceOf(UnreadableSiteError);
|
||||
expect(deserializeMock).toHaveBeenCalledTimes(1);
|
||||
expect(deserializeMock).not.toHaveBeenCalledWith(EMPTY_CANVAS);
|
||||
|
||||
expect(errorSpy).toHaveBeenCalledWith('Failed to load page state:', expect.any(Error));
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { useCallback } from 'react';
|
||||
import { useEditor } from '@craftjs/core';
|
||||
import { useEditorConfig } from '../state/EditorConfigContext';
|
||||
import { usePages, EMPTY_CANVAS } from '../state/PageContext';
|
||||
import { usePages } from '../state/PageContext';
|
||||
import { useSiteDesign, SiteDesign } from '../state/SiteDesignContext';
|
||||
import { exportBodyHtml } from '../utils/html-export';
|
||||
import { repairOrphanNodes } from '../utils/orphan-repair';
|
||||
@@ -196,6 +196,14 @@ export function buildSavePayload(input: BuildSavePayloadInput) {
|
||||
};
|
||||
}
|
||||
|
||||
/** A saved page's state could not be read; load() fails rather than opening a blank canvas. */
|
||||
export class UnreadableSiteError extends Error {
|
||||
constructor() {
|
||||
super('Saved page state could not be deserialized');
|
||||
this.name = 'UnreadableSiteError';
|
||||
}
|
||||
}
|
||||
|
||||
export interface SaveOptions {
|
||||
forceEmpty?: boolean;
|
||||
}
|
||||
@@ -344,6 +352,19 @@ export function useWhpApi() {
|
||||
|
||||
// Restore pages and load the first page into the canvas
|
||||
if (proj.pages_craft_state && Array.isArray(proj.pages_craft_state) && proj.pages_craft_state.length > 0) {
|
||||
// Validate every page up front so one unreadable page fails the whole
|
||||
// load instead of surfacing later, on a page switch, as a blank canvas.
|
||||
for (const p of proj.pages_craft_state) {
|
||||
if (!p.craftState) continue;
|
||||
try {
|
||||
const parsed = typeof p.craftState === 'string' ? JSON.parse(p.craftState) : p.craftState;
|
||||
if (!parsed || typeof parsed !== 'object' || !parsed.ROOT) throw new Error('missing ROOT');
|
||||
} catch (e) {
|
||||
console.error('Failed to load page state:', e);
|
||||
throw new UnreadableSiteError();
|
||||
}
|
||||
}
|
||||
|
||||
setPagesCraftState(proj.pages_craft_state.map((p: { id: string; name: string; slug: string; craftState: string | null; seo?: PageData['seo'] }) => ({
|
||||
id: p.id, name: p.name, slug: p.slug, craftState: p.craftState || null, seo: p.seo,
|
||||
})));
|
||||
@@ -374,16 +395,10 @@ export function useWhpApi() {
|
||||
}
|
||||
actions.deserialize(state);
|
||||
} catch (e) {
|
||||
// I5: this is the initial load that decides whether the user
|
||||
// sees a working editor at all -- align with `loadState`'s
|
||||
// behaviour (console.error + a known-safe fallback) instead of
|
||||
// warning quietly and leaving the Frame on whatever it last had.
|
||||
// Never fall back to a blank canvas: autosave would then replace
|
||||
// the customer's real (if unreadable) page with it. Fail the load.
|
||||
console.error('Failed to load page state:', e);
|
||||
try {
|
||||
actions.deserialize(EMPTY_CANVAS);
|
||||
} catch (_e2) {
|
||||
// give up
|
||||
}
|
||||
throw new UnreadableSiteError();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,9 +2,9 @@ import React from 'react';
|
||||
import { useSitesmith } from '../../hooks/useSitesmith';
|
||||
import { useEditorConfig } from '../../state/EditorConfigContext';
|
||||
|
||||
interface Props { onClick: () => void; }
|
||||
interface Props { onClick: () => void; disabled?: boolean; }
|
||||
|
||||
export const SitesmithButton: React.FC<Props> = ({ onClick }) => {
|
||||
export const SitesmithButton: React.FC<Props> = ({ onClick, disabled }) => {
|
||||
const cfg = useEditorConfig();
|
||||
const siteId = cfg.whpConfig?.siteId ?? 0;
|
||||
const { summary } = useSitesmith(siteId);
|
||||
@@ -14,6 +14,7 @@ export const SitesmithButton: React.FC<Props> = ({ onClick }) => {
|
||||
<button
|
||||
type="button"
|
||||
onClick={onClick}
|
||||
disabled={disabled}
|
||||
className="topbar-btn sitesmith-btn"
|
||||
title={locked ? 'Sitesmith — paid addon (click to learn more)' : 'Sitesmith AI Builder'}
|
||||
style={{
|
||||
|
||||
@@ -185,10 +185,6 @@ export const TemplateModal: React.FC<TemplateModalProps> = ({ open, onClose }) =
|
||||
const tpl = confirmTemplate;
|
||||
setConfirmTemplate(null);
|
||||
|
||||
// A blank template replaces the whole site with an empty one; the user
|
||||
// just confirmed that, so the next save must not be refused as accidental.
|
||||
if (tpl.pages.every((p) => !p.content.components?.length)) armForceEmptyNextSave();
|
||||
|
||||
try {
|
||||
// 1. Optionally apply design tokens
|
||||
if (applyDesign && tpl.design) {
|
||||
@@ -237,6 +233,12 @@ export const TemplateModal: React.FC<TemplateModalProps> = ({ open, onClose }) =
|
||||
if (keepId) {
|
||||
await applyHeaderFooter(tpl, keepId);
|
||||
}
|
||||
|
||||
// A blank template replaces the whole site with an empty one; the user
|
||||
// confirmed that, so the next save must not be refused as accidental.
|
||||
// Armed only after the apply succeeded -- a failed apply must not
|
||||
// leave a force_empty pending for an unrelated later save.
|
||||
if (tpl.pages.every((p) => !p.content.components?.length)) armForceEmptyNextSave();
|
||||
} catch (e) {
|
||||
console.error('Failed to load template:', e);
|
||||
} finally {
|
||||
|
||||
@@ -260,7 +260,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
||||
<button
|
||||
className="topbar-btn icon-only"
|
||||
onClick={() => actions.history.undo()}
|
||||
disabled={!canUndo}
|
||||
disabled={!canUndo || !canSave}
|
||||
aria-label="Undo"
|
||||
data-tooltip="Undo"
|
||||
>
|
||||
@@ -269,7 +269,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
||||
<button
|
||||
className="topbar-btn icon-only"
|
||||
onClick={() => actions.history.redo()}
|
||||
disabled={!canRedo}
|
||||
disabled={!canRedo || !canSave}
|
||||
aria-label="Redo"
|
||||
data-tooltip="Redo"
|
||||
>
|
||||
@@ -320,7 +320,8 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
||||
onOpenHeadCode={() => setHeadCodeModalOpen(true)}
|
||||
onOpenReportIssue={() => setReportOpen(true)}
|
||||
onPreview={handlePreview}
|
||||
sitesmithNode={<SitesmithButton onClick={() => openSitesmith()} />}
|
||||
sitesmithNode={<SitesmithButton onClick={() => openSitesmith()} disabled={!canSave} />}
|
||||
editingDisabled={!canSave}
|
||||
/>
|
||||
|
||||
<TemplateModal open={templateModalOpen} onClose={() => setTemplateModalOpen(false)} />
|
||||
@@ -333,7 +334,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
||||
return (
|
||||
<nav className="topbar">
|
||||
<PublishWarnings warnings={publishWarnings} onDismiss={() => setPublishWarnings([])} />
|
||||
{emptyBanner}
|
||||
{emptyBanner}
|
||||
<div className="topbar-left">
|
||||
{isWHP && (
|
||||
<a href={whpConfig!.backUrl} className="topbar-btn back-btn" aria-label="Back to Panel">
|
||||
@@ -374,19 +375,20 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
||||
</div>
|
||||
|
||||
<div className="topbar-right">
|
||||
<button className="topbar-btn" onClick={() => actions.history.undo()} disabled={!canUndo} title="Undo" aria-label="Undo">
|
||||
<button className="topbar-btn" onClick={() => actions.history.undo()} disabled={!canUndo || !canSave} title="Undo" aria-label="Undo">
|
||||
<i className="fa fa-undo" />
|
||||
</button>
|
||||
<button className="topbar-btn" onClick={() => actions.history.redo()} disabled={!canRedo} title="Redo" aria-label="Redo">
|
||||
<button className="topbar-btn" onClick={() => actions.history.redo()} disabled={!canRedo || !canSave} title="Redo" aria-label="Redo">
|
||||
<i className="fa fa-repeat" />
|
||||
</button>
|
||||
<span className="topbar-divider" />
|
||||
<button className="topbar-btn" title="Templates" aria-label="Templates" onClick={() => setTemplateModalOpen(true)}>
|
||||
<button className="topbar-btn" disabled={!canSave} title="Templates" aria-label="Templates" onClick={() => setTemplateModalOpen(true)}>
|
||||
<i className="fa fa-th-large" /> Templates
|
||||
</button>
|
||||
<button
|
||||
className="topbar-btn icon-only"
|
||||
aria-label="Custom Head Code"
|
||||
disabled={!canSave}
|
||||
data-tooltip="Code"
|
||||
onClick={() => setHeadCodeModalOpen(true)}
|
||||
>
|
||||
@@ -434,7 +436,7 @@ export const TopBar: React.FC<TopBarProps> = ({ device, onDeviceChange, showGuid
|
||||
</span>
|
||||
)}
|
||||
|
||||
<SitesmithButton onClick={() => openSitesmith()} />
|
||||
<SitesmithButton onClick={() => openSitesmith()} disabled={!canSave} />
|
||||
|
||||
<button
|
||||
className="topbar-btn primary"
|
||||
|
||||
@@ -16,6 +16,8 @@ export interface TopBarOverflowMenuProps {
|
||||
* re-implemented here so the mobile menu reuses the exact same
|
||||
* lock/cap-aware button the desktop topbar shows. */
|
||||
sitesmithNode: React.ReactNode;
|
||||
/** Canvas-changing items (Templates, Head Code) are disabled until the site has loaded. */
|
||||
editingDisabled?: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -37,6 +39,7 @@ export const TopBarOverflowMenu: React.FC<TopBarOverflowMenuProps> = ({
|
||||
onOpenReportIssue,
|
||||
onPreview,
|
||||
sitesmithNode,
|
||||
editingDisabled,
|
||||
}) => {
|
||||
const menuRef = useRef<HTMLDivElement>(null);
|
||||
|
||||
@@ -84,13 +87,13 @@ export const TopBarOverflowMenu: React.FC<TopBarOverflowMenuProps> = ({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<button type="button" className="topbar-overflow-item" role="menuitem" onClick={runAndClose(onOpenTemplates)}>
|
||||
<button type="button" className="topbar-overflow-item" role="menuitem" disabled={editingDisabled} onClick={runAndClose(onOpenTemplates)}>
|
||||
<i className="fa fa-th-large" aria-hidden="true" /> Templates
|
||||
</button>
|
||||
<button type="button" className="topbar-overflow-item" role="menuitem" onClick={runAndClose(onPreview)}>
|
||||
<i className="fa fa-eye" aria-hidden="true" /> Preview
|
||||
</button>
|
||||
<button type="button" className="topbar-overflow-item" role="menuitem" onClick={runAndClose(onOpenHeadCode)}>
|
||||
<button type="button" className="topbar-overflow-item" role="menuitem" disabled={editingDisabled} onClick={runAndClose(onOpenHeadCode)}>
|
||||
<i className="fa fa-code" aria-hidden="true" /> Head Code
|
||||
</button>
|
||||
<button type="button" className="topbar-overflow-item" role="menuitem" onClick={runAndClose(onOpenReportIssue)}>
|
||||
|
||||
@@ -1934,7 +1934,7 @@ body {
|
||||
everything below the topbar so nothing is editable behind it. */
|
||||
.load-gate {
|
||||
position: fixed;
|
||||
top: 48px;
|
||||
top: var(--topbar-height);
|
||||
left: 0;
|
||||
right: 0;
|
||||
bottom: 0;
|
||||
|
||||
Reference in new issue
Block a user