diff --git a/backend/app/models/user.py b/backend/app/models/user.py index 2b7099e..1cbd220 100644 --- a/backend/app/models/user.py +++ b/backend/app/models/user.py @@ -22,11 +22,19 @@ class User(Base): reminders_disabled = Column(Boolean, default=False, nullable=False, server_default=sa_false()) created_at = Column(DateTime, default=datetime.utcnow) - documents = relationship("PDFDocument", back_populates="user") - quizzes = relationship("Quiz", back_populates="user") - attempts = relationship("QuizAttempt", back_populates="user") - favorites = relationship("Favorite", back_populates="user", cascade="all, delete-orphan") - note = relationship("UserNote", back_populates="user", cascade="all, delete-orphan", uselist=False) + # passive_deletes leaves the child rows to the database, which already + # knows what to do with every one of them — the foreign keys are CASCADE or + # SET NULL. Without it SQLAlchemy insists on emptying each relationship + # itself first, by writing NULL into columns that are NOT NULL, and + # deleting a user failed with a constraint violation from a table nobody + # was looking at. + documents = relationship("PDFDocument", back_populates="user", passive_deletes=True) + quizzes = relationship("Quiz", back_populates="user", passive_deletes=True) + attempts = relationship("QuizAttempt", back_populates="user", passive_deletes=True) + favorites = relationship("Favorite", back_populates="user", + cascade="all, delete-orphan", passive_deletes=True) + note = relationship("UserNote", back_populates="user", cascade="all, delete-orphan", + uselist=False, passive_deletes=True) @property def is_admin(self): diff --git a/backend/app/routers/admin.py b/backend/app/routers/admin.py index a5d1918..41ca78b 100644 --- a/backend/app/routers/admin.py +++ b/backend/app/routers/admin.py @@ -82,9 +82,15 @@ def delete_user( raise HTTPException(status_code=404, detail="User not found") if user.id == admin.id: raise HTTPException(status_code=400, detail="Cannot delete yourself") - # Nullify non-cascading FKs before delete - db.execute(text("UPDATE question_categories SET user_id = NULL WHERE user_id = :uid"), {"uid": user_id}) - db.execute(text("UPDATE quiz_categories SET user_id = NULL WHERE user_id = :uid"), {"uid": user_id}) + # Two tables name a user and refuse to forget one: question_categories and + # quiz_categories are NOT NULL and NO ACTION. The taxonomy is the site's, + # not the author's, so it is handed to the administrator doing the deleting + # rather than deleted with them. Everything else the database already knows + # what to do with — every other foreign key is CASCADE or SET NULL. + db.execute(text("UPDATE question_categories SET user_id = :new WHERE user_id = :uid"), + {"new": admin.id, "uid": user_id}) + db.execute(text("UPDATE quiz_categories SET user_id = :new WHERE user_id = :uid"), + {"new": admin.id, "uid": user_id}) db.delete(user) db.commit() diff --git a/backend/app/routers/tags.py b/backend/app/routers/tags.py index cd41a30..6321f2f 100644 --- a/backend/app/routers/tags.py +++ b/backend/app/routers/tags.py @@ -1,16 +1,25 @@ -"""AI-powered question tagging/classification — subjects, diseases, keywords. -Tables are created via raw SQL in setup_pgvector() to avoid race conditions with multiple workers. +"""Organ systems: the vocabulary, and how many questions reach each one. + +Once this served four vocabularies a model wrote per question — subjects, +diseases, keywords, systems. The first three became the category tree and have +been retired; what is left is the sixteen systems, which a topic points at. + +The table is created via raw SQL in setup_pgvector() to avoid race conditions +with multiple workers, so there is no ORM class for it. """ import logging - +from collections import defaultdict from typing import Literal from fastapi import APIRouter, Depends, HTTPException from pydantic import BaseModel, Field -from sqlalchemy import text +from sqlalchemy import select, text from sqlalchemy.orm import Session from app.database import get_db +from app.models.exam import QuestionExamLink +from app.models.question import Question +from app.models.question_category import QuestionCategory, QuestionCategoryLink from app.models.user import User from app.utils.auth import get_current_user, require_moderator @@ -20,39 +29,67 @@ log = logging.getLogger(__name__) @router.get("") def get_tags(db: Session = Depends(get_db), current_user: User = Depends(get_current_user)): - """Return all tags grouped by type, with question counts.""" - # Counts are computed within the learner's active exam, and tags scoped to a - # different exam are left out — an unscoped list would offer disciplines that - # cannot match anything they are studying. - exam_id = getattr(current_user, "active_exam_id", None) - rows = db.execute(text(""" - SELECT t.id, t.name, t.type, t.parent_id, t.sort_order, - COUNT(DISTINCT tl.question_id) AS count - FROM question_tags t - LEFT JOIN question_tag_links tl ON tl.tag_id = t.id - LEFT JOIN question_exam_links el - ON el.question_id = tl.question_id AND (:exam_id IS NULL OR el.exam_id = :exam_id) - WHERE (t.exam_id IS NULL OR :exam_id IS NULL OR t.exam_id = :exam_id) - AND (:exam_id IS NULL OR tl.question_id IS NULL OR el.question_id IS NOT NULL) - GROUP BY t.id, t.name, t.type, t.parent_id, t.sort_order - ORDER BY t.sort_order, count DESC, t.name - """), {"exam_id": exam_id}).fetchall() + """The organ systems, with how many questions reach each one. - result = {"subjects": [], "diseases": [], "keywords": [], "systems": []} - mapping = {"subject": "subjects", "disease": "diseases", - "keyword": "keywords", "system": "systems"} - # A parent with no direct questions still has to appear, or its children - # become unreachable in a tree picker. - has_children = {r.parent_id for r in rows if r.parent_id} - for r in rows: - key = mapping.get(r.type) - if not key: + A question reaches a system through the topic it is filed under, and + through every topic above that one — the same rule the analysis groups by, + so the number here is the number the filter will return. + + It used to come from a symptom keyword the question happened to mention. + Those keywords are retired, and with them the subject and disease + vocabularies that had already become the topic tree. The shape of the + response is kept so nothing that reads it has to be rewritten to find an + empty list where it expected one. + """ + exam_id = getattr(current_user, "active_exam_id", None) + systems = db.execute(text( + "SELECT id, name, sort_order FROM question_tags " + "WHERE type = 'system' AND parent_id IS NULL ORDER BY sort_order, name")).fetchall() + + categories = db.query(QuestionCategory).all() + parents = {cat.id: cat.parent_id for cat in categories} + own = {cat.id: cat.system_id for cat in categories} + + def system_of(cid): + """The topic's own system, or the nearest one above it.""" + walk, guard = cid, 0 + while walk is not None and guard < 20: + if own.get(walk): + return own[walk] + walk = parents.get(walk) + guard += 1 + return None + + effective = {cid: system_of(cid) for cid in parents} + + # Scoped to what the learner is studying, so the number matches what the + # filter will actually return. + live = db.query(Question.id).filter(Question.deleted_at.is_(None)) + if exam_id: + live = live.filter(Question.id.in_( + select(QuestionExamLink.question_id).where(QuestionExamLink.exam_id == exam_id))) + in_scope = {qid for (qid,) in live.all()} + + counts: dict[int, set[int]] = defaultdict(set) + filings = [ + *db.query(Question.id, Question.question_category_id).filter( + Question.question_category_id.isnot(None)).all(), + # A question counts towards every topic it is filed under, primary or + # not, exactly as the analysis counts it. + *db.query(QuestionCategoryLink.question_id, QuestionCategoryLink.category_id).all(), + ] + for qid, cid in filings: + if qid not in in_scope: continue - if exam_id and r.count == 0 and r.id not in has_children: - continue - result[key].append({"id": r.id, "name": r.name, "count": r.count, - "parent_id": r.parent_id}) - return result + system = effective.get(cid) + if system: + counts[system].add(qid) + + return { + "subjects": [], "diseases": [], "keywords": [], + "systems": [{"id": row.id, "name": row.name, "count": len(counts.get(row.id, ())), + "parent_id": None} for row in systems], + } class TagWrite(BaseModel): @@ -164,6 +201,11 @@ def delete_tag(tag_id: int, move_to: int | None = None, db: Session = Depends(ge db.execute(text("UPDATE question_tags SET parent_id = :p WHERE parent_id = :i"), {"p": tag.parent_id, "i": tag_id}) db.execute(text("DELETE FROM question_tag_links WHERE tag_id = :i"), {"i": tag_id}) + # Topics point at this row. Postgres would set them null on its own, but + # the column carries no ForeignKey in the model — question_tags has no ORM + # class — so saying it here is what makes it true everywhere. + db.query(QuestionCategory).filter(QuestionCategory.system_id == tag_id).update( + {"system_id": None}, synchronize_session=False) db.execute(text("DELETE FROM question_tags WHERE id = :i"), {"i": tag_id}) db.commit() diff --git a/backend/tests/test_exam_scoped_tags.py b/backend/tests/test_exam_scoped_tags.py index 2e63d2a..364d8ab 100644 --- a/backend/tests/test_exam_scoped_tags.py +++ b/backend/tests/test_exam_scoped_tags.py @@ -1,4 +1,9 @@ -"""Tags may be shared across exams or scoped to one, and facets reflect the choice. +"""System counts follow the exam the learner is studying for. + +The subject and keyword vocabularies these once scoped are retired; a question +reaches a system through the topic it is filed under. What still has to hold is +that the number beside a system is the number the filter will return — which +means counting only what is in scope. Run: DATABASE_URL=sqlite:///:memory: PYTHONPATH=backend python -m unittest discover -s backend/tests """ @@ -17,6 +22,7 @@ from app.database import Base, get_db from app.models.course import Course # noqa — Quiz.course_id FK needs the table in metadata. from app.models.exam import Exam, QuestionExamLink from app.models.question import Question +from app.models.question_category import QuestionCategory from app.models.user import User from app.routers import tags from app.utils.auth import get_current_user @@ -53,18 +59,21 @@ class ExamScopedTagTests(unittest.TestCase): question_type="mcq", options=["a", "b"], correct_answer="a")) self.db.flush() self.db.add(QuestionExamLink(question_id=qid, exam_id=exam_id)) - # Cardiology is shared; each Pediatrics is scoped to its own exam. self.db.execute(text(""" - INSERT INTO question_tags (id, name, type, exam_id) VALUES - (10, 'Cardiology', 'subject', NULL), - (11, 'Pediatrics', 'subject', 1), - (12, 'Pediatrics', 'subject', 2), - (13, 'Obstetrics', 'subject', NULL) - """)) - self.db.execute(text(""" - INSERT INTO question_tag_links (question_id, tag_id) VALUES - (1, 10), (1, 11), (2, 11), (3, 12), (3, 10) + INSERT INTO question_tags (id, name, type, parent_id) VALUES + (10, 'Cardiovascular System', 'system', NULL), + (11, 'Respiratory System', 'system', NULL), + (12, 'Immune System', 'system', NULL) """)) + # Questions 1 and 2 are cardiovascular, question 3 respiratory. Only + # question 3 belongs to the second exam. + self.db.add_all([ + QuestionCategory(id=1, name="Cardiology", user_id=1, system_id=10), + QuestionCategory(id=2, name="Pulmonology", user_id=1, system_id=11), + ]) + self.db.flush() + for qid, cid in ((1, 1), (2, 1), (3, 2)): + self.db.get(Question, qid).question_category_id = cid self.db.commit() app = FastAPI() @@ -78,38 +87,33 @@ class ExamScopedTagTests(unittest.TestCase): self.db.close() self.engine.dispose() - def subjects(self): - return {t["name"]: t["count"] for t in self.client.get("/tags").json()["subjects"]} + def systems(self): + return {t["name"]: t["count"] for t in self.client.get("/tags").json()["systems"]} - def test_the_same_name_can_exist_once_per_exam(self): - rows = self.db.execute(text( - "SELECT exam_id FROM question_tags WHERE name='Pediatrics' ORDER BY exam_id")).fetchall() - self.assertEqual([r[0] for r in rows], [1, 2]) + def test_without_an_exam_everything_is_counted(self): + self.assertEqual(self.systems(), + {"Cardiovascular System": 2, "Respiratory System": 1, + "Immune System": 0}) - def test_without_an_exam_every_tag_is_offered(self): - rows = self.client.get("/tags").json()["subjects"] - # Both Pediatrics rows appear, since neither exam is in scope. - self.assertEqual(sorted((t["id"], t["count"]) for t in rows), - [(10, 2), (11, 2), (12, 1), (13, 0)]) - - def test_choosing_an_exam_scopes_counts_and_hides_the_other_exams_tags(self): + def test_choosing_an_exam_narrows_the_counts_to_it(self): self.user.active_exam_id = 1 self.db.commit() - subjects = self.subjects() - # Cardiology is shared but only question 1 is in this exam. - self.assertEqual(subjects["Cardiology"], 1) - self.assertEqual(subjects["Pediatrics"], 2) - # Obstetrics matches nothing here, and Step 2's Pediatrics is out of scope. - self.assertNotIn("Obstetrics", subjects) - self.assertEqual(len([t for t in self.client.get("/tags").json()["subjects"] - if t["name"] == "Pediatrics"]), 1) + # Questions 1 and 2 are in this exam; question 3 is not. + self.assertEqual(self.systems()["Cardiovascular System"], 2) + self.assertEqual(self.systems()["Respiratory System"], 0) - def test_switching_exam_switches_which_pediatrics_is_offered(self): + def test_switching_exam_switches_the_counts(self): self.user.active_exam_id = 2 self.db.commit() - subjects = self.subjects() - self.assertEqual(subjects["Pediatrics"], 1) # question 3 only - self.assertEqual(subjects["Cardiology"], 1) + self.assertEqual(self.systems()["Cardiovascular System"], 0) + self.assertEqual(self.systems()["Respiratory System"], 1) + + def test_a_system_with_nothing_in_scope_is_still_offered(self): + self.user.active_exam_id = 2 + self.db.commit() + # Shown at nought rather than hidden: a filter that silently loses an + # option teaches nobody why their bank has no cardiology in it. + self.assertIn("Immune System", self.systems()) if __name__ == "__main__": diff --git a/backend/tests/test_tag_hierarchy.py b/backend/tests/test_tag_hierarchy.py index 30cf6a9..0be3641 100644 --- a/backend/tests/test_tag_hierarchy.py +++ b/backend/tests/test_tag_hierarchy.py @@ -17,6 +17,7 @@ from app.database import Base, get_db from app.models.course import Course # noqa — Quiz.course_id FK needs the table in metadata. from app.models.media import MediaAsset # noqa — health report walks every embeddable table. from app.models.question import Question +from app.models.question_category import QuestionCategory from app.models.user import User from app.routers import tags from app.utils.auth import get_current_user @@ -49,11 +50,17 @@ class TagHierarchyTests(unittest.TestCase): self.db.execute(text(""" INSERT INTO question_tags (id, name, type, parent_id, sort_order) VALUES (10, 'Nervous System & Special Senses', 'system', NULL, 0), - (11, 'Seizure', 'keyword', 10, 100), - (12, 'Headache', 'keyword', 10, 100), - (13, 'Cardiology', 'subject', NULL, 100) + (11, 'Respiratory System', 'system', NULL, 1) """)) - self.db.execute(text("INSERT INTO question_tag_links (question_id, tag_id) VALUES (1, 11), (2, 11)")) + # A question reaches a system through the topic it is filed under. + self.db.add_all([ + QuestionCategory(id=1, name="Neurology", user_id=1, system_id=10), + QuestionCategory(id=2, name="Seizure", parent_id=1, user_id=1), + QuestionCategory(id=3, name="Pulmonology", user_id=1, system_id=11), + ]) + self.db.flush() + for qid, cid in ((1, 2), (2, 2), (3, 3)): + self.db.get(Question, qid).question_category_id = cid self.db.commit() self.user = self.mod @@ -68,80 +75,53 @@ class TagHierarchyTests(unittest.TestCase): self.db.close() self.engine.dispose() - def test_the_tree_is_returned_with_parents(self): + def test_systems_are_counted_through_the_topics_that_carry_them(self): body = self.client.get("/tags").json() - systems = {t["name"]: t for t in body["systems"]} - self.assertIn("Nervous System & Special Senses", systems) - keywords = {t["name"]: t["parent_id"] for t in body["keywords"]} - # Symptoms hang off the system they present in. - self.assertEqual(keywords["Seizure"], 10) - self.assertEqual(keywords["Headache"], 10) + systems = {t["name"]: t["count"] for t in body["systems"]} + # Two questions under Seizure, whose own topic has no system, so they + # reach the one on Neurology above it. + self.assertEqual(systems["Nervous System & Special Senses"], 2) + self.assertEqual(systems["Respiratory System"], 1) - def test_a_moderator_can_add_a_top_level_entry_and_a_child(self): - top = self.client.post("/tags/", json={"name": "Respiratory System", "type": "system"}) - self.assertEqual(top.status_code, 201, top.text) - parent_id = top.json()["id"] + def test_the_retired_vocabularies_answer_empty_rather_than_missing(self): + body = self.client.get("/tags").json() + # Subjects, diseases and keywords became the topic tree. The shape of + # the response is kept so nothing reading it has to be rewritten. + for gone in ("subjects", "diseases", "keywords"): + self.assertEqual(body[gone], []) - child = self.client.post("/tags/", json={"name": "Wheeze", "type": "keyword", - "parent_id": parent_id}) - self.assertEqual(child.status_code, 201, child.text) + def test_a_moderator_can_add_a_system(self): + made = self.client.post("/tags/", json={"name": "Endocrine System", "type": "system"}) + self.assertEqual(made.status_code, 201, made.text) # It shows up immediately, which is what every picker reads. - body = self.client.get("/tags").json() - self.assertIn("Wheeze", [t["name"] for t in body["keywords"]]) - - def test_a_child_must_match_its_parents_kind(self): - response = self.client.post("/tags/", json={"name": "Nonsense", "type": "keyword", - "parent_id": 13}) - self.assertEqual(response.status_code, 400) + self.assertIn("Endocrine System", + [t["name"] for t in self.client.get("/tags").json()["systems"]]) def test_duplicate_names_in_the_same_place_are_refused(self): self.assertEqual(self.client.post("/tags/", json={ - "name": "Seizure", "type": "keyword", "parent_id": 10}).status_code, 409) - # The same name under a different parent is fine. - self.assertEqual(self.client.post("/tags/", json={ - "name": "Seizure", "type": "keyword"}).status_code, 201) + "name": "Respiratory System", "type": "system"}).status_code, 409) + + def test_renaming_a_system_keeps_the_topics_pointing_at_it(self): + self.assertEqual(self.client.patch( + "/tags/10", json={"name": "Neurological System"}).status_code, 200) + systems = {t["name"]: t["count"] for t in self.client.get("/tags").json()["systems"]} + # The topics point at the row, not the word, so the count survives. + self.assertEqual(systems["Neurological System"], 2) + + def test_deleting_a_system_leaves_its_topics_pointing_at_nothing(self): + from app.models.question_category import QuestionCategory + self.client.delete("/tags/10") + self.db.expire_all() + # The foreign key is SET NULL in Postgres. Null means nobody has said + # which system this topic belongs to — it is not a claim that it has + # none, and the analysis leaves such a topic out rather than guessing. + self.assertIsNone(self.db.get(QuestionCategory, 1).system_id) def test_a_tag_cannot_become_its_own_descendants_child(self): - self.assertEqual(self.client.patch("/tags/10", json={"parent_id": 11}).status_code, 400) self.assertEqual(self.client.patch("/tags/10", json={"parent_id": 10}).status_code, 400) - def test_renaming_and_reparenting(self): - self.assertEqual(self.client.patch("/tags/12", json={"name": "Head pain"}).status_code, 200) - self.assertEqual(self.db.execute(text( - "SELECT name FROM question_tags WHERE id = 12")).scalar(), "Head pain") - self.client.patch("/tags/12", json={"parent_id": None}) - self.assertIsNone(self.db.execute(text( - "SELECT parent_id FROM question_tags WHERE id = 12")).scalar()) - - def test_questions_attached_to_a_tag_appear_in_its_count(self): - response = self.client.post("/tags/12/questions", json={"question_ids": [1, 2, 3]}) - self.assertEqual(response.json()["added"], 3) - counts = {t["name"]: t["count"] for t in self.client.get("/tags").json()["keywords"]} - self.assertEqual(counts["Headache"], 3) - # Adding the same ones again is a no-op rather than a duplicate. - self.assertEqual(self.client.post("/tags/12/questions", - json={"question_ids": [1, 2]}).json()["added"], 0) - - def test_deleting_reparents_children_rather_than_orphaning_them(self): - self.client.delete("/tags/10") - # Seizure and Headache were under it; they rise to top level, not nowhere. - rows = self.db.execute(text( - "SELECT parent_id FROM question_tags WHERE id IN (11, 12)")).fetchall() - self.assertEqual([r[0] for r in rows], [None, None]) - - def test_deleting_can_move_its_questions_onto_another_tag(self): - self.client.delete("/tags/11", params={"move_to": 12}) - counts = {t["name"]: t["count"] for t in self.client.get("/tags").json()["keywords"]} - self.assertEqual(counts["Headache"], 2) - def test_editing_the_taxonomy_is_moderator_only(self): self.user = self.learner - self.assertEqual(self.client.post("/tags/", json={"name": "X", "type": "keyword"}).status_code, 403) + self.assertEqual(self.client.post("/tags/", json={"name": "X", "type": "system"}).status_code, 403) self.assertEqual(self.client.patch("/tags/11", json={"name": "X"}).status_code, 403) self.assertEqual(self.client.delete("/tags/11").status_code, 403) - self.assertEqual(self.client.post("/tags/11/questions", - json={"question_ids": [1]}).status_code, 403) - - -if __name__ == "__main__": - unittest.main() diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index 20e776a..f2ec8c7 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -77,6 +77,24 @@ function AppLayout() { ) } +/** + * The frame for a page anybody can reach without an account. + * + * No navigation — a Sign In link on the sign-in page is noise — but the footer + * stays, because About, Contact and the clinical disclaimer are not things to + * hide from someone who has not signed in yet. + */ +function PublicLayout() { + return ( +
+
+ +
+ +
+ ) +} + /** * Somewhere this account cannot go. * @@ -138,15 +156,19 @@ function AppRoutes() { return ( }> - {/* Always public */} + {/* Always public. The landing page carries its own frame; the rest sit + in a plain one that still has the footer — signing in is not a + reason to lose the way to About, Contact or the disclaimer. */} } /> - : } /> - : } /> - } /> - } /> - } /> - } /> - } /> + }> + : } /> + : } /> + } /> + } /> + } /> + } /> + } /> + {/* Authenticated app — wrapped in AppLayout */} }> diff --git a/frontend/src/components/DraftBatch.css b/frontend/src/components/DraftBatch.css index 6fefdd4..49dda50 100644 --- a/frontend/src/components/DraftBatch.css +++ b/frontend/src/components/DraftBatch.css @@ -57,3 +57,19 @@ } @media (min-width: 700px) { .db-edit textarea, .db-edit input { font-size: 0.86rem; } } .db-edit-actions { display: flex; gap: 8px; } + +/* Where the batch files into. Above the list, because it is true of every row + in it and nothing can leave until it is answered. */ +.db-filing { + display: flex; align-items: center; gap: 12px; flex-wrap: wrap; + padding: 11px 14px; margin-bottom: 10px; + background: var(--bg); border: 1px solid var(--border); border-radius: 10px; +} +.db-filing label { display: flex; align-items: center; gap: 9px; flex: 1 1 320px; min-width: 0; } +.db-filing span { font-size: 0.82rem; font-weight: 600; color: var(--text-muted); white-space: nowrap; } +.db-filing select { + flex: 1; min-width: 0; padding: 7px 10px; font: inherit; font-size: 0.84rem; + border: 1px solid var(--border); border-radius: 8px; + background: var(--input-bg); color: var(--text); +} +.db-filing-note { flex: 1 1 100%; font-size: 0.78rem; line-height: 1.5; color: var(--wrong-fg); } diff --git a/frontend/src/components/DraftBatch.jsx b/frontend/src/components/DraftBatch.jsx index 5e1cb88..98f7806 100644 --- a/frontend/src/components/DraftBatch.jsx +++ b/frontend/src/components/DraftBatch.jsx @@ -22,6 +22,10 @@ export default function DraftBatch({ batchId, onChanged }) { const [picked, setPicked] = useState(() => new Set()) const [editing, setEditing] = useState(null) const [draftEdit, setDraftEdit] = useState(null) + // Where the whole batch files into. Said once here rather than on every + // draft, and required before anything crosses over: a question filed nowhere + // reaches no discipline, no organ system and no row on any analysis tab. + const [categories, setCategories] = useState([]) const load = useCallback(() => { api.get(`/drafts/batches/${batchId}`) @@ -31,6 +35,25 @@ export default function DraftBatch({ batchId, onChanged }) { useEffect(() => { load() }, [load]) + useEffect(() => { + api.get('/question-categories/') + .then(res => setCategories(Array.isArray(res.data) ? res.data : [])) + .catch(() => setCategories([])) + }, []) + + /** Names shown as a path, so "Croup" is not ambiguous between two parents. */ + const paths = useMemo(() => { + const byId = Object.fromEntries(categories.map(c => [c.id, c])) + const trail = (cat) => { + const parts = [] + let walk = cat, guard = 0 + while (walk && guard < 20) { parts.unshift(walk.name); walk = byId[walk.parent_id]; guard += 1 } + return parts.join(' › ') + } + return categories.map(c => ({ id: c.id, label: trail(c) })) + .sort((a, b) => a.label.localeCompare(b.label)) + }, [categories]) + const pending = useMemo( () => (batch?.drafts || []).filter(d => d.status === 'pending'), [batch]) const ready = useMemo(() => pending.filter(d => d.problems.length === 0), [pending]) @@ -92,6 +115,26 @@ export default function DraftBatch({ batchId, onChanged }) {
{error &&

{error}

} +
+ + {!batch.category_id && ( + + Every question from this batch is filed here when it crosses over. + Nothing can cross over until it is set. + + )} +
+
@@ -275,8 +266,6 @@ export default function CustomQuizPage() {
setOpenFacet('organ-systems')} /> setOpenFacet('systems')} /> - setOpenFacet('disciplines')} /> - setOpenFacet('symptoms')} /> setOpenFacet('articles')} /> setOpenFacet('saved')} /> @@ -416,21 +405,11 @@ export default function CustomQuizPage() { )} - setOpenFacet(null)} - onReset={() => setTagIds(ids => ids.filter(id => !subjectTags.some(t => t.id === id)))} - selected={subjectTags.filter(t => tagIds.includes(t.id)).map(t => ({ id: t.id, label: t.name }))} - onRemove={id => toggleTag(id, false)} - helper="By default, all disciplines are included unless filters are selected."> - {query => checkList(subjectTags, t => tagIds.includes(t.id), (t, on) => toggleTag(t.id, on), query)} - - - setOpenFacet(null)} - onReset={() => setTagIds(ids => ids.filter(id => !keywordTags.some(t => t.id === id)))} - selected={keywordTags.filter(t => tagIds.includes(t.id)).map(t => ({ id: t.id, label: t.name }))} - onRemove={id => toggleTag(id, false)} - helper="By default, all keywords are included unless filters are selected."> - {query => checkList(keywordTags, t => tagIds.includes(t.id), (t, on) => toggleTag(t.id, on), query)} - + {/* Disciplines and Symptoms & keywords were two more pickers here, over + the 203 subject tags and 4,281 keyword tags a model had written per + question. Both vocabularies were converted into Topics — 673 of them, + in a tree, which is the same axis said once and said better — and + have been retired. Topics is where a discipline is chosen. */} setOpenFacet(null)} onReset={() => setArticleIds([])} diff --git a/frontend/src/pages/CustomQuizPage.test.jsx b/frontend/src/pages/CustomQuizPage.test.jsx index f07f8ae..9bfad56 100644 --- a/frontend/src/pages/CustomQuizPage.test.jsx +++ b/frontend/src/pages/CustomQuizPage.test.jsx @@ -12,7 +12,7 @@ const categories = [ { id: 1, name: 'Pediatrics', question_count: 30, breadcrumbs: [{ id: 1, name: 'Pediatrics' }] }, { id: 2, name: 'Neonatal', question_count: 10, breadcrumbs: [{ id: 1, name: 'Pediatrics' }, { id: 2, name: 'Neonatal' }] }, ] -const TAGS = { subjects: [{ id: 7, name: 'Cardiology' }], keywords: [{ id: 8, name: 'Board Review 2019' }] } +const TAGS = { subjects: [], keywords: [], systems: [{ id: 7, name: 'Cardiovascular System', count: 255 }] } function setupCount(count = 30) { api.get.mockImplementation(url => { if (url === '/question-categories/') return Promise.resolve({ data: categories }) @@ -152,9 +152,9 @@ describe('CustomQuizPage', () => { await screen.findByText('30 questions available') await userEvent.type(screen.getByLabelText('Filter search'), 'cardio') - const hit = await screen.findByRole('checkbox', { name: /Cardiology/ }) + const hit = await screen.findByRole('checkbox', { name: /Cardiovascular System/ }) await userEvent.click(hit) - expect(screen.getByRole('button', { name: /^Disciplines/ })).toHaveTextContent('Cardiology') + expect(screen.getByRole('button', { name: /^Systems/ })).toHaveTextContent('Cardiovascular System') await userEvent.clear(screen.getByLabelText('Filter search')) await userEvent.type(screen.getByLabelText('Filter search'), 'zzzz') @@ -221,16 +221,16 @@ it('shows what you picked inside the picker, and lets you take one off', async ( return Promise.resolve({ data: { count: 30 } }) }) renderBuilder() - await userEvent.click(await screen.findByRole('button', { name: /Disciplines/ })) + await userEvent.click(await screen.findByRole('button', { name: /Systems/ })) - const panel = screen.getByRole('dialog', { name: 'Disciplines' }) - await userEvent.click(within(panel).getByRole('checkbox', { name: /Cardiology/ })) + const panel = screen.getByRole('dialog', { name: 'Systems' }) + await userEvent.click(within(panel).getByRole('checkbox', { name: /Cardiovascular System/ })) // The chip is the way back out. Hunting the checkbox down again in a list of // several hundred is not, and Reset throws away every other choice. - const chip = within(panel).getByRole('button', { name: 'Remove Cardiology' }) + const chip = within(panel).getByRole('button', { name: 'Remove Cardiovascular System' }) await userEvent.click(chip) - expect(within(panel).queryByRole('button', { name: 'Remove Cardiology' })).not.toBeInTheDocument() - expect(within(panel).getByRole('checkbox', { name: /Cardiology/ })).not.toBeChecked() + expect(within(panel).queryByRole('button', { name: 'Remove Cardiovascular System' })).not.toBeInTheDocument() + expect(within(panel).getByRole('checkbox', { name: /Cardiovascular System/ })).not.toBeChecked() }) diff --git a/frontend/src/pages/LandingPage.jsx b/frontend/src/pages/LandingPage.jsx index 723df08..190bbf5 100644 --- a/frontend/src/pages/LandingPage.jsx +++ b/frontend/src/pages/LandingPage.jsx @@ -183,6 +183,8 @@ function AuthModal({ mode, onClose, onSwitch }) { const [resendSent, setResendSent] = useState(false) const [resending, setResending] = useState(false) const [registered, setRegistered] = useState(false) + // Typed twice. The same field exists on /register; this is the other door. + const [confirm, setConfirm] = useState('') // Whether this site is invite only. The register page asks the same // question; this form is the other door and used not to ask at all, so // turning the gate on left everyone here failing with "an invite code is @@ -199,7 +201,7 @@ function AuthModal({ mode, onClose, onSwitch }) { }, []) const reset = () => { setError(''); setUnverified(false); setResendSent(false); setRegistered(false) } - const switchMode = (m) => { reset(); setName(''); setEmail(''); setPassword(''); onSwitch(m) } + const switchMode = (m) => { reset(); setName(''); setEmail(''); setPassword(''); setConfirm(''); onSwitch(m) } const handleLogin = async (e) => { e.preventDefault() @@ -218,6 +220,7 @@ function AuthModal({ mode, onClose, onSwitch }) { const handleRegister = async (e) => { e.preventDefault() setError('') + if (password !== confirm) { setError('The two passwords do not match.'); return } setLoading(true) try { const res = await api.post('/auth/register', { @@ -339,8 +342,19 @@ function AuthModal({ mode, onClose, onSwitch }) { setEmail(e.target.value)} required />
- - setPassword(e.target.value)} required minLength={8} /> + + setPassword(e.target.value)} /> +
+
+ + setConfirm(e.target.value)} /> + {confirm && password !== confirm && ( + + These do not match yet. + + )}
{inviteRequired && (
@@ -359,7 +373,8 @@ function AuthModal({ mode, onClose, onSwitch }) { )} diff --git a/frontend/src/pages/LoginTurnstile.test.jsx b/frontend/src/pages/LoginTurnstile.test.jsx index 9cc8f42..f8e8e37 100644 --- a/frontend/src/pages/LoginTurnstile.test.jsx +++ b/frontend/src/pages/LoginTurnstile.test.jsx @@ -56,6 +56,10 @@ it('keeps registration protected on the landing page', async () => { const signup = screen.getByRole('button', { name: 'Sign Up', exact: true }) expect(signup).toBeDisabled() act(() => window.turnstile.render.mock.calls[1][1].callback('synthetic-valid-token')) + // Still not enough: the password has to be typed twice and agree. + expect(signup).toBeDisabled() + await userEvent.type(screen.getByLabelText('Password'), 'longenough1') + await userEvent.type(screen.getByLabelText('Confirm password'), 'longenough1') expect(signup).toBeEnabled() }) diff --git a/frontend/src/pages/QuizPage.jsx b/frontend/src/pages/QuizPage.jsx index b4cefc8..85c70a1 100644 --- a/frontend/src/pages/QuizPage.jsx +++ b/frontend/src/pages/QuizPage.jsx @@ -1323,19 +1323,50 @@ const timerStarted = timeLeft !== null screen only made it ambiguous which one you were writing in. The global note still lives on the dashboard. */} {tool && setTool(null)} />} - {showReview && setShowReview(false)}> - {/* They are not counted as wrong: the score is out of what you - answered. But they are questions you did not do, and that is worth - saying before you hand it in. */} -

{answeredCount} of {totalCount} questions answered.{' '} - {answeredCount < totalCount - ? `${totalCount - answeredCount} will be left unanswered — they are not marked wrong, but they are not marked.` - : 'Everything is answered.'}

-
{questions.map((question, index) => )}
- -
} + {showReview && (() => { + const missing = questions + .map((question, index) => ({ question, index })) + .filter(({ question }) => !answers[question.id]) + return ( + setShowReview(false)}> + {/* A confirmation with one button is not a confirmation. The way + back into the block is the first thing offered, and the count + is a sentence rather than a grid to be counted by eye. */} +

+ {missing.length === 0 + ? `All ${totalCount} questions are answered.` + : `${missing.length} of ${totalCount} question${totalCount === 1 ? '' : 's'} ` + + `${missing.length === 1 ? 'is' : 'are'} unanswered.`} +

+ {missing.length > 0 && ( + <> +

+ They are not marked wrong — the score is out of what you + answered — but they are not marked at all. +

+
+ {missing.map(({ question, index }) => ( + + ))} +
+ + )} +
+ + +
+
+ ) + })()} {submitError &&
{submitError}
} {progressError &&
{progressError}
} {timeUp && ( diff --git a/frontend/src/pages/QuizPage.test.jsx b/frontend/src/pages/QuizPage.test.jsx index e443dfe..0ed9d34 100644 --- a/frontend/src/pages/QuizPage.test.jsx +++ b/frontend/src/pages/QuizPage.test.jsx @@ -461,13 +461,17 @@ describe('quiz player', () => { expect(screen.queryByText(/Full explanation/)).not.toBeInTheDocument() expect(api.get.mock.calls.some(([url]) => url.startsWith('/study-tools/attempts/'))).toBe(false) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - const review = screen.getByRole('dialog', { name: 'Review & Complete' }) - expect(within(review).getByText(/1 of 2 questions answered/)).toBeInTheDocument() - await userEvent.click(within(review).getByRole('button', { name: '2 · Unanswered' })) + const review = screen.getByRole('dialog', { name: 'End block?' }) + // The sentence names what is missing, not what is done — that is the + // decision being confirmed. + expect(within(review).getByText(/1 of 2 questions is unanswered/)).toBeInTheDocument() + // And there is a way back into the block, not only a way out of it. + expect(within(review).getByRole('button', { name: 'Go back and answer them' })).toBeInTheDocument() + await userEvent.click(within(review).getByRole('button', { name: '2' })) await findStem('Full second clinical question.') expect(api.post.mock.calls.some(([url]) => url === '/attempts/50/submit')).toBe(false) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Complete test' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ answers: [{ question_id: 1, user_answer: 'First answer' }], @@ -482,7 +486,7 @@ describe('quiz player', () => { expect(screen.getByRole('note')).toHaveTextContent('extrathoracic') fireEvent.keyDown(window, { key: '1' }) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Complete test' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ hints: [1], @@ -499,7 +503,7 @@ describe('quiz player', () => { fireEvent.keyDown(window, { key: '1' }) await userEvent.click(await screen.findByRole('button', { name: 'stridor' })) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Complete test' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) expect(await screen.findByText('Submitted results')).toBeInTheDocument() expect(api.post).toHaveBeenCalledWith('/attempts/50/submit', expect.objectContaining({ hints: [], @@ -576,7 +580,7 @@ describe('quiz player', () => { return originalPost(url, ...args) }) await userEvent.click(screen.getAllByRole('button', { name: 'Review & Complete' })[0]) - await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'Complete test' })) + await userEvent.click(within(screen.getByRole('dialog')).getByRole('button', { name: 'End block' })) expect(await screen.findByRole('alert')).toHaveTextContent('Try again safely') expect(api.delete).not.toHaveBeenCalled() await userEvent.click(screen.getByRole('button', { name: 'Retry submission' })) diff --git a/frontend/src/pages/QuizPlayer.css b/frontend/src/pages/QuizPlayer.css index daaf119..92222ad 100644 --- a/frontend/src/pages/QuizPlayer.css +++ b/frontend/src/pages/QuizPlayer.css @@ -440,3 +440,19 @@ body:has(.quiz-player.is-boxed) .site-footer { display: none; } border-radius: 8px; } .quiz-drawer-menu a:hover { background: var(--bg); } + +/* ── Ending a block ─────────────────────────────────────────────────── + A confirmation with one button is not a confirmation. The way back into + the block is offered first, and the unanswered are numbers you can press + rather than a list to be read. */ +.quiz-end-lead { margin: 0 0 8px; font-size: 1rem; font-weight: 600; } +.quiz-end-note { margin: 0 0 14px; font-size: 0.86rem; line-height: 1.6; color: var(--text-muted); } +.quiz-end-missing { display: flex; flex-wrap: wrap; gap: 8px; margin-bottom: 18px; } +.quiz-end-missing button { + min-width: 42px; min-height: 42px; padding: 0 12px; + font: inherit; font-weight: 700; cursor: pointer; + background: var(--wrong-bg); color: var(--wrong-fg); + border: 1px solid var(--wrong-bd); border-radius: 9px; +} +.quiz-end-missing button:hover { border-color: var(--wrong-fg); } +.quiz-end-actions { display: flex; gap: 10px; flex-wrap: wrap; justify-content: flex-end; } diff --git a/frontend/src/pages/RegisterInvite.test.jsx b/frontend/src/pages/RegisterInvite.test.jsx index bb0614a..cf8f731 100644 --- a/frontend/src/pages/RegisterInvite.test.jsx +++ b/frontend/src/pages/RegisterInvite.test.jsx @@ -17,6 +17,8 @@ const fill = async () => { await userEvent.type(screen.getByLabelText('Name'), 'Ada') await userEvent.type(screen.getByLabelText('Email'), 'ada@example.test') await userEvent.type(screen.getByLabelText('Password'), 'longenough1') + // Typed twice, as a real person would. + await userEvent.type(screen.getByLabelText('Confirm password'), 'longenough1') } describe('registering when the site is invite only', () => { @@ -60,6 +62,7 @@ describe('registering when the site is invite only', () => { render() // No code asked for, rather than a form nobody can complete. await waitFor(() => expect(screen.queryByLabelText('Invite code')).not.toBeInTheDocument()) + await fill() expect(screen.getByRole('button', { name: /Sign Up/ })).toBeEnabled() }) }) diff --git a/frontend/src/pages/RegisterPage.jsx b/frontend/src/pages/RegisterPage.jsx index 6c2f837..d8fedca 100644 --- a/frontend/src/pages/RegisterPage.jsx +++ b/frontend/src/pages/RegisterPage.jsx @@ -33,6 +33,9 @@ export default function RegisterPage() { const [name, setName] = useState('') const [email, setEmail] = useState('') const [password, setPassword] = useState('') + // Typed twice, because a password you cannot see is a password you can + // mistype into an account you then cannot get into. + const [confirm, setConfirm] = useState('') const [error, setError] = useState('') const [loading, setLoading] = useState(false) const [done, setDone] = useState(false) @@ -54,6 +57,7 @@ export default function RegisterPage() { const handleSubmit = async (e) => { e.preventDefault() setError('') + if (password !== confirm) { setError('The two passwords do not match.'); return } setLoading(true) try { const res = await api.post('/auth/register', { @@ -119,6 +123,17 @@ export default function RegisterPage() { setPassword(e.target.value)} required minLength={8} />
+
+ + setConfirm(e.target.value)} /> + {confirm && password !== confirm && ( + + These do not match yet. + + )} +
{inviteRequired && (
@@ -136,7 +151,8 @@ export default function RegisterPage() { )} diff --git a/frontend/src/pages/ToolsPage.test.jsx b/frontend/src/pages/ToolsPage.test.jsx index 359ea30..5682f9f 100644 --- a/frontend/src/pages/ToolsPage.test.jsx +++ b/frontend/src/pages/ToolsPage.test.jsx @@ -20,7 +20,8 @@ const BATCHES = [ ] const BATCH = { - id: 1, title: 'Nelson ch. 12', status: 'open', counts: { pending: 2, accepted: 1 }, + id: 1, title: 'Nelson ch. 12', status: 'open', category_id: 4, + counts: { pending: 2, accepted: 1 }, drafts: [ { id: 11, position: 0, question_text: 'A neonate with cyanosis…', options: ['TGA', 'ASD'], correct_answer: 'TGA', explanation: 'Because.', status: 'pending', problems: [], question_id: null }, @@ -105,3 +106,22 @@ describe('the question workbench', () => { expect.objectContaining({ correct_answer: 'two' }))) }) }) + +it('will not let a batch cross over until it is filed somewhere', async () => { + api.get.mockImplementation(url => { + if (url === '/drafts/batches') return Promise.resolve({ data: BATCHES }) + if (url === '/drafts/batches/1') return Promise.resolve({ data: { ...BATCH, category_id: null } }) + if (url === '/question-categories/') return Promise.resolve({ data: [{ id: 4, name: 'Neonatology', parent_id: null }] }) + return Promise.resolve({ data: [] }) + }) + render() + await userEvent.click(await screen.findByRole('button', { name: /Nelson ch\. 12/ })) + await userEvent.click(await screen.findByRole('checkbox', { name: 'Select draft 1' })) + // A question filed nowhere reaches no discipline, no organ system and no row + // on any analysis tab, so it cannot leave until somebody says where it goes. + expect(screen.getByRole('button', { name: /Move 1 into the bank/ })).toBeDisabled() + + await userEvent.selectOptions(screen.getByRole('combobox'), '4') + await waitFor(() => expect(api.patch).toHaveBeenCalledWith( + '/drafts/batches/1', { category_id: 4 })) +})