feat: question version history with restore; TODO list
Editing a question now snapshots its previous state. The last 5 are kept — the value is undoing a recent mistake, not an audit trail, and an uncapped history of full question bodies grows without bound (migration a9b0c1d2e3f4). A restore snapshots the current state first, so the restore is itself undoable. History is gated by the same per-category grant that gates editing, so it cannot be read by someone who could not have made the edit. The question editor shows the versions with their dates and a Restore action. Also added docs/TODO.md tracking everything requested and not yet delivered: AI Mode and its citation contract, global search, study-plan editing and articles-in-blocks, admin settings revamp, image libraries and question folders, media management, nested article sections with references and per-section notes and feedback, per-question notes and feedback in the runner, tutorial mode, the per-question performance table, the Overview dashboard, systems subsystems, and dropping "Pediatrics" as a discipline. Tests: 6 new backend (snapshot on edit, cap at five newest-first, restore, restore is undoable, refused without edit rights, unknown version). Full suites green: 119 backend, 136 frontend, build clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PpfzbZ1QTLMeVYxM2kyq8m
This commit is contained in:
parent
8613fd1e5e
commit
817cff569d
7 changed files with 401 additions and 1 deletions
31
backend/alembic/versions/a9b0c1d2e3f4_question_versions.py
Normal file
31
backend/alembic/versions/a9b0c1d2e3f4_question_versions.py
Normal file
|
|
@ -0,0 +1,31 @@
|
|||
"""Keep the last few versions of each question, so an edit can be undone.
|
||||
|
||||
Capped per question rather than kept forever: the value is undoing a recent
|
||||
mistake, and an uncapped history of full question bodies grows without bound.
|
||||
|
||||
Revision ID: a9b0c1d2e3f4
|
||||
Revises: z8f9a0b1c2d3
|
||||
"""
|
||||
from alembic import op
|
||||
|
||||
revision = "a9b0c1d2e3f4"
|
||||
down_revision = "z8f9a0b1c2d3"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade():
|
||||
op.execute("""
|
||||
CREATE TABLE IF NOT EXISTS question_versions (
|
||||
id SERIAL PRIMARY KEY,
|
||||
question_id INTEGER NOT NULL REFERENCES questions(id) ON DELETE CASCADE,
|
||||
snapshot JSONB NOT NULL,
|
||||
edited_by INTEGER REFERENCES users(id) ON DELETE SET NULL,
|
||||
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
|
||||
)
|
||||
""")
|
||||
op.execute("CREATE INDEX IF NOT EXISTS ix_qv_question ON question_versions(question_id, created_at DESC)")
|
||||
|
||||
|
||||
def downgrade():
|
||||
op.execute("DROP TABLE IF EXISTS question_versions")
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
from datetime import datetime
|
||||
from pgvector.sqlalchemy import Vector
|
||||
from sqlalchemy import Column, DateTime, Integer, String, Text, JSON, ForeignKey
|
||||
from sqlalchemy.orm import relationship, deferred
|
||||
|
|
@ -37,3 +38,19 @@ class Question(Base):
|
|||
|
||||
question_category = relationship("QuestionCategory", back_populates="questions",
|
||||
foreign_keys=[question_category_id])
|
||||
|
||||
|
||||
class QuestionVersion(Base):
|
||||
"""A snapshot of a question as it was before an edit.
|
||||
|
||||
Only the last MAX_VERSIONS are kept: the point is undoing a recent mistake,
|
||||
not an audit trail, and full question bodies add up.
|
||||
"""
|
||||
|
||||
__tablename__ = "question_versions"
|
||||
|
||||
id = Column(Integer, primary_key=True, index=True)
|
||||
question_id = Column(Integer, ForeignKey("questions.id", ondelete="CASCADE"), nullable=False, index=True)
|
||||
snapshot = Column(JSON, nullable=False)
|
||||
edited_by = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True)
|
||||
created_at = Column(DateTime, default=datetime.utcnow)
|
||||
|
|
|
|||
|
|
@ -101,6 +101,7 @@ def edit_question(
|
|||
if not question:
|
||||
raise HTTPException(status_code=404, detail="Question not found")
|
||||
assert_can_manage_questions(db, scope, [question_id])
|
||||
_snapshot_question(db, question, current_user.id)
|
||||
if "question_category_id" in data.model_fields_set:
|
||||
# Moving a question out of your scope would put it beyond your reach.
|
||||
assert_can_manage_category(scope, data.question_category_id)
|
||||
|
|
@ -702,6 +703,85 @@ def bulk_question_action(
|
|||
return {"updated": updated, "action": data.action}
|
||||
|
||||
|
||||
# Enough to undo a recent mistake without storing an unbounded history of full
|
||||
# question bodies.
|
||||
MAX_VERSIONS = 5
|
||||
|
||||
VERSIONED_FIELDS = ("question_text", "question_type", "options", "correct_answer",
|
||||
"explanation", "option_explanations", "key_points", "difficulty",
|
||||
"question_category_id", "image_path", "explanation_image_path")
|
||||
|
||||
|
||||
def _snapshot_question(db, question, user_id) -> None:
|
||||
"""Store the question as it is now, then trim to the most recent MAX_VERSIONS."""
|
||||
from app.models.question import QuestionVersion
|
||||
|
||||
db.add(QuestionVersion(
|
||||
question_id=question.id,
|
||||
snapshot={field: getattr(question, field, None) for field in VERSIONED_FIELDS},
|
||||
edited_by=user_id,
|
||||
))
|
||||
db.flush()
|
||||
keep = [row[0] for row in db.query(QuestionVersion.id).filter(
|
||||
QuestionVersion.question_id == question.id
|
||||
).order_by(QuestionVersion.created_at.desc(), QuestionVersion.id.desc()).limit(MAX_VERSIONS).all()]
|
||||
if keep:
|
||||
db.query(QuestionVersion).filter(
|
||||
QuestionVersion.question_id == question.id,
|
||||
~QuestionVersion.id.in_(keep),
|
||||
).delete(synchronize_session=False)
|
||||
|
||||
|
||||
@router.get("/detail/{question_id}/versions")
|
||||
def list_question_versions(
|
||||
question_id: int,
|
||||
db: Session = Depends(get_db),
|
||||
current_user: User = Depends(get_current_user),
|
||||
):
|
||||
"""Recent snapshots of a question, newest first."""
|
||||
from app.models.question import QuestionVersion
|
||||
|
||||
scope = require_question_manager(db, current_user)
|
||||
assert_can_manage_questions(db, scope, [question_id])
|
||||
rows = db.query(QuestionVersion).filter(
|
||||
QuestionVersion.question_id == question_id
|
||||
).order_by(QuestionVersion.created_at.desc(), QuestionVersion.id.desc()).limit(MAX_VERSIONS).all()
|
||||
return [{
|
||||
"id": row.id,
|
||||
"created_at": row.created_at.isoformat() if row.created_at else None,
|
||||
"edited_by": row.edited_by,
|
||||
"question_text": (row.snapshot or {}).get("question_text"),
|
||||
} for row in rows]
|
||||
|
||||
|
||||
@router.post("/detail/{question_id}/versions/{version_id}/restore")
|
||||
def restore_question_version(
|
||||
question_id: int,
|
||||
version_id: int,
|
||||
db: Session = Depends(get_db),
|
||||
current_user: User = Depends(get_current_user),
|
||||
):
|
||||
"""Roll a question back, snapshotting the current state first so it is undoable too."""
|
||||
from app.models.question import QuestionVersion
|
||||
|
||||
scope = require_question_manager(db, current_user)
|
||||
assert_can_manage_questions(db, scope, [question_id])
|
||||
question = db.query(Question).filter(Question.id == question_id).first()
|
||||
if not question:
|
||||
raise HTTPException(404, "Question not found")
|
||||
version = db.query(QuestionVersion).filter_by(id=version_id, question_id=question_id).first()
|
||||
if not version:
|
||||
raise HTTPException(404, "Version not found")
|
||||
|
||||
_snapshot_question(db, question, current_user.id)
|
||||
for field, value in (version.snapshot or {}).items():
|
||||
if field in VERSIONED_FIELDS:
|
||||
setattr(question, field, value)
|
||||
db.commit()
|
||||
db.refresh(question)
|
||||
return {"id": question.id, "restored_from": version_id}
|
||||
|
||||
|
||||
@router.get("/detail/{question_id}")
|
||||
def get_question_detail(
|
||||
question_id: int,
|
||||
|
|
|
|||
99
backend/tests/test_question_versions.py
Normal file
99
backend/tests/test_question_versions.py
Normal file
|
|
@ -0,0 +1,99 @@
|
|||
"""Editing a question keeps a short, capped history that can be rolled back.
|
||||
|
||||
Run: DATABASE_URL=sqlite:///:memory: PYTHONPATH=backend python -m unittest discover -s backend/tests
|
||||
"""
|
||||
import os
|
||||
os.environ["DATABASE_URL"] = "sqlite:///:memory:"
|
||||
|
||||
import unittest
|
||||
|
||||
from fastapi import FastAPI
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import create_engine
|
||||
from sqlalchemy.orm import Session
|
||||
from sqlalchemy.pool import StaticPool
|
||||
|
||||
from app.database import Base, get_db
|
||||
from app.models.course import Course # noqa — Quiz.course_id FK needs the table in metadata.
|
||||
from app.models.media import MediaAsset # noqa — health report walks every embeddable table.
|
||||
from app.models.question import Question, QuestionVersion
|
||||
from app.models.user import User
|
||||
from app.routers import questions
|
||||
from app.utils.auth import get_current_user
|
||||
|
||||
|
||||
class QuestionVersionTests(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)
|
||||
Base.metadata.create_all(self.engine)
|
||||
self.db = Session(self.engine)
|
||||
|
||||
self.mod = User(id=1, name="Mod", email="mod@example.test", hashed_password="unused", role="moderator")
|
||||
self.learner = User(id=2, name="Learner", email="learner@example.test", hashed_password="unused")
|
||||
self.db.add_all([self.mod, self.learner])
|
||||
self.db.add(Question(id=1, user_id=1, is_shared=1, question_text="Original stem",
|
||||
question_type="mcq", options=["yes", "no"], correct_answer="yes",
|
||||
explanation="Original explanation"))
|
||||
self.db.commit()
|
||||
|
||||
self.user = self.mod
|
||||
app = FastAPI()
|
||||
app.include_router(questions.router, prefix="/questions")
|
||||
app.dependency_overrides[get_db] = lambda: self.db
|
||||
app.dependency_overrides[get_current_user] = lambda: self.user
|
||||
self.client = TestClient(app)
|
||||
|
||||
def tearDown(self):
|
||||
self.client.close()
|
||||
self.db.close()
|
||||
self.engine.dispose()
|
||||
|
||||
def edit(self, text):
|
||||
return self.client.patch("/questions/1", json={"question_text": text})
|
||||
|
||||
def test_an_edit_snapshots_the_previous_state(self):
|
||||
self.assertEqual(self.edit("Second stem").status_code, 200)
|
||||
versions = self.client.get("/questions/detail/1/versions").json()
|
||||
self.assertEqual([v["question_text"] for v in versions], ["Original stem"])
|
||||
self.assertEqual(self.db.get(Question, 1).question_text, "Second stem")
|
||||
|
||||
def test_history_is_capped_at_five_newest_first(self):
|
||||
for n in range(2, 10):
|
||||
self.edit(f"Stem {n}")
|
||||
versions = self.client.get("/questions/detail/1/versions").json()
|
||||
self.assertEqual(len(versions), questions.MAX_VERSIONS)
|
||||
# Newest snapshot first, and the oldest states have been dropped.
|
||||
self.assertEqual(versions[0]["question_text"], "Stem 8")
|
||||
self.assertNotIn("Original stem", [v["question_text"] for v in versions])
|
||||
self.assertEqual(self.db.query(QuestionVersion).count(), questions.MAX_VERSIONS)
|
||||
|
||||
def test_restoring_brings_back_the_earlier_wording(self):
|
||||
self.edit("Second stem")
|
||||
self.edit("Third stem")
|
||||
versions = self.client.get("/questions/detail/1/versions").json()
|
||||
oldest = versions[-1]
|
||||
self.assertEqual(oldest["question_text"], "Original stem")
|
||||
|
||||
response = self.client.post(f"/questions/detail/1/versions/{oldest['id']}/restore")
|
||||
self.assertEqual(response.status_code, 200, response.text)
|
||||
self.assertEqual(self.db.get(Question, 1).question_text, "Original stem")
|
||||
|
||||
def test_a_restore_is_itself_undoable(self):
|
||||
self.edit("Second stem")
|
||||
versions = self.client.get("/questions/detail/1/versions").json()
|
||||
self.client.post(f"/questions/detail/1/versions/{versions[0]['id']}/restore")
|
||||
# The state before the restore was captured, so the restore can be undone.
|
||||
after = self.client.get("/questions/detail/1/versions").json()
|
||||
self.assertIn("Second stem", [v["question_text"] for v in after])
|
||||
|
||||
def test_history_is_refused_to_someone_who_cannot_edit(self):
|
||||
self.user = self.learner
|
||||
self.assertEqual(self.client.get("/questions/detail/1/versions").status_code, 403)
|
||||
self.assertEqual(self.client.post("/questions/detail/1/versions/1/restore").status_code, 403)
|
||||
|
||||
def test_an_unknown_version_is_refused(self):
|
||||
self.assertEqual(self.client.post("/questions/detail/1/versions/999/restore").status_code, 404)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
113
docs/TODO.md
Normal file
113
docs/TODO.md
Normal file
|
|
@ -0,0 +1,113 @@
|
|||
# PedsHub — outstanding work
|
||||
|
||||
Everything requested and not yet delivered. Ordered roughly by dependency, not
|
||||
priority — say which to take and I'll reorder.
|
||||
|
||||
Updated 2026-09-10.
|
||||
|
||||
---
|
||||
|
||||
## In flight
|
||||
|
||||
- [ ] **Question version history** — snapshot on edit, keep last 5, restore from
|
||||
the question edit page. Migration `a9b0c1d2e3f4` and the model exist; the
|
||||
snapshot-on-write hook, restore endpoint and UI are not wired yet.
|
||||
|
||||
## Design agreed, not built
|
||||
|
||||
- [ ] **AI Mode (RAG chat)** — see "AI Mode design" below. Needs: conversation +
|
||||
message tables, the retrieval step, the ID-citation contract, and the
|
||||
chat UI with a thread rail.
|
||||
- [ ] **Global search page** — one query across questions, articles, sections,
|
||||
cards and media. Results grouped by article with the matching *sections*
|
||||
listed beneath (section index already exists). Typeahead with "Go to" and
|
||||
"Search for". Search / AI Mode toggle.
|
||||
|
||||
## Content and editing
|
||||
|
||||
- [ ] **Admin can edit everything** — study plans (rename, reorder, add/remove
|
||||
blocks, move questions between blocks) and attach articles to a block.
|
||||
- [ ] **Study plan blocks carry articles**, not only questions: "Articles" with
|
||||
*Mark as read*, then "Sessions" with Study/Exam mode.
|
||||
- [ ] **Admin settings page revamp** — currently ugly; needs restructuring.
|
||||
- [ ] **Image libraries** — group images into libraries; grant a person access to
|
||||
one, several, or all. Same shape as the existing per-category question
|
||||
grants.
|
||||
- [ ] **Question folders** — collect questions into folders for assignment and
|
||||
access, alongside category grants.
|
||||
- [ ] **Media management page** — browse the image bank, show each image's id on
|
||||
hover, edit caption/alt/tags, attach to a question.
|
||||
|
||||
## Article reading
|
||||
|
||||
- [ ] **Nested sections** — sub-sections under a section, with a breadcrumb
|
||||
(`Article › Section`) and per-section collapse.
|
||||
- [ ] **References** — numbered list per article, with superscript markers in the
|
||||
body linking down to them.
|
||||
- [ ] **Per-section notes and feedback** — a learner's own note attached to a
|
||||
section, and a feedback channel to the educator.
|
||||
- [ ] **High-yield / key-exam-info toggles** — mark spans and let the reader show
|
||||
or hide them.
|
||||
|
||||
## Quiz runner
|
||||
|
||||
- [ ] **Per-question notes in study mode**, replacing the global notes tab that is
|
||||
currently on the quiz page.
|
||||
- [ ] **Per-question feedback** to the educator.
|
||||
- [ ] **Tutorial mode** — first-run coach marks ("Step 2 of 6", Skip / Next).
|
||||
|
||||
## Analysis
|
||||
|
||||
- [ ] **Per-question performance table** — number, stem excerpt, difficulty, time
|
||||
per question, percentile; sortable, paginated.
|
||||
- [ ] **Session analysis tab** — per-session results with study recommendations
|
||||
grouped by Articles / Disciplines / Systems.
|
||||
|
||||
## Dashboard
|
||||
|
||||
- [ ] **Overview page for signed-in users** — search hero with Search / AI Mode
|
||||
toggle, "Continue your study", and a study-analysis donut. The current
|
||||
dashboard becomes this; a separate signed-out landing page comes later.
|
||||
|
||||
## Taxonomy
|
||||
|
||||
- [ ] **Systems need subsystems** — the current tree came from the old subject
|
||||
tags and is flat where it should nest. Disciplines are fine.
|
||||
- [ ] **Drop "Pediatrics" as a discipline** — it duplicates the exam. Exams are
|
||||
the top level now (Pediatrics Boards, USMLE Step 2 CK), so a discipline
|
||||
called Pediatrics is redundant.
|
||||
|
||||
---
|
||||
|
||||
## AI Mode design
|
||||
|
||||
Retrieval decides what the model may cite; the model only writes prose.
|
||||
|
||||
1. Embed the learner's message, search every corpus (`hybrid_ids` already covers
|
||||
questions, articles, sections, cards, media).
|
||||
2. Put the retrieved rows in the prompt as the *only* permitted sources, each
|
||||
with its kind and id.
|
||||
3. The model cites by id from that list — `[[article:7#features]]` — never a URL.
|
||||
4. The server rewrites citations to links and **drops any id that was not
|
||||
retrieved**. A citation the model invented cannot survive.
|
||||
|
||||
That last step is the safety property, and it is enforced by the system rather
|
||||
than by the model behaving well — the same discipline as the article page no
|
||||
longer printing answers.
|
||||
|
||||
Open questions:
|
||||
- Persist conversations (a thread rail with named threads)? Needs
|
||||
`conversations` + `messages`.
|
||||
- Cards should carry links too, resolved the same way.
|
||||
|
||||
---
|
||||
|
||||
## Done this session
|
||||
|
||||
Hybrid search (full text + BGE-M3, RRF-fused) · embedding provenance and retry
|
||||
job · articles, cards, sections and media as searchable corpora · exams as real
|
||||
data with a per-user active exam · AI-mode matching from description or upload ·
|
||||
category management page · tag vocabulary sanitised · question manager with bulk
|
||||
editing · per-category educator grants · full-page question editor · session
|
||||
rail with gradual reveal · articles read as one page · practise-this-topic ·
|
||||
continue-study panel · PREP study plans.
|
||||
|
|
@ -102,3 +102,10 @@
|
|||
.qe-save { flex: 1; margin-left: 0; }
|
||||
.qe-bar-status { width: 100%; }
|
||||
}
|
||||
|
||||
/* ── Version history ──────────────────────────────────────────────── */
|
||||
.qe-versions { list-style: none; margin: 10px 0 0; padding: 0; display: flex; flex-direction: column; gap: 7px; }
|
||||
.qe-versions li { display: flex; flex-direction: column; gap: 4px; padding: 9px 10px; background: var(--bg); border-radius: 8px; }
|
||||
.qe-version-when { font-size: 0.72rem; color: var(--text-subtle); }
|
||||
.qe-version-text { font-size: 0.82rem; overflow-wrap: anywhere; }
|
||||
.qe-versions li .btn { align-self: flex-start; }
|
||||
|
|
|
|||
|
|
@ -33,6 +33,8 @@ export default function QuestionEditPage({ mode = 'edit' }) {
|
|||
const [error, setError] = useState('')
|
||||
const [status, setStatus] = useState('')
|
||||
const [copying, setCopying] = useState(false)
|
||||
const [versions, setVersions] = useState([])
|
||||
const [showVersions, setShowVersions] = useState(false)
|
||||
|
||||
useEffect(() => {
|
||||
api.get('/question-categories').then(res => setCategories(res.data || [])).catch(() => setCategories([]))
|
||||
|
|
@ -62,7 +64,23 @@ export default function QuestionEditPage({ mode = 'edit' }) {
|
|||
.finally(() => setLoading(false))
|
||||
}, [id, isCreate])
|
||||
|
||||
useEffect(() => { load() }, [load])
|
||||
const loadVersions = useCallback(() => {
|
||||
if (isCreate) return
|
||||
api.get(`/questions/detail/${id}/versions`)
|
||||
.then(res => setVersions(res.data || [])).catch(() => setVersions([]))
|
||||
}, [id, isCreate])
|
||||
|
||||
useEffect(() => { load(); loadVersions() }, [load, loadVersions])
|
||||
|
||||
const restore = async (versionId) => {
|
||||
setSaving(true); setError('')
|
||||
try {
|
||||
await api.post(`/questions/detail/${id}/versions/${versionId}/restore`)
|
||||
setStatus('Restored')
|
||||
load(); loadVersions()
|
||||
} catch (err) { setError(apiError(err, 'Could not restore that version')) }
|
||||
finally { setSaving(false) }
|
||||
}
|
||||
|
||||
const setField = (key, value) => setForm(f => ({ ...f, [key]: value }))
|
||||
|
||||
|
|
@ -316,6 +334,41 @@ export default function QuestionEditPage({ mode = 'edit' }) {
|
|||
</div>
|
||||
</section>
|
||||
|
||||
{!isCreate && (
|
||||
<section className="qe-card">
|
||||
<h2>History</h2>
|
||||
<div className="qe-card-body">
|
||||
{versions.length === 0 ? (
|
||||
<p className="qe-primary-note">No earlier versions yet. The last {5} edits are kept.</p>
|
||||
) : (
|
||||
<>
|
||||
<button type="button" className="btn btn-secondary btn-sm"
|
||||
aria-expanded={showVersions} onClick={() => setShowVersions(v => !v)}>
|
||||
{versions.length} earlier version{versions.length === 1 ? '' : 's'} {showVersions ? '▲' : '▼'}
|
||||
</button>
|
||||
{showVersions && (
|
||||
<ul className="qe-versions">
|
||||
{versions.map(version => (
|
||||
<li key={version.id}>
|
||||
<span className="qe-version-when">
|
||||
{version.created_at ? new Date(version.created_at).toLocaleString() : 'Unknown date'}
|
||||
</span>
|
||||
<span className="qe-version-text">
|
||||
{(version.question_text || '').slice(0, 70)}
|
||||
{(version.question_text || '').length > 70 ? '…' : ''}
|
||||
</span>
|
||||
<button type="button" className="btn btn-secondary btn-sm" disabled={saving}
|
||||
onClick={() => restore(version.id)}>Restore</button>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
</section>
|
||||
)}
|
||||
|
||||
<section className="qe-card">
|
||||
<h2>Images</h2>
|
||||
<div className="qe-card-body">
|
||||
|
|
|
|||
Loading…
Reference in a new issue