Skip to content
Merged
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
2 changes: 2 additions & 0 deletions frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,10 @@ export default function App() {
<Routes>
<Route path="/" element={<DashboardPage />} />
<Route path="/articles" element={<ArticleView excludeType="developing-stories" />} />
<Route path="/articles/:id/:slug/edit" element={<EditArticleView />} />
<Route path="/articles/:slug/edit" element={<EditArticleView />} />
<Route path="/developing-stories" element={<DevelopingStoriesView />} />
<Route path="/developing-stories/:id/:slug/edit" element={<EditArticleView />} />
<Route path="/developing-stories/:slug/edit" element={<EditArticleView />} />
<Route path="/articles/new" element={<EditArticleView />} />
<Route path="/developing-stories/new" element={<ComingSoon page="New developing story" />} />
Expand Down
15 changes: 12 additions & 3 deletions frontend/src/pages/DashboardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ interface RecentArticle {
published_date: string | null
}

const editArticlePath = (article: Pick<RecentArticle, "id" | "slug">) =>
`/articles/${encodeURIComponent(String(article.id))}/${encodeURIComponent(article.slug)}/edit`

interface ApiStats {
totalArticles: number | null
publishedArticles: number | null
Expand Down Expand Up @@ -244,9 +247,11 @@ export default function DashboardPage() {
throw new Error(`Save failed (${response.status})`)
}

const created = (await response.json().catch(() => null)) as { id?: number | string; slug?: string } | null
const createdSlug = created?.slug || slug
let confirmed = false
for (let attempt = 0; attempt < 5; attempt += 1) {
const verifyResponse = await apiFetch(`/v1/articles/${encodeURIComponent(slug)}`)
const verifyResponse = await apiFetch(`/v1/articles/${encodeURIComponent(createdSlug)}`)
if (verifyResponse.ok) {
confirmed = true
break
Expand All @@ -270,7 +275,11 @@ export default function DashboardPage() {
}
}

navigate(`/articles/${encodeURIComponent(slug)}/edit`)
if (created?.id !== undefined && created.id !== null) {
navigate(`/articles/${encodeURIComponent(String(created.id))}/${encodeURIComponent(createdSlug)}/edit`)
} else {
navigate(`/articles/${encodeURIComponent(createdSlug)}/edit`)
}
} catch (err) {
const message = err instanceof Error ? err.message : "Unable to save draft."
setDraftError(message)
Expand Down Expand Up @@ -436,7 +445,7 @@ export default function DashboardPage() {
<tr
key={article.id}
className="hover:bg-muted/40 cursor-pointer transition-colors group"
onClick={() => navigate(`/articles/${article.slug}/edit`)}
onClick={() => navigate(editArticlePath(article))}
>
<td className="px-5 py-2.5 font-medium group-hover:text-primary transition-colors max-w-[240px] truncate">
{article.title}
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/pages/articleView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -488,10 +488,10 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
const effectiveTotalCount = Math.max(totalArticleCount, (page * pageSize) + articles.length)
const totalPages = Math.max(1, Math.ceil(effectiveTotalCount / pageSize))
const listLabel = pageTitle.toLowerCase()
const editPathForSlug = (slug: string) =>
const editPathForArticle = (item: ArticleItem) =>
fixedType === "developing-stories"
? `/developing-stories/${encodeURIComponent(slug)}/edit`
: `/articles/${encodeURIComponent(slug)}/edit`
? `/developing-stories/${encodeURIComponent(item.id)}/${encodeURIComponent(item.slug ?? "")}/edit`
: `/articles/${encodeURIComponent(item.id)}/${encodeURIComponent(item.slug ?? "")}/edit`

// subsectionRows walks the section, then each subsection on the trail, and
// keeps the levels that actually have children to offer.
Expand Down Expand Up @@ -562,7 +562,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
setDeleteError(null)
setDeletingArticleId(item.id)
try {
const response = await apiFetch(`/v1/articles/${encodeURIComponent(item.slug)}`, {
const response = await apiFetch(`/v1/articles/${encodeURIComponent(item.slug)}?id=${encodeURIComponent(item.id)}`, {
method: "DELETE",
})
if (!response.ok) {
Expand All @@ -589,7 +589,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
setDeleteError(null)
setDeletingArticleId(item.id)
try {
const response = await apiFetch(`/v1/articles/${encodeURIComponent(item.slug)}/restore`, {
const response = await apiFetch(`/v1/articles/${encodeURIComponent(item.slug)}/restore?id=${encodeURIComponent(item.id)}`, {
method: "PATCH",
})
if (!response.ok) {
Expand Down Expand Up @@ -830,7 +830,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
<Link
className="block min-w-0 truncate rounded-sm hover:text-primary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-primary/40"
title={item.title}
to={editPathForSlug(item.slug)}
to={editPathForArticle(item)}
>
{item.title}
</Link>
Expand Down Expand Up @@ -883,7 +883,7 @@ function ArticleView({ pageTitle = "Articles", fixedType, excludeType }: Article
disabled={!item.slug}
onClick={() => {
if (!item.slug) return
navigate(editPathForSlug(item.slug))
navigate(editPathForArticle(item))
}}
title={item.slug ? "Edit" : "Edit unavailable"}
type="button"
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/pages/commentsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ export default function CommentsView() {
{comments.map((comment) => {
const actionsDisabled = busyCommentId !== null
const hasMappedArticle = comment.article_slug && comment.article_id > 0
const articlePath = hasMappedArticle ? `/articles/${encodeURIComponent(comment.article_slug)}/edit` : ""
const articlePath = hasMappedArticle ? `/articles/${encodeURIComponent(String(comment.article_id))}/${encodeURIComponent(comment.article_slug)}/edit` : ""
const publicPath = comment.article_slug ? `${siteUrl}/article/${comment.article_slug}` : ""

return (
Expand Down
25 changes: 22 additions & 3 deletions frontend/src/pages/editArticleView.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@ vi.mock("../components/SeoAnalysis", () => ({ default: () => null }))

const patchCalls = () => apiCalls.filter((call) => call.method === "PATCH")

const renderEditor = async () => {
const renderEditor = async (
initialEntry = `/articles/${ARTICLE_SLUG}/edit`,
routePath = "/articles/:slug/edit",
) => {
const user = userEvent.setup({ advanceTimers: vi.advanceTimersByTime })
render(
<MemoryRouter initialEntries={[`/articles/${ARTICLE_SLUG}/edit`]}>
<MemoryRouter initialEntries={[initialEntry]}>
<Routes>
<Route path="/articles/:slug/edit" element={<EditArticleView />} />
<Route path={routePath} element={<EditArticleView />} />
</Routes>
</MemoryRouter>,
)
Expand Down Expand Up @@ -168,6 +171,22 @@ describe("EditArticleView autosave", () => {
expect(patches[0].body?.content).toContain("and more")
})

it("loads and saves by id when the editor route carries one", async () => {
const user = await renderEditor(
`/articles/42/${ARTICLE_SLUG}/edit`,
"/articles/:id/:slug/edit",
)

expect(apiCalls.some((call) => call.method === "GET" && call.url === `/v1/articles/${ARTICLE_SLUG}?id=42`)).toBe(true)

await user.type(screen.getByTestId("article-body"), " and more")
await waitOutAutosave()

const patches = patchCalls()
expect(patches).toHaveLength(1)
expect(patches[0].url).toBe(`/v1/articles/${ARTICLE_SLUG}?id=42`)
})

it("does not unpublish a live article when the editor only selects Draft", async () => {
articleStatus = "published"
articlePublishedDate = "2025-03-04T15:30:00Z"
Expand Down
43 changes: 30 additions & 13 deletions frontend/src/pages/editArticleView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,11 @@
function EditArticleView() {
const navigate = useNavigate()
const apiFetch = useApiFetch()
const { slug: rawSlug } = useParams<{ slug: string }>()
const { id: rawID, slug: rawSlug } = useParams<{ id?: string; slug: string }>()
const slug = useMemo(() => (rawSlug ? decodeURIComponent(rawSlug) : ""), [rawSlug])
const articleID = useMemo(() => (rawID && /^\d+$/.test(rawID) ? rawID : ""), [rawID])
const articleQuery = articleID ? `?id=${encodeURIComponent(articleID)}` : ""
const articleApiPath = slug ? `/v1/articles/${encodeURIComponent(slug)}${articleQuery}` : ""
const isNew = !rawSlug

const [slugInput, setSlugInput] = useState("")
Expand Down Expand Up @@ -423,7 +426,7 @@
setError(null)
setSuccessMessage(null)
try {
const response = await apiFetch(`/v1/articles/${encodeURIComponent(slug)}`)
const response = await apiFetch(articleApiPath)
if (!response.ok) {
throw new Error(await readErrorMessage(response, `Could not load article (${response.status})`))
}
Expand Down Expand Up @@ -496,7 +499,7 @@
return () => {
cancelled = true
}
}, [apiFetch, slug, isNew])
}, [apiFetch, articleApiPath, slug, isNew])

// Try to claim an advisory edit lock while this article is open. If someone
// else already holds it we surface who, block editing, and keep re-checking
Expand All @@ -507,7 +510,7 @@
if (isNew || !slug) return
setLockChecking(true)
try {
const response = await apiFetch(`/v1/articles/${encodeURIComponent(slug)}/edit-lock`, { method: "PUT" })
const response = await apiFetch(`/v1/articles/${encodeURIComponent(slug)}/edit-lock${articleQuery}`, { method: "PUT" })
if (response.status === 409) {
const payload = (await response.json().catch(() => null)) as { holder_name?: string } | null
setLockedBy(payload?.holder_name?.trim() || "another editor")
Expand All @@ -520,7 +523,7 @@
} finally {
setLockChecking(false)
}
}, [apiFetch, slug, isNew])
}, [apiFetch, articleQuery, slug, isNew])

useEffect(() => {
if (isNew || !slug) return
Expand All @@ -532,7 +535,7 @@
const release = () => {
if (released) return
released = true
void apiFetch(`/v1/articles/${encodeURIComponent(slug)}/edit-lock`, {
void apiFetch(`/v1/articles/${encodeURIComponent(slug)}/edit-lock${articleQuery}`, {
method: "DELETE",
keepalive: true,
}).catch(() => {})
Expand All @@ -544,7 +547,7 @@
window.removeEventListener("beforeunload", release)
release()
}
}, [apiFetch, slug, isNew, acquireLock])
}, [apiFetch, articleQuery, slug, isNew, acquireLock])

useEffect(() => {
let cancelled = false
Expand Down Expand Up @@ -817,11 +820,21 @@
body: JSON.stringify(createPayload),
})
if (!response.ok) {
throw new Error(`Create failed (${response.status})`)
throw new Error(await readErrorMessage(response, `Create failed (${response.status})`))
}
clearArticleListCache()
setSuccessMessage("Article created.")
navigate("/articles")
// The server owns the slug: a title or slug another article already uses
// comes back with a suffix. Land on the new article rather than the list
// so the editor is looking at the slug that was actually stored.
const created = (await response.json().catch(() => null)) as { id?: number; slug?: string } | null
if (created?.id && created.slug) {
navigate(`/articles/${encodeURIComponent(String(created.id))}/${encodeURIComponent(created.slug)}/edit`, {
replace: true,
})
} else {
navigate("/articles")
}
return
}

Expand Down Expand Up @@ -856,15 +869,18 @@
noindex: noIndex,
}

const response = await apiFetch(`/v1/articles/${encodeURIComponent(slug)}`, {
const response = await apiFetch(articleApiPath, {
method: "PATCH",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify(payload),
})
if (!response.ok) {
throw new Error(`Save failed (${response.status})`)
// A rename onto a slug another article holds comes back 409 with a
// message worth showing: the generic text would read as a save that
// failed for no reason.
throw new Error(await readErrorMessage(response, `Save failed (${response.status})`))
}
if (nextTiming) {
setPublishTiming(nextTiming)
Expand All @@ -886,7 +902,8 @@
// 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 })
const basePath = articleID ? `/articles/${encodeURIComponent(articleID)}` : "/articles"
navigate(`${basePath}/${encodeURIComponent(slugToSave)}/edit`, { replace: true })
}
} catch (err) {
const message = err instanceof Error ? err.message : "Unable to save article."
Expand Down Expand Up @@ -918,7 +935,7 @@
}, AUTOSAVE_DELAY_MS)

return () => window.clearTimeout(timer)
}, [articleSnapshot, isAutoSaving, isLoading, isNew, isSaving, lockedBy, selectedAuthorIds, selectedCategorySlugs])
}, [articleApiPath, articleID, articleSnapshot, isAutoSaving, isLoading, isNew, isSaving, lockedBy, selectedAuthorIds, selectedCategorySlugs])

Check warning on line 938 in frontend/src/pages/editArticleView.tsx

View workflow job for this annotation

GitHub Actions / frontend

React Hook useEffect has a missing dependency: 'saveArticle'. Either include it or remove the dependency array

const inputClass ="w-full px-3 py-2 rounded-lg border border-border bg-background text-sm text-foreground placeholder:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-primary/40 focus:border-primary transition"
const selectClass = "w-full px-3 py-2 rounded-lg border border-border bg-background text-sm text-foreground focus:outline-none focus:ring-2 focus:ring-primary/40 focus:border-primary transition"
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/pages/seoView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export default function SeoView() {
<button
key={`${issue.article_id}-${idx}`}
type="button"
onClick={() => navigate(`/articles/${encodeURIComponent(issue.slug)}/edit`)}
onClick={() => navigate(`/articles/${encodeURIComponent(String(issue.article_id))}/${encodeURIComponent(issue.slug)}/edit`)}
className={`flex items-start gap-3 rounded-xl border p-4 text-left transition-colors ${
issue.type === "error"
? "border-destructive/30 bg-destructive/5 hover:bg-destructive/10"
Expand Down
Loading
Loading