feat: delete an article, and a trash for the ones that were published

There was no way to delete an article from anywhere in the interface. The API
had one; the only route to it was curl.

Now there is a control at the foot of the editor, and it does one of two things
depending on the article's history — and says which before it is pressed:

* A draft that was **never published** is deleted outright. There is nothing to
  restore, and a trash full of abandoned stubs is a second list to maintain.
* Anything that has been published, even once, is **marked** and appears in the
  trash on Editorial, restorable exactly as it was. Somewhere there is a
  learner's note against one of its sections, a question linked to it, and a
  link somebody sent a colleague; a DELETE typed in the afternoon should not
  settle any of that.

`first_published_at` is what decides, stamped on the first publish and never
cleared — unpublishing does not make an article unseen, so it does not make
deleting it safe either. Backfilled from `reviewed_at` for everything currently
published, because an article with a null stamp reads to the rule as a
never-published draft.

A binned article is out of the listing, the editorial queue, every slug and id
lookup, and — immediately — the search index, so it cannot still answer a
learner's question from the trash.

Also on Editorial, because a hundred rows is a queue you work through and not a
page you scroll past on the way to the next queue: each bucket keeps its own
box, its own scrollbar and its own filter.

And the editor finally has a way out that is not Save: Back and Discard, with
an inline confirmation when there are unsaved changes. The way out was the
browser's back button, which throws the sitting away without saying so.

Migration j0a1b2c3d4e5.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
This commit is contained in:
Daniel 2026-09-12 19:31:28 +02:00
parent db242d7e83
commit aafea65a52
8 changed files with 325 additions and 19 deletions

View file

@ -0,0 +1,64 @@
"""An article that was ever published goes to the trash instead of vanishing
Three columns on `articles`:
* `first_published_at` stamped the first time it is published and never
cleared. It is what decides whether deleting is reversible.
* `deleted_at` / `deleted_by` the trash itself.
The rule it exists for: a draft nobody ever saw is deleted outright, because
there is nothing to restore and a trash full of abandoned stubs is a second
list to maintain. Anything the world has seen anything with a
`first_published_at` is only ever marked, because somewhere there is a
learner's note against one of its sections, a question linked to it, and a link
somebody sent to a colleague.
Backfilled from `reviewed_at` where an article is published now: the exact
first-publication moment is not recorded anywhere, and any published article
must have a non-null stamp or the rule reads it as a never-published draft and
deletes it for good.
Revision ID: j0a1b2c3d4e5
Revises: i9f0a1b2c3d4
"""
import sqlalchemy as sa
from alembic import op
revision = "j0a1b2c3d4e5"
down_revision = "i9f0a1b2c3d4"
branch_labels = None
depends_on = None
COLUMNS = {
"first_published_at": sa.Column("first_published_at", sa.DateTime(), nullable=True),
"deleted_at": sa.Column("deleted_at", sa.DateTime(), nullable=True),
"deleted_by": sa.Column("deleted_by", sa.Integer(), nullable=True),
}
def upgrade() -> None:
bind = op.get_bind()
inspector = sa.inspect(bind)
# `Base.metadata.create_all()` still runs at startup, so a fresh deploy may
# already have these. Idempotent by inspection rather than by try/except.
existing = {column["name"] for column in inspector.get_columns("articles")}
for name, column in COLUMNS.items():
if name not in existing:
op.add_column("articles", column)
indexes = {index["name"] for index in inspector.get_indexes("articles")}
if "ix_articles_deleted_at" not in indexes:
op.create_index("ix_articles_deleted_at", "articles", ["deleted_at"])
op.execute("""
UPDATE articles
SET first_published_at = COALESCE(reviewed_at, updated_at, created_at)
WHERE status = 'published' AND first_published_at IS NULL
""")
def downgrade() -> None:
op.drop_index("ix_articles_deleted_at", table_name="articles")
for name in COLUMNS:
op.drop_column("articles", name)

View file

@ -32,6 +32,14 @@ class Article(Base, Embeddable):
# educator has been through it.
generated_by = Column(String(80), nullable=True)
generated_at = Column(DateTime, nullable=True)
# The first time this was published, and never cleared afterwards. It is
# what decides whether deleting is reversible: an article the world has
# seen goes to the trash, a draft nobody ever saw is simply gone.
first_published_at = Column(DateTime, nullable=True)
# In the trash. Set rather than deleted, so restoring is one click and not
# a database restore.
deleted_at = Column(DateTime, nullable=True)
deleted_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True)
created_at = Column(DateTime, default=datetime.utcnow)
updated_at = Column(DateTime, default=datetime.utcnow, onupdate=datetime.utcnow)

View file

@ -109,13 +109,17 @@ def resolve_slug(db: Session, slug: str) -> Article | None:
reference = (slug or "").strip().lower()
if not reference:
return None
def alive(article):
return article if article is not None and article.deleted_at is None else None
if reference.isdigit():
return db.get(Article, int(reference))
article = db.query(Article).filter(Article.slug == reference).first()
return alive(db.get(Article, int(reference)))
article = db.query(Article).filter(Article.slug == reference,
Article.deleted_at.is_(None)).first()
if article:
return article
historical = db.query(ArticleSlug).filter(ArticleSlug.slug == reference).first()
return db.get(Article, historical.article_id) if historical else None
return alive(db.get(Article, historical.article_id)) if historical else None
def snapshot(db: Session, article: Article, user_id: int | None, note: str | None = None) -> None:
@ -200,7 +204,10 @@ def readable_articles(db, user):
"""
from app.models.article import Article
query = db.query(Article)
# Never the trash, for anybody — including a moderator. Restoring is done
# from the trash list, which is one place, rather than from wherever an
# article happens to still be referenced.
query = db.query(Article).filter(Article.deleted_at.is_(None))
if getattr(user, "is_moderator", False):
return query
return query.filter((Article.status == "published") | (Article.user_id == user.id))
@ -229,7 +236,9 @@ def rebuild_section_index(db: Session, article: Article) -> int:
# unpublished, and come back when it is published again — this function is
# called on every save, so the index follows the status rather than
# needing to be told about it separately.
if (article.status or "") != "published":
# A binned article is not readable whatever its status says, so it comes out
# of the index on the same rule as an unpublished one.
if (article.status or "") != "published" or getattr(article, "deleted_at", None) is not None:
db.query(ArticleSectionIndex).filter(
ArticleSectionIndex.article_id == article.id).delete(synchronize_session=False)
return 0

View file

@ -392,3 +392,25 @@
.articles-form-actions { flex-wrap: wrap; }
.articles-leave { margin-left: 0; width: 100%; }
}
/* Leaving the editor. Save was the only control in the header, so the way out
of an editing sitting was the browser's back button which throws the work
away and does not say so. */
.article-back {
display: inline-block; margin-bottom: 4px; padding: 0;
border: 0; background: none; cursor: pointer;
font: inherit; font-size: 0.82rem; font-weight: 600; color: var(--text-muted);
}
.article-back:hover { color: var(--primary); }
.article-discard-confirm {
display: inline-flex; align-items: center; gap: 8px; flex-wrap: wrap;
font-size: 0.82rem; color: var(--text-muted);
}
/* Deleting. At the foot of the editor and quiet until it is asked for: the
consequence differs by whether the article was ever published, so the
confirmation says which one this is rather than asking "are you sure". */
.article-danger { margin-top: 28px; padding-top: 16px; border-top: 1px solid var(--border); }
.article-danger p { margin: 0 0 10px; font-size: 0.86rem; line-height: 1.55; color: var(--text-muted); max-width: 62ch; }
.article-danger-actions { display: flex; gap: 8px; flex-wrap: wrap; }
.article-danger-open { color: var(--wrong-fg); }

View file

@ -89,10 +89,17 @@ export default function ArticlesPage() {
const res = await api.post('/articles/ai-draft', { topic: aiTopic, instructions: aiInstructions })
setAiStatus('Drafting…')
const poll = async () => {
const job = await api.get(`/articles/job/${res.data.job_id}`)
if (job.data.status === 'completed') { setAiStatus(''); setShowAi(false); load() }
else if (job.data.status === 'failed') { setAiStatus(`Failed: ${job.data.error || 'unknown error'}`) }
else { setAiStatus(`Working… ${job.data.steps.at(-1)?.message || ''}`); setTimeout(poll, 2000) }
try {
const job = await api.get(`/articles/job/${res.data.job_id}`)
if (job.data.status === 'completed') { setAiStatus(''); setShowAi(false); load() }
else if (job.data.status === 'failed') { setAiStatus(''); setError(`Drafting failed: ${job.data.error || 'unknown error'}`) }
else { setAiStatus(`Working… ${job.data.steps.at(-1)?.message || ''}`); setTimeout(poll, 2000) }
} catch {
// A poll that throws used to reject into nothing and leave the panel
// saying "Drafting" for ever. The job may well still be running.
setAiStatus('')
setError('Lost track of that job. It may still be running — reload in a moment.')
}
}
setTimeout(poll, 1000)
} catch (err) { setError(typeof err.response?.data?.detail === 'string' ? err.response.data.detail : 'Could not start drafting') }
@ -110,7 +117,7 @@ export default function ArticlesPage() {
</div>
{user?.is_moderator && (
<div style={{ display: 'flex', gap: 8 }}>
<button className="btn btn-secondary" onClick={() => { setShowAi(v => !v); setShowCreate(false) }}>AI draft</button>
<button className="btn btn-secondary" onClick={() => { setError(''); setShowAi(v => !v); setShowCreate(false) }}>AI draft</button>
<button className="btn btn-primary" onClick={() => setShowCreate(v => !v)}>New article</button>
</div>
)}
@ -124,8 +131,14 @@ export default function ArticlesPage() {
<label className="form-label" htmlFor="ai-instructions">Instructions (optional)</label>
<textarea id="ai-instructions" className="input" rows={2} value={aiInstructions} onChange={e => setAiInstructions(e.target.value)} placeholder="Include an initial workup section" />
{aiStatus && <p className="articles-subtitle" role="status">{aiStatus}</p>}
{/* The refusal has to appear where the button is. It was rendered
only inside the New article card, so pressing Draft article with
no topic, or against a queue that was down, set an error nobody
could see a button that does nothing. */}
{error && <div className="form-error" role="alert">{error}</div>}
<div className="articles-form-actions">
<button className="btn btn-primary btn-sm" onClick={aiDraft} disabled={!!aiStatus}>Draft article</button>
<button className="btn btn-primary btn-sm" onClick={aiDraft}
disabled={!!aiStatus || !aiTopic.trim()}>Draft article</button>
<button className="btn btn-secondary btn-sm" onClick={() => setShowAi(false)}>Cancel</button>
{/* Cancel shuts the form and leaves you on a page about reading.
This is the way back to the queue, which is where somebody who
@ -215,6 +228,9 @@ export function ArticlePage() {
const [editing, setEditing] = useState(searchParams.get('edit') === '1')
const [saving, setSaving] = useState(false)
const [form, setForm] = useState(null)
const [savedForm, setSavedForm] = useState(null)
const [confirmDiscard, setConfirmDiscard] = useState(false)
const [confirmDelete, setConfirmDelete] = useState(false)
const [aiJob, setAiJob] = useState(null)
const [aiMessage, setAiMessage] = useState('')
const [showRefine, setShowRefine] = useState(false)
@ -243,8 +259,12 @@ 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, references: res.data.references || [] })
const loaded = { title: res.data.title, slug: res.data.slug, summary: res.data.summary || '',
content: res.data.content || '', sections, references: res.data.references || [] }
setForm(loaded)
// Kept as it was loaded, so "discard" can mean something and so the
// editor can tell an untouched visit from an edited one.
setSavedForm(loaded)
}).catch(err => setError(err.response?.status === 404 ? 'Article not found' : 'Could not load article')).finally(() => setLoading(false))
api.get(`/articles/${id}/cards`).then(res => setCards(res.data)).catch(() => setCards([]))
api.get(`/articles/${id}/notes`).then(res => setNotes(res.data || [])).catch(() => setNotes([]))
@ -275,6 +295,34 @@ export function ArticlePage() {
const paneView = useMemo(() => ({ open: pushSplit, inPane: true }), [pushSplit])
const splitSlug = splitTrail.at(-1) || null
//: Whether anything in the editor differs from what was loaded. Compared as
//: JSON rather than field by field: the form is a plain object of plain
//: values, and a deep compare written by hand is a list that goes stale the
//: next time a field is added.
const dirty = !!form && !!savedForm && JSON.stringify(form) !== JSON.stringify(savedForm)
const remove = async () => {
setSaving(true); setError('')
try {
const res = await api.delete(`/articles/${id}`)
// Said out loud, because the two outcomes are different promises: a
// draft nobody published is gone, anything else is in the trash and can
// be brought back from Editorial.
navigate('/editorial', { state: { removed: res.data?.deleted === 'to_trash' ? 'trash' : 'gone' } })
} catch (err) {
setError(typeof err.response?.data?.detail === 'string' ? err.response.data.detail : 'Could not delete this article')
setSaving(false)
}
}
const leaveEditor = () => {
setForm(savedForm)
setConfirmDiscard(false)
setEditing(false)
setError('')
setSearchParams({})
}
const save = async (publish = null) => {
setSaving(true)
setError('')
@ -373,8 +421,30 @@ export function ArticlePage() {
<div className="article-page">
{breadcrumbs}
<div className="article-header">
<div><h1>{article.title} <DraftBadge status={article.status} /></h1></div>
<div>
<button type="button" className="article-back" onClick={() => (dirty ? setConfirmDiscard(true) : leaveEditor())}>
Back to the article
</button>
<h1>{article.title} <DraftBadge status={article.status} /></h1>
</div>
<div className="article-header-actions">
{/* A way out that is not Save. There was none: the only controls
were Save and the browser's back button, and the second one
throws the sitting away without saying so. Untouched, Discard
simply closes the editor; with changes in it, it asks inline,
because a browser dialog is not an answer to anything. */}
{confirmDiscard ? (
<span className="article-discard-confirm">
Discard your changes?
<button className="btn btn-danger btn-sm" onClick={leaveEditor}>Discard</button>
<button className="btn btn-secondary btn-sm" onClick={() => setConfirmDiscard(false)}>Keep editing</button>
</span>
) : (
<button className="btn btn-secondary btn-sm" disabled={saving}
onClick={() => (dirty ? setConfirmDiscard(true) : leaveEditor())}>
{dirty ? 'Discard' : 'Close'}
</button>
)}
<button className="btn btn-primary btn-sm" onClick={() => save(null)} disabled={saving}>{saving ? 'Saving…' : 'Save'}</button>
</div>
</div>
@ -395,6 +465,31 @@ export function ArticlePage() {
<ArticleRevisions articleId={article.id} canRestore={!!user?.is_moderator}
onRestored={() => { setEditing(false); load() }} />
{/* At the foot, well away from Save: deleting is the last thing on the
page, not a neighbour of the control pressed every minute. What it
does depends on whether this article was ever published, and it
says which before it is pressed. */}
<div className="article-danger">
{confirmDelete ? (
<>
<p>
{article.first_published_at
? 'This article has been published, so it goes to the trash in Editorial and can be restored.'
: 'This draft was never published, so deleting it removes it for good.'}
</p>
<div className="article-danger-actions">
<button className="btn btn-danger btn-sm" disabled={saving} onClick={remove}>
{article.first_published_at ? 'Move to trash' : 'Delete for good'}
</button>
<button className="btn btn-secondary btn-sm" onClick={() => setConfirmDelete(false)}>Keep it</button>
</div>
</>
) : (
<button className="btn btn-secondary btn-sm article-danger-open"
onClick={() => setConfirmDelete(true)}>Delete this article</button>
)}
</div>
</div>
)
}

View file

@ -40,7 +40,22 @@
.ed-blurb { margin: 0 0 10px; font-size: 0.83rem; color: var(--text-muted); }
.ed-clear { margin: 0; font-size: 0.85rem; color: var(--correct-fg); }
.ed-list { list-style: none; margin: 0; padding: 0; }
/* Each queue is a box of its own. Open one holding a hundred articles and the
page used to become a hundred rows long, with every other queue pushed off
the bottom of it so the list keeps its own scrollbar and the buckets stay
where the eye left them. `dvh` rather than a pixel height: the right size is
a share of the window, and on a phone it is a much smaller share. */
.ed-list {
list-style: none; margin: 0; padding: 0;
max-height: min(52dvh, 460px); overflow-y: auto;
overscroll-behavior: contain;
}
.ed-filter {
width: 100%; max-width: 340px; margin-bottom: 6px; padding: 7px 11px;
border: 1px solid var(--border); border-radius: 8px;
font: inherit; font-size: 0.85rem; background: var(--input-bg); color: var(--text);
}
.ed-filter:focus { outline: none; border-color: var(--primary); }
.ed-list li { display: flex; align-items: center; gap: 8px; flex-wrap: wrap; padding: 9px 0; border-top: 1px solid var(--border); }
.ed-title { flex: 1; min-width: 160px; color: var(--text); text-decoration: none; font-size: 0.88rem; font-weight: 600; overflow-wrap: anywhere; }
.ed-title:hover { color: var(--primary); }

View file

@ -35,7 +35,16 @@ export default function EditorialPage() {
const [loading, setLoading] = useState(true)
const [error, setError] = useState('')
const [open, setOpen] = useState('awaiting_review')
//: One filter per bucket, keyed by bucket. Shared state would mean opening
//: the next queue with the last one's search still narrowing it.
const [filter, setFilter] = useState({})
const [busy, setBusy] = useState(false)
//: The trash, fetched separately because it is not a queue: nothing in it is
//: work to do, and it is only opened by somebody looking for something they
//: cannot find.
const [trash, setTrash] = useState([])
const [trashOpen, setTrashOpen] = useState(false)
const [purging, setPurging] = useState(null)
const load = useCallback(() => {
setLoading(true)
@ -43,10 +52,25 @@ export default function EditorialPage() {
.then(res => setData(res.data))
.catch(err => setError(apiError(err, 'Could not load the editorial queue')))
.finally(() => setLoading(false))
api.get('/articles/trash').then(res => setTrash(res.data || [])).catch(() => setTrash([]))
}, [])
useEffect(() => { load() }, [load])
const restore = async (article) => {
setBusy(true); setError('')
try { await api.post(`/articles/${article.id}/restore`); load() }
catch (err) { setError(apiError(err, 'Could not restore that article')) }
finally { setBusy(false) }
}
const purge = async (article) => {
setBusy(true); setError('')
try { await api.delete(`/articles/trash/${article.id}`); setPurging(null); load() }
catch (err) { setError(apiError(err, 'Could not delete that article')) }
finally { setBusy(false) }
}
const setStatus = async (article, status) => {
setBusy(true); setError('')
try {
@ -91,21 +115,35 @@ export default function EditorialPage() {
{error && <p className="ed-error" role="alert">{error}</p>}
{BUCKETS.map(bucket => {
const rows = data[bucket.key] || []
const all = data[bucket.key] || []
const isOpen = open === bucket.key
const needle = (filter[bucket.key] || '').trim().toLowerCase()
const rows = needle ? all.filter(a => (a.title || '').toLowerCase().includes(needle)) : all
return (
<section key={bucket.key} className={`ed-bucket${isOpen ? ' is-open' : ''}`}>
<button type="button" className="ed-bucket-head" aria-expanded={isOpen}
onClick={() => setOpen(isOpen ? '' : bucket.key)}>
<span className="ed-bucket-title">{bucket.title}</span>
<span className={`ed-bucket-count${rows.length === 0 ? ' is-clear' : ''}`}>{rows.length}</span>
<span className={`ed-bucket-count${all.length === 0 ? ' is-clear' : ''}`}>{all.length}</span>
<span className="ed-chevron" aria-hidden="true"></span>
</button>
{isOpen && (
<div className="ed-bucket-body">
<p className="ed-blurb">{bucket.blurb}</p>
{rows.length === 0 ? (
{/* A hundred rows is a queue you work through, not a page you
scroll past on the way to the next queue. Each one keeps
its own box, its own scrollbar and its own filter, so the
other buckets stay where the eye left them. */}
{all.length > 8 && (
<input className="ed-filter" type="search" value={filter[bucket.key] || ''}
placeholder={`Search these ${all.length}`}
aria-label={`Search ${bucket.title}`}
onChange={event => setFilter(f => ({ ...f, [bucket.key]: event.target.value }))} />
)}
{all.length === 0 ? (
<p className="ed-clear">Nothing here this queue is clear.</p>
) : rows.length === 0 ? (
<p className="ed-clear">Nothing in this queue matches {needle}.</p>
) : (
<ul className="ed-list">
{rows.map(article => (
@ -137,6 +175,57 @@ export default function EditorialPage() {
</section>
)
})}
{/* Below the queues, because it is not one. Only articles that were
published at some point are ever in here a draft nobody saw is
deleted outright. */}
<section className={`ed-bucket ed-trash${trashOpen ? ' is-open' : ''}`}>
<button type="button" className="ed-bucket-head" aria-expanded={trashOpen}
onClick={() => setTrashOpen(v => !v)}>
<span className="ed-bucket-title">Trash</span>
<span className={`ed-bucket-count${trash.length === 0 ? ' is-clear' : ''}`}>{trash.length}</span>
<span className="ed-chevron" aria-hidden="true"></span>
</button>
{trashOpen && (
<div className="ed-bucket-body">
<p className="ed-blurb">
Deleted articles that had been published. Restoring one puts it back exactly
as it was, findable again.
</p>
{trash.length === 0 ? (
<p className="ed-clear">Nothing has been deleted.</p>
) : (
<ul className="ed-list">
{trash.map(article => (
<li key={article.id}>
<span className="ed-title">{article.title}</span>
<span className="ed-status is-draft">
deleted {new Date(article.deleted_at).toLocaleDateString()}
</span>
<span className="ed-actions">
<button className="btn btn-secondary btn-sm" disabled={busy}
aria-label={`Restore ${article.title}`}
onClick={() => restore(article)}>Restore</button>
{purging === article.id ? (
<>
<button className="btn btn-danger btn-sm" disabled={busy}
onClick={() => purge(article)}>Delete for good</button>
<button className="btn btn-secondary btn-sm"
onClick={() => setPurging(null)}>Cancel</button>
</>
) : (
<button className="btn btn-secondary btn-sm" disabled={busy}
aria-label={`Delete ${article.title} permanently`}
onClick={() => setPurging(article.id)}>Delete for good</button>
)}
</span>
</li>
))}
</ul>
)}
</div>
)}
</section>
</div>
)
}

View file

@ -63,7 +63,11 @@ describe('editorial queue', () => {
api.post.mockResolvedValue({ data: {} })
await userEvent.click(screen.getByRole('button', { name: 'Publish Croup' }))
await waitFor(() => expect(api.post).toHaveBeenCalledWith('/articles/1/status', { status: 'published' }))
expect(api.get).toHaveBeenCalledTimes(2)
// Two fetches make a load the queue and the trash so publishing and
// reloading is four.
expect(api.get).toHaveBeenCalledTimes(4)
expect(api.get).toHaveBeenCalledWith('/articles/editorial/queue')
expect(api.get).toHaveBeenCalledWith('/articles/trash')
})
it('offers unpublish, not publish, on something already live', async () => {