fix: close custom-test review gaps and verify grading

Handle ownerless question revocation, legacy hide sharing, private deletion, selected-set grading including skips, UI validation/reparent/delete safeguards and offline-safe hierarchy migration. Verified 14 backend tests in deployed image, 13 frontend tests/build and real disposable PostgreSQL migration round-trip. Related tutor/image privacy work remains before deployment.
This commit is contained in:
Daniel 2026-09-07 02:07:58 +02:00
parent affd7177b5
commit f696b99569
14 changed files with 362 additions and 138 deletions

View file

@ -1,6 +1,5 @@
"""Optional category hierarchy; existing categories remain roots."""
from alembic import op
import sqlalchemy as sa
revision = "c82d19e4a601"
down_revision = "5f8c1c2a9d40"
@ -9,13 +8,23 @@ depends_on = None
def upgrade():
# create_all may already have created this column on a fresh install.
if "parent_id" not in {c["name"] for c in sa.inspect(op.get_bind()).get_columns("question_categories")}:
op.add_column("question_categories", sa.Column("parent_id", sa.Integer(), nullable=True))
op.create_foreign_key("fk_question_categories_parent", "question_categories", "question_categories",
["parent_id"], ["id"], ondelete="RESTRICT")
# SQL stays renderable offline and tolerates fresh create_all schemas.
op.execute("ALTER TABLE question_categories ADD COLUMN IF NOT EXISTS parent_id INTEGER")
op.execute("""
DO $$ BEGIN
IF NOT EXISTS (
SELECT 1 FROM pg_constraint
WHERE conrelid = 'question_categories'::regclass
AND conname = 'fk_question_categories_parent'
) THEN
ALTER TABLE question_categories
ADD CONSTRAINT fk_question_categories_parent
FOREIGN KEY (parent_id) REFERENCES question_categories(id) ON DELETE RESTRICT;
END IF;
END $$
""")
def downgrade():
op.drop_constraint("fk_question_categories_parent", "question_categories", type_="foreignkey")
op.drop_column("question_categories", "parent_id")
op.execute("ALTER TABLE question_categories DROP CONSTRAINT IF EXISTS fk_question_categories_parent")
op.execute("ALTER TABLE question_categories DROP COLUMN IF EXISTS parent_id")

View file

@ -24,7 +24,7 @@ from app.schemas.attempt import (
)
from app.utils.quiz_access import can_access_quiz, general_quiz_visibility, require_quiz_access
from app.utils.auth import get_current_user
from app.utils.quiz_questions import get_quiz_questions
from app.utils.quiz_questions import get_quiz_questions, grade_quiz_answers
router = APIRouter()
@ -109,7 +109,7 @@ def submit_attempt(
attempt = db.query(QuizAttempt).filter(
QuizAttempt.id == attempt_id,
QuizAttempt.user_id == current_user.id,
).first()
).with_for_update().first()
if not attempt:
raise HTTPException(status_code=404, detail="Attempt not found")
if attempt.completed_at:
@ -118,38 +118,20 @@ def submit_attempt(
quiz = db.query(Quiz).filter(Quiz.id == attempt.quiz_id).first()
require_quiz_access(db, quiz, current_user)
# Get all questions for this quiz via junction table
questions = {q.id: q for q in get_quiz_questions(db, attempt.quiz_id)}
grades = grade_quiz_answers(get_quiz_questions(db, attempt.quiz_id),
[(ans.question_id, ans.user_answer) for ans in submission.answers], attempt.selected_question_ids)
score = sum(correct for _, _, correct in grades)
for question, user_answer, is_correct in grades:
db.add(AttemptAnswer(attempt_id=attempt_id, question_id=question.id,
user_answer=user_answer, is_correct=is_correct))
attempt.total_questions = len(grades)
score = 0
submitted = {ans.question_id: ans.user_answer for ans in submission.answers}
# Save submitted answers
for ans in submission.answers:
question = questions.get(ans.question_id)
if not question:
continue
is_correct = bool(question.correct_answer) and ans.user_answer.strip().lower() == question.correct_answer.strip().lower()
if is_correct:
score += 1
db.add(AttemptAnswer(
attempt_id=attempt_id,
question_id=ans.question_id,
user_answer=ans.user_answer,
is_correct=is_correct,
))
# Check if review is allowed (course quizzes may disallow)
quiz = db.query(Quiz).filter(Quiz.id == attempt.quiz_id).first()
is_course_quiz = quiz and quiz.course_id is not None
# Review and grading use the same selected set, including skipped outcomes.
is_course_quiz = quiz.course_id is not None
review_allowed = not is_course_quiz or (quiz.allow_review == 1)
# Build full review — include ALL questions, unanswered marked as incorrect
answer_details = []
if review_allowed:
for q in get_quiz_questions(db, attempt.quiz_id):
user_answer = submitted.get(q.id, "")
is_correct = bool(user_answer) and bool(q.correct_answer) and user_answer.strip().lower() == q.correct_answer.strip().lower()
for q, user_answer, is_correct in grades:
answer_details.append(AnswerDetail(
question_id=q.id,
question_text=q.question_text,
@ -171,7 +153,7 @@ def submit_attempt(
import redis as redis_lib
from app.config import settings
r = redis_lib.from_url(settings.REDIS_URL, decode_responses=True)
r.delete(f"quiz_progress:{current_user.id}:{attempt.quiz_id}")
r.delete(f"quiz_progress:{current_user.id}:{attempt.id}", f"quiz_active:{current_user.id}:{attempt.id}")
except Exception:
logger.warning("Failed to clear quiz progress from Redis", exc_info=True)
@ -350,30 +332,26 @@ def get_progress(
started_at = datetime.fromisoformat(started_at_str.replace('Z', '+00:00'))
elapsed = (datetime.now(timezone.utc) - started_at).total_seconds()
if elapsed >= total_time:
# Timer expired — auto-submit
questions = {q.id: q for q in get_quiz_questions(db, quiz_id)}
score = 0
submitted_answers = saved.get("answers", {})
for qid_str, user_ans in submitted_answers.items():
qid = int(qid_str)
question = questions.get(qid)
if question:
correct = (question.correct_answer or "").strip().lower()
if (user_ans or "").strip().lower() == correct:
score += 1
db.add(AttemptAnswer(
attempt_id=attempt.id,
question_id=qid,
user_answer=user_ans,
is_correct=(user_ans or "").strip().lower() == correct,
))
attempt.score = score
# Serialize expiry with explicit submission and use identical grading.
db.refresh(attempt, with_for_update=True)
if attempt.completed_at:
r.delete(key)
return None
grades = grade_quiz_answers(get_quiz_questions(db, quiz_id),
[(int(qid), answer) for qid, answer in saved.get("answers", {}).items()],
attempt.selected_question_ids)
for question, answer, correct in grades:
db.add(AttemptAnswer(attempt_id=attempt.id, question_id=question.id,
user_answer=answer, is_correct=correct))
attempt.score = sum(correct for _, _, correct in grades)
attempt.total_questions = len(grades)
attempt.completed_at = datetime.utcnow()
attempt.expired = 1 # mark as timer-expired; exclude from history
db.commit()
r.delete(key)
return None # no progress to resume — already submitted
except Exception:
db.rollback()
logger.warning("Failed to check quiz timer expiration", exc_info=True)
return saved
@ -610,11 +588,14 @@ def get_attempt(
is_course_quiz = quiz and quiz.course_id is not None
review_allowed = not is_course_quiz or (quiz.allow_review == 1)
# Show ALL questions in review, not just answered ones
# Never reveal review content for an unfinished attempt or an unselected pool question.
review_allowed = review_allowed and attempt.completed_at is not None
submitted_map = {ans.question_id: ans for ans in attempt.answers}
answer_details = []
if review_allowed:
for q in get_quiz_questions(db, attempt.quiz_id):
if attempt.selected_question_ids is not None and q.id not in attempt.selected_question_ids:
continue
ans = submitted_map.get(q.id)
answer_details.append(AnswerDetail(
question_id=q.id,
@ -625,6 +606,7 @@ def get_attempt(
correct_answer=q.correct_answer,
is_correct=ans.is_correct if ans else False,
explanation=q.explanation,
explanation_image_path=q.explanation_image_path,
))
percentage = (attempt.score / attempt.total_questions * 100) if attempt.total_questions > 0 else 0

View file

@ -13,7 +13,7 @@ from app.models.user import User
from app.schemas.auth import Token
from app.utils.quiz_access import general_quiz_visibility
from app.utils.auth import create_access_token, get_current_user, verify_password
from app.utils.quiz_questions import get_quiz_questions
from app.utils.quiz_questions import get_quiz_questions, grade_quiz_answers
router = APIRouter()
@ -202,10 +202,11 @@ def upload_mobile_attempt(
if not quiz:
raise HTTPException(status_code=404, detail="Quiz not found")
question_map = {question.id: question for question in get_quiz_questions(db, quiz.id)}
selected_ids = data.selected_question_ids or [answer.question_id for answer in data.answers]
total_questions = len(selected_ids) if selected_ids else len(question_map)
score = 0
grades = grade_quiz_answers(get_quiz_questions(db, quiz.id),
[(answer.question_id, answer.user_answer) for answer in data.answers], data.selected_question_ids)
selected_ids = [question.id for question, _, _ in grades]
total_questions = len(grades)
score = sum(correct for _, _, correct in grades)
attempt = QuizAttempt(
quiz_id=quiz.id,
@ -218,19 +219,9 @@ def upload_mobile_attempt(
db.add(attempt)
db.flush()
for answer in data.answers:
question = question_map.get(answer.question_id)
if not question:
continue
is_correct = bool(answer.user_answer) and answer.user_answer.strip().lower() == question.correct_answer.strip().lower()
if is_correct:
score += 1
db.add(AttemptAnswer(
attempt_id=attempt.id,
question_id=question.id,
user_answer=answer.user_answer,
is_correct=is_correct,
))
for question, answer, correct in grades:
db.add(AttemptAnswer(attempt_id=attempt.id, question_id=question.id,
user_answer=answer, is_correct=correct))
attempt.score = score
db.commit()

View file

@ -49,6 +49,8 @@ def delete_question(
raise HTTPException(status_code=404, detail="Question not found")
is_mod = current_user.role in ("admin", "moderator")
if not is_mod:
if question.user_id != current_user.id:
raise HTTPException(status_code=403, detail="Not authorized to delete this 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")

View file

@ -534,8 +534,10 @@ def set_quiz_published(
if not quiz:
raise HTTPException(status_code=404, detail="Quiz not found")
quiz.is_published = 1 if published else 0
if not published:
quiz.is_shared = 0
db.commit()
return {"quiz_id": quiz_id, "is_published": quiz.is_published}
return {"quiz_id": quiz_id, "is_published": quiz.is_published, "is_shared": quiz.is_shared}
@router.get("/trash", response_model=list[QuizResponse])

View file

@ -15,7 +15,7 @@ from app.services.quiz_builder import shareable_question_predicate, bank_questio
def quiz_shareable_predicate(user=None):
allowed = bank_question_predicate(user) if user is not None else shareable_question_predicate()
return ~select(QuizQuestionLink.quiz_id).join(Question, Question.id == QuizQuestionLink.question_id).where(
QuizQuestionLink.quiz_id == Quiz.id, ~allowed,
QuizQuestionLink.quiz_id == Quiz.id, allowed.is_not(True),
).exists()

View file

@ -1,4 +1,5 @@
"""Helpers for managing the quiz ↔ question junction table."""
from fastapi import HTTPException
from sqlalchemy.orm import Session
from app.models.question import Question
@ -22,6 +23,24 @@ def get_quiz_questions(db: Session, quiz_id: int) -> list[Question]:
return [q_map[l.question_id] for l in links if l.question_id in q_map]
def grade_quiz_answers(questions, answers, selected_ids=None):
"""Grade each selected question once; omissions are recorded as incorrect."""
question_map = {q.id: q for q in questions}
ids = list(question_map) if selected_ids is None else selected_ids
if not ids or len(ids) != len(set(ids)) or set(ids) - question_map.keys():
raise HTTPException(400, "Question selection is invalid or no longer available")
submitted = dict(answers)
if len(submitted) != len(answers) or submitted.keys() - set(ids) or any(not isinstance(value, str) for value in submitted.values()):
raise HTTPException(400, "Answers contain duplicate or unselected question IDs")
grades = []
for qid in ids:
question = question_map[qid]
answer = submitted.get(qid, "")
correct = bool(answer.strip()) and bool(question.correct_answer) and answer.strip().lower() == question.correct_answer.strip().lower()
grades.append((question, answer, correct))
return grades
def add_questions_to_quiz(db: Session, quiz_id: int, question_ids: list[int], start_pos: int = 0):
"""Add question links to a quiz (skips duplicates)."""
existing = {

View file

@ -1,46 +1,40 @@
"""Migration graph and PostgreSQL DDL checks without a database connection."""
import importlib.util
"""Exercise Alembic's real offline path; never connect to an external database."""
import io
import os
from pathlib import Path
import unittest
from unittest.mock import Mock, patch
from unittest.mock import patch
os.environ["DATABASE_URL"] = "sqlite:///:memory:"
from app.database import Base # Initialize the app engine with disposable SQLite before offline URL override.
from alembic import command
from alembic.config import Config
from alembic.migration import MigrationContext
from alembic.operations import Operations
from alembic.script import ScriptDirectory
class CategoryMigrationTests(unittest.TestCase):
def test_graph_upgrade_and_downgrade(self):
backend = Path(__file__).resolve().parents[1]
config = Config()
output = io.StringIO()
config = Config(output_buffer=output)
config.set_main_option("script_location", str(backend / "alembic"))
scripts = ScriptDirectory.from_config(config)
self.assertEqual(scripts.get_heads(), ["c82d19e4a601"])
self.assertEqual(len(scripts.get_heads()), 1)
self.assertEqual(scripts.get_revision("c82d19e4a601").down_revision, "5f8c1c2a9d40")
list(scripts.walk_revisions()) # raises on a broken chain
spec = importlib.util.spec_from_file_location("category_migration", backend / "alembic/versions/c82d19e4a601_category_hierarchy.py")
migration = importlib.util.module_from_spec(spec)
spec.loader.exec_module(migration)
output = io.StringIO()
ops = Operations(MigrationContext.configure(dialect_name="postgresql", opts={"as_sql": True, "output_buffer": output}))
inspector = Mock()
inspector.get_columns.return_value = [{"name": "id"}, {"name": "name"}]
with patch.object(migration, "op", ops), patch.object(migration.sa, "inspect", return_value=inspector):
migration.upgrade()
list(scripts.walk_revisions())
# sql=True uses no engine/connection; the URL selects PostgreSQL DDL only.
with patch.dict(os.environ, {"DATABASE_URL": "postgresql://unused@127.0.0.1/offline_only"}):
command.upgrade(config, "5f8c1c2a9d40:c82d19e4a601", sql=True)
sql = output.getvalue()
self.assertIn("ADD COLUMN parent_id INTEGER", sql)
self.assertIn("REFERENCES question_categories (id) ON DELETE RESTRICT", sql)
self.assertNotIn("UPDATE", sql) # existing IDs/assignments remain intact
self.assertIn("ADD COLUMN IF NOT EXISTS parent_id INTEGER", sql)
self.assertIn("WHERE conrelid = 'question_categories'::regclass", sql)
self.assertIn("FOREIGN KEY (parent_id) REFERENCES question_categories(id) ON DELETE RESTRICT", sql)
self.assertNotIn("UPDATE question_categories", sql)
output.seek(0)
output.truncate()
inspector.get_columns.return_value.append({"name": "parent_id"})
migration.upgrade() # fresh create_all already includes parent_id
self.assertEqual(output.getvalue(), "")
migration.downgrade()
self.assertIn("DROP CONSTRAINT fk_question_categories_parent", output.getvalue())
self.assertIn("DROP COLUMN parent_id", output.getvalue())
command.downgrade(config, "c82d19e4a601:5f8c1c2a9d40", sql=True)
self.assertIn("DROP CONSTRAINT IF EXISTS fk_question_categories_parent", output.getvalue())
self.assertIn("DROP COLUMN IF EXISTS parent_id", output.getvalue())
if __name__ == "__main__":

View file

@ -8,7 +8,7 @@ import sys
import unittest
from datetime import datetime, timedelta
from types import ModuleType
from unittest.mock import patch
from unittest.mock import Mock, patch
from fastapi import FastAPI
from fastapi.testclient import TestClient
@ -246,6 +246,110 @@ class BuilderTests(unittest.TestCase):
self.assertEqual(self.client.patch("/quizzes/2/share?shared=true").status_code, 400)
self.assertEqual(self.client.get("/quizzes/1").status_code, 200)
def test_ownerless_question_revocation_denies_saved_owner(self):
self.db.get(Question, 1).user_id = None
self.db.commit()
saved = self.generate(is_shared=True, category_ids=[1], count=2).json()["id"]
self.db.get(Question, 1).is_shared = 0
self.db.commit()
for user in (self.owner, self.peer):
self.user = user
self.assertEqual(self.client.get(f"/quizzes/{saved}").status_code, 403)
self.assertEqual(self.client.post(f"/attempts/start?quiz_id={saved}").status_code, 403)
self.assertEqual(self.client.get(f"/mobile/quizzes/{saved}").status_code, 404)
self.assertNotIn(saved, [q["id"] for q in self.client.get("/quizzes/").json()])
def test_legacy_hide_revokes_both_flags(self):
self.db.get(Quiz, 1).is_shared = 1
self.db.commit()
self.user = self.mod
response = self.client.patch("/quizzes/1/publish?published=false")
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json()["is_shared"], 0)
self.user = self.peer
self.assertNotIn(1, [q["id"] for q in self.client.get("/quizzes/").json()])
self.assertEqual(self.client.get("/quizzes/1").status_code, 403)
self.assertEqual(self.client.get("/mobile/quizzes/1").status_code, 404)
self.assertEqual(self.client.post("/attempts/start?quiz_id=1").status_code, 403)
def test_peer_cannot_delete_private_manual_question_or_history(self):
saved = self.generate(category_ids=[3], count=1).json()["id"]
attempt = self.answer(3, True, quiz_id=saved)
self.user = self.peer
self.assertEqual(self.client.delete("/questions/3").status_code, 403)
self.assertIsNotNone(self.db.get(Question, 3))
self.assertEqual(self.db.query(QuizQuestionLink).filter_by(quiz_id=saved, question_id=3).count(), 1)
self.assertEqual(self.db.query(AttemptAnswer).filter_by(attempt_id=attempt.id, question_id=3).count(), 1)
def test_real_submissions_record_skips_and_latest_outcome(self):
saved = self.generate(is_shared=True, category_ids=[1], count=2).json()["id"]
first = self.client.post(f"/attempts/start?quiz_id={saved}").json()["id"]
self.assertEqual(self.client.get(f"/attempts/{first}").json()["answers"], [])
redis = Mock()
with patch.dict(sys.modules, {"redis": redis}):
result = self.client.post(f"/attempts/{first}/submit", json={"answers": [{"question_id": 1, "user_answer": "yes"}]})
self.assertEqual(result.status_code, 200, result.text)
self.assertEqual((result.json()["score"], result.json()["total_questions"]), (1, 2))
self.assertEqual(self.db.query(AttemptAnswer).filter_by(attempt_id=first).count(), 2)
skipped = self.db.query(AttemptAnswer).filter_by(attempt_id=first, question_id=2).one()
self.assertEqual((skipped.user_answer, skipped.is_correct), ("", False))
self.assertEqual(self.count(state="incorrect"), 1)
self.assertEqual(self.count(state="unused"), 2)
second = self.client.post(f"/attempts/start?quiz_id={saved}&fresh=true").json()["id"]
with patch.dict(sys.modules, {"redis": redis}):
result = self.client.post(f"/attempts/{second}/submit", json={"answers": []})
self.assertEqual(result.status_code, 200, result.text)
self.assertEqual(result.json()["score"], 0)
self.assertEqual(self.count(state="incorrect"), 2)
self.assertEqual(self.count(state="unused"), 2)
def test_submission_rejects_duplicates_and_out_of_pool_atomically(self):
saved = self.generate(is_shared=True, category_ids=[1], count=2).json()["id"]
aid = self.client.post(f"/attempts/start?quiz_id={saved}").json()["id"]
attempt = self.db.get(QuizAttempt, aid)
attempt.selected_question_ids = [1]
attempt.total_questions = 1
self.db.commit()
for ids in ([1, 1], [2], [999]):
result = self.client.post(f"/attempts/{aid}/submit", json={"answers": [{"question_id": qid, "user_answer": "yes"} for qid in ids]})
self.assertEqual(result.status_code, 400, result.text)
self.assertEqual(self.db.query(AttemptAnswer).filter_by(attempt_id=aid).count(), 0)
self.assertIsNone(self.db.get(QuizAttempt, aid).completed_at)
with patch.dict(sys.modules, {"redis": Mock()}):
result = self.client.post(f"/attempts/{aid}/submit", json={"answers": [{"question_id": 1, "user_answer": "yes"}]})
self.assertEqual(result.status_code, 200, result.text)
self.assertEqual((result.json()["score"], result.json()["total_questions"], result.json()["percentage"]), (1, 1, 100))
self.assertEqual([q["question_id"] for q in result.json()["answers"]], [1])
self.assertEqual([q["question_id"] for q in self.client.get(f"/attempts/{aid}").json()["answers"]], [1])
def test_mobile_and_expiry_use_same_selected_question_grading(self):
import json
saved = self.generate(is_shared=True, category_ids=[1], count=2).json()["id"]
before = self.db.query(QuizAttempt).count()
for selected, answers in [([1, 1], []), ([999], []), ([1], [2]), ([1], [1, 1])]:
result = self.client.post("/mobile/attempts", json={"quiz_id": saved, "selected_question_ids": selected,
"answers": [{"question_id": qid, "user_answer": "yes"} for qid in answers]})
self.assertEqual(result.status_code, 400, result.text)
self.assertEqual(self.db.query(QuizAttempt).count(), before)
result = self.client.post("/mobile/attempts", json={"quiz_id": saved, "answers": [{"question_id": 1, "user_answer": "yes"}]})
self.assertEqual(result.status_code, 200, result.text)
self.assertEqual((result.json()["score"], result.json()["total_questions"]), (1, 2))
self.assertEqual(self.db.query(AttemptAnswer).filter_by(attempt_id=result.json()["id"]).count(), 2)
aid = self.client.post(f"/attempts/start?quiz_id={saved}&fresh=true").json()["id"]
attempt = self.db.get(QuizAttempt, aid)
attempt.selected_question_ids = [1]
attempt.total_questions = 1
self.db.commit()
redis = Mock()
redis.from_url.return_value.get.return_value = json.dumps({"answers": {}, "total_time": 1, "started_at": "2000-01-01T00:00:00+00:00"})
with patch.dict(sys.modules, {"redis": redis}):
response = self.client.get(f"/attempts/progress?quiz_id={saved}")
self.assertEqual(response.status_code, 200, response.text)
self.assertIsNone(response.json())
self.assertEqual(self.db.get(QuizAttempt, aid).expired, 1)
self.assertEqual(self.db.query(AttemptAnswer).filter_by(attempt_id=aid).count(), 1)
self.assertFalse(self.db.query(AttemptAnswer).filter_by(attempt_id=aid).one().is_correct)
if __name__ == "__main__":
unittest.main()

View file

@ -0,0 +1,36 @@
# Quiz revamp progress
## Milestone 1 — custom tests and categories
Implemented in `affd717` plus the follow-up review-fix commit:
- Learner custom-test builder: multiple categories and descendants, exact available counts, 1200 questions, study/exam modes, unused/incorrect/bookmarked filters, fixed saved membership and optional sharing.
- Existing categories retain IDs/assignments; optional hierarchy, cycle guards, breadcrumbs and educator reparenting support added.
- Central general-quiz visibility and sharing checks across web, attempts and mobile; ownerless-question revocation is NULL-safe and both Hide and Unshare revoke public access.
- Explicit question selection rejects private/course/missing IDs. Peer deletion of private/manual questions is denied.
- Web, mobile and expiry use one grading function. Duplicate/out-of-pool submissions are rejected; skips have recorded incorrect outcomes; reviews use selected questions only, and unfinished attempts do not return answer review.
- Category UI handles validation arrays, refreshes reparented filters and offers relocation even when the visible question count is zero. Private quiz titles are keyboard-accessible links.
- Category migration supports real offline SQL rendering and existing/fresh schemas.
### Verification
Initial implementation received two independent read-only reviews (access/correctness and UI/migration). The parent applied the accepted fixes and added behavioral regressions.
- Backend: **14 tests passed** inside the exact deployed backend image `sha256:77d9af981537a09396fb5a511f27b4efd00794443823db8eb522beb3aa70a5b9` (Python 3.11 and deployed library versions). Disposable SQLite only; network disabled; no production data.
- Frontend: **13 tests passed in four suites** with `NODE_ENV=test npm test`; production build passed with `NODE_ENV=production npm run build`.
- PostgreSQL 16: actual migration upgrade, FK enforcement, existing-row/question-assignment preservation, repeated upgrade, downgrade and re-upgrade passed in a disposable network-isolated container. Container/data cleaned up automatically.
- Alembic offline SQL: explicit `5f8c1c2a9d40:c82d19e4a601` upgrade and reverse downgrade exercised without mocking inspection.
- Changed Python syntax and `git diff --check` passed.
- Regression negative-control run against the unfixed `affd717` is tracked separately; do not infer its result from the passing fixed-code suite.
The first Docker validation attempt failed before tests because a read-only mountpoint was absent; setup was corrected. One frontend regression initially used an ambiguous Create Quiz selector; the modal was labeled and the test scoped to it, then all tests passed.
### Release boundaries
This milestone is source work on the feature branch, **not a production deployment or completion of the whole revamp**. Apply migrations explicitly before serving updated code against the existing production DB; startup does not automatically upgrade existing tables.
Older tutor-context and question-image delivery authorization gaps identified by review remain a release blocker and have their own tracked privacy task. Already-downloaded offline content cannot be recalled by server revocation. No new AI provider calls or production database/service changes were performed.
## Next
Continue with the Orthobullets-inspired runner/results UI, question navigation and study tools; then article/subsection reading, linked flashcards, educator AI authoring and moderated comments. Complete related-content privacy work and end-to-end desktop/mobile validation before deployment.

View file

@ -94,7 +94,7 @@ export default function CustomQuizPage() {
</select></label>
{mode === 'timed' && <label>Time limit (minutes, optional)<input type="number" min="1" step="1" value={time} onChange={e => setTime(e.target.value)} /></label>}
</div>
<p>Unused means never answered in a completed, nonexpired bank attempt. Incorrect uses your latest such answer.</p>
<p>Unused means no completed, nonexpired bank attempt outcome. Incorrect uses your latest outcome, including skipped questions.</p>
<label className="custom-test-share"><input type="checkbox" checked={shared} onChange={e => setShared(e.target.checked)} /> Share with other learners (only shareable questions)</label>
<p role="status" aria-live="polite">{ready ? `${available} questions available` : 'Counting available questions…'}</p>
{ready && available === 0 && <p>No questions match these filters.</p>}

View file

@ -8,6 +8,13 @@ import { useDialog } from '../hooks/useDialog'
const TeachChat = lazy(() => import('../components/TeachChat'))
const RichEditor = lazy(() => import('../components/RichEditor'))
function apiError(err, fallback) {
const detail = err?.response?.data?.detail
if (typeof detail === 'string') return detail
if (Array.isArray(detail)) return detail.map(item => typeof item?.msg === 'string' ? item.msg : '').filter(Boolean).join('; ') || fallback
return fallback
}
/** Strip HTML tags for plain text display (truncated cards). */
function stripHtml(html) {
if (!html) return ''
@ -119,7 +126,7 @@ function CreateQuizModal({ selectedIds, categories, onClose, onCreated }) {
})
navigate(`/quizzes/${res.data.id}`)
}
} catch (err) { setError(err.response?.data?.detail || 'Failed') }
} catch (err) { setError(apiError(err, 'Could not create quiz')) }
finally { setLoading(false) }
}
@ -127,8 +134,8 @@ function CreateQuizModal({ selectedIds, categories, onClose, onCreated }) {
return (
<div style={{ position: 'fixed', inset: 0, background: 'rgba(0,0,0,0.5)', zIndex: 1000, display: 'flex', alignItems: 'center', justifyContent: 'center', padding: 16 }}>
<div style={{ background: 'var(--card-bg)', borderRadius: 12, padding: 24, maxWidth: 420, width: '100%' }}>
<h2 style={{ marginBottom: 16 }}>Create Quiz</h2>
<div role="dialog" aria-modal="true" aria-labelledby="create-bank-quiz-heading" style={{ background: 'var(--card-bg)', borderRadius: 12, padding: 24, maxWidth: 420, width: '100%' }}>
<h2 id="create-bank-quiz-heading" style={{ marginBottom: 16 }}>Create Quiz</h2>
{error && <div className="alert alert-error">{error}</div>}
<div className="form-group">
<label>Source</label>
@ -138,8 +145,8 @@ function CreateQuizModal({ selectedIds, categories, onClose, onCreated }) {
</select>
</div>
<div className="form-group">
<label>Quiz Title</label>
<input type="text" value={form.title} onChange={e => setForm(f => ({ ...f, title: e.target.value }))} placeholder="Enter title..." />
<label htmlFor="bank-quiz-title">Quiz Title</label>
<input id="bank-quiz-title" type="text" maxLength={200} value={form.title} onChange={e => setForm(f => ({ ...f, title: e.target.value }))} placeholder="Enter title..." />
</div>
<div className="form-group">
<label>Mode</label>
@ -240,7 +247,7 @@ function QuestionEditModal({ question, categories, onSaved, onClose }) {
onSaved({ ...question, ...res.data,
question_category_name: categories.find(c => c.id === payload.question_category_id)?.name || null })
onClose()
} catch (err) { setError(err.response?.data?.detail || 'Save failed') }
} catch (err) { setError(apiError(err, 'Save failed')) }
finally { setSaving(false) }
}
@ -347,7 +354,7 @@ function CreateQuestionModal({ categories, onCreated, onClose }) {
const res = await api.post('/questions/upload-image', fd)
setForm(f => ({ ...f, image_path: res.data.image_path }))
} catch (err) {
setError(err.response?.data?.detail || 'Image upload failed')
setError(apiError(err, 'Image upload failed'))
} finally {
setUploadingImage(false)
}
@ -372,7 +379,7 @@ function CreateQuestionModal({ categories, onCreated, onClose }) {
const res = await api.post('/questions/create', payload)
onCreated(res.data)
onClose()
} catch (err) { setError(err.response?.data?.detail || 'Failed to create question') }
} catch (err) { setError(apiError(err, 'Failed to create question')) }
finally { setSaving(false) }
}
@ -615,7 +622,7 @@ export default function QuestionBankPage() {
} catch { }
}, 2000)
} catch (err) {
await openAlert(err.response?.data?.detail || 'Failed to start classification', { title: 'Error' })
await openAlert(apiError(err, 'Failed to start classification'), { title: 'Error' })
}
}
@ -661,7 +668,7 @@ export default function QuestionBankPage() {
setAssignCatId('')
api.get('/question-categories/').then(res => setCategories(res.data))
} catch (err) {
setBulkError(err.response?.data?.detail || 'Failed to assign category')
setBulkError(apiError(err, 'Failed to assign category'))
}
}
@ -674,7 +681,11 @@ export default function QuestionBankPage() {
const res = await api.get('/question-categories/')
setCategories(res.data)
setNewCatName(''); setCatParent(''); setEditingCategory(null); setShowCatForm(false)
} catch (err) { await openAlert(err.response?.data?.detail || 'Failed', { title: 'Error' }) }
clearSelection()
setQuestions([])
setTotal(0)
await loadQuestions()
} catch (err) { await openAlert(apiError(err, 'Could not save category'), { title: 'Error' }) }
}
const [deletingCatId, setDeletingCatId] = useState(null)
@ -689,7 +700,7 @@ export default function QuestionBankPage() {
setCategories(prev => prev.filter(c => c.id !== catId))
setFilterCatIds(prev => prev.filter(c => c !== catId))
loadQuestions(searchQuery, 0, filterCatIds.filter(c => c !== catId), showUncategorized, showFavorites)
} catch (err) { await openAlert(err.response?.data?.detail || 'Could not delete category', { title: 'Error' }) }
} catch (err) { await openAlert(apiError(err, 'Could not delete category'), { title: 'Error' }) }
}
const deleteCategory = (catId) => {
@ -711,7 +722,7 @@ export default function QuestionBankPage() {
api.get('/question-categories/').then(r => setCategories(r.data))
}
} catch (err) {
setImportResult({ error: err.response?.data?.detail || 'Import failed' })
setImportResult({ error: apiError(err, 'Import failed') })
} finally {
setImporting(false)
}
@ -734,7 +745,7 @@ export default function QuestionBankPage() {
openAlert(`Imported ${res.data.imported} of ${res.data.total_items} questions.${errors}`, { title: 'QTI Import Complete' })
loadQuestions()
} catch (err) {
openAlert(err.response?.data?.detail || 'QTI import failed', { title: 'Import Failed' })
openAlert(apiError(err, 'QTI import failed'), { title: 'Import Failed' })
} finally {
setQtiImporting(false)
}
@ -754,7 +765,7 @@ export default function QuestionBankPage() {
a.click()
URL.revokeObjectURL(url)
} catch (err) {
openAlert(err.response?.data?.detail || 'QTI export failed', { title: 'Export Failed' })
openAlert(apiError(err, 'QTI export failed'), { title: 'Export Failed' })
} finally {
setQtiExporting(false)
}
@ -771,7 +782,7 @@ export default function QuestionBankPage() {
setFavorites(prev => [...prev, questionId])
}
} catch (err) {
await openAlert(err.response?.data?.detail || 'Failed to update favorite', { title: 'Error' })
await openAlert(apiError(err, 'Failed to update favorite'), { title: 'Error' })
}
}
@ -787,15 +798,14 @@ export default function QuestionBankPage() {
<div style={{ position: 'fixed', inset: 0, background: 'rgba(0,0,0,0.5)', zIndex: 1000, display: 'flex', alignItems: 'center', justifyContent: 'center', padding: 16 }}>
<div style={{ background: 'var(--card-bg)', borderRadius: 12, padding: 24, maxWidth: 400, width: '100%' }}>
<h2 style={{ marginBottom: 12, fontSize: '1.1rem' }}>Delete "{cat?.name}"?</h2>
{cat?.question_count > 0 && (
<div className="form-group">
<label>Move {cat.question_count} questions to:</label>
<select value={moveToCatId} onChange={e => setMoveToCatId(e.target.value)}>
<option value="">Leave uncategorized</option>
{others.map(c => <option key={c.id} value={c.id}>{(c.breadcrumbs || []).map(b => b.name).join(' ') || c.name}</option>)}
</select>
</div>
)}
<p>This affects all assigned questions, including private and course questions excluded from the visible count.</p>
<div className="form-group">
<label htmlFor="category-delete-target">Move all assigned questions to:</label>
<select id="category-delete-target" value={moveToCatId} onChange={e => setMoveToCatId(e.target.value)}>
<option value="">Leave uncategorized</option>
{others.map(c => <option key={c.id} value={c.id}>{(c.breadcrumbs || []).map(b => b.name).join(' ') || c.name}</option>)}
</select>
</div>
<div style={{ display: 'flex', gap: 8, marginTop: 16 }}>
<button className="btn btn-danger" onClick={confirmDeleteCategory}>Delete category</button>
<button className="btn btn-secondary" onClick={() => { setDeletingCatId(null); setMoveToCatId('') }}>Cancel</button>
@ -881,7 +891,7 @@ export default function QuestionBankPage() {
{showCatForm && (
<div style={{ marginTop: 12, display: 'flex', gap: 8, flexWrap: 'wrap' }}>
<input aria-label="Category name" type="text" value={newCatName} onChange={e => setNewCatName(e.target.value)} placeholder="New category name..."
<input aria-label="Category name" type="text" maxLength={200} value={newCatName} onChange={e => setNewCatName(e.target.value)} placeholder="New category name..."
onKeyDown={e => e.key === 'Enter' && addCategory()}
style={{ flex: 1, padding: '7px 12px', border: '1px solid var(--border)', borderRadius: 8, fontSize: '0.875rem', background: 'var(--input-bg)', color: 'var(--text)' }} />
<select aria-label="Parent category" value={catParent} onChange={e => setCatParent(e.target.value)}>
@ -1104,7 +1114,7 @@ export default function QuestionBankPage() {
await api.delete(`/questions/${q.id}`)
setQuestions(prev => prev.filter(x => x.id !== q.id))
setTotal(t => t - 1)
} catch (err) { await openAlert(err.response?.data?.detail || 'Delete failed', { title: 'Error' }) }
} catch (err) { await openAlert(apiError(err, 'Delete failed'), { title: 'Error' }) }
}}>Delete</button>}
</div>
</div>

View file

@ -1,4 +1,4 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react'
import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import { MemoryRouter } from 'react-router-dom'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
@ -10,6 +10,7 @@ vi.mock('../api/client', () => ({
default: {
get: vi.fn(),
post: vi.fn(),
patch: vi.fn(),
delete: vi.fn(),
},
}))
@ -100,6 +101,80 @@ describe('QuestionBankPage QTI actions', () => {
})
})
describe('QuestionBankPage review regressions', () => {
beforeEach(() => { vi.resetAllMocks(); mockInitialRequests() })
it('renders validation arrays for category creation without crashing', async () => {
api.post.mockRejectedValue({ response: { data: { detail: [{ msg: 'Category name is too long' }] } } })
renderPage()
await userEvent.click(await screen.findByRole('button', { name: '+ Category' }))
const name = screen.getByLabelText('Category name')
expect(name).toHaveAttribute('maxlength', '200')
await userEvent.type(name, 'New category')
await userEvent.click(screen.getByRole('button', { name: 'Add', exact: true }))
expect(await screen.findByText('Category name is too long')).toBeInTheDocument()
})
it('handles selected-quiz validation arrays and bounds the title', async () => {
const initial = api.get.getMockImplementation()
api.get.mockImplementation(url => url === '/questions/bank' ? Promise.resolve({ data: {
total: 1, questions: [{ id: 10, question_text: 'A full question', options: ['Yes', 'No'], correct_answer: 'Yes', question_type: 'mcq' }],
} }) : initial(url))
api.post.mockRejectedValue({ response: { data: { detail: [{ msg: 'Quiz title is invalid' }] } } })
renderPage()
await screen.findByText('A full question')
await userEvent.click(screen.getByRole('checkbox'))
await userEvent.click(screen.getByRole('button', { name: 'Create Quiz (1 selected)' }))
const title = screen.getByLabelText('Quiz Title')
expect(title).toHaveAttribute('maxlength', '200')
await userEvent.type(title, 'My quiz')
await userEvent.click(within(screen.getByRole('dialog', { name: 'Create Quiz' })).getByRole('button', { name: 'Create Quiz', exact: true }))
expect(await screen.findByText('Quiz title is invalid')).toBeInTheDocument()
})
it('reloads active parent results after moving its child', async () => {
let moved = false
const cats = [
{ id: 1, name: 'Root', question_count: 1, breadcrumbs: [{ id: 1, name: 'Root' }] },
{ id: 2, name: 'Child', parent_id: 1, question_count: 1, breadcrumbs: [{ id: 1, name: 'Root' }, { id: 2, name: 'Child' }] },
{ id: 3, name: 'Other', question_count: 0, breadcrumbs: [{ id: 3, name: 'Other' }] },
]
const initial = api.get.getMockImplementation()
api.get.mockImplementation((url, options) => {
if (url === '/question-categories/') return Promise.resolve({ data: cats })
if (url === '/questions/bank') {
const questions = moved && options?.params?.category_ids === '1' ? [] : [{ id: 10, question_text: 'Child question', options: ['Yes', 'No'], question_type: 'mcq', correct_answer: 'Yes' }]
return Promise.resolve({ data: { questions, total: questions.length } })
}
return initial(url)
})
api.patch.mockImplementation(() => { moved = true; return Promise.resolve({ data: {} }) })
renderPage()
await userEvent.click(await screen.findByRole('button', { name: 'Root (1)', exact: true }))
await screen.findByText('Child question')
await waitFor(() => expect(api.get).toHaveBeenCalledWith('/questions/bank', expect.objectContaining({ params: expect.objectContaining({ category_ids: '1' }) })))
await userEvent.click(screen.getByRole('button', { name: 'Edit category Child' }))
await userEvent.selectOptions(screen.getByLabelText('Parent category'), '3')
await userEvent.click(screen.getByRole('button', { name: 'Save category' }))
await waitFor(() => expect(screen.queryByText('Child question')).not.toBeInTheDocument())
expect(await screen.findByText('0 questions total')).toBeInTheDocument()
})
it('offers relocation even when visible count is zero', async () => {
const initial = api.get.getMockImplementation()
api.get.mockImplementation(url => url === '/question-categories/' ? Promise.resolve({ data: [
{ id: 1, name: 'Hidden assignments', question_count: 0 }, { id: 3, name: 'Destination', question_count: 0 },
] }) : initial(url))
api.delete.mockResolvedValue({})
renderPage()
await userEvent.click(await screen.findByRole('button', { name: 'Delete category Hidden assignments' }))
expect(screen.getByText(/including private and course questions excluded/)).toBeInTheDocument()
await userEvent.selectOptions(screen.getByLabelText('Move all assigned questions to:'), '3')
await userEvent.click(screen.getByRole('button', { name: 'Delete category', exact: true }))
await waitFor(() => expect(api.delete).toHaveBeenCalledWith('/question-categories/1', { params: { move_to: 3 } }))
})
})
describe('QuestionBankPage category hierarchy', () => {
it('edits parent assignment with breadcrumb choices and excludes descendants', async () => {
vi.clearAllMocks()

View file

@ -217,8 +217,8 @@ function QuizCard({ quiz, isModerator, categories, onDelete, onCategoryChange })
e.stopPropagation()
try {
const newVal = quiz.is_published === 0
await api.patch(`/quizzes/${quiz.id}/publish`, null, { params: { published: newVal } })
onCategoryChange(quiz.id, quiz.category_id, newVal ? 1 : 0) // reuse callback to update state
const res = await api.patch(`/quizzes/${quiz.id}/publish`, null, { params: { published: newVal } })
onCategoryChange(quiz.id, quiz.category_id, res.data.is_published, res.data.is_shared)
} catch { }
}
@ -233,7 +233,7 @@ function QuizCard({ quiz, isModerator, categories, onDelete, onCategoryChange })
style={{ background: 'none', border: 'none', cursor: 'pointer', color: '#cbd5e1', fontSize: '0.9rem', padding: '4px 6px', borderRadius: 4, textDecoration: 'none', display: 'flex', alignItems: 'center' }}
onMouseEnter={e => e.currentTarget.style.color = 'var(--primary)'}
onMouseLeave={e => e.currentTarget.style.color = '#cbd5e1'}></Link>
<button title={quiz.is_published === 0 ? 'Unpublished (sharing may still grant access) — click to publish' : 'Published — click to unpublish'} onClick={togglePublish}
<button title={quiz.is_published === 0 ? 'Unpublished — click to publish' : 'Hide from learners (also revokes sharing)'} onClick={togglePublish}
style={{ background: 'none', border: 'none', cursor: 'pointer', color: quiz.is_published === 0 ? '#ef4444' : '#cbd5e1', fontSize: '0.9rem', padding: '4px 6px', borderRadius: 4 }}
onMouseEnter={e => e.currentTarget.style.opacity = '0.7'} onMouseLeave={e => e.currentTarget.style.opacity = '1'}>
{quiz.is_published === 0 ? '🙈' : '👁'}
@ -268,7 +268,7 @@ function QuizCard({ quiz, isModerator, categories, onDelete, onCategoryChange })
{quiz.mode === 'learning' ? '📖' : '📝'}
</div>
<div style={{ paddingRight: isModerator ? 48 : 0 }}>
<div style={{ fontWeight: 700, fontSize: '1rem', color: 'var(--text)', lineHeight: 1.3 }}>{quiz.title}</div>
<Link to={`/quizzes/${quiz.id}`} onClick={e => e.stopPropagation()} style={{ fontWeight: 700, fontSize: '1rem', color: 'var(--text)', lineHeight: 1.3 }}>{quiz.title}</Link>
</div>
<div style={{ marginTop: 'auto', display: 'flex', flexDirection: 'column', gap: 4 }}>
<div style={{ fontSize: '0.82rem', color: '#64748b' }}>{quiz.questions_count} question{quiz.questions_count !== 1 ? 's' : ''}</div>