Coverage for server / services / common / question_bank.py: 96%
70 statements
« prev ^ index » next coverage.py v7.13.4, created at 2026-10-04 09:33 +0000
« prev ^ index » next coverage.py v7.13.4, created at 2026-10-04 09:33 +0000
1"""Resolving an assignment's questions from the live question banks.
3WHY THIS MODULE EXISTS
4----------------------
5Question documents live in three places, checked in order:
71. ``teacher_questionbank`` — the live bank (teacher-authored items)
82. ``global_questionbank`` — staff-authored items, in the ADMIN-STAFF database
93. ``question_collection`` — a legacy collection that no longer exists on any
10 EruditionTX database; kept as a last-resort lookup for historical ids only
12``server/services/student/student_assignment.py`` already resolved questions this
13way, but ``assignment_view_fetch`` / ``assignment_review_fetch`` in
14``server/services/common/assignments.py`` read ``question_collection`` DIRECTLY —
15so they answered 200 with ``"questions": []`` for every assignment, however many
16questions it actually held. The lookup lives here now so there is one
17implementation rather than a third copy.
19Developer: Allan Ninal
20Date: 2026-09-23
21"""
23import json
25from bson.objectid import ObjectId
27from server.connection.database import db, staff_admin_db
28from server.utilities import model_parser
30# Defense-in-depth: bound the cross-bank $in fan-out. Assignments cap at 100
31# questions, so this never bites a legitimate assignment — it only guards
32# against a corrupt or abusive oversized id set.
33MAX_QUESTION_FETCH = 500
36def question_object_ids(assignment) -> list:
37 """The ObjectIds of an assignment's questions, in the assignment's own order.
39 ``Assignment.questions`` is a ``List[Union[QuestionModel, str]]`` — each item
40 is a QuestionModel (``.id`` is a PydanticObjectId), a ``{"id": ...}`` dict, or
41 a bare id string. There is no ``question_ids`` field on the model; code that
42 referenced one raised AttributeError and 500'd.
43 """
44 questions = (
45 assignment.get("questions", [])
46 if isinstance(assignment, dict)
47 else (getattr(assignment, "questions", None) or [])
48 )
49 ids = []
50 for entry in questions:
51 raw = (
52 entry.get("id") if isinstance(entry, dict) else getattr(entry, "id", entry)
53 )
54 if raw is None:
55 continue
56 if isinstance(raw, ObjectId):
57 ids.append(raw)
58 elif ObjectId.is_valid(str(raw)):
59 ids.append(ObjectId(str(raw)))
60 return ids
63async def fetch_questions_by_ids(question_ids) -> dict:
64 """Map ``str(question id) -> question document``, across all three banks.
66 Ids that exist in none of them are simply absent from the map: a question a
67 teacher deleted must not break the whole assignment view.
68 """
69 if not question_ids:
70 return {}
72 if len(question_ids) > MAX_QUESTION_FETCH:
73 print(
74 f"[question_bank] question_ids count {len(question_ids)} exceeds cap "
75 f"{MAX_QUESTION_FETCH}; truncating cross-bank fetch."
76 )
77 question_ids = question_ids[:MAX_QUESTION_FETCH]
79 question_map = {}
81 results = (
82 await db["teacher_questionbank"]
83 .aggregate(
84 [
85 {"$match": {"_id": {"$in": question_ids}}},
86 {"$addFields": {"_id_str": {"$toString": "$_id"}}},
87 ]
88 )
89 .to_list(length=len(question_ids))
90 )
91 for q in results:
92 question_map[str(q["_id"])] = q
94 missing = [qid for qid in question_ids if str(qid) not in question_map]
96 # The GLOBAL bank lives in the admin-staff database, not this service's own.
97 if missing and staff_admin_db is not None:
98 global_results = (
99 await staff_admin_db["global_questionbank"]
100 .aggregate(
101 [
102 {"$match": {"_id": {"$in": missing}}},
103 {"$addFields": {"_id_str": {"$toString": "$_id"}}},
104 ]
105 )
106 .to_list(length=len(missing))
107 )
108 for q in global_results:
109 question_map[str(q["_id"])] = q
110 missing = [qid for qid in question_ids if str(qid) not in question_map]
112 if missing:
113 legacy_results = (
114 await db["question_collection"]
115 .aggregate(
116 [
117 {"$match": {"_id": {"$in": missing}}},
118 {"$addFields": {"_id_str": {"$toString": "$_id"}}},
119 ]
120 )
121 .to_list(length=len(missing))
122 )
123 for q in legacy_results:
124 question_map[str(q["_id"])] = q
126 return question_map
129def project_question(document: dict, points_override=None) -> dict:
130 """One question document, shaped for an assignment response.
132 The same field set the student assignment-taking path returns
133 (``StudentAssignmentsService._prepare_questions``), minus the shuffling,
134 which belongs to taking an attempt rather than viewing an assignment.
136 `points_override` (from the assignment's `question_points_overrides`) wins
137 over the bank document's own `points`, so a teacher's per-assignment point
138 edit is reflected without mutating the shared question document.
139 """
140 return {
141 "_id": document["_id"],
142 "points": (
143 points_override if points_override is not None else document.get("points")
144 ),
145 "choices": document.get("choices"),
146 "question": document.get("question"),
147 "questionType": document.get("questionType"),
148 "correctAnswer": document.get("correctAnswer"),
149 "category": document.get("category"),
150 "groups": document.get("groups"),
151 "rows": document.get("rows"),
152 "rowHeaderLabel": document.get("rowHeaderLabel"),
153 }
156async def resolve_assignment_questions(assignment) -> list:
157 """An assignment's questions, resolved and projected, in the assignment's order.
159 Ids missing from every bank are skipped rather than raising — one deleted
160 question must not take the whole response down with it.
161 """
162 ids = question_object_ids(assignment)
163 question_map = await fetch_questions_by_ids(ids)
165 overrides = (
166 assignment.get("question_points_overrides")
167 if isinstance(assignment, dict)
168 else getattr(assignment, "question_points_overrides", None)
169 ) or {}
171 resolved = []
172 for qid in ids:
173 document = question_map.get(str(qid))
174 if document is None:
175 continue
176 resolved.append(project_question(document, overrides.get(str(qid))))
177 return resolved
180def id_match_forms(raw_id) -> list:
181 """The same id in both shapes Mongo may be holding it in.
183 Ownership columns here are inconsistent by history — `createdBy` on questions
184 and `created_by` on assignments are a string on most rows and an ObjectId on
185 thousands of older ones. A filter that matches only one shape silently misses
186 the other.
187 """
188 forms = [str(raw_id)]
189 if ObjectId.is_valid(str(raw_id)):
190 forms.append(ObjectId(str(raw_id)))
191 return forms
194def _json_safe(document: dict) -> dict:
195 """A question document FastAPI can actually serialise.
197 Added by Allan Ninal — 2026-09-23 (EI-T115)
198 WHAT: run the raw Mongo document through the house JSONEncoder before it
199 leaves this module.
200 WHY: a real question carries `_id` and `createdBy` as **ObjectId** and
201 `createdDate` as a datetime. The adaptive routes return the document
202 straight to the client, and FastAPI's jsonable_encoder cannot handle a
203 bare ObjectId — it died with
204 `TypeError: 'ObjectId' object is not iterable` AFTER the picker had
205 correctly found a question, so the endpoint still answered 500.
206 Doing it here covers both service copies and any future caller.
208 My own picker tests missed this because they used simplified fixtures
209 with no ObjectId fields; test_adaptive_question_picker.py now builds a
210 document with the real field types.
211 """
212 return json.loads(model_parser.JSONEncoder().encode(document))
215async def pick_adaptive_question(
216 *, difficulty: str, classification: str, exclude_ids: list, creator_id
217) -> dict | None:
218 """One unseen question at `difficulty` and `classification`, or None.
220 WHERE IT LOOKS, and why only there
221 ----------------------------------
222 1. The assignment CREATOR's own questions in `teacher_questionbank`.
223 2. The curated `global_questionbank` (in the admin-staff database), which is
224 shared by design.
226 It never reaches another teacher's private questions. That bank holds 40,279
227 rows from **1,166 distinct authors**; sampling it unscoped would hand one
228 teacher's material to another teacher's students. It also never returns a
229 soft-deleted question — 18,646 of those rows are `deleted: true`, and the
230 previous picker filtered neither.
232 The old implementation read `db["question_collection"]` matching a
233 `classification` field. That collection exists in NO database on this
234 cluster, and no question document carries a `classification` field, so it
235 could never return anything — the endpoint answered 400 "Something wrong
236 fetching a new question." for every input that got that far (and 500 before
237 PR #345). The live banks carry `assignmentType` (STAAR / SAT / TSI / ACT /
238 TEACHER TEST / Quiz / Homework), which is what the route's
239 `question_classification` parameter actually selects on.
241 `difficulty` arrives lowercase from the ladder ("easy"/"average"/"advance")
242 and is title-cased to match the stored values.
244 Developer: Allan Ninal — 2026-09-23 (EI-T115)
245 """
246 match = {
247 "difficulty": (difficulty or "").title(),
248 "assignmentType": classification,
249 "deleted": {"$ne": True},
250 }
251 if exclude_ids:
252 match["_id"] = {"$nin": list(exclude_ids)}
254 # 1. the creator's own bank
255 owned = await (
256 db["teacher_questionbank"]
257 .aggregate(
258 [
259 {"$match": {**match, "createdBy": {"$in": id_match_forms(creator_id)}}},
260 {"$sample": {"size": 1}},
261 ]
262 )
263 .to_list(length=1)
264 )
265 if owned:
266 return _json_safe(owned[0])
268 # 2. the curated global bank, shared by design
269 if staff_admin_db is not None:
270 shared = await (
271 staff_admin_db["global_questionbank"]
272 .aggregate([{"$match": match}, {"$sample": {"size": 1}}])
273 .to_list(length=1)
274 )
275 if shared:
276 return _json_safe(shared[0])
278 return None