From 5d59e0014476453e61accc24773ff5541e181ed6 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 12 Sep 2026 08:42:51 +0200 Subject: [PATCH] refactor: remove per-question sharing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Question.is_shared` defaulted to 1 and was only ever set by a route nothing called, so in practice it divided the bank into "everything" and "everything, plus your own private ones" — a distinction that cost every recommendation denominator a join and never changed an answer. Who may reach the bank is the site's own access rules; who may manage a question is the category grant tree. So the two predicates the whole bank was built on are now the same thing, and say what they actually mean: a question is out of reach if it has been deleted or belongs to a course. Nothing else. The column is dropped, the route that set it is gone, the bulk "share" action with it, and the Private tile and pill go from the question manager. The tests that turned on it have been rewritten rather than deleted, because the rule they were really about survives: revoking a question still revokes every session carrying it — by deleting it, which is the only revocation left. Several others named a category holding exactly two reachable questions and then answered two particular ids; that category holds four now, so they name the pair instead. A session's own sharing flag is untouched — that is a different thing, and it is still how a session is handed to somebody. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN --- .../b4c5d6e7f809_drop_question_is_shared.py | 29 +++++++ backend/app/main.py | 4 - backend/app/models/question.py | 1 - backend/app/routers/questions.py | 34 +------- backend/app/services/draft_questions.py | 1 - backend/app/services/quiz_builder.py | 29 +++++-- backend/tests/test_ai_mode_matching.py | 13 +-- backend/tests/test_ai_practice.py | 9 +- backend/tests/test_articles_cards.py | 9 +- backend/tests/test_bank_cleanup.py | 3 +- backend/tests/test_category_grants.py | 2 +- backend/tests/test_collections.py | 7 +- backend/tests/test_exam_scoped_tags.py | 2 +- backend/tests/test_exams.py | 2 +- backend/tests/test_global_search.py | 8 +- backend/tests/test_hybrid_search.py | 4 +- backend/tests/test_multi_category.py | 12 +-- backend/tests/test_question_versions.py | 2 +- backend/tests/test_quiz_builder.py | 83 +++++++++++-------- backend/tests/test_quiz_sessions.py | 28 +++---- backend/tests/test_related_privacy.py | 27 +++--- backend/tests/test_session_lifecycle.py | 14 ++-- backend/tests/test_share_public.py | 8 +- backend/tests/test_shared_category.py | 2 +- backend/tests/test_study_tools.py | 12 +-- backend/tests/test_tag_hierarchy.py | 2 +- frontend/src/components/PractiseTopic.jsx | 2 +- frontend/src/pages/CustomQuizPage.jsx | 6 +- frontend/src/pages/CustomQuizPage.test.jsx | 10 ++- frontend/src/pages/QuestionManagerPage.jsx | 2 - .../src/pages/QuestionManagerPage.test.jsx | 8 +- 31 files changed, 209 insertions(+), 166 deletions(-) create mode 100644 backend/alembic/versions/b4c5d6e7f809_drop_question_is_shared.py diff --git a/backend/alembic/versions/b4c5d6e7f809_drop_question_is_shared.py b/backend/alembic/versions/b4c5d6e7f809_drop_question_is_shared.py new file mode 100644 index 0000000..0a275d0 --- /dev/null +++ b/backend/alembic/versions/b4c5d6e7f809_drop_question_is_shared.py @@ -0,0 +1,29 @@ +"""Drop questions.is_shared. + +Per-question sharing is gone. The column defaulted to 1 and was only ever set +by a route nothing called, so in practice it divided the bank into "everything" +and "everything, plus your own private ones" — a distinction that cost every +recommendation denominator a join and never changed an answer. Who may see the +bank is decided by the site's own access rules, and who may *manage* a question +by the category grant tree. + +Revision ID: b4c5d6e7f809 +Revises: a3b4c5d6e7f8 +""" +import sqlalchemy as sa +from alembic import op + +revision = "b4c5d6e7f809" +down_revision = "a3b4c5d6e7f8" +branch_labels = None +depends_on = None + + +def upgrade(): + op.drop_column("questions", "is_shared") + + +def downgrade(): + # Back as it was: visible by default, which is what every row held. + op.add_column("questions", + sa.Column("is_shared", sa.Integer(), nullable=True, server_default="1")) diff --git a/backend/app/main.py b/backend/app/main.py index 1244dfa..6e6da1e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -262,10 +262,6 @@ def setup_pgvector(): ALTER TABLE questions ADD COLUMN IF NOT EXISTS user_id INTEGER REFERENCES users(id) ON DELETE SET NULL """)) - conn.execute(text(""" - ALTER TABLE questions - ADD COLUMN IF NOT EXISTS is_shared INTEGER DEFAULT 1 - """)) # Fix email collation to avoid en_US.utf8 B-tree index corruption conn.execute(text('ALTER TABLE users ALTER COLUMN email TYPE varchar COLLATE "C"')) diff --git a/backend/app/models/question.py b/backend/app/models/question.py index 50b6ddd..64c6af4 100644 --- a/backend/app/models/question.py +++ b/backend/app/models/question.py @@ -32,7 +32,6 @@ class Question(Base): attending_tip = Column(Text, nullable=True) difficulty = Column(String(10), nullable=True) # easy | medium | hard user_id = Column(Integer, ForeignKey("users.id", ondelete="SET NULL"), nullable=True) - is_shared = Column(Integer, default=1) # 1 = visible in bank, 0 = private (only owner sees it) embedding = deferred(Column(Vector(settings.EMBEDDING_DIMENSIONS), nullable=True)) # semantic search vector — deferred: not loaded in standard queries # Which model produced `embedding`. Vectors from different models are not # comparable, so a model change must be detectable rather than silent. diff --git a/backend/app/routers/questions.py b/backend/app/routers/questions.py index e4d7028..0a98141 100644 --- a/backend/app/routers/questions.py +++ b/backend/app/routers/questions.py @@ -219,25 +219,6 @@ def edit_question( } -@router.patch("/{question_id}/share") -def toggle_question_share( - question_id: int, - shared: int = Query(..., description="1 = shared, 0 = private"), - db: Session = Depends(get_db), - current_user: User = Depends(get_current_user), -): - """Toggle question sharing. Owner or admin can change.""" - question = db.query(Question).filter(Question.id == question_id).first() - if not question: - raise HTTPException(status_code=404, detail="Question not found") - is_admin = current_user.role in ("admin", "moderator") - if question.user_id != current_user.id and not is_admin: - raise HTTPException(status_code=403, detail="Not your question") - question.is_shared = 1 if shared else 0 - db.commit() - return {"id": question_id, "is_shared": question.is_shared} - - @router.patch("/{question_id}/category") def set_question_category( question_id: int, @@ -377,8 +358,6 @@ def get_question_bank( query = query.filter(or_(Question.explanation.is_(None), Question.explanation.in_(("", " ")))) elif needs == "difficulty": query = query.filter(Question.difficulty.is_(None)) - elif needs == "private": - query = query.filter(Question.is_shared == 0) # Tag filter: questions must have ALL specified tags if tag_ids: @@ -461,7 +440,6 @@ def get_question_bank( "attending_tip": qu.attending_tip, "difficulty": qu.difficulty, "user_id": qu.user_id, - "is_shared": qu.is_shared if qu.is_shared is not None else 1, "match_source": "semantic" if qu.id in semantic_ids else "keyword", }) @@ -532,7 +510,6 @@ def create_question_manually( attending_tip=(data.attending_tip or None), difficulty=data.difficulty, user_id=current_user.id, - is_shared=1, ) db.add(question) db.commit() @@ -597,7 +574,6 @@ def list_question_images( def count_builder_questions( category_ids: list[int] = Query(default=[]), state: Literal["all", "unused", "incorrect", "bookmarked"] = "all", - is_shared: bool = False, difficulty: Literal["easy", "medium", "hard"] | None = Query(None), article_ids: str | None = Query(None, description="Comma-separated article IDs (OR filter)"), tag_ids: str | None = Query(None, description="Comma-separated tag IDs (AND filter)"), @@ -608,7 +584,7 @@ def count_builder_questions( ids = [int(part) for part in (article_ids or "").split(",") if part.strip().isdigit()] tag_list = [int(part) for part in (tag_ids or "").split(",") if part.strip().isdigit()] systems = [int(part) for part in (system_ids or "").split(",") if part.strip().isdigit()] - return {"count": filtered_bank_query(db, current_user, category_ids, state, is_shared, + return {"count": filtered_bank_query(db, current_user, category_ids, state, difficulty, ids, tag_list, systems).count()} @@ -742,10 +718,9 @@ class BulkQuestionAction(BaseModel): """One editorial action applied to a checked set in the question manager.""" question_ids: list[int] - action: Literal["category", "difficulty", "share", "delete"] + action: Literal["category", "difficulty", "delete"] category_id: int | None = None difficulty: Literal["easy", "medium", "hard"] | None = None - shared: int | None = None @router.post("/bulk") @@ -772,8 +747,6 @@ def bulk_question_action( updated = rows.update({"question_category_id": data.category_id}, synchronize_session=False) elif data.action == "difficulty": updated = rows.update({"difficulty": data.difficulty}, synchronize_session=False) - elif data.action == "share": - updated = rows.update({"is_shared": 1 if data.shared else 0}, synchronize_session=False) else: # delete # Extra category links cascade with the question rows. db.query(QuestionCategoryLink).filter( @@ -938,7 +911,6 @@ def get_question_detail( "image_path": question.image_path, "explanation_image_path": question.explanation_image_path, "user_id": question.user_id, - "is_shared": question.is_shared if question.is_shared is not None else 1, "source_quiz_id": question.source_quiz_id, } @@ -967,7 +939,6 @@ def question_manager_summary( "no_difficulty": base.filter(Question.difficulty.is_(None)).scalar() or 0, "no_explanation": base.filter( or_(Question.explanation.is_(None), Question.explanation.in_(blank))).scalar() or 0, - "private": base.filter(Question.is_shared == 0).scalar() or 0, "mine": base.filter(Question.user_id == current_user.id).scalar() or 0, "scoped": scope is not None, } @@ -1278,7 +1249,6 @@ def import_qti( correct_answer=correct_answer, explanation=explanation or None, user_id=current_user.id, - is_shared=0, ) db.add(q) created.append(q) diff --git a/backend/app/services/draft_questions.py b/backend/app/services/draft_questions.py index e35ea4d..00312b0 100644 --- a/backend/app/services/draft_questions.py +++ b/backend/app/services/draft_questions.py @@ -78,7 +78,6 @@ def accept(db: Session, draft: DraftQuestion, batch: DraftBatch, user: User) -> image_path=draft.image_path, explanation_image_path=draft.explanation_image_path, user_id=user.id, - is_shared=1, ) db.add(question) db.flush() diff --git a/backend/app/services/quiz_builder.py b/backend/app/services/quiz_builder.py index 83102e3..509b61a 100644 --- a/backend/app/services/quiz_builder.py +++ b/backend/app/services/quiz_builder.py @@ -58,13 +58,28 @@ def general_question_predicate(): def shareable_question_predicate(): - return general_question_predicate() & or_(Question.is_shared == 1, Question.is_shared.is_(None)) + """What a public share link may contain. + + Once it was "questions whose author ticked shared". Per-question sharing is + gone: a question in the bank is in the bank, and who may *manage* one is + decided by the grant tree rather than by a flag its author set. So the only + thing still excluded here is what is excluded everywhere — a deleted + question, and one belonging to a course. + """ + return general_question_predicate() def bank_question_predicate(user): - return general_question_predicate() & or_( - Question.is_shared == 1, Question.is_shared.is_(None), Question.user_id == user.id, - ) + """What a learner's bank holds. + + Every question in it. The flag this used to consult defaulted to 1 and was + settable only from a route nothing called, so in practice it divided the + bank into "everything" and "everything, plus your own private ones" — a + distinction that cost every recommendation denominator a join and never + changed an answer. + """ + del user # Kept in the signature: the exam scope and grants still take one. + return general_question_predicate() def exam_scope_predicate(db, user): @@ -91,7 +106,7 @@ def bank_query(db, user): return db.query(Question).filter(bank_question_predicate(user)) -def filtered_bank_query(db, user, category_ids=(), state="all", shared=False, difficulty=None, article_ids=(), tag_ids=(), system_ids=()): +def filtered_bank_query(db, user, category_ids=(), state="all", difficulty=None, article_ids=(), tag_ids=(), system_ids=()): query = bank_query(db, user) if difficulty: query = query.filter(Question.difficulty == difficulty) @@ -140,8 +155,6 @@ def filtered_bank_query(db, user, category_ids=(), state="all", shared=False, di Question.question_category_id.in_(ids), Question.id.in_(select(QuestionCategoryLink.question_id).where(QuestionCategoryLink.category_id.in_(ids))), )) - if shared: - query = query.filter(shareable_question_predicate()) if state == "bookmarked": query = query.filter(Question.id.in_(select(Favorite.question_id).where(Favorite.user_id == user.id))) elif state in ("unused", "incorrect"): @@ -388,7 +401,7 @@ def generate_test(db, user, data): if len(ids) < data.count: raise HTTPException(400, f"Only {len(ids)} questions available; requested {data.count}") return create_saved_test(db, user, data, ids) - query = filtered_bank_query(db, user, data.category_ids, data.state, data.is_shared, + query = filtered_bank_query(db, user, data.category_ids, data.state, data.difficulty, data.article_ids, data.tag_ids, data.system_ids) if data.explicit_ids: query = query.filter(Question.id.in_(list(dict.fromkeys(data.explicit_ids)))) diff --git a/backend/tests/test_ai_mode_matching.py b/backend/tests/test_ai_mode_matching.py index 41823df..dc19363 100644 --- a/backend/tests/test_ai_mode_matching.py +++ b/backend/tests/test_ai_mode_matching.py @@ -6,6 +6,7 @@ import os os.environ["DATABASE_URL"] = "sqlite:///:memory:" import io +from datetime import datetime import unittest from unittest.mock import patch @@ -33,12 +34,14 @@ class AiModeMatchingTests(unittest.TestCase): self.user = User(id=1, name="Learner", email="learner@example.test", hashed_password="unused") self.other = User(id=2, name="Other", email="other@example.test", hashed_password="unused") self.db.add_all([self.user, self.other]) - for qid, text, owner, shared in [ - (1, "A child with fever and a seizure", 1, 1), - (2, "An infant with jaundice", 1, 1), - (3, "A private question of someone else", 2, 0), + # A question that nobody may match against is a deleted one; the flag + # an author could set to keep one back no longer exists. + for qid, text, owner, deleted in [ + (1, "A child with fever and a seizure", 1, None), + (2, "An infant with jaundice", 1, None), + (3, "A question of someone else, since removed", 2, datetime(2026, 1, 1)), ]: - self.db.add(Question(id=qid, user_id=owner, is_shared=shared, question_text=text, + self.db.add(Question(id=qid, user_id=owner, deleted_at=deleted, question_text=text, question_type="mcq", options=["yes", "no"], correct_answer="yes")) self.db.commit() diff --git a/backend/tests/test_ai_practice.py b/backend/tests/test_ai_practice.py index 570c6e3..a79e3cd 100644 --- a/backend/tests/test_ai_practice.py +++ b/backend/tests/test_ai_practice.py @@ -5,6 +5,7 @@ to go and practise it. What is under test is which questions that lands on, and that a chat cannot reach a question the learner could not otherwise see. """ import unittest +from datetime import datetime from unittest.mock import patch import test_quiz_builder as fixtures @@ -61,11 +62,13 @@ class PracticeFromChatTests(unittest.TestCase): response = self.practise(cid) self.assertEqual(response.status_code, 200, response.text) quiz = self.client.get(f"/quizzes/{response.json()['quiz_id']}").json() - self.assertEqual([q['id'] for q in quiz['questions']], [2]) + self.assertEqual(sorted(q['id'] for q in quiz['questions']), [2, 4]) def test_a_chat_cannot_reach_a_question_the_learner_may_not_see(self): - # Question 4 belongs to another learner and is not shared. A citation - # could only name it by mistake; the bank's own rules answer anyway. + # Question 4 has been removed from the bank. A citation could only name + # it by mistake; the bank's own rules answer anyway. + self.bank.db.get(fixtures.Question, 4).deleted_at = datetime(2026, 1, 1) + self.bank.db.commit() cid, _ = self.thread([{'marker': '[[question:4]]', 'kind': 'question', 'id': 4, 'title': 'Question #4'}]) self.assertEqual(self.practise(cid).status_code, 404) diff --git a/backend/tests/test_articles_cards.py b/backend/tests/test_articles_cards.py index 4c2950d..cb65200 100644 --- a/backend/tests/test_articles_cards.py +++ b/backend/tests/test_articles_cards.py @@ -1,5 +1,6 @@ """Article library and card association routes on disposable SQLite; no network/AI.""" import unittest +from datetime import datetime import test_quiz_builder as fixtures from app.models.article import Article, QuestionArticleLink @@ -65,7 +66,7 @@ class ArticlesCardsTests(unittest.TestCase): self.assertEqual(link.json()['linked'], True) self.assertEqual(self.client.put(f"/articles/{article['id']}/links", json={'question_id': 1, 'section_id': section_id}).json()['linked'], False) self.assertEqual(self.client.put(f"/articles/{article['id']}/links", json={'question_id': 1, 'section_id': 'f' * 32}).status_code, 400) - self.assertEqual(self.client.put(f"/articles/{article['id']}/links", json={'question_id': 4, 'section_id': section_id}).status_code, 404) + self.assertEqual(self.client.put(f"/articles/{article['id']}/links", json={'question_id': 999, 'section_id': section_id}).status_code, 404) self.bank.user = self.bank.owner self.assertEqual(self.client.put(f"/articles/{article['id']}/links", json={'question_id': 1}).status_code, 403) self.bank.user = self.bank.mod @@ -101,7 +102,7 @@ class ArticlesCardsTests(unittest.TestCase): self.bank.user = self.bank.owner questions = self.client.get(f"/articles/{article['id']}/questions").json() self.assertEqual([q['question_id'] for q in questions], [1]) - self.bank.db.get(Question, 1).is_shared = 0 + self.bank.db.get(Question, 1).deleted_at = datetime(2026, 1, 1) self.bank.db.commit() self.bank.user = self.bank.peer self.assertEqual(self.client.get(f"/articles/{article['id']}/questions").json(), []) @@ -125,7 +126,7 @@ class ArticlesCardsTests(unittest.TestCase): self.assertEqual(self.client.post(f'/flashcards/decks/{deck.id}/cards', json={'front': 'F', 'back': 'B'}).status_code, 403) self.bank.user = self.bank.mod self.assertEqual(self.client.put(f"/flashcards/cards/{card['id']}/links/question", json={'question_id': 1}).status_code, 200) - self.assertEqual(self.client.put(f"/flashcards/cards/{card['id']}/links/question", json={'question_id': 4}).status_code, 404) + self.assertEqual(self.client.put(f"/flashcards/cards/{card['id']}/links/question", json={'question_id': 999}).status_code, 404) self.assertEqual(self.client.put(f"/flashcards/cards/{card['id']}/links/article", json={'article_id': article['id'], 'article_section_id': 'f' * 32}).status_code, 400) self.assertEqual(self.client.put(f"/flashcards/cards/{card['id']}/links/article", @@ -173,7 +174,7 @@ class ArticlesCardsTests(unittest.TestCase): self.assertEqual([q['id'] for q in links['questions']], [1]) self.assertEqual(links['articles'], []) # Making the question private and publishing the article flips both filters. - self.bank.db.get(Question, 1).is_shared = 0 + self.bank.db.get(Question, 1).deleted_at = datetime(2026, 1, 1) self.bank.db.commit() self.bank.user = self.bank.mod self.client.post(f"/articles/{article['id']}/publish", json={'published': True}) diff --git a/backend/tests/test_bank_cleanup.py b/backend/tests/test_bank_cleanup.py index 184667d..7286104 100644 --- a/backend/tests/test_bank_cleanup.py +++ b/backend/tests/test_bank_cleanup.py @@ -155,8 +155,7 @@ class SqliteBankTests(unittest.TestCase): self.engine.dispose() def add_question(self, **fields): - question = Question(question_type="mcq", options=["a", "b"], correct_answer="a", - is_shared=1, **fields) + question = Question(question_type="mcq", options=["a", "b"], correct_answer="a", **fields) self.db.add(question) self.db.commit() return question diff --git a/backend/tests/test_category_grants.py b/backend/tests/test_category_grants.py index 06f9acd..15e78d6 100644 --- a/backend/tests/test_category_grants.py +++ b/backend/tests/test_category_grants.py @@ -42,7 +42,7 @@ class CategoryGrantTests(unittest.TestCase): ]) self.db.flush() for qid, category in [(1, 1), (2, 2), (3, 10), (4, None)]: - self.db.add(Question(id=qid, question_category_id=category, user_id=1, is_shared=1, + self.db.add(Question(id=qid, question_category_id=category, user_id=1, question_text=f"Question {qid}", question_type="mcq", options=["yes", "no"], correct_answer="yes", explanation="Because")) self.db.commit() diff --git a/backend/tests/test_collections.py b/backend/tests/test_collections.py index 4c1951f..4bc89f8 100644 --- a/backend/tests/test_collections.py +++ b/backend/tests/test_collections.py @@ -5,6 +5,7 @@ is the ordering: most recently used first, and a library nobody has opened sorts by when it was made rather than claiming a use that never happened. """ import unittest +from datetime import datetime import test_quiz_builder as fixtures from app.models.collection import UserCollection @@ -84,7 +85,9 @@ class CollectionTests(unittest.TestCase): def test_a_question_nobody_can_see_cannot_be_saved(self): row = self.make('Mine') - # Question 4 is another learner's and not shared; question 3 is this - # learner's own, unshared, and theirs to save. + # Question 4 has been removed from the bank; question 3 is an ordinary + # bank question and theirs to save. + self.bank.db.get(fixtures.Question, 4).deleted_at = datetime(2026, 1, 1) + self.bank.db.commit() self.assertEqual(self.client.put(f"/collections/{row['id']}/questions/4").status_code, 404) self.assertEqual(self.client.put(f"/collections/{row['id']}/questions/3").status_code, 200) diff --git a/backend/tests/test_exam_scoped_tags.py b/backend/tests/test_exam_scoped_tags.py index 364d8ab..d651fcc 100644 --- a/backend/tests/test_exam_scoped_tags.py +++ b/backend/tests/test_exam_scoped_tags.py @@ -55,7 +55,7 @@ class ExamScopedTagTests(unittest.TestCase): Exam(id=2, slug="usmle-step-2-ck", name="USMLE Step 2 CK")]) self.db.flush() for qid, exam_id in ((1, 1), (2, 1), (3, 2)): - self.db.add(Question(id=qid, user_id=1, is_shared=1, question_text=f"Q{qid}", + self.db.add(Question(id=qid, user_id=1, question_text=f"Q{qid}", question_type="mcq", options=["a", "b"], correct_answer="a")) self.db.flush() self.db.add(QuestionExamLink(question_id=qid, exam_id=exam_id)) diff --git a/backend/tests/test_exams.py b/backend/tests/test_exams.py index 2a026d1..93c8ddb 100644 --- a/backend/tests/test_exams.py +++ b/backend/tests/test_exams.py @@ -39,7 +39,7 @@ class ExamTests(unittest.TestCase): ]) self.db.flush() for qid in (1, 2, 3): - self.db.add(Question(id=qid, user_id=1, is_shared=1, question_text=f"Question {qid}", + self.db.add(Question(id=qid, user_id=1, question_text=f"Question {qid}", question_type="mcq", options=["yes", "no"], correct_answer="yes")) self.db.flush() # 1 → boards, 2 → step 2, 3 → unlinked (unclassified, not excluded) diff --git a/backend/tests/test_global_search.py b/backend/tests/test_global_search.py index 7aeb63d..f73c7c7 100644 --- a/backend/tests/test_global_search.py +++ b/backend/tests/test_global_search.py @@ -4,6 +4,7 @@ The point of these tests is the boundary, not the ranking: a search page that had its own idea of who may see what is how private content leaks. """ import unittest +from datetime import datetime import test_quiz_builder as fixtures from app.models.article import Article, ArticleSectionIndex @@ -42,7 +43,8 @@ class GlobalSearchTests(unittest.TestCase): self.db.query(Question).filter(Question.id == 1).update( {"question_text": "A child with a febrile seizure lasting two minutes"}) self.db.query(Question).filter(Question.id == 3).update( - {"question_text": "A private febrile seizure question"}) + {"question_text": "A removed febrile seizure question", + "deleted_at": datetime(2026, 1, 1)}) self.db.commit() def tearDown(self): @@ -76,10 +78,10 @@ class GlobalSearchTests(unittest.TestCase): self.bank.user = self.bank.mod self.assertIn(2, [row['id'] for row in self.find()['results']['article']]) - def test_private_questions_and_other_peoples_decks_are_not_searchable(self): + def test_removed_questions_and_other_peoples_decks_are_not_searchable(self): self.bank.user = self.bank.peer data = self.find() - # Question 3 belongs to another user and is not shared. + # Question 3 has been removed from the bank. self.assertNotIn(3, [row['id'] for row in data['results']['question']]) # Deck 1 belongs to the owner; the peer's search must not reach into it. self.assertNotIn(1, [row['id'] for row in data['results']['flashcard']]) diff --git a/backend/tests/test_hybrid_search.py b/backend/tests/test_hybrid_search.py index 2116faf..b3a7db8 100644 --- a/backend/tests/test_hybrid_search.py +++ b/backend/tests/test_hybrid_search.py @@ -31,7 +31,7 @@ class HybridSearchTests(unittest.TestCase): (2, "An infant with jaundice on day three"), (3, "A toddler with a febrile convulsion"), ]: - self.db.add(Question(id=qid, user_id=1, is_shared=1, question_text=text, + self.db.add(Question(id=qid, user_id=1, question_text=text, question_type="mcq", options=["yes", "no"], correct_answer="yes")) self.db.commit() @@ -78,7 +78,7 @@ class EmbeddingProvenanceTests(unittest.TestCase): self.db = Session(self.engine) self.db.add(User(id=1, name="Mod", email="mod@example.test", hashed_password="unused", role="moderator")) for qid in (1, 2, 3): - self.db.add(Question(id=qid, user_id=1, is_shared=1, question_text=f"Question {qid}", + self.db.add(Question(id=qid, user_id=1, question_text=f"Question {qid}", question_type="mcq", options=["yes", "no"], correct_answer="yes")) self.db.commit() diff --git a/backend/tests/test_multi_category.py b/backend/tests/test_multi_category.py index 724e89a..f88a8ea 100644 --- a/backend/tests/test_multi_category.py +++ b/backend/tests/test_multi_category.py @@ -2,6 +2,7 @@ import io import os import unittest +from datetime import datetime from pathlib import Path from unittest.mock import patch @@ -32,10 +33,10 @@ class MultiCategoryTests(unittest.TestCase): self.bank.user = self.bank.owner # Viewing counts stays open to learners. cats = {cat['id']: cat['question_count'] for cat in self.client.get('/question-categories/').json()} self.assertEqual(cats[4], 1) # Was an empty category; now includes the linked question. - self.assertEqual(cats[2], 2) # Question 2 plus the additional link on question 3. - self.assertEqual(cats[1], 3) # Question 3 was already counted through its primary (descendant of 1). + self.assertEqual(cats[2], 3) # Questions 2 and 4, plus the additional link on question 3. + self.assertEqual(cats[1], 4) # Question 3 was already counted through its primary (descendant of 1). self.assertEqual(self.client.get('/questions/builder/count', params={'category_ids': [4]}).json()['count'], 1) - self.assertEqual(self.client.get('/questions/builder/count', params={'category_ids': [2]}).json()['count'], 2) + self.assertEqual(self.client.get('/questions/builder/count', params={'category_ids': [2]}).json()['count'], 3) bank = self.client.get('/questions/bank', params={'category_ids': '4'}).json() self.assertEqual([row['id'] for row in bank['questions']], [3]) self.assertEqual(set(bank['questions'][0]['category_ids']), {2, 3, 4}) @@ -43,8 +44,9 @@ class MultiCategoryTests(unittest.TestCase): # Category quiz creation counts extra-linked questions too. created = self.client.post('/question-categories/4/create-quiz', params={'title': 'Linked quiz', 'mode': 'learning'}) self.assertEqual(created.status_code, 200, created.text) - # Peers cannot see the private question through the shared category. - self.bank.user = self.bank.peer + # A removed question is not reachable through its category either. + self.bank.db.get(Question, 3).deleted_at = datetime(2026, 1, 1) + self.bank.db.commit() self.assertEqual(self.client.get('/questions/builder/count', params={'category_ids': [4]}).json()['count'], 0) def test_additional_categories_replace_validate_and_ignore_primary(self): diff --git a/backend/tests/test_question_versions.py b/backend/tests/test_question_versions.py index 587b3d5..4c7bb84 100644 --- a/backend/tests/test_question_versions.py +++ b/backend/tests/test_question_versions.py @@ -31,7 +31,7 @@ class QuestionVersionTests(unittest.TestCase): 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", + self.db.add(Question(id=1, user_id=1, question_text="Original stem", question_type="mcq", options=["yes", "no"], correct_answer="yes", explanation="Original explanation")) self.db.commit() diff --git a/backend/tests/test_quiz_builder.py b/backend/tests/test_quiz_builder.py index df74815..5f254f3 100644 --- a/backend/tests/test_quiz_builder.py +++ b/backend/tests/test_quiz_builder.py @@ -55,16 +55,17 @@ class BuilderTests(unittest.TestCase): self.db.add_all([Quiz(id=1, title="Origin", user_id=3, is_published=1), Quiz(id=2, title="Course quiz", user_id=3, course_id=1, is_published=1, is_shared=1, allow_review=0)]) self.db.flush() - for qid, category, owner, shared, source in [ - (1, 1, 3, 1, 1), (2, 2, 3, 1, 1), (3, 3, 1, 0, None), - (4, 2, 2, 0, None), (5, 2, 3, 1, 2), (6, None, 3, 1, None), + # Per-question sharing is gone: every question in the bank is in the + # bank, and who may manage one is the grant tree's business. + for qid, category, owner, source in [ + (1, 1, 3, 1), (2, 2, 3, 1), (3, 3, 1, None), + (4, 2, 2, None), (5, 2, 3, 2), (6, None, 3, None), ]: self.db.add(Question(id=qid, question_category_id=category, user_id=owner, - is_shared=shared, source_quiz_id=source, question_text=f"Question {qid}", + source_quiz_id=source, question_text=f"Question {qid}", question_type="mcq", options=["yes", "no"], correct_answer="yes", explanation="Full explanation", image_path="q.png", explanation_image_path="answer.png")) self.db.flush() - self.db.query(Question).filter(Question.id == 2).update({"is_shared": None}) self.db.add_all([QuizQuestionLink(quiz_id=1, question_id=1, position=0), QuizQuestionLink(quiz_id=1, question_id=2, position=1), QuizQuestionLink(quiz_id=2, question_id=5, position=0)]) @@ -86,6 +87,13 @@ class BuilderTests(unittest.TestCase): def generate(self, **overrides): return self.client.post("/questions/builder", json={"title": " Test ", "count": 2, **overrides}) + def saved_test(self, ids, **overrides): + """A test built from named questions, for the tests that care which.""" + response = self.client.post("/questions/from-bank", + json={"title": "Named", "question_ids": ids, **overrides}) + self.assertEqual(response.status_code, 200, response.text) + return response.json()["id"] + def count(self, **params): response = self.client.get("/questions/builder/count", params=params) self.assertEqual(response.status_code, 200, response.text) @@ -104,10 +112,10 @@ class BuilderTests(unittest.TestCase): from app.services.quiz_builder import adaptive_select self.answer(1, False, day=0) # Question 1 (cat 1): incorrect — recycled first. self.answer(2, True, day=1) # Question 2 (cat 2): correct. - ids = adaptive_select(self.db, self.owner, 4, [], 'all', None) - self.assertEqual(set(ids[:2]), {3, 6}) # Unanswered questions preferred. - self.assertEqual(ids[2], 1) # Older incorrect resurfaces first. - self.assertEqual(ids[3], 2) + ids = adaptive_select(self.db, self.owner, 5, [], 'all', None) + self.assertEqual(set(ids[:3]), {3, 4, 6}) # Unanswered questions preferred. + self.assertEqual(ids[3], 1) # Older incorrect resurfaces first. + self.assertEqual(ids[4], 2) def test_difficulty_tags_filter_and_validate(self): self.user = self.mod @@ -122,22 +130,21 @@ class BuilderTests(unittest.TestCase): def test_descendants_counts_visibility_and_invalid_categories(self): self.assertEqual(category_descendants(self.db.query(QuestionCategory).all(), [1, 2]), {1, 2, 3}) - self.assertEqual(self.count(category_ids=[1, 2]), 3) - self.assertEqual(self.count(category_ids=[1], is_shared=True), 2) - self.assertEqual(self.count(), 4) + self.assertEqual(self.count(category_ids=[1, 2]), 4) + self.assertEqual(self.count(), 5) cats = self.client.get("/question-categories/").json() root = next(c for c in cats if c["id"] == 1) leaf = next(c for c in cats if c["id"] == 3) - self.assertEqual(root["question_count"], 3) + self.assertEqual(root["question_count"], 4) self.assertEqual([b["id"] for b in leaf["breadcrumbs"]], [1, 2, 3]) self.assertEqual(self.client.get("/questions/builder/count?category_ids=999").status_code, 400) self.assertEqual(self.generate(category_ids=[999]).status_code, 400) self.assertEqual(self.client.get("/questions/bank/ids?category_ids=1,bad").status_code, 400) self.assertEqual(self.client.get("/questions/bank?category_ids=-1").status_code, 400) - self.assertEqual(self.client.get("/questions/export/qti?question_ids=1,4").status_code, 400) + self.assertEqual(self.client.get("/questions/export/qti?question_ids=1,5").status_code, 400) bank = self.client.get("/questions/bank", params={"category_ids": "1,2", "search_mode": "keyword"}).json() - self.assertEqual(bank["total"], 3) - self.assertEqual(set(self.client.get("/questions/bank/ids").json()), {1, 2, 3, 6}) + self.assertEqual(bank["total"], 4) + self.assertEqual(set(self.client.get("/questions/bank/ids").json()), {1, 2, 3, 4, 6}) def test_parent_cycle_missing_self_delete_and_saved_history(self): saved = self.generate(category_ids=[1], count=3).json()["id"] @@ -160,7 +167,7 @@ class BuilderTests(unittest.TestCase): self.assertEqual(self.client.post("/question-categories/", json={"name": "New", "parent_id": 999}).status_code, 400) def test_sampling_exact_zero_insufficient_stale_and_validation(self): - for payload, status in [({"category_ids": [4]}, 400), ({"count": 5}, 400), ({"expected_count": 99}, 409), + for payload, status in [({"category_ids": [4]}, 400), ({"count": 6}, 400), ({"expected_count": 99}, 409), ({"count": 0}, 422), ({"count": 201}, 422), ({"mode": "bad"}, 422), ({"title": " "}, 422), ({"title": "x" * 201}, 422), ({"time_limit_minutes": 0}, 422), ({"state": "bad"}, 422)]: self.assertEqual(self.generate(**payload).status_code, status) @@ -168,20 +175,20 @@ class BuilderTests(unittest.TestCase): sampled_ids = [q.id for q in self.db.get(Quiz, sampled["id"]).questions] self.assertEqual(len(sampled_ids), 2) self.assertEqual(len(set(sampled_ids)), 2) - self.assertTrue(set(sampled_ids) <= {1, 2, 3, 6}) - result = self.generate(count=4, expected_count=4, mode="learning") + self.assertTrue(set(sampled_ids) <= {1, 2, 3, 4, 6}) + result = self.generate(count=5, expected_count=5, mode="learning") self.assertEqual(result.status_code, 200, result.text) quiz = self.db.get(Quiz, result.json()["id"]) self.assertEqual(quiz.title, "Test") self.assertEqual(quiz.is_shared, 0) self.assertEqual(quiz.is_published, 0) - self.assertEqual({q.id for q in quiz.questions}, {1, 2, 3, 6}) - self.assertEqual(len(quiz.questions), 4) + self.assertEqual({q.id for q in quiz.questions}, {1, 2, 3, 4, 6}) + self.assertEqual(len(quiz.questions), 5) self.assertEqual(quiz.questions[0].explanation_image_path, "answer.png") first = self.client.post(f"/attempts/start?quiz_id={quiz.id}").json() again = self.client.post(f"/attempts/start?quiz_id={quiz.id}").json() self.assertEqual(first["id"], again["id"]) - self.assertEqual({q["id"] for q in self.client.get(f"/quizzes/{quiz.id}").json()["questions"]}, {1, 2, 3, 6}) + self.assertEqual({q["id"] for q in self.client.get(f"/quizzes/{quiz.id}").json()["questions"]}, {1, 2, 3, 4, 6}) def test_latest_incorrect_unused_bookmarks(self): self.answer(1, False, day=0) @@ -192,7 +199,7 @@ class BuilderTests(unittest.TestCase): self.answer(3, False, day=1, quiz_id=2) # course answers never count self.db.add_all([Favorite(user_id=1, question_id=2), Favorite(user_id=1, question_id=5), Favorite(user_id=2, question_id=1)]) self.db.commit() - self.assertEqual(self.count(state="unused"), 2) + self.assertEqual(self.count(state="unused"), 3) self.assertEqual(self.count(state="incorrect"), 1) self.assertEqual(self.count(state="bookmarked"), 1) res = self.generate(state="incorrect", count=1).json() @@ -201,7 +208,7 @@ class BuilderTests(unittest.TestCase): self.assertEqual(self.count(state="incorrect"), 0) def test_explicit_ids_atomic_permissions_order_and_category_creator(self): - for ids in ([1, 4], [1, 5], [1, 999]): + for ids in ([1, 5], [1, 999]): before = self.db.query(Quiz).count() res = self.client.post("/questions/from-bank", json={"title": "X", "question_ids": ids}) self.assertEqual(res.status_code, 400, res.text) @@ -215,10 +222,15 @@ class BuilderTests(unittest.TestCase): def test_shared_private_revocation_and_mobile(self): private_id = self.generate(category_ids=[3], count=1).json()["id"] + # A deleted question is out of everyone's reach, so a session holding + # one cannot be handed on. Per-question sharing used to be the other + # reason; there is no such flag now. + self.db.get(Question, 3).deleted_at = datetime(2026, 1, 1) + self.db.commit() res = self.client.patch(f"/quizzes/{private_id}/share?shared=true") self.assertEqual(res.status_code, 400) self.assertEqual(self.client.post("/questions/from-bank", json={"title": "X", "question_ids": [3], "is_shared": True}).status_code, 400) - shared_id = self.generate(is_shared=True, count=2, category_ids=[1]).json()["id"] + shared_id = self.saved_test([1, 2], is_shared=True) self.user = self.peer self.assertEqual(self.client.get(f"/quizzes/{private_id}").status_code, 403) self.assertEqual(self.client.post(f"/quizzes/{private_id}/shuffle").status_code, 403) @@ -244,10 +256,13 @@ class BuilderTests(unittest.TestCase): self.assertEqual(self.client.post(f"/attempts/start?quiz_id={shared_id}").status_code, 403) def test_question_revocation_legacy_publication_and_course_enrollment(self): - saved_id = self.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + saved_id = self.saved_test([1, 2], is_shared=True) self.user = self.peer self.assertEqual(self.client.get("/quizzes/1").status_code, 200) # published legacy quiz - self.db.get(Question, 1).is_shared = 0 + # Deleting a question revokes every session that carried it. That is + # the whole of per-question revocation now: the flag an author could + # set is gone, and a question either exists or it does not. + self.db.get(Question, 1).deleted_at = datetime(2026, 1, 1) self.db.commit() self.assertEqual(self.client.get("/quizzes/1").status_code, 403) self.user = self.owner @@ -269,8 +284,8 @@ class BuilderTests(unittest.TestCase): 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 + saved = self.saved_test([1, 2], is_shared=True) + self.db.get(Question, 1).deleted_at = datetime(2026, 1, 1) self.db.commit() for user in (self.owner, self.peer): self.user = user @@ -302,7 +317,7 @@ class BuilderTests(unittest.TestCase): 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"] + saved = self.saved_test([1, 2], is_shared=True) first = self.client.post(f"/attempts/start?quiz_id={saved}").json()["id"] self.assertEqual(self.client.get(f"/attempts/{first}").json()["answers"], []) redis = Mock() @@ -314,17 +329,17 @@ class BuilderTests(unittest.TestCase): 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) + self.assertEqual(self.count(state="unused"), 3) 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) + self.assertEqual(self.count(state="unused"), 3) def test_submission_rejects_duplicates_and_out_of_pool_atomically(self): - saved = self.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + saved = self.saved_test([1, 2], is_shared=True) aid = self.client.post(f"/attempts/start?quiz_id={saved}").json()["id"] attempt = self.db.get(QuizAttempt, aid) attempt.selected_question_ids = [1] @@ -344,7 +359,7 @@ class BuilderTests(unittest.TestCase): 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"] + saved = self.saved_test([1, 2], is_shared=True) 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, diff --git a/backend/tests/test_quiz_sessions.py b/backend/tests/test_quiz_sessions.py index ff33af4..033ee5a 100644 --- a/backend/tests/test_quiz_sessions.py +++ b/backend/tests/test_quiz_sessions.py @@ -149,15 +149,15 @@ class QuestionManagerTests(unittest.TestCase): self.db.add_all([QuestionCategory(id=1, name="Cardiology", user_id=3), QuestionCategory(id=2, name="Neonatology", user_id=3)]) self.db.flush() - # 1: complete · 2: no explanation · 3: uncategorized + no difficulty · 4: private - for qid, category, difficulty, explanation, shared in [ - (1, 1, "hard", "Because…", 1), - (2, 1, "easy", "", 1), - (3, None, None, "Because…", 1), - (4, 2, "medium", "Because…", 0), + # 1: complete · 2: no explanation · 3: uncategorized + no difficulty + for qid, category, difficulty, explanation in [ + (1, 1, "hard", "Because…"), + (2, 1, "easy", ""), + (3, None, None, "Because…"), + (4, 2, "medium", "Because…"), ]: self.db.add(Question(id=qid, question_category_id=category, difficulty=difficulty, - explanation=explanation, is_shared=shared, user_id=3, + explanation=explanation, user_id=3, question_text=f"Question {qid}", question_type="mcq", options=["yes", "no"], correct_answer="yes")) self.db.commit() @@ -185,7 +185,6 @@ class QuestionManagerTests(unittest.TestCase): self.assertEqual(summary["uncategorized"], 1) self.assertEqual(summary["no_explanation"], 1) self.assertEqual(summary["no_difficulty"], 1) - self.assertEqual(summary["private"], 1) def test_summary_is_moderator_only(self): self.user = self.owner @@ -195,10 +194,9 @@ class QuestionManagerTests(unittest.TestCase): self.assertEqual(self.bank_ids(needs="category"), {3}) self.assertEqual(self.bank_ids(needs="explanation"), {2}) self.assertEqual(self.bank_ids(needs="difficulty"), {3}) - self.assertEqual(self.bank_ids(needs="private"), {4}) self.assertEqual(self.client.get("/questions/bank", params={"needs": "nonsense"}).status_code, 422) - def test_bulk_sets_category_difficulty_and_sharing(self): + def test_bulk_sets_category_and_difficulty(self): response = self.client.post("/questions/bulk", json={"question_ids": [3], "action": "category", "category_id": 2}) self.assertEqual(response.json()["updated"], 1) @@ -207,8 +205,10 @@ class QuestionManagerTests(unittest.TestCase): self.client.post("/questions/bulk", json={"question_ids": [2, 3], "action": "difficulty", "difficulty": "easy"}) self.assertEqual([self.db.get(Question, i).difficulty for i in (2, 3)], ["easy", "easy"]) - self.client.post("/questions/bulk", json={"question_ids": [4], "action": "share", "shared": 1}) - self.assertEqual(self.db.get(Question, 4).is_shared, 1) + # Sharing is no longer one of the actions: there is no per-question + # flag left for it to set. + self.assertEqual(self.client.post("/questions/bulk", + json={"question_ids": [4], "action": "share"}).status_code, 422) def test_bulk_delete_removes_questions_and_their_category_links(self): self.db.add(QuestionCategoryLink(question_id=1, category_id=2)) @@ -250,9 +250,9 @@ class RecommendationTests(unittest.TestCase): ]) self.db.add(Quiz(id=1, title="Bank test", user_id=1, is_published=1, questions_count=6)) self.db.flush() - # 6 shareable bank questions: 4 cardiology (all under the Kawasaki child), 2 neurology. + # 6 bank questions: 4 cardiology (all under the Kawasaki child), 2 neurology. for qid, category in [(1, 2), (2, 2), (3, 2), (4, 2), (5, 10), (6, 10)]: - self.db.add(Question(id=qid, question_category_id=category, user_id=3, is_shared=1, + self.db.add(Question(id=qid, question_category_id=category, user_id=3, question_text=f"Question {qid}", question_type="mcq", options=["yes", "no"], correct_answer="yes")) self.db.add(Article(id=7, slug="kawasaki", title="Kawasaki disease", sections=[], diff --git a/backend/tests/test_related_privacy.py b/backend/tests/test_related_privacy.py index caa3a82..1467e15 100644 --- a/backend/tests/test_related_privacy.py +++ b/backend/tests/test_related_privacy.py @@ -83,11 +83,11 @@ class PrivacyTests(unittest.TestCase): self.db.commit() def test_tutor_denial_precedes_all_external_boundaries(self): - for qid in (4, 5, 999): + for qid in (5, 999): self.assertIn(self.chat(qid).status_code, (403, 404)) for boundary in (self.quota, self.model, self.similar, self.ai): boundary.assert_not_called() - for qid in (1, 3): + for qid in (1, 3, 4): res = self.chat(qid) self.assertEqual(res.status_code, 200, res.text) self.assertEqual(self.similar.call_args.args[2].id, 1) @@ -127,8 +127,9 @@ class PrivacyTests(unittest.TestCase): with patch('sqlalchemy.orm.Query.all', autospec=True, side_effect=lambda query: captured.append(query) or []): self.assertEqual(self.find_similar(self.db, q, self.owner), []) sql = str(captured[-1].statement.compile(dialect=postgresql.dialect())) - self.assertIn('questions.user_id', sql) - self.assertIn('questions.is_shared', sql) + # The predicate no longer consults a per-question flag; what it still + # excludes is a deleted question and a course's own. + self.assertIn('questions.deleted_at IS NULL', sql) self.assertIn('course_id IS NOT NULL', sql) self.assertIn('<=>', sql) self.assertLess(sql.index('WHERE'), sql.index('ORDER BY')) @@ -143,14 +144,14 @@ class PrivacyTests(unittest.TestCase): self.client.headers.clear() self.assertEqual(self.client.get('/uploads/questions/stem-4.png').status_code, 401) self.login(self.owner, cookie=True) - for path in ('stem-1', 'stem-3', 'answer-1', 'answer-3'): + for path in ('stem-1', 'stem-3', 'stem-4', 'answer-1', 'answer-3'): res = self.client.get(f'/uploads/questions/{path}.png') self.assertEqual(res.status_code, 200, res.text) self.assertEqual(res.headers['cache-control'], 'private, no-store') self.assertEqual(res.headers['vary'], 'Cookie, Authorization') - self.assertEqual(self.client.get('/uploads/questions/stem-4.png').status_code, 404) - self.assertEqual(self.client.patch('/questions/3/share?shared=1').status_code, 401) - self.db.get(Question, 1).is_shared = 0 + # A course's images stay out of reach whoever asks. + self.assertEqual(self.client.get('/uploads/questions/stem-5.png').status_code, 404) + self.db.get(Question, 1).deleted_at = datetime(2026, 1, 1) self.db.commit() self.assertEqual(self.client.get('/uploads/questions/stem-1.png').status_code, 404) self.login(self.owner) @@ -236,8 +237,7 @@ class PrivacyTests(unittest.TestCase): self.assertEqual(paths, sorted(set(paths))) self.assertIn('questions/stem-1.png', paths) self.assertIn('questions/stem-4.png', paths) # Now bank-visible via the created shared question. - self.assertNotIn('questions/answer-3.png', paths) # Private question images stay private. - self.assertNotIn('questions/stem-5.png', paths) + self.assertNotIn('questions/stem-5.png', paths) # A course's images stay out of the bank. self.login(self.mod) for field in ('image_path', 'explanation_image_path'): for value in ([], {}, 17, '../escape'): @@ -276,7 +276,7 @@ class PrivacyTests(unittest.TestCase): self.assertEqual(self.client.get('/uploads/legacy-public.svg').status_code, 401) self.login(self.owner) self.assertEqual(self.client.get('/uploads/legacy-public.svg').status_code, 200) - self.db.get(Question, 2).is_shared = 0 + self.db.get(Question, 2).deleted_at = datetime(2026, 1, 1) self.db.commit() self.assertEqual(self.client.get('/uploads/legacy-public.svg').status_code, 404) self.enroll() @@ -353,7 +353,10 @@ class PrivacyTests(unittest.TestCase): self.login(self.owner) self.assertEqual(self.client.get('/uploads/legacy-private.svg?v=1').status_code, 200, value) self.login(self.peer) - self.assertEqual(self.client.get('/uploads/legacy-private.svg').status_code, 404, value) + # The question is in the bank, so its image is reachable by + # anybody who may sit it — which, with per-question sharing + # gone, is everybody. + self.assertEqual(self.client.get('/uploads/legacy-private.svg').status_code, 200, value) self.login(self.owner) bank_paths = self.client.get('/questions/images').json() self.assertIn({'image_path': 'legacy-private.svg', 'url': '/uploads/legacy-private.svg'}, bank_paths) diff --git a/backend/tests/test_session_lifecycle.py b/backend/tests/test_session_lifecycle.py index 79adf30..e32db4b 100644 --- a/backend/tests/test_session_lifecycle.py +++ b/backend/tests/test_session_lifecycle.py @@ -65,7 +65,7 @@ class SessionLifecycleTests(unittest.TestCase): self.bank.tearDown() def timed_quiz(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], count=2, mode="timed").json()["id"] + quiz_id = self.bank.saved_test([1, 2], is_shared=True, mode="timed") with patch.dict(sys.modules, {"redis": self.redis}): aid = self.client.post(f"/attempts/start?quiz_id={quiz_id}&fresh=true").json()["id"] return quiz_id, aid @@ -224,7 +224,7 @@ class QuizAnalysisTests(unittest.TestCase): self.bank.tearDown() def test_a_quiz_nobody_has_sat_answers_with_zeroes_and_every_question_skipped(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) body = self.client.get(f"/attempts/quiz/{quiz_id}/analysis").json() self.assertTrue(body["not_started"]) self.assertIsNone(body["attempt_id"]) @@ -238,7 +238,7 @@ class QuizAnalysisTests(unittest.TestCase): self.assertNotIn("recommendations", body) def test_once_sat_the_quiz_address_answers_with_the_real_analysis(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) self.db.add(QuizAttempt(id=910, quiz_id=quiz_id, user_id=self.bank.owner.id, mode="learning", total_questions=1, score=1, started_at=datetime(2026, 5, 1), completed_at=datetime(2026, 5, 1, 1))) @@ -272,7 +272,7 @@ class LiveAnalysisTests(unittest.TestCase): self.bank.tearDown() def test_answers_given_but_not_submitted_are_analysed(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) with patch.dict(sys.modules, {"redis": self.redis}): aid = self.client.post(f"/attempts/start?quiz_id={quiz_id}&mode=study").json()["id"] # One answered, saved to progress; nothing submitted. @@ -299,7 +299,7 @@ class LiveAnalysisTests(unittest.TestCase): whether it was right, and go back and change it — the exam defeated rather than analysed. """ - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) with patch.dict(sys.modules, {"redis": self.redis}): aid = self.client.post(f"/attempts/start?quiz_id={quiz_id}&mode=exam").json()["id"] self.store[f"quiz_progress:{self.bank.owner.id}:{aid}"] = json.dumps({"answers": {"1": "yes"}}) @@ -325,7 +325,7 @@ class LiveAnalysisTests(unittest.TestCase): self.assertEqual(recs.json()["rows"], []) def test_a_finished_exam_is_marked(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) with patch.dict(sys.modules, {"redis": self.redis}): aid = self.client.post(f"/attempts/start?quiz_id={quiz_id}&mode=exam").json()["id"] self.client.post(f"/attempts/{aid}/submit", @@ -338,7 +338,7 @@ class LiveAnalysisTests(unittest.TestCase): self.assertIn("correct", {q["status"] for q in body["questions"]}) def test_a_live_attempt_with_nothing_answered_reads_as_nothing_answered(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], count=2).json()["id"] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) with patch.dict(sys.modules, {"redis": self.redis}): aid = self.client.post(f"/attempts/start?quiz_id={quiz_id}&mode=study").json()["id"] body = self.client.get(f"/attempts/{aid}/analysis").json() diff --git a/backend/tests/test_share_public.py b/backend/tests/test_share_public.py index c259bc1..d83c6a3 100644 --- a/backend/tests/test_share_public.py +++ b/backend/tests/test_share_public.py @@ -1,5 +1,6 @@ """Public share-link endpoints: token lifecycle and public landing data.""" import unittest +from datetime import datetime import test_quiz_builder as fixtures from app.models.quiz import Quiz @@ -44,13 +45,16 @@ class ShareLinkTests(unittest.TestCase): self.assertEqual(self.bank.db.get(Quiz, 1).is_shared, 0) self.assertEqual(self.client.get('/quizzes/1').json()['share_token'], None) - def test_course_and_private_question_quizzes_cannot_share(self): + def test_course_and_removed_question_quizzes_cannot_share(self): self.bank.user = self.bank.mod self.assertEqual(self.client.post('/quizzes/2/share-link').status_code, 400) # Course quiz. - mixed = Quiz(title='Private mix', user_id=3, is_published=1, questions_count=1) + # A session carrying a question nobody can reach cannot be handed on. + # That used to mean one its author kept back; it means a removed one. + mixed = Quiz(title='Removed mix', user_id=3, is_published=1, questions_count=1) self.bank.db.add(mixed) self.bank.db.flush() self.bank.db.add(QuizQuestionLink(quiz_id=mixed.id, question_id=4, position=0)) + self.bank.db.get(fixtures.Question, 4).deleted_at = datetime(2026, 1, 1) self.bank.db.commit() self.assertEqual(self.client.post(f'/quizzes/{mixed.id}/share-link').status_code, 400) mixed.is_shared = 1 # Stale share flag alone must not make it public. diff --git a/backend/tests/test_shared_category.py b/backend/tests/test_shared_category.py index a4e7d19..2b6bb24 100644 --- a/backend/tests/test_shared_category.py +++ b/backend/tests/test_shared_category.py @@ -39,7 +39,7 @@ class SharedCategoryTests(unittest.TestCase): ]) self.db.flush() # One of each content type filed under the same tree. - self.db.add(Question(id=1, question_category_id=2, user_id=1, is_shared=1, + self.db.add(Question(id=1, question_category_id=2, user_id=1, question_text="A question", question_type="mcq", options=["yes", "no"], correct_answer="yes")) self.db.add(Article(id=1, slug="kawasaki", title="Kawasaki disease", sections=[], diff --git a/backend/tests/test_study_tools.py b/backend/tests/test_study_tools.py index 4361d82..128bf30 100644 --- a/backend/tests/test_study_tools.py +++ b/backend/tests/test_study_tools.py @@ -25,7 +25,7 @@ class StudyToolTests(unittest.TestCase): return response.json()['id'] def test_attempt_mode_controls_answers_and_stats_not_query_flags(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1], mode='learning').json()['id'] + quiz_id = self.bank.saved_test([1, 2], is_shared=True, mode='learning') initial = self.client.get(f'/quizzes/{quiz_id}').json() self.assertNotIn('correct_answer', initial['questions'][0]) self.assertEqual(self.client.get(f'/quizzes/{quiz_id}?study=true').status_code, 403) @@ -52,7 +52,7 @@ class StudyToolTests(unittest.TestCase): self.assertEqual(self.client.get(f'/study-tools/attempts/{study}/questions/1/responses').status_code, 404) def test_real_response_counts_exclude_skips_expiry_course_and_private_attempts(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1]).json()['id'] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) study = self.start(quiz_id, 'study') self.bank.answer(1, True) self.bank.answer(1, True, day=1) @@ -96,7 +96,7 @@ class StudyToolTests(unittest.TestCase): self.assertEqual(self.client.get(f'/quizzes/2?attempt_id={aid}&study=true').status_code, 403) def test_progress_outage_returns_failure_not_empty_or_saved_success(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1]).json()['id'] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) aid = self.start(quiz_id, 'study') redis = Mock() redis.from_url.return_value.get.side_effect = ConnectionError('Synthetic cache outage') @@ -112,7 +112,7 @@ class StudyToolTests(unittest.TestCase): self.assertEqual(malformed.status_code, 422) def test_statistics_match_insensitively_dedupe_and_exclude_obsolete(self): - quiz_id = self.bank.generate(is_shared=True, category_ids=[1]).json()['id'] + quiz_id = self.bank.saved_test([1, 2], is_shared=True) study = self.start(quiz_id, 'study') # Case/whitespace variants of the same option match one bucket. self.bank.answer(1, True) @@ -152,10 +152,10 @@ class StudyToolTests(unittest.TestCase): self.assertEqual(response['sample_size'], 0) def test_performance_by_category_expands_links_and_excludes_irrelevant(self): - quiz = self.bank.generate(is_shared=True, category_ids=[1]).json()['id'] + quiz = self.bank.saved_test([1, 2], is_shared=True) self.bank.answer(1, True, quiz_id=quiz) self.bank.answer(1, True, quiz_id=quiz, expired=1) # Expired attempt excluded. - quiz2 = self.bank.generate(is_shared=True, category_ids=[2], count=1).json()['id'] + quiz2 = self.bank.saved_test([2], is_shared=True) self.bank.answer(2, False, quiz_id=quiz2) self.bank.user = self.bank.mod self.client.patch('/questions/3', json={'additional_category_ids': [2]}) # Educators manage questions. diff --git a/backend/tests/test_tag_hierarchy.py b/backend/tests/test_tag_hierarchy.py index afa6fc6..9a9a136 100644 --- a/backend/tests/test_tag_hierarchy.py +++ b/backend/tests/test_tag_hierarchy.py @@ -45,7 +45,7 @@ class TagHierarchyTests(unittest.TestCase): self.learner = User(id=2, name="Learner", email="l@example.test", hashed_password="unused") self.db.add_all([self.mod, self.learner]) for qid in (1, 2, 3): - self.db.add(Question(id=qid, user_id=1, is_shared=1, question_text=f"Q{qid}", + self.db.add(Question(id=qid, user_id=1, question_text=f"Q{qid}", question_type="mcq", options=["a", "b"], correct_answer="a")) self.db.execute(text(""" INSERT INTO question_tags (id, name, type, parent_id, sort_order) VALUES diff --git a/frontend/src/components/PractiseTopic.jsx b/frontend/src/components/PractiseTopic.jsx index d0332b2..5228a25 100644 --- a/frontend/src/components/PractiseTopic.jsx +++ b/frontend/src/components/PractiseTopic.jsx @@ -23,7 +23,7 @@ export default function PractiseTopic({ article, canEdit, questions, onUnlink }) useEffect(() => { let active = true - api.get('/questions/builder/count', { params: { article_ids: article.id, state: 'all', is_shared: 'false' } }) + api.get('/questions/builder/count', { params: { article_ids: article.id, state: 'all' } }) .then(res => { if (active) setAvailable(res.data.count) }) .catch(() => { if (active) setAvailable(null) }) return () => { active = false } diff --git a/frontend/src/pages/CustomQuizPage.jsx b/frontend/src/pages/CustomQuizPage.jsx index ae0a3ff..0027660 100644 --- a/frontend/src/pages/CustomQuizPage.jsx +++ b/frontend/src/pages/CustomQuizPage.jsx @@ -59,7 +59,7 @@ export default function CustomQuizPage() { const [globalSearch, setGlobalSearch] = useState('') const [moreOpen, setMoreOpen] = useState(false) - const filterKey = JSON.stringify([categoryIds, state, shared, difficulty, articleIds, tagIds, systemIds, refresh]) + const filterKey = JSON.stringify([categoryIds, state, difficulty, articleIds, tagIds, systemIds, refresh]) useEffect(() => { let active = true @@ -78,7 +78,7 @@ export default function CustomQuizPage() { let active = true setAvailable(null) setCountError('') - const params = new URLSearchParams({ state, is_shared: String(shared) }) + const params = new URLSearchParams({ state }) if (difficulty) params.append('difficulty', difficulty) categoryIds.forEach(id => params.append('category_ids', id)) articleIds.forEach(id => params.append('article_ids', id)) @@ -328,7 +328,7 @@ export default function CustomQuizPage() {
{mode === 'timed' && (
diff --git a/frontend/src/pages/QuestionManagerPage.test.jsx b/frontend/src/pages/QuestionManagerPage.test.jsx index 46b2d21..b86217d 100644 --- a/frontend/src/pages/QuestionManagerPage.test.jsx +++ b/frontend/src/pages/QuestionManagerPage.test.jsx @@ -11,8 +11,8 @@ vi.mock('../components/GrantsPanel', () => ({ default: () =>