diff --git a/backend/alembic/versions/c82d19e4a601_category_hierarchy.py b/backend/alembic/versions/c82d19e4a601_category_hierarchy.py index a93a23d..7806909 100644 --- a/backend/alembic/versions/c82d19e4a601_category_hierarchy.py +++ b/backend/alembic/versions/c82d19e4a601_category_hierarchy.py @@ -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") diff --git a/backend/app/routers/attempts.py b/backend/app/routers/attempts.py index e46e646..141c92e 100644 --- a/backend/app/routers/attempts.py +++ b/backend/app/routers/attempts.py @@ -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 diff --git a/backend/app/routers/mobile.py b/backend/app/routers/mobile.py index d9a94ac..661ab81 100644 --- a/backend/app/routers/mobile.py +++ b/backend/app/routers/mobile.py @@ -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() diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index 290cad6..e91221e 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -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") diff --git a/backend/app/routers/quizzes.py b/backend/app/routers/quizzes.py index 0542c77..fbc4934 100644 --- a/backend/app/routers/quizzes.py +++ b/backend/app/routers/quizzes.py @@ -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]) diff --git a/backend/app/utils/quiz_access.py b/backend/app/utils/quiz_access.py index 164fa6f..6f87f37 100644 --- a/backend/app/utils/quiz_access.py +++ b/backend/app/utils/quiz_access.py @@ -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() diff --git a/backend/app/utils/quiz_questions.py b/backend/app/utils/quiz_questions.py index e0fc9e7..f2d01ac 100644 --- a/backend/app/utils/quiz_questions.py +++ b/backend/app/utils/quiz_questions.py @@ -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 = { diff --git a/backend/tests/test_category_migration.py b/backend/tests/test_category_migration.py index a6608c7..c73b73e 100644 --- a/backend/tests/test_category_migration.py +++ b/backend/tests/test_category_migration.py @@ -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__": diff --git a/backend/tests/test_quiz_builder.py b/backend/tests/test_quiz_builder.py index 8b559e4..00aa39a 100644 --- a/backend/tests/test_quiz_builder.py +++ b/backend/tests/test_quiz_builder.py @@ -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() diff --git a/docs/quiz-revamp-progress.md b/docs/quiz-revamp-progress.md new file mode 100644 index 0000000..47be18d --- /dev/null +++ b/docs/quiz-revamp-progress.md @@ -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, 1–200 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. diff --git a/frontend/src/pages/CustomQuizPage.jsx b/frontend/src/pages/CustomQuizPage.jsx index 4504171..414c52f 100644 --- a/frontend/src/pages/CustomQuizPage.jsx +++ b/frontend/src/pages/CustomQuizPage.jsx @@ -94,7 +94,7 @@ export default function CustomQuizPage() { {mode === 'timed' && } -

Unused means never answered in a completed, nonexpired bank attempt. Incorrect uses your latest such answer.

+

Unused means no completed, nonexpired bank attempt outcome. Incorrect uses your latest outcome, including skipped questions.

{ready ? `${available} questions available` : 'Counting available questions…'}

{ready && available === 0 &&

No questions match these filters.

} diff --git a/frontend/src/pages/QuestionBankPage.jsx b/frontend/src/pages/QuestionBankPage.jsx index 304e64d..f363ea3 100644 --- a/frontend/src/pages/QuestionBankPage.jsx +++ b/frontend/src/pages/QuestionBankPage.jsx @@ -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 (
-
-

Create Quiz

+
+

Create Quiz

{error &&
{error}
}
@@ -138,8 +145,8 @@ function CreateQuizModal({ selectedIds, categories, onClose, onCreated }) {
- - setForm(f => ({ ...f, title: e.target.value }))} placeholder="Enter title..." /> + + setForm(f => ({ ...f, title: e.target.value }))} placeholder="Enter title..." />
@@ -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() {

Delete "{cat?.name}"?

- {cat?.question_count > 0 && ( -
- - -
- )} +

This affects all assigned questions, including private and course questions excluded from the visible count.

+
+ + +
@@ -881,7 +891,7 @@ export default function QuestionBankPage() { {showCatForm && (
- setNewCatName(e.target.value)} placeholder="New category name..." + 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)' }} />