diff --git a/frontend/src/pages/editArticleView.test.tsx b/frontend/src/pages/editArticleView.test.tsx index 1772d9d..b27929e 100644 --- a/frontend/src/pages/editArticleView.test.tsx +++ b/frontend/src/pages/editArticleView.test.tsx @@ -17,6 +17,9 @@ type ApiCall = { url: string; method: string; body: Record | nu let apiCalls: ApiCall[] = [] let articleStatus: string let articlePublishedDate: string | undefined +// Slugs belonging to other articles, so a GET for them answers 200 the way the +// API would rather than the 404 that means "free". +let takenSlugs: Set const jsonResponse = (payload: unknown, status = 200) => new Response(JSON.stringify(payload), { @@ -57,6 +60,12 @@ const apiFetchStub = vi.fn(async (url: string, init?: RequestInit): Promise { apiCalls = [] articleStatus = "draft" articlePublishedDate = undefined + takenSlugs = new Set() apiFetchStub.mockClear() // shouldAdvanceTime keeps Testing Library's own waitFor polling alive while // the autosave debounce stays under our control. @@ -166,6 +176,50 @@ describe("EditArticleView autosave", () => { expect(patches[0].body?.noindex).toBe(true) }) + // Regenerating rewrites the article's public URL, so it must wait for a + // deliberate save rather than riding along on the 2.5s autosave. + it("does not send a regenerated slug on autosave", async () => { + const user = await renderEditor() + + await user.clear(screen.getByLabelText("Title")) + await user.type(screen.getByLabelText("Title"), "Brand new headline") + await user.click(screen.getByRole("button", { name: /Regenerate from title/ })) + await waitFor(() => expect(screen.getByLabelText("Slug")).toHaveValue("brand-new-headline")) + await waitOutAutosave() + + const patches = patchCalls() + expect(patches.length).toBeGreaterThan(0) + for (const patch of patches) { + expect(patch.body).not.toHaveProperty("slug") + } + }) + + it("sends the regenerated slug on an explicit save", async () => { + const user = await renderEditor() + + await user.clear(screen.getByLabelText("Title")) + await user.type(screen.getByLabelText("Title"), "Brand new headline") + await user.click(screen.getByRole("button", { name: /Regenerate from title/ })) + await waitFor(() => expect(screen.getByLabelText("Slug")).toHaveValue("brand-new-headline")) + await user.click(screen.getByRole("button", { name: "Save Draft" })) + + await waitFor(() => expect(patchCalls().some((call) => call.body?.slug)).toBe(true)) + expect(patchCalls().find((call) => call.body?.slug)?.body?.slug).toBe("brand-new-headline") + }) + + // Two articles on one slug would make the public URL ambiguous, and the schema + // has no unique index to stop it. + it("suffixes a regenerated slug that another article already holds", async () => { + takenSlugs.add("brand-new-headline") + const user = await renderEditor() + + await user.clear(screen.getByLabelText("Title")) + await user.type(screen.getByLabelText("Title"), "Brand new headline") + await user.click(screen.getByRole("button", { name: /Regenerate from title/ })) + + await waitFor(() => expect(screen.getByLabelText("Slug")).toHaveValue("brand-new-headline-2")) + }) + it("publishes only when the publish button is pressed", async () => { const user = await renderEditor() diff --git a/frontend/src/pages/editArticleView.tsx b/frontend/src/pages/editArticleView.tsx index a6dc11f..575a988 100644 --- a/frontend/src/pages/editArticleView.tsx +++ b/frontend/src/pages/editArticleView.tsx @@ -1,6 +1,6 @@ import { lazy, Suspense, useCallback, useEffect, useMemo, useRef, useState } from "react" import type { KeyboardEvent } from "react" -import { ArrowLeft, Save, Image, Search, X, Copy, Check } from "lucide-react" +import { ArrowLeft, Save, Image, Search, X, Copy, Check, RefreshCw } from "lucide-react" import { useNavigate, useParams } from "react-router-dom" import { useApiFetch } from "../hooks/useApiFetch" import { articleUrl } from "../auth/urls" @@ -51,6 +51,9 @@ type PatchPayload = { title: string excerpt: string content: string + // Only sent when the editor regenerated the slug, and never by an autosave: + // changing it moves the article's public URL. + slug?: string // Both are omitted by autosave: a publish transition only happens when the // editor presses the publish button. status?: EditableStatus @@ -111,7 +114,10 @@ const isValidCanonicalUrl = (value: string): boolean => { return (parsed.protocol === "http:" || parsed.protocol === "https:") && parsed.host !== "" } -const slugifyCategory = (value: string): string => +// Mirrors db.CanonicalizeSlug on the server: lowercase, every run of +// non-alphanumerics collapsed to a single dash, no leading or trailing dash. +// Anything else is rejected as non-canonical by the API. +const slugify = (value: string): string => value .trim() .toLowerCase() @@ -212,6 +218,11 @@ function EditArticleView() { const isNew = !rawSlug const [slugInput, setSlugInput] = useState("") + // A slug regenerated for an existing article, held until the next explicit + // save: the URL an article is already reachable at must not move under a + // background autosave. Null means "keep the slug on file". + const [pendingSlug, setPendingSlug] = useState(null) + const [slugRegenerating, setSlugRegenerating] = useState(false) const [isLoading, setIsLoading] = useState(true) const [isSaving, setIsSaving] = useState(false) const [isAutoSaving, setIsAutoSaving] = useState(false) @@ -381,7 +392,7 @@ function EditArticleView() { const categorySlugs = (payload.categories ?? []) .map((category) => { const name = (category.name ?? "").trim() - const categorySlug = slugifyCategory(category.slug ?? name) + const categorySlug = slugify(category.slug ?? name) if (categorySlug && name) { legacyCategories[categorySlug] = name } @@ -670,10 +681,15 @@ function EditArticleView() { if (!slug) return + // Captured before the request so a regenerate landing mid-save cannot make + // the redirect below disagree with what was actually sent. + const slugToSave = !autosave && pendingSlug && pendingSlug !== slug ? pendingSlug : "" + const payload: PatchPayload = { title: title.trim(), excerpt: excerpt.trim(), content: content.trim(), + ...(slugToSave ? { slug: slugToSave } : {}), // An autosave omits both fields entirely; the handler leaves pub_date and // scheduled_pub_date untouched when neither is present, so the article // stays exactly as published (or as draft) as the editor left it. @@ -718,6 +734,12 @@ function EditArticleView() { } else { setSuccessMessage("Article saved.") } + if (slugToSave) { + // The route still points at the old slug, which no longer resolves, so + // move the editor onto the new one before anything refetches. + setPendingSlug(null) + navigate(`/articles/${encodeURIComponent(slugToSave)}/edit`, { replace: true }) + } } catch (err) { const message = err instanceof Error ? err.message : "Unable to save article." if (autosave) { @@ -925,6 +947,48 @@ function EditArticleView() { // On a new article the slug the server will assign is not known until it is // saved, so only offer the link once there is a real one. const effectiveSlug = isNew ? "" : slug + // Article slugs carry no uniqueness constraint, so handing two articles the + // same one would make the public URL ambiguous. Probe the API for the plain + // slug and fall back to -2, -3, ... the way WordPress did. + const findFreeSlug = async (base: string): Promise => { + for (let suffix = 1; suffix <= 20; suffix += 1) { + const candidate = suffix === 1 ? base : `${base}-${suffix}` + if (candidate === slug) return candidate + const response = await apiFetch(`/v1/articles/${encodeURIComponent(candidate)}`) + if (response.status === 404) return candidate + if (!response.ok) { + throw new Error(`Slug check failed (${response.status})`) + } + } + throw new Error(`Every slug from "${base}" to "${base}-20" is already taken.`) + } + + const regenerateSlug = async () => { + const base = slugify(title) + if (!base) { + setError("Add a title before regenerating the slug.") + return + } + setError(null) + setSuccessMessage(null) + setSlugRegenerating(true) + try { + const next = await findFreeSlug(base) + if (isNew) { + setSlugInput(next) + } else { + setPendingSlug(next === slug ? null : next) + if (next === slug) { + setSuccessMessage("The slug already matches the title.") + } + } + } catch (err) { + setError(err instanceof Error ? err.message : "Unable to regenerate the slug.") + } finally { + setSlugRegenerating(false) + } + } + const copyArticleLink = async () => { if (!effectiveSlug) return if (await copyText(articleUrl(effectiveSlug))) { @@ -1083,39 +1147,73 @@ function EditArticleView() {

Publish

-