feat: edit every view and its sources, with a way back to any earlier save
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
parent
e1b580386c
commit
18afb138dc
8 changed files with 569 additions and 37 deletions
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
29
frontend/src/components/ArticleEditor.css
Normal file
29
frontend/src/components/ArticleEditor.css
Normal file
|
|
@ -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%; }
|
||||
}
|
||||
174
frontend/src/components/ArticleEditor.jsx
Normal file
174
frontend/src/components/ArticleEditor.jsx
Normal file
|
|
@ -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 (
|
||||
<div className="article-edit">
|
||||
<label className="form-label" htmlFor="edit-title">Title</label>
|
||||
<input id="edit-title" className="input" value={form.title}
|
||||
onChange={e => setForm(f => ({ ...f, title: e.target.value }))} />
|
||||
|
||||
<label className="form-label" htmlFor="edit-slug">Slug</label>
|
||||
<input id="edit-slug" className="input" value={form.slug}
|
||||
onChange={e => setForm(f => ({ ...f, slug: e.target.value }))} />
|
||||
<p className="ae-hint">Renaming is safe — the old address keeps working and still finds this article.</p>
|
||||
|
||||
<label className="form-label" htmlFor="edit-summary">Summary</label>
|
||||
<textarea id="edit-summary" className="input" rows={2} value={form.summary}
|
||||
onChange={e => setForm(f => ({ ...f, summary: e.target.value }))} />
|
||||
|
||||
<label className="form-label">Introduction</label>
|
||||
<RichEditor value={form.content} onChange={content => setForm(f => ({ ...f, content }))} height={160} />
|
||||
|
||||
<div className="ae-views" role="tablist" aria-label="Which view to edit">
|
||||
{VIEWS.map(option => {
|
||||
const count = sections.filter(s => (s.variant || 'long') === option.key).length
|
||||
return (
|
||||
<button key={option.key} type="button" role="tab"
|
||||
aria-selected={view === option.key}
|
||||
className={`ae-view${view === option.key ? ' is-active' : ''}`}
|
||||
onClick={() => setView(option.key)}>
|
||||
{option.label}<span className="ae-view-count">{count}</span>
|
||||
</button>
|
||||
)
|
||||
})}
|
||||
</div>
|
||||
<p className="ae-hint">{VIEWS.find(v => v.key === view)?.hint}</p>
|
||||
|
||||
{inView.length === 0 && (
|
||||
<p className="ae-empty">This view has no sections yet. A view with nothing in it is not offered to readers.</p>
|
||||
)}
|
||||
|
||||
{inView.map(({ sec, index }) => (
|
||||
<div key={sec.id} className="card ae-section">
|
||||
<div className="ae-section-head">
|
||||
<input className="input ae-section-title" value={sec.title}
|
||||
aria-label={`Title for section ${index + 1}`} placeholder="Section heading"
|
||||
onChange={e => patchSection(index, { title: e.target.value })} />
|
||||
<input className="input ae-section-slug" value={sec.slug}
|
||||
aria-label={`Slug for section ${index + 1}`}
|
||||
onChange={e => patchSection(index, { slug: e.target.value })} />
|
||||
{/* Only an earlier top-level section of the same view: nesting is one
|
||||
level deep, and a sub-section cannot belong to another view. */}
|
||||
<select className="input" value={sec.parent_id || ''}
|
||||
aria-label={`Parent section for section ${index + 1}`}
|
||||
onChange={e => patchSection(index, { parent_id: e.target.value || null })}>
|
||||
<option value="">Top-level section</option>
|
||||
{sections.slice(0, index)
|
||||
.filter(c => !c.parent_id && (c.variant || 'long') === (sec.variant || 'long'))
|
||||
.map(c => <option key={c.id} value={c.id}>Under: {c.title || c.slug}</option>)}
|
||||
</select>
|
||||
<button className="btn btn-sm btn-danger" onClick={() => removeSection(index)}
|
||||
aria-label={`Remove section ${index + 1}`}
|
||||
title="Removing a section also removes links to it">Remove</button>
|
||||
</div>
|
||||
<RichEditor value={sec.content} height={200}
|
||||
onChange={content => patchSection(index, { content })} />
|
||||
</div>
|
||||
))}
|
||||
|
||||
<button className="btn btn-secondary btn-sm" onClick={addSection}>
|
||||
Add {VIEWS.find(v => v.key === view)?.label.toLowerCase()} section
|
||||
</button>
|
||||
|
||||
<h3 className="ae-heading">References</h3>
|
||||
<p className="ae-hint">
|
||||
Sources for the article as a whole. There are no markers in the prose, so
|
||||
this list is the whole of the provenance.
|
||||
</p>
|
||||
{(form.references || []).length === 0 && <p className="ae-empty">No sources listed.</p>}
|
||||
{(form.references || []).map((ref, index) => (
|
||||
<div key={index} className="ae-reference">
|
||||
<input className="input" value={ref.title || ''} placeholder="Book or guideline"
|
||||
aria-label={`Reference ${index + 1} title`}
|
||||
onChange={e => patchReference(index, { title: e.target.value })} />
|
||||
<input className="input" value={ref.author || ''} placeholder="Author (optional)"
|
||||
aria-label={`Reference ${index + 1} author`}
|
||||
onChange={e => patchReference(index, { author: e.target.value })} />
|
||||
<input className="input ae-pages" placeholder="Pages"
|
||||
value={pageDrafts[index] ?? (ref.pages || []).join(', ')}
|
||||
aria-label={`Reference ${index + 1} pages`}
|
||||
onChange={e => {
|
||||
setPageDrafts(d => ({ ...d, [index]: e.target.value }))
|
||||
patchReference(index, {
|
||||
pages: e.target.value.split(',')
|
||||
.map(p => parseInt(p.trim(), 10)).filter(Number.isFinite),
|
||||
})
|
||||
}}
|
||||
onBlur={() => setPageDrafts(d => {
|
||||
// Once you leave the field it shows what was actually stored, so a
|
||||
// dropped entry is visible rather than a silent difference.
|
||||
const { [index]: _gone, ...rest } = d
|
||||
return rest
|
||||
})} />
|
||||
<button className="btn btn-sm btn-danger" aria-label={`Remove reference ${index + 1}`}
|
||||
onClick={() => setForm(f => ({ ...f, references: f.references.filter((_, j) => j !== index) }))}>
|
||||
Remove
|
||||
</button>
|
||||
</div>
|
||||
))}
|
||||
<button className="btn btn-secondary btn-sm"
|
||||
onClick={() => setForm(f => ({ ...f, references: [...(f.references || []), { title: '', author: '', pages: [] }] }))}>
|
||||
Add reference
|
||||
</button>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
115
frontend/src/components/ArticleEditor.test.jsx
Normal file
115
frontend/src/components/ArticleEditor.test.jsx
Normal file
|
|
@ -0,0 +1,115 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { render, screen, within } from '@testing-library/react'
|
||||
import userEvent from '@testing-library/user-event'
|
||||
import { useState } from 'react'
|
||||
import ArticleEditor from './ArticleEditor'
|
||||
|
||||
vi.mock('./RichEditor', () => ({
|
||||
default: ({ value, onChange }) => (
|
||||
<textarea aria-label="Body" value={value || ''} onChange={e => onChange(e.target.value)} />
|
||||
),
|
||||
}))
|
||||
|
||||
const START = {
|
||||
title: 'Bronchiolitis', slug: 'bronchiolitis', summary: 'A summary', content: 'Intro',
|
||||
sections: [
|
||||
{ id: 'a'.repeat(32), slug: 'in-short', title: 'In short', content: '- a bullet', parent_id: null, variant: 'short' },
|
||||
{ id: 'b'.repeat(32), slug: 'definition', title: 'Definition', content: 'Body', parent_id: null, variant: 'long' },
|
||||
{ id: 'c'.repeat(32), slug: 'features', title: 'Clinical features', content: 'Body', parent_id: null, variant: 'long' },
|
||||
{ id: 'd'.repeat(32), slug: 'management', title: 'Management', content: 'Body', parent_id: null, variant: 'clinical' },
|
||||
],
|
||||
references: [{ title: 'Nelson Textbook of Pediatrics', author: 'Kliegman', pages: [1234, 1235] }],
|
||||
}
|
||||
|
||||
let latest
|
||||
function Harness({ initial = START }) {
|
||||
const [form, setForm] = useState(initial)
|
||||
latest = form
|
||||
return <ArticleEditor form={form} setForm={setForm} />
|
||||
}
|
||||
|
||||
const mount = (initial) => render(<Harness initial={initial} />)
|
||||
const openView = (name) => userEvent.click(screen.getByRole('tab', { name: new RegExp(`^${name}`) }))
|
||||
|
||||
describe('editing an article', () => {
|
||||
beforeEach(() => { latest = null })
|
||||
|
||||
it('edits one view at a time so a stray edit cannot land in the wrong one', async () => {
|
||||
mount()
|
||||
// Long opens first and shows only its own sections.
|
||||
expect(screen.getByDisplayValue('Definition')).toBeInTheDocument()
|
||||
expect(screen.queryByDisplayValue('In short')).not.toBeInTheDocument()
|
||||
expect(screen.queryByDisplayValue('Management')).not.toBeInTheDocument()
|
||||
|
||||
await openView('Short')
|
||||
expect(screen.getByDisplayValue('In short')).toBeInTheDocument()
|
||||
expect(screen.queryByDisplayValue('Definition')).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('counts what each view holds, so an empty one is visible before you open it', async () => {
|
||||
mount()
|
||||
expect(within(screen.getByRole('tab', { name: /^Short/ })).getByText('1')).toBeInTheDocument()
|
||||
expect(within(screen.getByRole('tab', { name: /^Long/ })).getByText('2')).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('adds a section to the view being edited, not to the article at large', async () => {
|
||||
mount()
|
||||
await openView('Clinical')
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Add clinical section' }))
|
||||
const added = latest.sections[latest.sections.length - 1]
|
||||
expect(added.variant).toBe('clinical')
|
||||
expect(added.id).toMatch(/^[0-9a-f]{32}$/)
|
||||
})
|
||||
|
||||
it('offers only earlier sections of the same view as a parent', async () => {
|
||||
mount()
|
||||
const parent = screen.getByLabelText('Parent section for section 3')
|
||||
const options = [...parent.options].map(o => o.text)
|
||||
// 'Definition' is earlier and also long; 'In short' is earlier but another view.
|
||||
expect(options).toEqual(['Top-level section', 'Under: Definition'])
|
||||
})
|
||||
|
||||
it('lifts children rather than deleting them with their parent', async () => {
|
||||
mount({
|
||||
...START,
|
||||
sections: [
|
||||
{ id: 'b'.repeat(32), slug: 'definition', title: 'Definition', content: '', parent_id: null, variant: 'long' },
|
||||
{ id: 'e'.repeat(32), slug: 'sub', title: 'A sub-section', content: '', parent_id: 'b'.repeat(32), variant: 'long' },
|
||||
],
|
||||
})
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Remove section 1' }))
|
||||
expect(latest.sections).toHaveLength(1)
|
||||
// The survivor would otherwise point at a section that no longer exists.
|
||||
expect(latest.sections[0].parent_id).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps references structured, so "published without sources" can be answered', async () => {
|
||||
mount()
|
||||
expect(screen.getByLabelText('Reference 1 title')).toHaveValue('Nelson Textbook of Pediatrics')
|
||||
expect(screen.getByLabelText('Reference 1 pages')).toHaveValue('1234, 1235')
|
||||
|
||||
await userEvent.clear(screen.getByLabelText('Reference 1 pages'))
|
||||
await userEvent.type(screen.getByLabelText('Reference 1 pages'), '77, 78')
|
||||
expect(latest.references[0].pages).toEqual([77, 78])
|
||||
})
|
||||
|
||||
it('drops page entries that are not numbers rather than storing rubbish', async () => {
|
||||
mount()
|
||||
await userEvent.clear(screen.getByLabelText('Reference 1 pages'))
|
||||
await userEvent.type(screen.getByLabelText('Reference 1 pages'), '12, abc, 14')
|
||||
expect(latest.references[0].pages).toEqual([12, 14])
|
||||
})
|
||||
|
||||
it('adds and removes a reference', async () => {
|
||||
mount()
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Add reference' }))
|
||||
expect(latest.references).toHaveLength(2)
|
||||
await userEvent.click(screen.getByRole('button', { name: 'Remove reference 2' }))
|
||||
expect(latest.references).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('says a view is empty rather than looking broken', async () => {
|
||||
mount({ ...START, sections: [] })
|
||||
expect(screen.getByText(/no sections yet/)).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
38
frontend/src/components/ArticleRevisions.css
Normal file
38
frontend/src/components/ArticleRevisions.css
Normal file
|
|
@ -0,0 +1,38 @@
|
|||
/* The way back. */
|
||||
|
||||
.ar-wrap { margin-top: 18px; border-top: 1px solid var(--border); padding-top: 12px; }
|
||||
.ar-toggle {
|
||||
background: none; border: 0; padding: 0; cursor: pointer; font: inherit;
|
||||
font-size: 0.78rem; font-weight: 700; letter-spacing: 0.05em;
|
||||
text-transform: uppercase; color: var(--text-muted);
|
||||
}
|
||||
.ar-toggle:hover { color: var(--primary); }
|
||||
.ar-body { margin-top: 10px; }
|
||||
.ar-error { color: var(--wrong-fg); font-size: 0.84rem; }
|
||||
.ar-empty { color: var(--text-muted); font-size: 0.85rem; margin: 0; }
|
||||
|
||||
.ar-list { list-style: none; margin: 0; padding: 0; }
|
||||
.ar-list li { display: flex; align-items: center; gap: 10px; flex-wrap: wrap; padding: 9px 0; border-bottom: 1px solid var(--border); }
|
||||
.ar-when { font-size: 0.8rem; color: var(--text-muted); font-variant-numeric: tabular-nums; min-width: 150px; }
|
||||
.ar-title { flex: 1; min-width: 140px; font-size: 0.87rem; font-weight: 600; overflow-wrap: anywhere; }
|
||||
.ar-meta, .ar-note { font-size: 0.72rem; color: var(--text-subtle); }
|
||||
.ar-note { font-style: italic; }
|
||||
.ar-actions { display: flex; gap: 6px; }
|
||||
|
||||
.ar-preview { margin-top: 12px; padding: 12px 14px; background: var(--bg); border: 1px solid var(--border); border-radius: 10px; }
|
||||
.ar-preview-head { display: flex; justify-content: space-between; align-items: center; gap: 10px; }
|
||||
.ar-preview-head h4 { margin: 0; font-size: 0.95rem; }
|
||||
.ar-preview-summary { margin: 6px 0; font-size: 0.85rem; color: var(--text-muted); }
|
||||
.ar-preview-sections { list-style: none; margin: 8px 0 0; padding: 0; display: flex; flex-direction: column; gap: 4px; }
|
||||
.ar-preview-sections li { display: flex; align-items: center; gap: 8px; font-size: 0.84rem; }
|
||||
.ar-preview-variant {
|
||||
font-size: 0.63rem; font-weight: 700; text-transform: uppercase; letter-spacing: 0.04em;
|
||||
padding: 1px 7px; border-radius: 9px; background: var(--option-sel-bg); color: var(--primary);
|
||||
}
|
||||
.ar-preview-refs { margin: 10px 0 0; font-size: 0.8rem; color: var(--text-muted); }
|
||||
|
||||
@media (max-width: 640px) {
|
||||
.ar-when { min-width: 0; width: 100%; }
|
||||
.ar-actions { width: 100%; }
|
||||
.ar-actions .btn { flex: 1; }
|
||||
}
|
||||
110
frontend/src/components/ArticleRevisions.jsx
Normal file
110
frontend/src/components/ArticleRevisions.jsx
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
import { useCallback, useEffect, useState } from 'react'
|
||||
import api from '../api/client'
|
||||
import './ArticleRevisions.css'
|
||||
|
||||
const when = (value) => (value
|
||||
? new Date(value).toLocaleString(undefined, { dateStyle: 'medium', timeStyle: 'short' })
|
||||
: '')
|
||||
|
||||
/**
|
||||
* Every save this article has been through, and the way back to any of them.
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
export default function ArticleRevisions({ articleId, canRestore, onRestored }) {
|
||||
const [rows, setRows] = useState([])
|
||||
const [open, setOpen] = useState(false)
|
||||
const [preview, setPreview] = useState(null)
|
||||
const [busy, setBusy] = useState(false)
|
||||
const [error, setError] = useState('')
|
||||
|
||||
const load = useCallback(() => {
|
||||
api.get(`/articles/${articleId}/revisions`)
|
||||
.then(res => setRows(res.data || []))
|
||||
.catch(() => setError('Could not load the history'))
|
||||
}, [articleId])
|
||||
|
||||
useEffect(() => { if (open) load() }, [open, load])
|
||||
|
||||
const show = async (revision) => {
|
||||
setError('')
|
||||
try {
|
||||
const res = await api.get(`/articles/${articleId}/revisions/${revision.id}`)
|
||||
setPreview(res.data)
|
||||
} catch { setError('Could not open that version') }
|
||||
}
|
||||
|
||||
const restore = async (revision) => {
|
||||
setBusy(true); setError('')
|
||||
try {
|
||||
await api.post(`/articles/${articleId}/revisions/${revision.id}/restore`)
|
||||
setPreview(null)
|
||||
load()
|
||||
onRestored?.()
|
||||
} catch (err) {
|
||||
const detail = err?.response?.data?.detail
|
||||
setError(typeof detail === 'string' ? detail : 'Could not restore that version')
|
||||
} finally { setBusy(false) }
|
||||
}
|
||||
|
||||
return (
|
||||
<div className="ar-wrap">
|
||||
<button type="button" className="ar-toggle" aria-expanded={open}
|
||||
onClick={() => setOpen(v => !v)}>
|
||||
Version history {open ? '▲' : '▼'}
|
||||
</button>
|
||||
|
||||
{open && (
|
||||
<div className="ar-body">
|
||||
{error && <p className="ar-error" role="alert">{error}</p>}
|
||||
{rows.length === 0 ? (
|
||||
<p className="ar-empty">No earlier versions — this article has not been edited since it was written.</p>
|
||||
) : (
|
||||
<ul className="ar-list">
|
||||
{rows.map(revision => (
|
||||
<li key={revision.id}>
|
||||
<span className="ar-when">{when(revision.created_at)}</span>
|
||||
<span className="ar-title">{revision.title}</span>
|
||||
<span className="ar-meta">{revision.section_count} sections</span>
|
||||
{revision.note && <span className="ar-note">{revision.note}</span>}
|
||||
<span className="ar-actions">
|
||||
<button className="btn btn-secondary btn-sm"
|
||||
aria-label={`View version from ${when(revision.created_at)}`}
|
||||
onClick={() => show(revision)}>View</button>
|
||||
{canRestore && (
|
||||
<button className="btn btn-secondary btn-sm" disabled={busy}
|
||||
aria-label={`Restore version from ${when(revision.created_at)}`}
|
||||
onClick={() => restore(revision)}>Restore</button>
|
||||
)}
|
||||
</span>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
)}
|
||||
|
||||
{preview && (
|
||||
<div className="ar-preview">
|
||||
<div className="ar-preview-head">
|
||||
<h4>{preview.title}</h4>
|
||||
<button className="btn btn-secondary btn-sm" onClick={() => setPreview(null)}>Close</button>
|
||||
</div>
|
||||
{preview.summary && <p className="ar-preview-summary">{preview.summary}</p>}
|
||||
<ul className="ar-preview-sections">
|
||||
{(preview.sections || []).map(section => (
|
||||
<li key={section.id}>
|
||||
<span className="ar-preview-variant">{section.variant || 'long'}</span>
|
||||
{section.title}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
{(preview.references || []).length > 0 && (
|
||||
<p className="ar-preview-refs">{preview.references.length} references</p>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
|
@ -4,6 +4,8 @@ import api from '../api/client'
|
|||
import { useAuth } from '../context/AuthContext'
|
||||
import { SplitViewProvider } from '../context/SplitViewContext'
|
||||
import RichEditor from '../components/RichEditor'
|
||||
import ArticleEditor from '../components/ArticleEditor'
|
||||
import ArticleRevisions from '../components/ArticleRevisions'
|
||||
import CategoryColumns from '../components/CategoryColumns'
|
||||
import { resolveArticleId } from '../components/ArticleLink'
|
||||
import ArticleReader from '../components/ArticleReader'
|
||||
|
|
@ -208,7 +210,8 @@ export function ArticlePage() {
|
|||
if (from && sections.some(s => s.id === from)) return from
|
||||
return from ? prev : '' // No param: keep the main article view.
|
||||
})
|
||||
setForm({ title: res.data.title, slug: res.data.slug, summary: res.data.summary || '', content: res.data.content || '', sections })
|
||||
setForm({ title: res.data.title, slug: res.data.slug, summary: res.data.summary || '',
|
||||
content: res.data.content || '', sections, references: res.data.references || [] })
|
||||
}).catch(err => setError(err.response?.status === 404 ? 'Article not found' : 'Could not load article')).finally(() => setLoading(false))
|
||||
api.get(`/articles/${id}/questions`).then(res => setQuestions(res.data)).catch(() => setQuestions([]))
|
||||
api.get(`/articles/${id}/cards`).then(res => setCards(res.data)).catch(() => setCards([]))
|
||||
|
|
@ -334,41 +337,9 @@ export function ArticlePage() {
|
|||
)}
|
||||
|
||||
{editing && form ? (
|
||||
<div className="article-edit">
|
||||
<label className="form-label" htmlFor="edit-title">Title</label>
|
||||
<input id="edit-title" className="input" value={form.title} onChange={e => setForm(f => ({ ...f, title: e.target.value }))} />
|
||||
<label className="form-label" htmlFor="edit-slug">Slug</label>
|
||||
<input id="edit-slug" className="input" value={form.slug} onChange={e => setForm(f => ({ ...f, slug: e.target.value }))} />
|
||||
<label className="form-label" htmlFor="edit-summary">Summary</label>
|
||||
<textarea id="edit-summary" className="input" rows={2} value={form.summary} onChange={e => setForm(f => ({ ...f, summary: e.target.value }))} />
|
||||
<label className="form-label">Introduction</label>
|
||||
<RichEditor value={form.content} onChange={content => setForm(f => ({ ...f, content }))} height={180} />
|
||||
<h3 style={{ marginTop: 20 }}>Sections</h3>
|
||||
{form.sections.map((sec, i) => (
|
||||
<div key={sec.id} className="card" style={{ marginBottom: 12 }}>
|
||||
<div style={{ display: 'flex', gap: 8, alignItems: 'center', flexWrap: 'wrap' }}>
|
||||
<input className="input" style={{ flex: 2 }} value={sec.title} aria-label="Section title"
|
||||
onChange={e => setForm(f => ({ ...f, sections: f.sections.map((s, j) => j === i ? { ...s, title: e.target.value } : s) }))} />
|
||||
<input className="input" style={{ flex: 1 }} value={sec.slug} aria-label="Section slug"
|
||||
onChange={e => setForm(f => ({ ...f, sections: f.sections.map((s, j) => j === i ? { ...s, slug: e.target.value } : s) }))} />
|
||||
{/* Only an earlier top-level section can be a parent, which is what
|
||||
keeps nesting one level deep and a sub-section below its heading. */}
|
||||
<select className="input" style={{ flex: 1 }} value={sec.parent_id || ''} aria-label={`Parent section for section ${i + 1}`}
|
||||
onChange={e => setForm(f => ({ ...f, sections: f.sections.map((s, j) => j === i ? { ...s, parent_id: e.target.value || null } : s) }))}>
|
||||
<option value="">Top-level section</option>
|
||||
{form.sections.slice(0, i).filter(candidate => !candidate.parent_id).map(candidate => (
|
||||
<option key={candidate.id} value={candidate.id}>Under: {candidate.title || candidate.slug}</option>
|
||||
))}
|
||||
</select>
|
||||
<button className="btn btn-sm btn-danger" onClick={() => setForm(f => ({ ...f, sections: f.sections.filter((_, j) => j !== i) }))}
|
||||
title="Removing a section also removes links to it">Remove</button>
|
||||
</div>
|
||||
<RichEditor value={sec.content} height={220} onChange={content => setForm(f => ({ ...f, sections: f.sections.map((s, j) => j === i ? { ...s, content } : s) }))} />
|
||||
</div>
|
||||
))}
|
||||
<button className="btn btn-secondary btn-sm" onClick={() => setForm(f => ({
|
||||
...f, sections: [...f.sections, { id: sectionId(), slug: `section-${f.sections.length + 1}`, title: '', content: '', parent_id: null }],
|
||||
}))}>Add section</button>
|
||||
<div>
|
||||
<ArticleEditor form={form} setForm={setForm} />
|
||||
|
||||
<div className="card" style={{ marginTop: 16 }}>
|
||||
<h4>Link a question</h4>
|
||||
<p style={{ color: 'var(--text-muted)', fontSize: '0.85rem' }}>Enter a bank question ID, optionally scoped to one section.</p>
|
||||
|
|
@ -382,6 +353,9 @@ export function ArticlePage() {
|
|||
</div>
|
||||
{linkError && <div className="form-error" role="alert">{linkError}</div>}
|
||||
</div>
|
||||
|
||||
<ArticleRevisions articleId={article.id} canRestore={!!user?.is_moderator}
|
||||
onRestored={() => { setEditing(false); load() }} />
|
||||
</div>
|
||||
) : (
|
||||
<div className={`article-split${splitSlug ? ' is-open' : ''}`}>
|
||||
|
|
|
|||
Loading…
Reference in a new issue