feat: questions are soft-deleted, and the trash holds them

Question ids come from a sequence and are never reissued, and fourteen
tables point at them — attempts, quiz membership, exam membership,
media, article links, notes, favourites, feedback. Deleting the row took
all of that with it, so "restore" could only ever have meant typing the
text in again as a different question.

DELETE now sets deleted_at. The question leaves the bank, the builder,
search and every share path at once, because the exclusion lives in
general_question_predicate rather than at each call site. Restoring puts
back the same id, so everything that pointed at it still does. Erasing
for real requires the trash first and a moderator, and the confirmation
says what goes with it.

The trash page holds questions instead of tests. A test is a selection
you can remake in a minute; nobody wanted those back.

Used and withdrawn invite codes can be removed — an unused one is still
withdrawn rather than deleted, so it stays visible as having been issued
and stopped.

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-11 20:12:58 +02:00
parent 6b6c5e1b49
commit d1de9589ad
11 changed files with 327 additions and 37 deletions

View file

@ -0,0 +1,27 @@
"""Questions are soft-deleted.
A question id comes from a sequence and is never reissued, and fourteen tables
reference it attempts, quiz membership, exam membership, media, article
links, notes, favourites, feedback. Deleting the row took all of that with it,
so "restore" could never have meant more than "type it in again with a new id".
Revision ID: f6a7b8c9d0e1
Revises: e5f6a7b8c9d0
"""
import sqlalchemy as sa
from alembic import op
revision = "f6a7b8c9d0e1"
down_revision = "e5f6a7b8c9d0"
branch_labels = None
depends_on = None
def upgrade() -> None:
op.add_column("questions", sa.Column("deleted_at", sa.DateTime(), nullable=True))
op.create_index("ix_questions_deleted_at", "questions", ["deleted_at"])
def downgrade() -> None:
op.drop_index("ix_questions_deleted_at", table_name="questions")
op.drop_column("questions", "deleted_at")

View file

@ -38,6 +38,12 @@ class Question(Base):
# comparable, so a model change must be detectable rather than silent.
embedding_model = Column(String(120), nullable=True, index=True)
embedded_at = Column(DateTime, nullable=True)
# Deleting is hiding, not erasing. Ids come from a sequence and are never
# reissued, and fourteen tables point at this one — attempts, quiz
# membership, exam membership, media, notes, feedback. A hard delete takes
# all of that with it and nothing can put it back, so the row stays and
# this column says it is gone.
deleted_at = Column(DateTime, nullable=True, index=True)
question_category = relationship("QuestionCategory", back_populates="questions",
foreign_keys=[question_category_id])

View file

@ -494,16 +494,21 @@ def create_invite(data: InviteIn, db: Session = Depends(get_db),
@router.delete("/invites/{invite_id}", status_code=204)
def revoke_invite(invite_id: int, db: Session = Depends(get_db),
admin: User = Depends(require_admin)):
"""Withdraw a code that has not been used.
"""Withdraw an unused code, or clear away a spent one.
A spent code is kept: who it let in is the record worth having, and
deleting it would lose that.
An unused code is withdrawn it stays listed, so it is clear that it was
issued and then stopped. A spent or already-withdrawn code has nothing left
to stop, and a list that only grows is a list nobody reads; removing it
loses who it let in, but that person has an account, which is the record
that matters.
"""
row = db.get(InviteCode, invite_id)
if not row:
raise HTTPException(404, "Invite not found")
if row.used_by is not None:
raise HTTPException(400, "That code has already been used")
if row.used_by is not None or row.revoked_at is not None:
db.delete(row)
db.commit()
return
row.revoked_at = datetime.utcnow()
db.commit()

View file

@ -5,6 +5,7 @@ import logging
import os
import re
import uuid
from datetime import datetime
from typing import Literal
logger = logging.getLogger(__name__)
@ -57,7 +58,14 @@ def delete_question(
db: Session = Depends(get_db),
current_user: User = Depends(get_current_user),
):
"""Delete a question. Moderators can delete any; regular users can delete their own."""
"""Move a question to the trash. Restore with PATCH /{id}/restore."""
question = _question_for_delete(db, question_id, current_user)
question.deleted_at = datetime.utcnow()
db.commit()
def _question_for_delete(db: Session, question_id: int, current_user: User) -> Question:
"""The question, if this person is allowed to remove it."""
question = db.query(Question).filter(Question.id == question_id).first()
if not question:
raise HTTPException(status_code=404, detail="Question not found")
@ -70,6 +78,70 @@ def delete_question(
# Regular users can only delete questions they created (no source quiz)
if question.source_quiz_id is not None:
raise HTTPException(status_code=403, detail="Only moderators can delete extracted questions")
return question
@router.get("/trash")
def list_trashed_questions(
limit: int = 100,
db: Session = Depends(get_db),
current_user: User = Depends(get_current_user),
):
"""What this person has deleted and could still put back.
A moderator sees everything deleted; anyone else sees their own, and an
educator sees what falls inside the categories they were granted.
"""
query = db.query(Question).filter(Question.deleted_at.isnot(None))
if current_user.role not in ("admin", "moderator"):
scope = manageable_categories(db, current_user)
clause = Question.user_id == current_user.id
if scope:
clause = clause | Question.question_category_id.in_(scope)
query = query.filter(clause)
rows = query.order_by(Question.deleted_at.desc()).limit(min(limit, 500)).all()
categories = {
c.id: c.name for c in db.query(QuestionCategory).filter(
QuestionCategory.id.in_([r.question_category_id for r in rows if r.question_category_id])
).all()
} if rows else {}
return [{
"id": row.id,
"question_text": (row.question_text or "")[:400],
"question_type": row.question_type,
"category": categories.get(row.question_category_id),
"deleted_at": row.deleted_at,
} for row in rows]
@router.patch("/{question_id}/restore")
def restore_question(
question_id: int,
db: Session = Depends(get_db),
current_user: User = Depends(get_current_user),
):
"""Put a question back. Its id never changed, so everything that pointed at
it attempts, quizzes, exams, media still does."""
question = _question_for_delete(db, question_id, current_user)
if question.deleted_at is None:
raise HTTPException(400, "That question is not in the trash")
question.deleted_at = None
db.commit()
return {"id": question.id, "restored": True}
@router.delete("/{question_id}/permanent", status_code=204)
def delete_question_permanently(
question_id: int,
db: Session = Depends(get_db),
current_user: User = Depends(require_moderator),
):
"""Erase it. This takes its answers, its quiz membership and its media with
it, and no restore can bring any of that back so only a moderator may,
and only something already in the trash."""
question = _question_for_delete(db, question_id, current_user)
if question.deleted_at is None:
raise HTTPException(400, "Move it to the trash first")
db.delete(question)
db.commit()

View file

@ -49,7 +49,11 @@ def validate_parent(categories, category_id, parent_id):
def general_question_predicate():
source = aliased(Quiz)
return ~select(source.id).where(source.id == Question.source_quiz_id, source.course_id.isnot(None)).exists()
# A deleted question is out of every path at once — bank, builder, search,
# recommendations, share — because it is excluded here rather than at each
# call site, where one of them would eventually be forgotten.
return Question.deleted_at.is_(None) & ~select(source.id).where(
source.id == Question.source_quiz_id, source.course_id.isnot(None)).exists()
def shareable_question_predicate():

View file

@ -162,7 +162,10 @@ class CategoryGrantTests(unittest.TestCase):
self.user = self.educator
self.assertEqual(self.client.delete("/questions/3").status_code, 403)
self.assertEqual(self.client.delete("/questions/1").status_code, 204)
self.assertIsNone(self.db.get(Question, 1))
# Deleting hides rather than erases — the row and its id have to
# survive or nothing that points at them can be put back.
self.db.expire_all()
self.assertIsNotNone(self.db.get(Question, 1).deleted_at)
def test_summary_is_scoped_to_the_grant(self):
self.grant(category_id=1)

View file

@ -0,0 +1,71 @@
"""Deleting a question hides it; restoring it puts back the same question.
The point of the whole arrangement is the id. Ids come from a sequence and are
never reissued, and fourteen tables point at this one attempts, quiz
membership, exam membership, media, notes, favourites, feedback. Erasing the
row takes all of that with it, so a "restore" that re-inserted the text would
be a different question wearing the same words.
"""
import unittest
import test_quiz_builder as fixtures
from app.models.question import Question
class QuestionTrashTests(unittest.TestCase):
def setUp(self):
self.bank = fixtures.BuilderTests()
self.bank.setUp()
self.client = self.bank.client
self.bank.user = self.bank.mod
def tearDown(self):
self.bank.tearDown()
def row(self, question_id):
self.bank.db.expire_all()
return self.bank.db.get(Question, question_id)
def test_delete_hides_the_question_but_keeps_the_row(self):
self.assertEqual(self.client.delete("/questions/1").status_code, 204)
kept = self.row(1)
self.assertIsNotNone(kept, "the row must survive or nothing can point at it")
self.assertIsNotNone(kept.deleted_at)
def test_a_deleted_question_leaves_the_bank_and_the_builder(self):
before = self.client.get("/questions/bank").json()
before_ids = [q["id"] for q in before["questions"]]
self.assertIn(1, before_ids)
self.client.delete("/questions/1")
after = self.client.get("/questions/bank").json()
self.assertNotIn(1, [q["id"] for q in after["questions"]])
# Excluded in the shared predicate, so the builder's count moves too
# rather than each caller having to remember.
self.assertEqual(self.client.get("/questions/builder/count", params={"state": "all"}).json()["count"],
len(before_ids) - 1)
def test_the_trash_lists_it_and_restore_brings_back_the_same_id(self):
self.client.delete("/questions/1")
trash = self.client.get("/questions/trash").json()
self.assertEqual([row["id"] for row in trash], [1])
self.assertEqual(self.client.patch("/questions/1/restore").status_code, 200)
self.assertIsNone(self.row(1).deleted_at)
self.assertIn(1, [q["id"] for q in self.client.get("/questions/bank").json()["questions"]])
self.assertEqual(self.client.get("/questions/trash").json(), [])
def test_restoring_something_that_is_not_deleted_is_refused(self):
self.assertEqual(self.client.patch("/questions/1/restore").status_code, 400)
def test_erasing_requires_the_trash_first(self):
# Straight to permanent would make the trash a formality.
self.assertEqual(self.client.delete("/questions/1/permanent").status_code, 400)
self.client.delete("/questions/1")
self.assertEqual(self.client.delete("/questions/1/permanent").status_code, 204)
self.assertIsNone(self.row(1))
if __name__ == "__main__":
unittest.main()

View file

@ -56,7 +56,8 @@ export default function SitePolicy() {
() => api.post('/admin/invites', { note: note.trim() || null }).then(res => { setNote(''); return res }),
'Could not create a code')
const revoke = (row) => run(() => api.delete(`/admin/invites/${row.id}`), 'Could not withdraw that code')
const revoke = (row) => run(() => api.delete(`/admin/invites/${row.id}`),
row.status === 'open' ? 'Could not withdraw that code' : 'Could not remove that code')
const copy = (code) => {
navigator.clipboard?.writeText(code)
@ -123,7 +124,7 @@ export default function SitePolicy() {
{row.status === 'revoked' && <small>Withdrawn {when(row.revoked_at)}</small>}
</span>
<span className="sp-code-actions">
{row.status === 'open' && (
{row.status === 'open' ? (
<>
<button type="button" className="btn btn-secondary btn-sm"
onClick={() => copy(row.code)}>{copied === row.code ? '✓ Copied' : 'Copy'}</button>
@ -131,6 +132,12 @@ export default function SitePolicy() {
disabled={busy} aria-label={`Withdraw ${row.code}`}
onClick={() => revoke(row)}>Withdraw</button>
</>
) : (
// A spent code has nothing left to stop, and a list that
// only grows is a list nobody reads.
<button type="button" className="btn btn-secondary btn-sm sp-revoke"
disabled={busy} aria-label={`Remove ${row.code}`}
onClick={() => revoke(row)}>Remove</button>
)}
</span>
</li>

View file

@ -232,7 +232,7 @@ function ToolsSection() {
{ to: '/categories', icon: '🗂️', label: 'Taxonomy', desc: 'Topics, systems, symptoms, diseases' },
{ to: '/editorial', icon: '✍️', label: 'Editorial', desc: 'Article drafts and references' },
{ to: '/access', icon: '🔑', label: 'Access', desc: 'Who may edit what' },
{ to: '/trash', icon: '🗑️', label: 'Trash', desc: 'Restore deleted tests' },
{ to: '/trash', icon: '🗑️', label: 'Trash', desc: 'Restore deleted questions' },
{ to: '/jobs', icon: '📋', label: 'Extraction jobs', desc: 'Extraction history' },
].map(item => (
<Link key={item.to} to={item.to} className="set-card">

View file

@ -1,61 +1,93 @@
import { useState, useEffect } from 'react'
import { useNavigate } from 'react-router-dom'
import { Link } from 'react-router-dom'
import api from '../api/client'
import ConfirmButton from '../components/ConfirmButton'
const when = value => (value ? new Date(value).toLocaleDateString(undefined,
{ day: '2-digit', month: 'short', year: 'numeric' }) : '')
/**
* Deleted questions.
*
* It used to hold deleted quizzes, which nobody wanted back a quiz is a
* selection you can make again in a minute. A question is not: it was written,
* classified, linked to articles, answered in other people's attempts and
* given media. Deleting one used to erase all of that, because the row itself
* went. It is hidden now instead, so restoring it puts back the same id and
* everything that pointed at it still does.
*/
export default function TrashPage() {
const [trashed, setTrashed] = useState([])
const [rows, setRows] = useState([])
const [loading, setLoading] = useState(true)
const navigate = useNavigate()
const [error, setError] = useState('')
useEffect(() => {
api.get('/quizzes/trash')
.then(res => setTrashed(res.data))
.catch(console.error)
api.get('/questions/trash')
.then(res => setRows(res.data || []))
.catch(() => setError('Could not load the trash'))
.finally(() => setLoading(false))
}, [])
const restore = async (id) => {
await api.patch(`/quizzes/${id}/restore`)
setTrashed(prev => prev.filter(q => q.id !== id))
const act = async (id, run, failure) => {
setError('')
try {
await run()
setRows(prev => prev.filter(row => row.id !== id))
} catch (err) {
const detail = err?.response?.data?.detail
setError(typeof detail === 'string' ? detail : failure)
}
}
const permanentDelete = async (id) => {
await api.delete(`/quizzes/${id}/permanent`)
setTrashed(prev => prev.filter(q => q.id !== id))
}
const restore = id => act(id, () => api.patch(`/questions/${id}/restore`), 'Could not restore that question')
const erase = id => act(id, () => api.delete(`/questions/${id}/permanent`), 'Could not delete that question')
if (loading) return <div className="loading"><div className="spinner" /></div>
return (
<div style={{ maxWidth: 700, margin: '0 auto' }}>
<div style={{ maxWidth: 820, margin: '0 auto' }}>
<div className="card" style={{ marginBottom: 16 }}>
<h2>Trash</h2>
<p style={{ color: 'var(--text-muted)', fontSize: '0.875rem', marginTop: 4 }}>
Deleted quizzes restore or permanently delete.
Deleted questions. Restoring one puts back the same question its
answers, the quizzes it was in and the articles it links to are all
still attached. Deleting it forever takes those with it.
</p>
</div>
{trashed.length === 0 ? (
<div className="card"><div className="empty-state">Trash is empty</div></div>
) : trashed.map(quiz => (
<div className="card" key={quiz.id} style={{ marginBottom: 10, display: 'flex', justifyContent: 'space-between', alignItems: 'center', gap: 12 }}>
<div>
<div style={{ fontWeight: 600 }}>{quiz.title}</div>
<div style={{ fontSize: '0.8rem', color: 'var(--text-muted)' }}>
{quiz.questions_count} questions · deleted {new Date(quiz.deleted_at).toLocaleDateString()}
{error && <div className="alert alert-error">{error}</div>}
{rows.length === 0 ? (
<div className="card"><div className="empty-state">Nothing deleted.</div></div>
) : rows.map(row => (
<div className="card" key={row.id}
style={{ marginBottom: 10, display: 'flex', justifyContent: 'space-between', alignItems: 'flex-start', gap: 12 }}>
<div style={{ minWidth: 0 }}>
<div style={{ fontWeight: 600, fontSize: '0.92rem' }}>
{(row.question_text || '').slice(0, 180)}
{(row.question_text || '').length > 180 && '…'}
</div>
<div style={{ fontSize: '0.8rem', color: 'var(--text-muted)', marginTop: 4 }}>
#{row.id}
{row.category && ` · ${row.category}`}
{` · deleted ${when(row.deleted_at)}`}
</div>
</div>
<div style={{ display: 'flex', gap: 8, flexShrink: 0 }}>
<button className="btn btn-primary btn-sm" onClick={() => restore(quiz.id)}>Restore</button>
<button className="btn btn-primary btn-sm" onClick={() => restore(row.id)}>Restore</button>
<ConfirmButton
label="Delete forever"
confirmLabel="Yes, permanently delete"
onConfirm={() => permanentDelete(quiz.id)}
confirmLabel="Yes, erase it and its answers"
onConfirm={() => erase(row.id)}
/>
</div>
</div>
))}
<p style={{ fontSize: '0.82rem', color: 'var(--text-subtle)', marginTop: 16 }}>
Looking for a deleted test? <Link to="/sessions">Sessions</Link> a test is a
selection you can make again, so they are not kept here.
</p>
</div>
)
}

View file

@ -0,0 +1,63 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { render, screen, waitFor } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { MemoryRouter } from 'react-router-dom'
import TrashPage from './TrashPage'
import api from '../api/client'
vi.mock('../api/client', () => ({
default: { get: vi.fn(), patch: vi.fn(), delete: vi.fn() },
}))
const ROWS = [
{ id: 42, question_text: 'A 3-week-old presents with bilious vomiting.', category: 'Neonatology',
deleted_at: '2026-09-10T10:00:00' },
]
const mount = () => render(<MemoryRouter><TrashPage /></MemoryRouter>)
beforeEach(() => {
vi.clearAllMocks()
api.get.mockResolvedValue({ data: ROWS })
api.patch.mockResolvedValue({ data: {} })
api.delete.mockResolvedValue({})
})
describe('trash', () => {
it('holds deleted questions, not deleted tests', async () => {
mount()
expect(await screen.findByText(/bilious vomiting/)).toBeInTheDocument()
expect(api.get).toHaveBeenCalledWith('/questions/trash')
// A test is a selection you can make again; a question is not.
expect(api.get).not.toHaveBeenCalledWith('/quizzes/trash')
})
it('shows the id, because restoring returns the same question', async () => {
mount()
expect(await screen.findByText(/#42/)).toBeInTheDocument()
expect(screen.getByText(/Neonatology/)).toBeInTheDocument()
})
it('restores one and takes it off the list', async () => {
mount()
await userEvent.click(await screen.findByRole('button', { name: 'Restore' }))
await waitFor(() => expect(api.patch).toHaveBeenCalledWith('/questions/42/restore'))
await waitFor(() => expect(screen.getByText('Nothing deleted.')).toBeInTheDocument())
})
it('says what erasing costs before it happens', async () => {
mount()
await userEvent.click(await screen.findByRole('button', { name: 'Delete forever' }))
await userEvent.click(screen.getByRole('button', { name: 'Yes, erase it and its answers' }))
await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/questions/42/permanent'))
})
it('reports a refusal rather than quietly dropping the row', async () => {
api.delete.mockRejectedValue({ response: { data: { detail: 'Move it to the trash first' } } })
mount()
await userEvent.click(await screen.findByRole('button', { name: 'Delete forever' }))
await userEvent.click(screen.getByRole('button', { name: 'Yes, erase it and its answers' }))
expect(await screen.findByText('Move it to the trash first')).toBeInTheDocument()
expect(screen.getByText(/bilious vomiting/)).toBeInTheDocument()
})
})