diff --git a/backend/alembic/versions/f6a7b8c9d0e1_question_soft_delete.py b/backend/alembic/versions/f6a7b8c9d0e1_question_soft_delete.py
new file mode 100644
index 0000000..54b3b3e
--- /dev/null
+++ b/backend/alembic/versions/f6a7b8c9d0e1_question_soft_delete.py
@@ -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")
diff --git a/backend/app/models/question.py b/backend/app/models/question.py
index 319a786..50b6ddd 100644
--- a/backend/app/models/question.py
+++ b/backend/app/models/question.py
@@ -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])
diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py
index 1f5ee7b..b6c1614 100644
--- a/backend/app/routers/admin.py
+++ b/backend/app/routers/admin.py
@@ -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()
diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py
index 40eec85..e4d7028 100644
--- a/backend/app/routers/questions.py
+++ b/backend/app/routers/questions.py
@@ -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()
diff --git a/backend/app/services/quiz_builder.py b/backend/app/services/quiz_builder.py
index 48d3e5b..e47dc88 100644
--- a/backend/app/services/quiz_builder.py
+++ b/backend/app/services/quiz_builder.py
@@ -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():
diff --git a/backend/tests/test_category_grants.py b/backend/tests/test_category_grants.py
index 5432ea4..06f9acd 100644
--- a/backend/tests/test_category_grants.py
+++ b/backend/tests/test_category_grants.py
@@ -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)
diff --git a/backend/tests/test_question_trash.py b/backend/tests/test_question_trash.py
new file mode 100644
index 0000000..6c4193a
--- /dev/null
+++ b/backend/tests/test_question_trash.py
@@ -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()
diff --git a/frontend/src/components/SitePolicy.jsx b/frontend/src/components/SitePolicy.jsx
index a890575..f7552a7 100644
--- a/frontend/src/components/SitePolicy.jsx
+++ b/frontend/src/components/SitePolicy.jsx
@@ -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' && Withdrawn {when(row.revoked_at)}}
- {row.status === 'open' && (
+ {row.status === 'open' ? (
<>
@@ -131,6 +132,12 @@ export default function SitePolicy() {
disabled={busy} aria-label={`Withdraw ${row.code}`}
onClick={() => revoke(row)}>Withdraw
+ ) : (
+ // A spent code has nothing left to stop, and a list that
+ // only grows is a list nobody reads.
+
)}
diff --git a/frontend/src/pages/SettingsPage.jsx b/frontend/src/pages/SettingsPage.jsx
index 9a83230..241f6a8 100644
--- a/frontend/src/pages/SettingsPage.jsx
+++ b/frontend/src/pages/SettingsPage.jsx
@@ -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 => (
diff --git a/frontend/src/pages/TrashPage.jsx b/frontend/src/pages/TrashPage.jsx
index 50f4c53..d64bfac 100644
--- a/frontend/src/pages/TrashPage.jsx
+++ b/frontend/src/pages/TrashPage.jsx
@@ -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
return (
-
+
Trash
- 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.