From 18afb138dcfb6ad3ec6d961b5190b7a748b3844a Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 10 Sep 2026 18:27:52 +0200 Subject: [PATCH] feat: edit every view and its sources, with a way back to any earlier save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An admin could already edit an article's prose, but not which view a section belonged to and not its references at all — generation attached those and nothing could touch them. And the revisions the API had been writing since the CMS landed were unreachable from the interface. Editing one view at a time Short, Long and Clinical are tabs, each showing only its own sections with a count on the tab. All three live in one list because they are one article, but editing them together made it impossible to tell which version you were changing, and a stray edit to the clinical view while meaning to fix the long one is a mistake nobody notices until a learner does. A parent can only be an earlier top-level section of the same view, which is what the server enforces. Deleting a section lifts its children rather than taking them with it: a survivor pointing at a section that no longer exists is worse than an orphan. References are editable and structured Title, author and pages, so the editorial queue's "published without sources" stays a truthful question. A save that does not mention references leaves them alone rather than clearing them, or an older client would silently strip the provenance generation attached. Version history Every save is listed with what it was, and any of them can be opened or put back. Restoring is itself a save, so the version you are leaving is kept too — a history you can only walk one way is not a safety net, it is a trapdoor. Someone else's draft returns 403 rather than being readable through its history. One bug this turned up: the page-number field was derived from the parsed array on every keystroke, so typing "12, 14" became "1214" the moment the comma landed. The field now holds what you are typing and the array holds what gets saved, and on blur it shows what was actually stored so a dropped entry is visible rather than a silent difference. 208 backend, 243 frontend green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- backend/app/routers/articles.py | 15 +- backend/tests/test_articles_cards.py | 79 ++++++++ frontend/src/components/ArticleEditor.css | 29 +++ frontend/src/components/ArticleEditor.jsx | 174 ++++++++++++++++++ .../src/components/ArticleEditor.test.jsx | 115 ++++++++++++ frontend/src/components/ArticleRevisions.css | 38 ++++ frontend/src/components/ArticleRevisions.jsx | 110 +++++++++++ frontend/src/pages/ArticlesPage.jsx | 46 +---- 8 files changed, 569 insertions(+), 37 deletions(-) create mode 100644 frontend/src/components/ArticleEditor.css create mode 100644 frontend/src/components/ArticleEditor.jsx create mode 100644 frontend/src/components/ArticleEditor.test.jsx create mode 100644 frontend/src/components/ArticleRevisions.css create mode 100644 frontend/src/components/ArticleRevisions.jsx diff --git a/backend/app/routers/articles.py b/backend/app/routers/articles.py index 6637ade..82b9846 100644 --- a/backend/app/routers/articles.py +++ b/backend/app/routers/articles.py @@ -2,7 +2,7 @@ import re from fastapi import APIRouter, Depends, HTTPException, Query -from pydantic import BaseModel, field_validator +from pydantic import BaseModel, Field, field_validator from sqlalchemy.orm import Session from app.database import get_db @@ -45,6 +45,14 @@ class ArticleSection(BaseModel): variant: str = "long" +class ArticleReference(BaseModel): + """Where a fact came from. Named sources, not markers in the prose.""" + + title: str = Field(min_length=1, max_length=300) + author: str | None = Field(default=None, max_length=300) + pages: list[int] = Field(default_factory=list, max_length=40) + + class ArticleWrite(BaseModel): title: str slug: str @@ -53,6 +61,9 @@ class ArticleWrite(BaseModel): sections: list[ArticleSection] = [] category_id: int | None = None section_id: int | None = None + # Absent means "leave them alone": a caller that predates references must not + # silently strip the ones generation attached. + references: list[ArticleReference] | None = None @field_validator("slug") @classmethod @@ -498,6 +509,8 @@ def update_article( for section in data.sections ]) article.category_id, article.section_id = data.category_id, data.section_id + if data.references is not None: + article.references_json = [ref.model_dump() for ref in data.references] article_service.record_slug(db, article) # Remediate links whose section was removed; whole-article links survive renames. kept = _section_ids(article) diff --git a/backend/tests/test_articles_cards.py b/backend/tests/test_articles_cards.py index b64a2fd..4c2950d 100644 --- a/backend/tests/test_articles_cards.py +++ b/backend/tests/test_articles_cards.py @@ -286,3 +286,82 @@ class ArticleReadingTests(unittest.TestCase): self.assertNotIn('##', excerpt) self.assertNotIn('/uploads/', excerpt) self.assertIn('the workup', excerpt) # a link keeps its words + + def test_all_three_views_round_trip_through_a_save(self): + payload = {"title": "Nested topic", "slug": "three-views", "summary": "S", "content": "I", + "sections": [ + {"id": "a" * 32, "slug": "short-1", "title": "In short", + "content": "- a bullet", "variant": "short"}, + {"id": "b" * 32, "slug": "long-1", "title": "Definition", + "content": "Body", "variant": "long"}, + {"id": "c" * 32, "slug": "clin-1", "title": "Management", + "content": "Give fluids", "variant": "clinical"}, + ]} + created = self.client.post('/articles/', json=payload).json() + self.assertEqual(created["variants"], ["short", "long", "clinical"]) + + # Editing one view leaves the other two exactly as they were. + payload["sections"][1]["content"] = "A better definition" + updated = self.client.patch(f"/articles/{created['id']}", json=payload).json() + by_variant = {s["variant"]: s["content"] for s in updated["sections"]} + self.assertEqual(by_variant["long"], "A better definition") + self.assertEqual(by_variant["short"], "- a bullet") + self.assertEqual(by_variant["clinical"], "Give fluids") + + def test_a_sub_section_cannot_belong_to_another_view(self): + response = self.make([ + {"id": "a" * 32, "slug": "s1", "title": "Definition", "content": "B", "variant": "long"}, + {"id": "b" * 32, "slug": "s2", "title": "Detail", "content": "B", + "variant": "clinical", "parent_id": "a" * 32}, + ], slug="mixed-nesting") + self.assertEqual(response.status_code, 400, response.text) + + def test_references_are_editable_and_absence_leaves_them_alone(self): + payload = {"title": "Refs", "slug": "refs-topic", "content": "I", "sections": [], + "references": [{"title": "Nelson", "author": "Kliegman", "pages": [12, 13]}]} + created = self.client.post('/articles/', json=payload).json() + + # Set on a save… + updated = self.client.patch(f"/articles/{created['id']}", json={ + **payload, "references": [{"title": "Mandell", "pages": [7]}]}).json() + self.assertEqual([r["title"] for r in updated["references"]], ["Mandell"]) + + # …and a caller that does not mention them must not wipe them, or an + # older client would silently strip the provenance generation attached. + no_refs = dict(payload) + no_refs.pop("references") + kept = self.client.patch(f"/articles/{created['id']}", json=no_refs).json() + self.assertEqual([r["title"] for r in kept["references"]], ["Mandell"]) + + def test_every_save_is_recoverable(self): + payload = {"title": "Versioned", "slug": "versioned", "content": "First draft", + "sections": [{"id": "a" * 32, "slug": "s", "title": "S", "content": "One", + "variant": "long"}]} + article = self.client.post('/articles/', json=payload).json() + + payload["content"] = "Second draft" + payload["sections"][0]["content"] = "Two" + self.client.patch(f"/articles/{article['id']}", json=payload) + + revisions = self.client.get(f"/articles/{article['id']}/revisions").json() + self.assertEqual(len(revisions), 1) + old = self.client.get(f"/articles/{article['id']}/revisions/{revisions[0]['id']}").json() + self.assertEqual(old["content"], "First draft") + + restored = self.client.post( + f"/articles/{article['id']}/revisions/{revisions[0]['id']}/restore").json() + self.assertEqual(restored["content"], "First draft") + self.assertEqual(restored["sections"][0]["content"], "One") + + # Restoring is itself a save: the version being left is kept, or the way + # back from a mistaken restore is gone. + after = self.client.get(f"/articles/{article['id']}/revisions").json() + self.assertEqual(len(after), 2) + self.assertIn("before restoring", after[0]["note"]) + + def test_history_is_not_a_way_into_someone_elses_draft(self): + article = self.make([self.section('a', 'A')], slug='private-history').json() + self.bank.user = self.bank.peer + self.assertEqual(self.client.get(f"/articles/{article['id']}/revisions").status_code, 403) + self.assertEqual(self.client.post( + f"/articles/{article['id']}/revisions/1/restore").status_code, 403) diff --git a/frontend/src/components/ArticleEditor.css b/frontend/src/components/ArticleEditor.css new file mode 100644 index 0000000..098df1d --- /dev/null +++ b/frontend/src/components/ArticleEditor.css @@ -0,0 +1,29 @@ +/* Editing an article: one view at a time, then its sources. */ + +.ae-hint { margin: 4px 0 12px; font-size: 0.8rem; color: var(--text-muted); } +.ae-empty { margin: 8px 0 12px; font-size: 0.85rem; color: var(--text-muted); } +.ae-heading { margin: 26px 0 4px; font-size: 1rem; } + +.ae-views { display: inline-flex; gap: 2px; margin-top: 20px; padding: 3px; background: var(--bg); border: 1px solid var(--border); border-radius: 9px; } +.ae-view { + display: inline-flex; align-items: center; gap: 6px; + min-height: 34px; padding: 6px 13px; border: 0; border-radius: 7px; cursor: pointer; + background: none; font: inherit; font-size: 0.83rem; font-weight: 600; color: var(--text-muted); +} +.ae-view:hover { color: var(--text); } +.ae-view.is-active { background: var(--card-bg); color: var(--primary); box-shadow: 0 1px 2px rgba(15,23,42,0.08); } +.ae-view-count { font-size: 0.7rem; padding: 0 6px; border-radius: 9px; background: var(--bg); color: var(--text-subtle); } +.ae-view.is-active .ae-view-count { background: var(--option-sel-bg); color: var(--primary); } + +.ae-section { margin-bottom: 12px; } +.ae-section-head { display: flex; gap: 8px; align-items: center; flex-wrap: wrap; margin-bottom: 8px; } +.ae-section-title { flex: 2; min-width: 160px; } +.ae-section-slug { flex: 1; min-width: 120px; } + +.ae-reference { display: flex; gap: 8px; align-items: center; flex-wrap: wrap; margin-bottom: 8px; } +.ae-reference .input { flex: 2; min-width: 150px; } +.ae-reference .ae-pages { flex: 1; min-width: 90px; } + +@media (max-width: 640px) { + .ae-section-head .btn, .ae-reference .btn { width: 100%; } +} diff --git a/frontend/src/components/ArticleEditor.jsx b/frontend/src/components/ArticleEditor.jsx new file mode 100644 index 0000000..f75262a --- /dev/null +++ b/frontend/src/components/ArticleEditor.jsx @@ -0,0 +1,174 @@ +import { useState } from 'react' +import RichEditor from './RichEditor' +import './ArticleEditor.css' + +const VIEWS = [ + { key: 'short', label: 'Short', hint: 'Bullets a learner could revise from the night before an exam.' }, + { key: 'long', label: 'Long', hint: 'The full article: definition through management.' }, + { key: 'clinical', label: 'Clinical', hint: 'What to do at the bedside, with doses and routes.' }, +] + +const sectionId = () => Array.from(crypto.getRandomValues(new Uint8Array(16)), + b => b.toString(16).padStart(2, '0')).join('') + +/** + * Editing one article's three views, its references, and nothing else. + * + * Sections are edited one view at a time. All three live in a single list — + * they are one article — but showing them together made it impossible to tell + * which version you were changing, and a stray edit to the clinical view while + * meaning to fix the long one is the kind of mistake nobody notices until a + * learner does. + * + * References are structured rather than free text so the editorial queue can + * ask "what is published without sources" and get a truthful answer. + */ +export default function ArticleEditor({ form, setForm }) { + const [view, setView] = useState('long') + // Page numbers are typed as text but stored as numbers. Deriving the field + // from the parsed array rewrites it on every keystroke, so "12, 1" becomes + // "121" the moment the comma is typed — the draft is what you are typing, the + // array is what gets saved. + const [pageDrafts, setPageDrafts] = useState({}) + + const sections = form.sections || [] + const inView = sections.map((sec, index) => ({ sec, index })) + .filter(({ sec }) => (sec.variant || 'long') === view) + + const patchSection = (index, patch) => setForm(f => ({ + ...f, sections: f.sections.map((s, j) => (j === index ? { ...s, ...patch } : s)), + })) + + const addSection = () => setForm(f => ({ + ...f, + sections: [...f.sections, { + id: sectionId(), slug: `${view}-section-${f.sections.length + 1}`, + title: '', content: '', parent_id: null, variant: view, + }], + })) + + const removeSection = (index) => setForm(f => ({ + ...f, + // Children of a removed section would point at nothing, so they come up a + // level rather than being deleted along with it. + sections: f.sections + .filter((_, j) => j !== index) + .map(s => (s.parent_id === f.sections[index].id ? { ...s, parent_id: null } : s)), + })) + + const patchReference = (index, patch) => setForm(f => ({ + ...f, references: (f.references || []).map((r, j) => (j === index ? { ...r, ...patch } : r)), + })) + + return ( +
+ + setForm(f => ({ ...f, title: e.target.value }))} /> + + + setForm(f => ({ ...f, slug: e.target.value }))} /> +

Renaming is safe — the old address keeps working and still finds this article.

+ + +