From 4c67931587c49b2d401ebfb8a0fca765ac8f98c8 Mon Sep 17 00:00:00 2001 From: ssavutu Date: Thu, 6 Aug 2026 20:24:21 -0400 Subject: [PATCH] feat(editor): add a regenerate-slug button Rebuilds the slug from the current title. On a new article it just fills the slug box; on an existing one the regenerated slug is held as pending and only committed by an explicit save, so an article's public URL can never move under the 2.5s autosave. The editor redirects itself to the new /articles//edit afterwards, since the route it was on no longer resolves. articles.slug carries no unique index, so two articles could otherwise land on one public URL: the button probes GET /v1/articles/ and falls back to -2, -3, ... up to -20 rather than reusing a slug that another article already holds. The pending state warns that the old /article/ will 404 -- there is no redirect table, so nothing catches the old URL. Co-Authored-By: Claude Opus 5 --- frontend/src/pages/editArticleView.test.tsx | 54 +++++++ frontend/src/pages/editArticleView.tsx | 158 ++++++++++++++++---- 2 files changed, 182 insertions(+), 30 deletions(-) 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

-