Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/autosave-stops-after-rejected-save.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@emdash-cms/admin": patch
---

Fixes the content editor's autosave so that a draft that the server rejected, such as a field value that exceeds its `maxLength`, is not resent every few seconds. The editor keeps the unsaved changes and tries again only after the content changes.
36 changes: 36 additions & 0 deletions e2e/tests/autosave.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,4 +127,40 @@ test.describe("Autosave", () => {
const latestRevision = data2.data.items?.[0];
expect(latestRevision?.data?.title).toBe("Edit Two");
});

test("does not resend a rejected autosave until the content changes", async ({ admin }) => {
await fetch(`${baseUrl}/_emdash/api/schema/collections/${collectionSlug}/fields`, {
method: "POST",
headers,
body: JSON.stringify({
slug: "summary",
type: "string",
label: "Summary",
validation: { maxLength: 10 },
}),
});

const contentUrl = `/_emdash/api/content/${collectionSlug}/${postId}`;
const isPut = (res: any) => res.url().includes(contentUrl) && res.request().method() === "PUT";
const putStatuses: number[] = [];
admin.page.on("response", (res) => {
if (isPut(res)) putStatuses.push(res.status());
});

await admin.goToEditContent(collectionSlug, postId);
await admin.waitForLoading();

const summaryInput = admin.page.locator("#field-summary");
const rejectedPut = admin.page.waitForResponse(isPut, { timeout: 10000 });
await summaryInput.fill("well over ten characters");
expect((await rejectedPut).status()).toBe(400);

await admin.page.waitForTimeout(7000);
expect(putStatuses).toEqual([400]);
await expect(summaryInput).toHaveValue("well over ten characters");

const acceptedPut = admin.page.waitForResponse(isPut, { timeout: 10000 });
await summaryInput.fill("short");
expect((await acceptedPut).status()).toBe(200);
});
});
20 changes: 20 additions & 0 deletions packages/admin/src/components/ContentEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ export interface ContentEditorProps {
isAutosaveFeedbackActive?: boolean;
/** Entry-scoped token advanced after a successful autosave. */
autosaveCompletionToken?: number;
/** Entry-scoped token advanced after the server rejected an autosave payload. */
autosaveRejectionToken?: number;
onPublish?: () => void;
onUnpublish?: () => void;
/** Callback to discard draft changes (revert to published version) */
Expand Down Expand Up @@ -235,6 +237,7 @@ export function ContentEditor({
isAutosaving,
isAutosaveFeedbackActive,
autosaveCompletionToken,
autosaveRejectionToken,
onPublish,
onUnpublish,
onDiscardDraft,
Expand Down Expand Up @@ -336,6 +339,7 @@ export function ContentEditor({
}),
);
const pendingAutosaveStateRef = React.useRef<string | null>(null);
const [rejectedAutosaveState, setRejectedAutosaveState] = React.useState<string | null>(null);

// Synchronously reset form state when the underlying item changes (e.g. a
// translation switch where TanStack Router keeps ContentEditor mounted but
Expand Down Expand Up @@ -366,6 +370,7 @@ export function ContentEditor({
}),
);
pendingAutosaveStateRef.current = null;
setRejectedAutosaveState(null);
setBylinesTouched(false);
}

Expand All @@ -392,6 +397,7 @@ export function ContentEditor({
}),
);
pendingAutosaveStateRef.current = null;
setRejectedAutosaveState(null);
setBylinesTouched(false);
}
}, [item?.updatedAt, itemDataString, itemBylinesString, item?.slug, item?.status]);
Expand Down Expand Up @@ -457,6 +463,15 @@ export function ContentEditor({
pendingAutosaveStateRef.current = null;
}, [autosaveCompletionToken]);

React.useEffect(() => {
if (!autosaveRejectionToken || !pendingAutosaveStateRef.current) {
return;
}

setRejectedAutosaveState(pendingAutosaveStateRef.current);
pendingAutosaveStateRef.current = null;
}, [autosaveRejectionToken]);

const hasInvalidUrls = React.useCallback(
(data: Record<string, unknown>) => {
for (const [name, field] of Object.entries(fields)) {
Expand All @@ -481,6 +496,10 @@ export function ContentEditor({
return;
}

if (currentData === rejectedAutosaveState) {
return;
}

// Clear any pending autosave
if (autosaveTimeoutRef.current) {
clearTimeout(autosaveTimeoutRef.current);
Expand Down Expand Up @@ -523,6 +542,7 @@ export function ContentEditor({
bylinesTouched,
hasInvalidUrls,
hasUnsupportedPortableTextMarks,
rejectedAutosaveState,
]);

// Cancel pending autosave on manual save
Expand Down
20 changes: 20 additions & 0 deletions packages/admin/src/lib/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,26 @@ function formatValidationIssues(error: Record<string, unknown>): string | undefi
return messages.length > 0 ? messages.join("; ") : undefined;
}

/**
* Client errors that pass no verdict on the request body, so resending it
* unchanged can still succeed. Every other 4xx repeats its verdict on every
* attempt.
*/
const RETRYABLE_CLIENT_ERROR_STATUSES: ReadonlySet<number> = new Set([
408, // Request Timeout: the server gave up waiting for the request
421, // Misdirected Request: another connection can be routed correctly
425, // Too Early: sent as TLS early data, replayable after the handshake
429, // Too Many Requests: succeeds once the rate limit window has passed
]);

/** Whether retrying the same request unchanged can never succeed. */
export function isTerminalRequestError(error: unknown): boolean {
if (!(error instanceof ApiResponseError)) return false;
return (
error.status >= 400 && error.status < 500 && !RETRYABLE_CLIENT_ERROR_STATUSES.has(error.status)
);
}

/**
* Throw an error with the message from the API response body if available,
* falling back to a generic message. All API error responses use the shape
Expand Down
1 change: 1 addition & 0 deletions packages/admin/src/lib/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export {
API_BASE,
ApiResponseError,
apiFetch,
isTerminalRequestError,
parseApiResponse,
throwResponseError,
type FindManyResult,
Expand Down
11 changes: 10 additions & 1 deletion packages/admin/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ import {
renameMediaFolder,
deleteMediaFolder,
ApiResponseError,
isTerminalRequestError,
useCurrentUser,
type CreateCollectionInput,
type UpdateCollectionInput,
Expand Down Expand Up @@ -941,6 +942,8 @@ function ContentEditPage() {
const itemLocale = rawItem?.locale ?? undefined;
const autosaveCompletionSequenceRef = React.useRef(0);
const [autosaveCompletion, setAutosaveCompletion] = React.useState({ entryId: "", token: 0 });
const autosaveRejectionSequenceRef = React.useRef(0);
const [autosaveRejection, setAutosaveRejection] = React.useState({ entryId: "", token: 0 });
const [editorSavePendingCounts, setEditorSavePendingCounts] = React.useState<
ReadonlyMap<string, number>
>(new Map());
Expand All @@ -957,6 +960,10 @@ function ContentEditPage() {
autosaveCompletionSequenceRef.current += 1;
setAutosaveCompletion({ entryId, token: autosaveCompletionSequenceRef.current });
}, []);
const recordAutosaveRejection = React.useCallback((entryId: string) => {
autosaveRejectionSequenceRef.current += 1;
setAutosaveRejection({ entryId, token: autosaveRejectionSequenceRef.current });
}, []);
const { data: bylinesData, isSuccess: bylinesLoaded } = useQuery({
queryKey: ["bylines", "picker", itemLocale ?? null],
queryFn: () => fetchBylines({ locale: itemLocale, limit: 100 }),
Expand Down Expand Up @@ -1066,7 +1073,8 @@ function ContentEditPage() {
// Keep the cache fresh without refetching older server state back into the form
// while the user is still typing.
},
onError: (err) => {
onError: (err, variables) => {
if (isTerminalRequestError(err)) recordAutosaveRejection(variables.targetId);
toastManager.add({
title: t`Autosave failed`,
description: err instanceof Error ? err.message : t`An error occurred`,
Expand Down Expand Up @@ -1344,6 +1352,7 @@ function ContentEditPage() {
autosaveMutation.isPending && autosaveMutation.variables?.targetId === id
}
autosaveCompletionToken={autosaveCompletion.entryId === id ? autosaveCompletion.token : 0}
autosaveRejectionToken={autosaveRejection.entryId === id ? autosaveRejection.token : 0}
onPublish={handlePublish}
onUnpublish={handleUnpublish}
onDiscardDraft={handleDiscardDraft}
Expand Down
49 changes: 49 additions & 0 deletions packages/admin/tests/components/ContentEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,55 @@ describe("ContentEditor", () => {
vi.useRealTimers();
}
});

it("does not resend a rejected autosave payload until the content changes", async () => {
vi.useFakeTimers();

try {
const item = makeItem();
const onAutosave = vi.fn();
const props: ContentEditorProps = {
collection: "posts",
collectionLabel: "Post",
fields: defaultFields,
isNew: false,
item,
onSave: vi.fn(),
onAutosave,
isAutosaving: false,
autosaveCompletionToken: 0,
autosaveRejectionToken: 0,
};

const screen = await render(<ContentEditor {...props} />);
const titleInput = screen.getByLabelText("Title");
await titleInput.fill("Too long");

await vi.advanceTimersByTimeAsync(2000);
expect(onAutosave).toHaveBeenCalledTimes(1);

await screen.rerender(<ContentEditor {...props} isAutosaving={true} />);
await screen.rerender(
<ContentEditor {...props} isAutosaving={false} autosaveRejectionToken={1} />,
);

await vi.advanceTimersByTimeAsync(10_000);
expect(onAutosave).toHaveBeenCalledTimes(1);
await expect.element(screen.getByLabelText("Title")).toHaveValue("Too long");
await expect
.element(screen.getByRole("button", { name: "Save", exact: true }).first())
.toBeEnabled();

await titleInput.fill("Short");
await vi.advanceTimersByTimeAsync(2000);
expect(onAutosave).toHaveBeenCalledTimes(2);
expect(onAutosave).toHaveBeenLastCalledWith(
expect.objectContaining({ data: expect.objectContaining({ title: "Short" }) }),
);
} finally {
vi.useRealTimers();
}
});
});

describe("delete", () => {
Expand Down
29 changes: 29 additions & 0 deletions packages/admin/tests/lib/api-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
ApiResponseError,
apiFetch,
fetchManifest,
isTerminalRequestError,
throwResponseError,
} from "../../src/lib/api/client";

Expand Down Expand Up @@ -167,3 +168,31 @@ describe("throwResponseError", () => {
);
});
});

describe("isTerminalRequestError", () => {
const apiError = (status: number, code: string, message: string) =>
new ApiResponseError(status, code, message);

it("treats a client error outside the retryable list as terminal", () => {
expect(isTerminalRequestError(apiError(400, "VALIDATION_ERROR", "rejected"))).toBe(true);
expect(isTerminalRequestError(apiError(401, "UNAUTHORIZED", "unauthorized"))).toBe(true);
expect(isTerminalRequestError(apiError(404, "NOT_FOUND", "not found"))).toBe(true);
expect(isTerminalRequestError(apiError(409, "CONFLICT", "conflict"))).toBe(true);
expect(isTerminalRequestError(apiError(413, "PAYLOAD_TOO_LARGE", "payload too large"))).toBe(
true,
);
});

it("keeps the retryable client errors retryable", () => {
expect(isTerminalRequestError(apiError(408, "REQUEST_TIMEOUT", "request timeout"))).toBe(false);
expect(isTerminalRequestError(apiError(421, "MISDIRECTED_REQUEST", "misdirected"))).toBe(false);
expect(isTerminalRequestError(apiError(425, "TOO_EARLY", "too early"))).toBe(false);
expect(isTerminalRequestError(apiError(429, "RATE_LIMITED", "slow down"))).toBe(false);
});

it("keeps server errors and network failures retryable", () => {
expect(isTerminalRequestError(apiError(500, "INTERNAL_ERROR", "boom"))).toBe(false);
expect(isTerminalRequestError(apiError(504, "GATEWAY_TIMEOUT", "gateway timeout"))).toBe(false);
expect(isTerminalRequestError(new TypeError("Failed to fetch"))).toBe(false);
});
});
55 changes: 55 additions & 0 deletions packages/admin/tests/router.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ vi.mock("../src/components/ContentEditor", () => ({
isSaveFeedbackActive,
isUpdatingPublishedAt,
autosaveCompletionToken,
autosaveRejectionToken,
}: {
item?: { data?: { title?: string }; slug?: string | null };
onSave?: (payload: { data: Record<string, unknown> }) => void;
Expand All @@ -65,6 +66,7 @@ vi.mock("../src/components/ContentEditor", () => ({
isSaveFeedbackActive?: boolean;
isUpdatingPublishedAt?: boolean;
autosaveCompletionToken?: number;
autosaveRejectionToken?: number;
}) => (
<div data-testid="content-editor">
<div data-testid="mock-title">{item?.data?.title ?? ""}</div>
Expand All @@ -73,6 +75,7 @@ vi.mock("../src/components/ContentEditor", () => ({
<div data-testid="manual-save-blocked">{isSaving ? "blocked" : "ready"}</div>
<div data-testid="autosave-blocked">{isSaving || isAutosaving ? "blocked" : "ready"}</div>
<div data-testid="autosave-completion-token">{autosaveCompletionToken ?? 0}</div>
<div data-testid="autosave-rejection-token">{autosaveRejectionToken ?? 0}</div>
<form
onSubmit={(e) => {
e.preventDefault();
Expand Down Expand Up @@ -1665,4 +1668,56 @@ describe("ContentEditPage – autosave cache patching", () => {
globalThis.fetch = fetchWithMocks;
}
});

it("signals a rejected autosave to the editor", async () => {
mockFetch.on(
"PUT",
"/_emdash/api/content/posts/post_1?locale=en",
{ error: { code: "VALIDATION_ERROR", message: "title: Too big" } },
400,
);
const { router, TestApp } = buildRouter();
await router.navigate({
to: "/content/$collection/$id",
params: { collection: "posts", id: "post_1" },
});
const screen = await render(<TestApp />);
await waitFor(() => {
expect(screen.getByTestId("mock-title").element().textContent).toBe("Draft Title");
});

await screen.getByRole("button", { name: "Trigger Draft Sync" }).click();

await waitFor(() => {
expect(screen.getByTestId("autosave-rejection-token").element().textContent).toBe("1");
});
expect(screen.getByTestId("autosave-completion-token").element().textContent).toBe("0");
expect(screen.getByTestId("mock-title").element().textContent).toBe("Draft Title");
});

it("does not signal a rejection for a server error", async () => {
mockFetch.on(
"PUT",
"/_emdash/api/content/posts/post_1?locale=en",
{ error: { code: "INTERNAL_ERROR", message: "boom" } },
500,
);
const { router, TestApp } = buildRouter();
await router.navigate({
to: "/content/$collection/$id",
params: { collection: "posts", id: "post_1" },
});
const screen = await render(<TestApp />);
await waitFor(() => {
expect(screen.getByTestId("mock-title").element().textContent).toBe("Draft Title");
});

await screen.getByRole("button", { name: "Trigger Draft Sync" }).click();

await expect.element(screen.getByText("Autosave failed")).toBeInTheDocument();
await waitFor(() => {
expect(screen.getByTestId("autosave-blocked").element().textContent).toBe("ready");
});
expect(screen.getByTestId("autosave-rejection-token").element().textContent).toBe("0");
});
});
Loading