diff --git a/.changeset/autosave-stops-after-rejected-save.md b/.changeset/autosave-stops-after-rejected-save.md new file mode 100644 index 0000000000..80265fb4a3 --- /dev/null +++ b/.changeset/autosave-stops-after-rejected-save.md @@ -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. diff --git a/e2e/tests/autosave.spec.ts b/e2e/tests/autosave.spec.ts index 69891c9428..998759cfc3 100644 --- a/e2e/tests/autosave.spec.ts +++ b/e2e/tests/autosave.spec.ts @@ -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); + }); }); diff --git a/packages/admin/src/components/ContentEditor.tsx b/packages/admin/src/components/ContentEditor.tsx index abf09240b3..a755df8c1c 100644 --- a/packages/admin/src/components/ContentEditor.tsx +++ b/packages/admin/src/components/ContentEditor.tsx @@ -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) */ @@ -235,6 +237,7 @@ export function ContentEditor({ isAutosaving, isAutosaveFeedbackActive, autosaveCompletionToken, + autosaveRejectionToken, onPublish, onUnpublish, onDiscardDraft, @@ -336,6 +339,7 @@ export function ContentEditor({ }), ); const pendingAutosaveStateRef = React.useRef(null); + const [rejectedAutosaveState, setRejectedAutosaveState] = React.useState(null); // Synchronously reset form state when the underlying item changes (e.g. a // translation switch where TanStack Router keeps ContentEditor mounted but @@ -366,6 +370,7 @@ export function ContentEditor({ }), ); pendingAutosaveStateRef.current = null; + setRejectedAutosaveState(null); setBylinesTouched(false); } @@ -392,6 +397,7 @@ export function ContentEditor({ }), ); pendingAutosaveStateRef.current = null; + setRejectedAutosaveState(null); setBylinesTouched(false); } }, [item?.updatedAt, itemDataString, itemBylinesString, item?.slug, item?.status]); @@ -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) => { for (const [name, field] of Object.entries(fields)) { @@ -481,6 +496,10 @@ export function ContentEditor({ return; } + if (currentData === rejectedAutosaveState) { + return; + } + // Clear any pending autosave if (autosaveTimeoutRef.current) { clearTimeout(autosaveTimeoutRef.current); @@ -523,6 +542,7 @@ export function ContentEditor({ bylinesTouched, hasInvalidUrls, hasUnsupportedPortableTextMarks, + rejectedAutosaveState, ]); // Cancel pending autosave on manual save diff --git a/packages/admin/src/lib/api/client.ts b/packages/admin/src/lib/api/client.ts index b928e297c8..0656ab0461 100644 --- a/packages/admin/src/lib/api/client.ts +++ b/packages/admin/src/lib/api/client.ts @@ -58,6 +58,26 @@ function formatValidationIssues(error: Record): 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 = 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 diff --git a/packages/admin/src/lib/api/index.ts b/packages/admin/src/lib/api/index.ts index 1a55c6d1f5..d8f09fb0e6 100644 --- a/packages/admin/src/lib/api/index.ts +++ b/packages/admin/src/lib/api/index.ts @@ -9,6 +9,7 @@ export { API_BASE, ApiResponseError, apiFetch, + isTerminalRequestError, parseApiResponse, throwResponseError, type FindManyResult, diff --git a/packages/admin/src/router.tsx b/packages/admin/src/router.tsx index f5e4328d48..5d3fed2d1c 100644 --- a/packages/admin/src/router.tsx +++ b/packages/admin/src/router.tsx @@ -121,6 +121,7 @@ import { renameMediaFolder, deleteMediaFolder, ApiResponseError, + isTerminalRequestError, useCurrentUser, type CreateCollectionInput, type UpdateCollectionInput, @@ -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 >(new Map()); @@ -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 }), @@ -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`, @@ -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} diff --git a/packages/admin/tests/components/ContentEditor.test.tsx b/packages/admin/tests/components/ContentEditor.test.tsx index 8cedb1d292..a099e3efc3 100644 --- a/packages/admin/tests/components/ContentEditor.test.tsx +++ b/packages/admin/tests/components/ContentEditor.test.tsx @@ -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(); + const titleInput = screen.getByLabelText("Title"); + await titleInput.fill("Too long"); + + await vi.advanceTimersByTimeAsync(2000); + expect(onAutosave).toHaveBeenCalledTimes(1); + + await screen.rerender(); + await screen.rerender( + , + ); + + 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", () => { diff --git a/packages/admin/tests/lib/api-client.test.ts b/packages/admin/tests/lib/api-client.test.ts index 0c80e0fe5c..4cace4544c 100644 --- a/packages/admin/tests/lib/api-client.test.ts +++ b/packages/admin/tests/lib/api-client.test.ts @@ -4,6 +4,7 @@ import { ApiResponseError, apiFetch, fetchManifest, + isTerminalRequestError, throwResponseError, } from "../../src/lib/api/client"; @@ -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); + }); +}); diff --git a/packages/admin/tests/router.test.tsx b/packages/admin/tests/router.test.tsx index 0a56747bb4..a4f8f20564 100644 --- a/packages/admin/tests/router.test.tsx +++ b/packages/admin/tests/router.test.tsx @@ -54,6 +54,7 @@ vi.mock("../src/components/ContentEditor", () => ({ isSaveFeedbackActive, isUpdatingPublishedAt, autosaveCompletionToken, + autosaveRejectionToken, }: { item?: { data?: { title?: string }; slug?: string | null }; onSave?: (payload: { data: Record }) => void; @@ -65,6 +66,7 @@ vi.mock("../src/components/ContentEditor", () => ({ isSaveFeedbackActive?: boolean; isUpdatingPublishedAt?: boolean; autosaveCompletionToken?: number; + autosaveRejectionToken?: number; }) => (
{item?.data?.title ?? ""}
@@ -73,6 +75,7 @@ vi.mock("../src/components/ContentEditor", () => ({
{isSaving ? "blocked" : "ready"}
{isSaving || isAutosaving ? "blocked" : "ready"}
{autosaveCompletionToken ?? 0}
+
{autosaveRejectionToken ?? 0}
{ e.preventDefault(); @@ -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(); + 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(); + 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"); + }); });