diff --git a/backend/routes/calendar.py b/backend/routes/calendar.py index d14f6136..494eaa94 100644 --- a/backend/routes/calendar.py +++ b/backend/routes/calendar.py @@ -18,6 +18,7 @@ ) from db.connection import table from models import SaveAssignmentsBody, StudyBlockBody, ExportBody, SyncBody +from services import academics from services.auth_guard import require_self, get_session_user_id from services.calendar_service import extract_assignments_from_file, insert_new_assignments from services.encryption import encrypt, encrypt_if_present, decrypt, decrypt_if_present @@ -87,6 +88,62 @@ def _require_google_creds(user_id: str) -> "Credentials": return _get_refreshed_credentials(token_rows[0]) +def _course_meta_cached(offering_id, cache): + if not offering_id: + return {} + if offering_id not in cache: + course_id = academics.offering_course_id(offering_id) + course = {} + if course_id: + rows = table("courses").select( + "id,course_code,course_name", + filters={"id": f"eq.{course_id}"}, limit=1, + ) + course = rows[0] if rows else {} + cache[offering_id] = { + "course_id": course_id, + "course_code": course.get("course_code"), + "course_name": course.get("course_name"), + } + return cache[offering_id] + + +def _owned_enrollment_ids(user_id) -> set: + return {e["id"] for e in academics.user_enrollment_ids(user_id)} + + +def _read_assignments(user_id, *, due_gte=None, limit=None): + enrollments = academics.user_enrollment_ids(user_id) + if not enrollments: + return [] + offering_by_enrollment = {e["id"]: e.get("offering_id") for e in enrollments} + ids = ",".join(offering_by_enrollment.keys()) + filters = {"enrollment_id": f"in.({ids})"} + if due_gte: + filters["due_date"] = f"gte.{due_gte}" + rows = table("assignments").select( + "id,enrollment_id,title,due_date,assignment_type,notes,google_event_id,source", + filters=filters, order="due_date.asc", limit=limit, + ) + cache = {} + out = [] + for r in rows: + meta = _course_meta_cached(offering_by_enrollment.get(r.get("enrollment_id")), cache) + out.append({ + "id": r["id"], + "user_id": user_id, + "title": r["title"], + "due_date": r["due_date"], + "assignment_type": r.get("assignment_type"), + "notes": decrypt_if_present(r.get("notes")), + "google_event_id": r.get("google_event_id"), + "course_id": meta.get("course_id"), + "course_code": meta.get("course_code") or "", + "course_name": meta.get("course_name") or "", + }) + return out + + # ── Syllabus extraction ─────────────────────────────────────────────────────── @router.post("/extract") @@ -125,11 +182,11 @@ def save_assignments(body: SaveAssignmentsBody, request: FastAPIRequest): "course_id": a.course_id, "due_date": a.due_date, "assignment_type": a.assignment_type, - "notes": encrypt_if_present(a.notes), + "notes": a.notes, # raw; insert_new_assignments encrypts } for a in body.assignments ] - saved = insert_new_assignments(body.user_id, payload) + saved = insert_new_assignments(body.user_id, payload, source="manual") return {"saved_count": saved} @@ -137,55 +194,14 @@ def save_assignments(body: SaveAssignmentsBody, request: FastAPIRequest): def get_upcoming(user_id: str, request: FastAPIRequest): require_self(user_id, request) today = datetime.utcnow().strftime("%Y-%m-%d") - rows = table("assignments").select( - "id,user_id,title,due_date,assignment_type,notes,google_event_id,course_id,courses!left(course_code,course_name)", - filters={"user_id": f"eq.{user_id}", "due_date": f"gte.{today}"}, - order="due_date.asc", - limit=20, - ) - assignments = [] - for r in rows: - course = r.get("courses", {}) if isinstance(r.get("courses"), dict) else {} - assignments.append({ - "id": r["id"], - "user_id": r["user_id"], - "title": r["title"], - "due_date": r["due_date"], - "assignment_type": r.get("assignment_type"), - "notes": decrypt_if_present(r.get("notes")), - "google_event_id": r.get("google_event_id"), - "course_id": r.get("course_id"), - "course_code": course.get("course_code") or "", - "course_name": course.get("course_name") or "", - }) - return {"assignments": assignments} + return {"assignments": _read_assignments(user_id, due_gte=today, limit=20)} @router.get("/all/{user_id}") def get_all_assignments(user_id: str, request: FastAPIRequest): """Return all assignments for a user (past and future) for the calendar view.""" require_self(user_id, request) - rows = table("assignments").select( - "id,user_id,title,due_date,assignment_type,notes,google_event_id,course_id,courses!left(course_code,course_name)", - filters={"user_id": f"eq.{user_id}"}, - order="due_date.asc", - ) - assignments = [] - for r in rows: - course = r.get("courses", {}) if isinstance(r.get("courses"), dict) else {} - assignments.append({ - "id": r["id"], - "user_id": r["user_id"], - "title": r["title"], - "due_date": r["due_date"], - "assignment_type": r.get("assignment_type"), - "notes": decrypt_if_present(r.get("notes")), - "google_event_id": r.get("google_event_id"), - "course_id": r.get("course_id"), - "course_code": course.get("course_code") or "", - "course_name": course.get("course_name") or "", - }) - return {"assignments": assignments} + return {"assignments": _read_assignments(user_id)} @router.patch("/assignments/{assignment_id}") @@ -195,25 +211,23 @@ def update_assignment(assignment_id: str, body: dict, request: FastAPIRequest): raise HTTPException(status_code=400, detail="user_id is required") require_self(user_id, request) + owned = _owned_enrollment_ids(user_id) + if not owned: + raise HTTPException(status_code=404, detail="Assignment not found") existing = table("assignments").select( - "id", filters={"id": f"eq.{assignment_id}", "user_id": f"eq.{user_id}"}, limit=1, + "id", + filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"}, + limit=1, ) if not existing: raise HTTPException(status_code=404, detail="Assignment not found") - # Whitelist non-sensitive fields. `notes` is excluded; edit notes via /save flow. - ALLOWED = {"title", "course_id", "due_date", "assignment_type"} + ALLOWED = {"title", "due_date", "assignment_type"} # course_id no longer settable here patch = {k: v for k, v in body.items() if k in ALLOWED} if not patch: return {"updated": False} - - if "course_id" in patch and patch["course_id"] == "": - patch["course_id"] = None - - # Scope the write by user_id too (defense in depth): the scoped SELECT above - # already 404s a non-owned id, but don't rely on that guard alone (#123). table("assignments").update( - patch, filters={"id": f"eq.{assignment_id}", "user_id": f"eq.{user_id}"} + patch, filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"} ) return {"updated": True} @@ -221,14 +235,18 @@ def update_assignment(assignment_id: str, body: dict, request: FastAPIRequest): @router.delete("/assignments/{assignment_id}") def delete_assignment(assignment_id: str, request: FastAPIRequest, user_id: str = Query(...)): require_self(user_id, request) + owned = _owned_enrollment_ids(user_id) + if not owned: + raise HTTPException(status_code=404, detail="Assignment not found") existing = table("assignments").select( - "id", filters={"id": f"eq.{assignment_id}", "user_id": f"eq.{user_id}"}, limit=1, + "id", + filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"}, + limit=1, ) if not existing: raise HTTPException(status_code=404, detail="Assignment not found") - # Scope the delete by user_id too (defense in depth), not just the guard above. table("assignments").delete( - filters={"id": f"eq.{assignment_id}", "user_id": f"eq.{user_id}"} + filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"} ) return {"deleted": True} @@ -237,16 +255,11 @@ def delete_assignment(assignment_id: str, request: FastAPIRequest, user_id: str def suggest_study_blocks(body: StudyBlockBody, request: FastAPIRequest): require_self(body.user_id, request) today = datetime.utcnow().strftime("%Y-%m-%d") - assignments = table("assignments").select( - "id,title,due_date,courses!left(course_code,course_name)", - filters={"user_id": f"eq.{body.user_id}", "due_date": f"gte.{today}"}, - order="due_date.asc", - ) + assignments = _read_assignments(body.user_id, due_gte=today) blocks = [] for a in assignments: - course = a.get("courses", {}) if isinstance(a.get("courses"), dict) else {} - cc = course.get("course_code") or "" - cn = course.get("course_name") or "" + cc = a.get("course_code") or "" + cn = a.get("course_name") or "" course_label = f"[{cc}] " if cc else (f"{cn}: " if cn else "") blocks.append({ "topic": f"{course_label}{a['title']}" if course_label else a["title"], @@ -330,32 +343,29 @@ def sync_to_google(body: SyncBody, request: FastAPIRequest): creds = _require_google_creds(body.user_id) service = build("calendar", "v3", credentials=creds) + owned = _owned_enrollment_ids(body.user_id) + if not owned: + return {"synced_count": 0} + in_clause = f"in.({','.join(owned)})" unsynced = table("assignments").select( - "id,title,due_date,notes,google_event_id,courses!left(course_code,course_name)", - filters={ - "user_id": f"eq.{body.user_id}", - "google_event_id": "is.null", - }, + "id,enrollment_id,title,due_date,notes,google_event_id", + filters={"enrollment_id": in_clause, "google_event_id": "is.null"}, ) - # Also catch empty-string google_event_id unsynced += table("assignments").select( - "id,title,due_date,notes,google_event_id,courses!left(course_code,course_name)", - filters={ - "user_id": f"eq.{body.user_id}", - "google_event_id": "eq.", - }, + "id,enrollment_id,title,due_date,notes,google_event_id", + filters={"enrollment_id": in_clause, "google_event_id": "eq."}, ) + enr_to_offering = {e["id"]: e.get("offering_id") for e in academics.user_enrollment_ids(body.user_id)} + cache = {} synced = 0 for a in unsynced: if not a.get("due_date"): continue - - course = a.get("courses", {}) if isinstance(a.get("courses"), dict) else {} - course_code = course.get("course_code") or "" - course_name = course.get("course_name") or "" - course_label = f"[{course_code}] " if course_code else (f"{course_name}: " if course_name else "") - + meta = _course_meta_cached(enr_to_offering.get(a.get("enrollment_id")), cache) + cc = meta.get("course_code") or "" + cn = meta.get("course_name") or "" + course_label = f"[{cc}] " if cc else (f"{cn}: " if cn else "") event = { "summary": f"{course_label}{a['title']}" if course_label else a["title"], "description": decrypt_if_present(a.get("notes")) or "", @@ -363,10 +373,9 @@ def sync_to_google(body: SyncBody, request: FastAPIRequest): "end": {"date": a["due_date"]}, } created = service.events().insert(calendarId="primary", body=event).execute() - # Scope the write-back by user_id too (defense in depth), matching export. table("assignments").update( {"google_event_id": created["id"]}, - filters={"id": f"eq.{a['id']}", "user_id": f"eq.{body.user_id}"}, + filters={"id": f"eq.{a['id']}", "enrollment_id": in_clause}, ) synced += 1 @@ -381,18 +390,24 @@ def export_to_google(body: ExportBody, request: FastAPIRequest): creds = _require_google_creds(body.user_id) service = build("calendar", "v3", credentials=creds) + owned = _owned_enrollment_ids(body.user_id) + enr_to_offering = {e["id"]: e.get("offering_id") for e in academics.user_enrollment_ids(body.user_id)} + in_clause = f"in.({','.join(owned)})" if owned else "in.()" + cache = {} + exported = 0 skipped = 0 for aid in body.assignment_ids: - # #123: scope by user_id, not just id. Without this an authenticated - # caller could pass another user's assignment UUIDs to read+decrypt - # their private notes, push them into the caller's calendar, and stamp - # google_event_id onto the victim's row. Every sibling endpoint - # (update/delete/sync) already scopes by user_id; a non-owned id now - # returns no row and is skipped. + # #123: scope by enrollment_id membership, not just id. Without this an + # authenticated caller could pass another user's assignment UUIDs to + # read+decrypt their private notes, push them into the caller's calendar, + # and stamp google_event_id onto the victim's row. Scoping to the caller's + # own enrollment ids means a non-owned id returns no row and is skipped. + if not owned: + continue rows = table("assignments").select( - "id,title,due_date,notes,google_event_id,courses!left(course_code,course_name)", - filters={"id": f"eq.{aid}", "user_id": f"eq.{body.user_id}"}, + "id,enrollment_id,title,due_date,notes,google_event_id", + filters={"id": f"eq.{aid}", "enrollment_id": in_clause}, ) if not rows: continue @@ -402,10 +417,10 @@ def export_to_google(body: ExportBody, request: FastAPIRequest): skipped += 1 continue - course = a.get("courses", {}) if isinstance(a.get("courses"), dict) else {} - course_code = course.get("course_code") or "" - course_name = course.get("course_name") or "" - course_label = f"[{course_code}] " if course_code else (f"{course_name}: " if course_name else "") + meta = _course_meta_cached(enr_to_offering.get(a.get("enrollment_id")), cache) + cc = meta.get("course_code") or "" + cn = meta.get("course_name") or "" + course_label = f"[{cc}] " if cc else (f"{cn}: " if cn else "") event = { "summary": f"{course_label}{a['title']}" if course_label else a["title"], @@ -414,11 +429,11 @@ def export_to_google(body: ExportBody, request: FastAPIRequest): "end": {"date": a["due_date"]}, } created = service.events().insert(calendarId="primary", body=event).execute() - # Scope the write-back by user_id too (defense in depth): never stamp - # google_event_id onto a row the caller doesn't own. + # Scope the write-back by enrollment_id membership (defense in depth): + # never stamp google_event_id onto a row the caller doesn't own. table("assignments").update( {"google_event_id": created["id"]}, - filters={"id": f"eq.{aid}", "user_id": f"eq.{body.user_id}"}, + filters={"id": f"eq.{aid}", "enrollment_id": in_clause}, ) exported += 1 diff --git a/backend/routes/documents.py b/backend/routes/documents.py index 24e80653..807e3b44 100644 --- a/backend/routes/documents.py +++ b/backend/routes/documents.py @@ -472,7 +472,7 @@ def _save_orchestrator_syllabus(*, user_id: str, course_id: str, filename: str, }) if legacy: try: - save_assignments_to_db(user_id, legacy) + save_assignments_to_db(user_id, legacy, source="syllabus") except Exception: logger.exception("Assignment save failed for '%s' (best-effort)", filename) @@ -985,7 +985,7 @@ async def _legacy_upload_pipeline( try: for a in ai["assignments"]: a["course_id"] = course_id - save_assignments_to_db(user_id, ai["assignments"]) + save_assignments_to_db(user_id, ai["assignments"], source="syllabus") except Exception: logger.exception("Assignment save failed for '%s' (best-effort)", filename) diff --git a/backend/services/academics.py b/backend/services/academics.py index 8a91c1b0..c4f08a6f 100644 --- a/backend/services/academics.py +++ b/backend/services/academics.py @@ -135,3 +135,61 @@ def term_for_offering(offering_id: str) -> dict | None: return None terms = table("terms").select("*", filters={"id": f"eq.{term_id}"}, limit=1) return terms[0] if terms else None + + +def user_enrollment_ids(user_id: str) -> list[dict]: + """The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper).""" + if not user_id: + return [] + return table("enrollments").select( + "id,offering_id", filters={"user_id": f"eq.{user_id}"} + ) or [] + + +def enrollment_id_for(user_id: str, course_id: str, *, create: bool = False) -> str | None: + """Resolve (user, abstract course) → the user's current-term enrollment id. + + Prefer the user's enrollment in the course's current-term offering, else + their only offering of the course. With ``create=True``, ensure an offering + (current term) and an enrollment row exist so a write never silently drops. + """ + if not user_id or not course_id: + return None + + offering_ids = user_offering_ids_for_course(user_id, course_id) + if offering_ids: + chosen = offering_ids[0] + cur = current_term() + cur_id = cur["id"] if cur else None + if cur_id: + for oid in offering_ids: + t = term_for_offering(oid) + if t and t.get("id") == cur_id: + chosen = oid + break + rows = table("enrollments").select( + "id", + filters={"user_id": f"eq.{user_id}", "offering_id": f"eq.{chosen}"}, + limit=1, + ) + if rows: + return rows[0]["id"] + + if not create: + return None + + offering_id = resolve_offering(course_id, create=True) + if not offering_id: + return None + existing = table("enrollments").select( + "id", + filters={"user_id": f"eq.{user_id}", "offering_id": f"eq.{offering_id}"}, + limit=1, + ) + if existing: + return existing[0]["id"] + new_id = str(uuid.uuid4()) + table("enrollments").insert( + {"id": new_id, "user_id": user_id, "offering_id": offering_id} + ) + return new_id diff --git a/backend/services/calendar_service.py b/backend/services/calendar_service.py index a62be325..5836a35d 100644 --- a/backend/services/calendar_service.py +++ b/backend/services/calendar_service.py @@ -20,21 +20,23 @@ PROMPT_PATH = os.path.join(os.path.dirname(os.path.dirname(__file__)), "prompts", "syllabus_extraction.txt") -def load_existing_assignment_keys(user_id: str) -> set[tuple[str, str]]: - """All (title, due_date) keys for a user, using the same normalization as assignment_dedupe_key.""" +def load_existing_assignment_keys(user_id: str) -> set: + from services.academics import user_enrollment_ids + enrollments = user_enrollment_ids(user_id) + if not enrollments: + return set() + ids = ",".join(e["id"] for e in enrollments) existing_rows = table("assignments").select( - "title,due_date", - filters={"user_id": f"eq.{user_id}"}, + "title,due_date", filters={"enrollment_id": f"in.({ids})"}, ) return {assignment_dedupe_key(r.get("title"), r.get("due_date")) for r in (existing_rows or [])} -def insert_new_assignments(user_id: str, assignments: list[dict]) -> int: - """ - Insert assignments that are not already present for this user (#16). - Same trimmed title + same calendar day (see assignment_dedupe_key) → skip. - Returns number of rows inserted. - """ +def insert_new_assignments(user_id: str, assignments: list[dict], *, source: str = "manual") -> int: + """Insert assignments (deduped per the user's enrollment set, #16) on the + enrollment-keyed schema. Each assignment must carry a ``course_id`` — it is + resolved to the user's enrollment (created if missing). Returns rows inserted.""" + from services.academics import enrollment_id_for existing_keys = load_existing_assignment_keys(user_id) rows = [] for a in assignments: @@ -45,20 +47,19 @@ def insert_new_assignments(user_id: str, assignments: list[dict]) -> int: key = assignment_dedupe_key(title, due_raw) if key in existing_keys: continue + course_id = a.get("course_id") + enrollment_id = enrollment_id_for(user_id, course_id, create=True) if course_id else None + if not enrollment_id: + continue # decision: every assignment is course-tied existing_keys.add(key) rows.append({ "id": str(uuid.uuid4()), - "user_id": user_id, + "enrollment_id": enrollment_id, "title": title, - "course_id": a.get("course_id") or None, "due_date": key[1], "assignment_type": a.get("assignment_type") or "other", - # #126: encrypt at the write boundary. assignments.notes is an - # encrypted column; every other writer (calendar.py, gradebook.py) - # encrypts. Syllabus-extracted notes were being persisted as - # plaintext, defeating column encryption and spamming decrypt - # fallback warnings on read. - "notes": encrypt_if_present(a.get("notes")), + "notes": encrypt_if_present(a.get("notes")), # #126: encrypt at write + "source": source, }) if rows: table("assignments").insert(rows) @@ -108,9 +109,9 @@ async def _extract_via_agent( return syllabus_to_wire_dict(result.output, raw_text=extracted_text) -def save_assignments_to_db(user_id: str, assignments: list) -> int: - """Write extracted assignment dicts to the DB (deduped via insert_new_assignments).""" - return insert_new_assignments(user_id, assignments) +def save_assignments_to_db(user_id: str, assignments: list, *, source: str = "syllabus") -> int: + """Write extracted assignment dicts (deduped via insert_new_assignments).""" + return insert_new_assignments(user_id, assignments, source=source) async def extract_assignments_from_file( diff --git a/backend/tests/test_academics_enrollment_resolver.py b/backend/tests/test_academics_enrollment_resolver.py new file mode 100644 index 00000000..ae3b78ed --- /dev/null +++ b/backend/tests/test_academics_enrollment_resolver.py @@ -0,0 +1,49 @@ +# tests/test_academics_enrollment_resolver.py +from unittest.mock import MagicMock, patch +import services.academics as ac + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestUserEnrollmentIds: + def test_returns_rows(self): + with patch("services.academics.table", side_effect=_dispatch({ + "enrollments": _tbl(select=[{"id": "e1", "offering_id": "o1"}]), + })): + assert ac.user_enrollment_ids("user_andres") == [{"id": "e1", "offering_id": "o1"}] + + def test_empty_user(self): + assert ac.user_enrollment_ids("") == [] + +class TestEnrollmentIdFor: + def test_existing_enrollment_current_term(self): + # user_offering_ids_for_course -> ["o1"]; term match; enrollment e1 + tables = { + "course_offerings": _tbl(select=[{"id": "o1"}]), + "enrollments": _tbl(select=[{"id": "e1"}]), + } + with patch("services.academics.table", side_effect=_dispatch(tables)), \ + patch("services.academics.user_offering_ids_for_course", return_value=["o1"]), \ + patch("services.academics.current_term", return_value=None): + assert ac.enrollment_id_for("user_andres", "CS101") == "e1" + + def test_create_when_missing(self): + with patch("services.academics.user_offering_ids_for_course", return_value=[]), \ + patch("services.academics.resolve_offering", return_value="o9"), \ + patch("services.academics.table", side_effect=_dispatch({ + "enrollments": _tbl(select=[], insert=[]), + })): + eid = ac.enrollment_id_for("user_andres", "CS101", create=True) + assert isinstance(eid, str) and eid + + def test_missing_no_create_returns_none(self): + with patch("services.academics.user_offering_ids_for_course", return_value=[]): + assert ac.enrollment_id_for("user_andres", "CS101", create=False) is None diff --git a/backend/tests/test_assignment_notes_encryption.py b/backend/tests/test_assignment_notes_encryption.py index b476384d..a196a770 100644 --- a/backend/tests/test_assignment_notes_encryption.py +++ b/backend/tests/test_assignment_notes_encryption.py @@ -6,6 +6,10 @@ This is an encryption-boundary correctness test (not a cross-user-access test): it asserts the value handed to the DB insert is ciphertext, and that it round-trips back to the original plaintext via decrypt. + +Assignments are now enrollment-keyed (no user_id/course_id columns). Each +assignment must carry a course_id so insert_new_assignments can resolve the +enrollment; tests mock enrollment_id_for and user_enrollment_ids accordingly. """ from unittest.mock import patch @@ -23,14 +27,16 @@ def _insert(rows): captured["rows"] = rows return rows - with patch("services.calendar_service.table") as t: + with patch("services.calendar_service.table") as t, \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): # No existing rows → nothing deduped; capture what gets inserted. t.return_value.select.return_value = [] t.return_value.insert.side_effect = _insert n = insert_new_assignments( "user_andres", - [{"title": "Midterm", "due_date": "2026-03-01", "notes": PLAINTEXT_NOTES}], + [{"title": "Midterm", "due_date": "2026-03-01", "notes": PLAINTEXT_NOTES, "course_id": "CS101"}], ) assert n == 1 @@ -43,11 +49,13 @@ def _insert(rows): def test_none_notes_stay_none(self): captured = {} - with patch("services.calendar_service.table") as t: + with patch("services.calendar_service.table") as t, \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): t.return_value.select.return_value = [] t.return_value.insert.side_effect = lambda rows: captured.setdefault("rows", rows) insert_new_assignments( "user_andres", - [{"title": "Reading", "due_date": "2026-03-02", "notes": None}], + [{"title": "Reading", "due_date": "2026-03-02", "notes": None, "course_id": "CS101"}], ) assert captured["rows"][0]["notes"] is None diff --git a/backend/tests/test_calendar_export_idor.py b/backend/tests/test_calendar_export_idor.py index 328fcba5..89801eee 100644 --- a/backend/tests/test_calendar_export_idor.py +++ b/backend/tests/test_calendar_export_idor.py @@ -5,12 +5,15 @@ authenticated user could pass ANOTHER user's assignment UUIDs to read+decrypt their private notes, push them into the caller's Google Calendar, and stamp google_event_id onto the victim's row. The fix scopes the select (and the -write-back) by user_id, so a non-owned id returns no row and is skipped. +write-back) by enrollment_id membership, so a non-owned id returns no row and +is skipped. The `user_id` column no longer exists on the `assignments` table; +the new security boundary is the caller's own enrollment ids. The cross-user test makes the DB mock behave like a real row-scoped store: the -victim's row is only returned when the query is NOT scoped to the caller -(pre-fix) or is scoped to the victim. The fix queries scoped to the caller, so -the victim row is never returned, never decrypted, never pushed. +victim's row is only returned when the query is NOT scoped to the caller's +enrollment ids (pre-fix) or is scoped to the victim's enrollment id. The fix +queries scoped to the caller's owned enrollment ids, so the victim row is never +returned, never decrypted, never pushed. """ from unittest.mock import MagicMock, patch @@ -23,24 +26,26 @@ ATTACKER = "user_attacker" VICTIM = "user_victim" VICTIM_ASSIGNMENT_ID = "assignment_owned_by_victim" +VICTIM_ENROLLMENT_ID = "enr_victim" +ATTACKER_ENROLLMENT_ID = "enr_attacker" VICTIM_ROW = { "id": VICTIM_ASSIGNMENT_ID, + "enrollment_id": VICTIM_ENROLLMENT_ID, "title": "Victim private assignment", "due_date": "2026-03-01", "notes": "ENC_secret_victim_notes", "google_event_id": None, - "courses": {"course_code": "BIO101", "course_name": "Biology"}, } def _row_scoped_select(*_args, **kwargs): - """Simulate a row-scoped table: return the victim row only for an unscoped - query (pre-fix) or one scoped to the victim — never for one scoped to the - attacker.""" + """Simulate a row-scoped table: return the victim row only when the + enrollment_id filter includes the victim's enrollment — never when it's + scoped to the attacker's enrollment.""" filters = kwargs.get("filters", {}) - uid = filters.get("user_id") - if uid is None or uid == f"eq.{VICTIM}": + enr_filter = filters.get("enrollment_id", "") + if not enr_filter or VICTIM_ENROLLMENT_ID in enr_filter: return [VICTIM_ROW] return [] @@ -50,11 +55,16 @@ def test_cannot_export_another_users_assignment(self): with patch("routes.calendar._require_google_creds", return_value=MagicMock()), \ patch("routes.calendar.build") as build, \ patch("routes.calendar.decrypt_if_present") as decrypt, \ - patch("routes.calendar.table") as t: + patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: service = MagicMock() service.events.return_value.insert.return_value.execute.return_value = {"id": "evt_new"} build.return_value = service t.return_value.select.side_effect = _row_scoped_select + # Attacker only owns their own enrollment, NOT the victim's. + ac.user_enrollment_ids.return_value = [ + {"id": ATTACKER_ENROLLMENT_ID, "offering_id": "o1"} + ] # Attacker authenticates as themselves but targets the victim's id. r = client.post( @@ -63,7 +73,7 @@ def test_cannot_export_another_users_assignment(self): ) assert r.status_code == 200 - # Nothing exported: the user_id-scoped query found no row → skipped. + # Nothing exported: the enrollment-scoped query found no row → skipped. assert r.json()["exported_count"] == 0 # The victim's notes were never decrypted and never pushed to a calendar. decrypt.assert_not_called() @@ -72,21 +82,25 @@ def test_cannot_export_another_users_assignment(self): t.return_value.update.assert_not_called() def test_owner_can_export_their_own_assignment(self): - """Control: scoping by user_id must not break the legitimate path.""" - own_row = {**VICTIM_ROW, "id": "my_assignment"} + """Control: scoping by enrollment_id must not break the legitimate path.""" + own_row = {**VICTIM_ROW, "id": "my_assignment", "enrollment_id": VICTIM_ENROLLMENT_ID} def _select(*_args, **kwargs): - uid = kwargs.get("filters", {}).get("user_id") - return [own_row] if uid == f"eq.{VICTIM}" else [] + enr_filter = kwargs.get("filters", {}).get("enrollment_id", "") + return [own_row] if VICTIM_ENROLLMENT_ID in enr_filter else [] with patch("routes.calendar._require_google_creds", return_value=MagicMock()), \ patch("routes.calendar.build") as build, \ patch("routes.calendar.decrypt_if_present", return_value="secret"), \ - patch("routes.calendar.table") as t: + patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: service = MagicMock() service.events.return_value.insert.return_value.execute.return_value = {"id": "evt_new"} build.return_value = service t.return_value.select.side_effect = _select + ac.user_enrollment_ids.return_value = [ + {"id": VICTIM_ENROLLMENT_ID, "offering_id": "o1"} + ] r = client.post( "/api/calendar/export", diff --git a/backend/tests/test_calendar_read_enrollment.py b/backend/tests/test_calendar_read_enrollment.py new file mode 100644 index 00000000..804be3c5 --- /dev/null +++ b/backend/tests/test_calendar_read_enrollment.py @@ -0,0 +1,47 @@ +# tests/test_calendar_read_enrollment.py +from unittest.mock import MagicMock, patch +from fastapi.testclient import TestClient +from main import app + +client = TestClient(app) + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestUpcomingEnrollmentKeyed: + def test_empty_when_no_enrollments(self): + with patch("routes.calendar.table", side_effect=_dispatch({"enrollments": _tbl(select=[])})), \ + patch("services.academics.table", side_effect=_dispatch({"enrollments": _tbl(select=[])})): + r = client.get("/api/calendar/upcoming/user_andres") + assert r.status_code == 200 + assert r.json() == {"assignments": []} + + def test_decorates_with_course_meta(self): + tables = { + "enrollments": _tbl(select=[{"id": "e1", "offering_id": "o1"}]), + "assignments": _tbl(select=[{ + "id": "a1", "enrollment_id": "e1", "title": "HW1", + "due_date": "2999-01-01", "assignment_type": "homework", + "notes": None, "google_event_id": None, "source": "manual", + }]), + "courses": _tbl(select=[{"id": "CS101", "course_code": "CS101", "course_name": "Intro"}]), + } + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + ac.offering_course_id.return_value = "CS101" + r = client.get("/api/calendar/upcoming/user_andres") + assert r.status_code == 200 + items = r.json()["assignments"] + assert len(items) == 1 + assert items[0]["course_code"] == "CS101" + assert items[0]["course_id"] == "CS101" + assert items[0]["user_id"] == "user_andres" diff --git a/backend/tests/test_calendar_routes.py b/backend/tests/test_calendar_routes.py index a3073009..29d40cd6 100644 --- a/backend/tests/test_calendar_routes.py +++ b/backend/tests/test_calendar_routes.py @@ -13,6 +13,14 @@ client = TestClient(app) +def _tbl(**rows_by_verb): + """Build a MagicMock table handle with canned per-verb return values.""" + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + + # ── GET /api/calendar/status/{user_id} ─────────────────────────────────────── class TestCalendarStatus: @@ -45,14 +53,16 @@ def test_connected_when_valid_token_exists(self): class TestSaveAssignments: def test_saves_multiple_assignments(self): - with patch("services.calendar_service.table") as t: + with patch("services.calendar_service.table") as t, \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): t.return_value.select.return_value = [] t.return_value.insert.return_value = [] body = { "user_id": "user_andres", "assignments": [ - {"title": "HW1", "due_date": "2026-03-01", "assignment_type": "homework"}, - {"title": "Quiz 1", "due_date": "2026-03-10", "assignment_type": "quiz"}, + {"title": "HW1", "due_date": "2026-03-01", "assignment_type": "homework", "course_id": "CS101"}, + {"title": "Quiz 1", "due_date": "2026-03-10", "assignment_type": "quiz", "course_id": "CS101"}, ], } r = client.post("/api/calendar/save", json=body) @@ -68,19 +78,23 @@ def test_save_empty_list_returns_zero(self): assert r.json()["saved_count"] == 0 def test_save_with_optional_fields_omitted(self): - with patch("services.calendar_service.table") as t: + with patch("services.calendar_service.table") as t, \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): t.return_value.select.return_value = [] t.return_value.insert.return_value = [] body = { "user_id": "user_andres", - "assignments": [{"title": "Midterm", "due_date": "2026-04-01"}], + "assignments": [{"title": "Midterm", "due_date": "2026-04-01", "course_id": "CS101"}], } r = client.post("/api/calendar/save", json=body) assert r.status_code == 200 assert r.json()["saved_count"] == 1 def test_save_skips_duplicate_title_and_date(self): - with patch("services.calendar_service.table") as t: + with patch("services.calendar_service.table") as t, \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): t.return_value.select.return_value = [ {"title": "HW1", "due_date": "2026-03-01"}, ] @@ -88,8 +102,8 @@ def test_save_skips_duplicate_title_and_date(self): body = { "user_id": "user_andres", "assignments": [ - {"title": "HW1", "due_date": "2026-03-01", "assignment_type": "homework"}, - {"title": "HW2", "due_date": "2026-03-02", "assignment_type": "homework"}, + {"title": "HW1", "due_date": "2026-03-01", "assignment_type": "homework", "course_id": "CS101"}, + {"title": "HW2", "due_date": "2026-03-02", "assignment_type": "homework", "course_id": "CS101"}, ], } r = client.post("/api/calendar/save", json=body) @@ -98,7 +112,9 @@ def test_save_skips_duplicate_title_and_date(self): def test_save_skips_when_iso_datetime_matches_existing_date(self): """#16: same title + same calendar day (ISO date vs datetime) → one row.""" - with patch("services.calendar_service.table") as t: + with patch("services.calendar_service.table") as t, \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): t.return_value.select.return_value = [ {"title": "Final Exam", "due_date": "2026-05-01"}, ] @@ -106,7 +122,7 @@ def test_save_skips_when_iso_datetime_matches_existing_date(self): body = { "user_id": "user_andres", "assignments": [ - {"title": "Final Exam", "due_date": "2026-05-01T09:00:00", "assignment_type": "exam"}, + {"title": "Final Exam", "due_date": "2026-05-01T09:00:00", "assignment_type": "exam", "course_id": "CS101"}, ], } r = client.post("/api/calendar/save", json=body) @@ -118,16 +134,20 @@ def test_save_skips_when_iso_datetime_matches_existing_date(self): class TestGetUpcoming: def test_returns_assignments_from_db(self): + # New enrollment-keyed schema: rows carry enrollment_id, not user_id/course_id. mock_rows = [ - {"id": "a1", "user_id": "user_andres", "title": "HW1", + {"id": "a1", "enrollment_id": "e1", "title": "HW1", "due_date": "2026-03-01", "assignment_type": "homework", - "notes": None, "google_event_id": None, "course_id": None, "courses": None}, - {"id": "a2", "user_id": "user_andres", "title": "Quiz", + "notes": None, "google_event_id": None, "source": None}, + {"id": "a2", "enrollment_id": "e1", "title": "Quiz", "due_date": "2026-03-10", "assignment_type": "quiz", - "notes": None, "google_event_id": None, "course_id": None, "courses": None}, + "notes": None, "google_event_id": None, "source": None}, ] - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: t.return_value.select.return_value = mock_rows + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + ac.offering_course_id.return_value = None # no course → empty strings r = client.get("/api/calendar/upcoming/user_andres") assert r.status_code == 200 @@ -139,7 +159,9 @@ def test_returns_assignments_from_db(self): assert assignments[0]["course_name"] == "" def test_returns_empty_list_when_none(self): - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [] t.return_value.select.return_value = [] r = client.get("/api/calendar/upcoming/user_andres") assert r.status_code == 200 @@ -151,10 +173,15 @@ def test_returns_empty_list_when_none(self): class TestSuggestStudyBlocks: def test_returns_at_most_5_blocks(self): many_assignments = [ - {"id": f"a{i}", "title": f"Task {i}", "due_date": f"2026-03-{i:02d}", "course_name": "CS"} + {"id": f"a{i}", "enrollment_id": "e1", "title": f"Task {i}", + "due_date": f"2026-03-{i:02d}", "assignment_type": None, + "notes": None, "google_event_id": None, "source": None} for i in range(1, 9) ] - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + ac.offering_course_id.return_value = None t.return_value.select.return_value = many_assignments r = client.post("/api/calendar/suggest-study-blocks", json={"user_id": "user_andres"}) @@ -162,8 +189,13 @@ def test_returns_at_most_5_blocks(self): assert len(r.json()["study_blocks"]) <= 5 def test_block_shape_is_correct(self): - assignments = [{"id": "a1", "title": "HW1", "due_date": "2026-03-01", "course_name": "Math"}] - with patch("routes.calendar.table") as t: + assignments = [{"id": "a1", "enrollment_id": "e1", "title": "HW1", + "due_date": "2026-03-01", "assignment_type": None, + "notes": None, "google_event_id": None, "source": None}] + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + ac.offering_course_id.return_value = None t.return_value.select.return_value = assignments r = client.post("/api/calendar/suggest-study-blocks", json={"user_id": "user_andres"}) @@ -174,7 +206,9 @@ def test_block_shape_is_correct(self): assert block["duration_minutes"] == 60 def test_empty_assignments_returns_empty_blocks(self): - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] t.return_value.select.return_value = [] r = client.post("/api/calendar/suggest-study-blocks", json={"user_id": "user_andres"}) assert r.json()["study_blocks"] == [] @@ -195,7 +229,9 @@ def test_deletes_oauth_token_and_returns_disconnected(self): class TestUpdateAssignment: def test_updates_whitelisted_fields(self): - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] t.return_value.select.return_value = [{"id": "a1"}] t.return_value.update.return_value = [{}] r = client.patch( @@ -210,7 +246,9 @@ def test_missing_user_id_returns_400(self): assert r.status_code == 400 def test_unknown_assignment_returns_404(self): - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] t.return_value.select.return_value = [] r = client.patch( "/api/calendar/assignments/missing", @@ -218,26 +256,23 @@ def test_unknown_assignment_returns_404(self): ) assert r.status_code == 404 - def test_empty_course_id_is_nulled(self): - captured = {} - def table_side_effect(name): - m = MagicMock() - m.select.return_value = [{"id": "a1"}] - def _update(patch, filters=None): - captured["patch"] = patch - return [{}] - m.update.side_effect = _update - return m - with patch("routes.calendar.table", side_effect=table_side_effect): + def test_course_id_no_longer_settable(self): + """course_id is derived from enrollment; patching it via PATCH is intentionally blocked.""" + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + t.return_value.select.return_value = [{"id": "a1"}] r = client.patch( "/api/calendar/assignments/a1", - json={"user_id": "u1", "course_id": ""}, + json={"user_id": "u1", "course_id": "some-course"}, ) assert r.status_code == 200 - assert captured["patch"]["course_id"] is None + assert r.json() == {"updated": False} def test_no_valid_fields_returns_updated_false(self): - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] t.return_value.select.return_value = [{"id": "a1"}] r = client.patch( "/api/calendar/assignments/a1", @@ -251,7 +286,9 @@ def test_no_valid_fields_returns_updated_false(self): class TestDeleteAssignment: def test_deletes_assignment(self): - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] t.return_value.select.return_value = [{"id": "a1"}] t.return_value.delete.return_value = [] r = client.delete("/api/calendar/assignments/a1?user_id=u1") @@ -259,7 +296,9 @@ def test_deletes_assignment(self): assert r.json() == {"deleted": True} def test_missing_returns_404(self): - with patch("routes.calendar.table") as t: + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] t.return_value.select.return_value = [] r = client.delete("/api/calendar/assignments/a1?user_id=u1") assert r.status_code == 404 diff --git a/backend/tests/test_calendar_scoping_enrollment.py b/backend/tests/test_calendar_scoping_enrollment.py new file mode 100644 index 00000000..9c40d99f --- /dev/null +++ b/backend/tests/test_calendar_scoping_enrollment.py @@ -0,0 +1,38 @@ +from unittest.mock import MagicMock, patch +from fastapi.testclient import TestClient +from main import app + +client = TestClient(app) + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestUpdateScoping: + def test_404_when_assignment_not_in_user_enrollments(self): + tables = { + "assignments": _tbl(select=[]), # no row owned by user's enrollments + } + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + r = client.patch("/api/calendar/assignments/a-other", + json={"user_id": "user_andres", "title": "x"}) + assert r.status_code == 404 + + def test_updates_owned_assignment(self): + tables = {"assignments": _tbl(select=[{"id": "a1"}], update=[])} + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + r = client.patch("/api/calendar/assignments/a1", + json={"user_id": "user_andres", "title": "new"}) + assert r.status_code == 200 + assert r.json() == {"updated": True} diff --git a/backend/tests/test_calendar_sibling_write_scoping.py b/backend/tests/test_calendar_sibling_write_scoping.py index a3b3dc8b..07ddcfca 100644 --- a/backend/tests/test_calendar_sibling_write_scoping.py +++ b/backend/tests/test_calendar_sibling_write_scoping.py @@ -1,10 +1,9 @@ """ Defense-in-depth follow-up to #123: the assignment mutation endpoints -(update / delete / sync) do a user_id-scoped SELECT and 404 a non-owned id -before writing, so they're not exploitable today — but their write filters -were id-only, relying solely on that guard. These tests assert the write/delete -now also scope by user_id, so a future change to the read-guard can't silently -reopen the IDOR. +(update / delete / sync) scope reads AND writes by enrollment_id membership, +so a caller cannot touch another user's assignments even if they know the UUID. +These tests assert both the SELECT guard and the write/delete filter scope by +enrollment_id (not user_id, which no longer exists on the assignments table). """ from unittest.mock import MagicMock, patch @@ -16,49 +15,64 @@ OWNER = "user_andres" AID = "assignment_1" +ENROLLMENT_ID = "e1" class TestSiblingWriteScoping: - def test_update_scopes_write_by_user_id(self): - with patch("routes.calendar.table") as t: + def test_update_scopes_write_by_enrollment_id(self): + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": ENROLLMENT_ID, "offering_id": "o1"}] t.return_value.select.return_value = [{"id": AID}] # owner's row exists r = client.patch( f"/api/calendar/assignments/{AID}", json={"user_id": OWNER, "title": "New title"}, ) assert r.status_code == 200 - # The UPDATE filter must include user_id, not just id. + # The UPDATE filter must scope by enrollment_id, not user_id. update_filters = t.return_value.update.call_args.kwargs["filters"] - assert update_filters.get("user_id") == f"eq.{OWNER}" + assert "enrollment_id" in update_filters + assert ENROLLMENT_ID in update_filters["enrollment_id"] assert update_filters.get("id") == f"eq.{AID}" - def test_delete_scopes_delete_by_user_id(self): - with patch("routes.calendar.table") as t: + def test_delete_scopes_delete_by_enrollment_id(self): + with patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": ENROLLMENT_ID, "offering_id": "o1"}] t.return_value.select.return_value = [{"id": AID}] r = client.delete(f"/api/calendar/assignments/{AID}?user_id={OWNER}") assert r.status_code == 200 delete_filters = t.return_value.delete.call_args.kwargs["filters"] - assert delete_filters.get("user_id") == f"eq.{OWNER}" + assert "enrollment_id" in delete_filters + assert ENROLLMENT_ID in delete_filters["enrollment_id"] assert delete_filters.get("id") == f"eq.{AID}" - def test_sync_scopes_writeback_by_user_id(self): + def test_sync_scopes_writeback_by_enrollment_id(self): unsynced = [{ - "id": AID, "title": "HW", "due_date": "2026-03-01", - "notes": None, "google_event_id": None, "courses": {}, + "id": AID, "enrollment_id": ENROLLMENT_ID, "title": "HW", + "due_date": "2026-03-01", "notes": None, "google_event_id": None, }] with patch("routes.calendar._require_google_creds", return_value=MagicMock()), \ patch("routes.calendar.build") as build, \ patch("routes.calendar.decrypt_if_present", return_value=""), \ - patch("routes.calendar.table") as t: + patch("routes.calendar.table") as t, \ + patch("routes.calendar.academics") as ac: service = MagicMock() service.events.return_value.insert.return_value.execute.return_value = {"id": "evt_1"} build.return_value = service + ac.user_enrollment_ids.return_value = [{"id": ENROLLMENT_ID, "offering_id": "o1"}] + # offering_course_id returns None so _course_meta_cached skips the + # courses table select (keeps select side_effect list simple). + ac.offering_course_id.return_value = None # select returns the unsynced row on the first call, [] thereafter. t.return_value.select.side_effect = [unsynced, []] r = client.post("/api/calendar/sync", json={"user_id": OWNER}) assert r.status_code == 200 update_filters = t.return_value.update.call_args.kwargs["filters"] - assert update_filters.get("user_id") == f"eq.{OWNER}" + # Write-back must scope by enrollment_id (not user_id, which no longer + # exists on the assignments table) — same IDOR guarantee, new key. + assert "enrollment_id" in update_filters + assert ENROLLMENT_ID in update_filters["enrollment_id"] assert update_filters.get("id") == f"eq.{AID}" diff --git a/backend/tests/test_calendar_sync_export_enrollment.py b/backend/tests/test_calendar_sync_export_enrollment.py new file mode 100644 index 00000000..b46666a7 --- /dev/null +++ b/backend/tests/test_calendar_sync_export_enrollment.py @@ -0,0 +1,32 @@ +# tests/test_calendar_sync_export_enrollment.py +from unittest.mock import MagicMock, patch +from fastapi.testclient import TestClient +from main import app + +client = TestClient(app) + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestExportScoping: + def test_export_skips_unowned_id(self): + tables = {"assignments": _tbl(select=[], update=[])} # id not owned -> no row + creds = MagicMock() + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar._require_google_creds", return_value=creds), \ + patch("routes.calendar.build") as build, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + r = client.post("/api/calendar/export", + json={"user_id": "user_andres", "assignment_ids": ["a-other"]}) + assert r.status_code == 200 + assert r.json() == {"exported_count": 0, "skipped_count": 0} + build.return_value.events.return_value.insert.assert_not_called() diff --git a/backend/tests/test_calendar_write_enrollment.py b/backend/tests/test_calendar_write_enrollment.py new file mode 100644 index 00000000..69100717 --- /dev/null +++ b/backend/tests/test_calendar_write_enrollment.py @@ -0,0 +1,44 @@ +# tests/test_calendar_write_enrollment.py +from unittest.mock import MagicMock, patch +import services.calendar_service as cs + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +class TestInsertNewAssignments: + def test_resolves_enrollment_and_inserts(self): + assignments_tbl = _tbl(select=[], insert=[]) + with patch("services.calendar_service.table", return_value=assignments_tbl), \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1") as eif: + n = cs.insert_new_assignments("user_andres", [ + {"title": "HW1", "due_date": "2026-03-01", "course_id": "CS101", "assignment_type": "homework"}, + ], source="manual") + assert n == 1 + eif.assert_called_with("user_andres", "CS101", create=True) + inserted = assignments_tbl.insert.call_args[0][0] + assert inserted[0]["enrollment_id"] == "e1" + assert inserted[0]["source"] == "manual" + assert "user_id" not in inserted[0] and "course_id" not in inserted[0] + + def test_skips_when_no_course(self): + with patch("services.calendar_service.table", return_value=_tbl(select=[], insert=[])), \ + patch("services.academics.user_enrollment_ids", return_value=[]): + n = cs.insert_new_assignments("user_andres", [ + {"title": "HW1", "due_date": "2026-03-01"}, # no course_id + ]) + assert n == 0 + + def test_dedup_against_enrollment_set(self): + # existing row in the user's enrollment has same title+day -> skip + existing = _tbl(select=[{"title": "HW1", "due_date": "2026-03-01"}], insert=[]) + with patch("services.calendar_service.table", return_value=existing), \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): + n = cs.insert_new_assignments("user_andres", [ + {"title": "HW1", "due_date": "2026-03-01", "course_id": "CS101"}, + ]) + assert n == 0 diff --git a/backend/tests/test_documents_routes.py b/backend/tests/test_documents_routes.py index c81fca73..b6fbe1db 100644 --- a/backend/tests/test_documents_routes.py +++ b/backend/tests/test_documents_routes.py @@ -379,7 +379,7 @@ def test_syllabus_triggers_assignment_extraction(self): r = _make_upload(filename="syllabus.pdf") assert r.status_code == 200 - mock_save.assert_called_once_with("u1", assignments) + mock_save.assert_called_once_with("u1", assignments, source="syllabus") def test_non_syllabus_skips_assignment_extraction(self): ai_result = { diff --git a/docs/superpowers/plans/2026-06-28-calendar-assignments-enrollment-rewire.md b/docs/superpowers/plans/2026-06-28-calendar-assignments-enrollment-rewire.md new file mode 100644 index 00000000..aa76be50 --- /dev/null +++ b/docs/superpowers/plans/2026-06-28-calendar-assignments-enrollment-rewire.md @@ -0,0 +1,835 @@ +# Calendar / assignments enrollment-rewire Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make `routes/calendar.py` + `services/calendar_service.py` work against the enrollment-keyed `assignments` table (migration 0021) so the dashboard stops 500-ing and the calendar feature functions on the redesigned schema. + +**Architecture:** Assignments are always course-tied and key on `enrollment_id`. A new `services/academics.py` resolver turns `(user, abstract course) → enrollment_id` (creating an offering+enrollment when missing); reads fetch the user's enrollments then `assignments WHERE enrollment_id IN (...)`, decorating each row with abstract `course_id`/`course_code`/`course_name` via the existing `offering_course_id` bridge. HTTP request/response shapes are unchanged. + +**Tech Stack:** FastAPI, PostgREST via `db.connection.table()`, pytest + `unittest.mock` (mock Supabase per conftest), column encryption via `services/encryption.py`. + +## Global Constraints + +- All DB access via `db.connection.table()` — never instantiate httpx or import supabase. (CLAUDE.md) +- Enrollment/offering/term resolution lives in `services/academics.py`. (CLAUDE.md) +- `assignments.notes` is column-encrypted: `encrypt_if_present` at write, `decrypt_if_present` at read. (CLAUDE.md) +- Tests live in `backend/tests/`, run `python -m pytest tests/ -q` from `backend/`. Auth is auto-bypassed by `conftest.py` (`require_self` stubbed; path-param `user_id` flows through). +- New `assignments` columns only: `id, enrollment_id, category_id, title, due_date, assignment_type, notes, points_possible, points_earned, source, google_event_id, gradescope_*, curve_*`. There is no `user_id`/`course_id`/`courses` relationship on this table. +- `source` ∈ `{'manual','syllabus'}`. `assignment_type` ∈ `{'homework','exam','reading','project','quiz','other'}`. +- Run all commands from `backend/` using `venv/bin/python` (e.g. `venv/bin/python -m pytest ...`). + +--- + +## Test helper: multi-table mock dispatch + +Several handlers now touch >1 table per call, so the old single-`return_value` mock is insufficient. Each test below builds a dispatch: `table(name)` returns a per-name `MagicMock`. + +```python +from unittest.mock import MagicMock + +def _tbl(**rows_by_verb): + """A per-table mock. e.g. _tbl(select=[...], insert=[], update=[], delete=[]).""" + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables: dict): + """Return a side_effect callable mapping table(name) -> its mock. + Unlisted names get an empty-select mock so stray reads don't explode.""" + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table +``` + +Place this at the top of each new test module (or import from a shared `tests/_dbmock.py` if you prefer; creating that file is optional and folded into Task 2). + +--- + +### Task 1: `enrollment_id_for` + `user_enrollment_ids` resolvers + +**Files:** +- Modify: `services/academics.py` (append two functions; `uuid` and `table` already imported) +- Test: `tests/test_academics_enrollment_resolver.py` (create) + +**Interfaces:** +- Produces: + - `academics.enrollment_id_for(user_id: str, course_id: str, *, create: bool = False) -> str | None` + - `academics.user_enrollment_ids(user_id: str) -> list[dict]` (each `{"id", "offering_id"}`) + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_academics_enrollment_resolver.py +from unittest.mock import MagicMock, patch +import services.academics as ac + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestUserEnrollmentIds: + def test_returns_rows(self): + with patch("services.academics.table", side_effect=_dispatch({ + "enrollments": _tbl(select=[{"id": "e1", "offering_id": "o1"}]), + })): + assert ac.user_enrollment_ids("user_andres") == [{"id": "e1", "offering_id": "o1"}] + + def test_empty_user(self): + assert ac.user_enrollment_ids("") == [] + +class TestEnrollmentIdFor: + def test_existing_enrollment_current_term(self): + # user_offering_ids_for_course -> ["o1"]; term match; enrollment e1 + tables = { + "course_offerings": _tbl(select=[{"id": "o1"}]), + "enrollments": _tbl(select=[{"id": "e1"}]), + } + with patch("services.academics.table", side_effect=_dispatch(tables)), \ + patch("services.academics.user_offering_ids_for_course", return_value=["o1"]), \ + patch("services.academics.current_term", return_value=None): + assert ac.enrollment_id_for("user_andres", "CS101") == "e1" + + def test_create_when_missing(self): + with patch("services.academics.user_offering_ids_for_course", return_value=[]), \ + patch("services.academics.resolve_offering", return_value="o9"), \ + patch("services.academics.table", side_effect=_dispatch({ + "enrollments": _tbl(select=[], insert=[]), + })): + eid = ac.enrollment_id_for("user_andres", "CS101", create=True) + assert isinstance(eid, str) and eid + + def test_missing_no_create_returns_none(self): + with patch("services.academics.user_offering_ids_for_course", return_value=[]): + assert ac.enrollment_id_for("user_andres", "CS101", create=False) is None +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `venv/bin/python -m pytest tests/test_academics_enrollment_resolver.py -q` +Expected: FAIL — `AttributeError: module 'services.academics' has no attribute 'enrollment_id_for'`. + +- [ ] **Step 3: Implement the resolvers** + +Append to `services/academics.py`: + +```python +def user_enrollment_ids(user_id: str) -> list[dict]: + """The user's enrollments as ``{id, offering_id}`` rows (read + scoping helper).""" + if not user_id: + return [] + return table("enrollments").select( + "id,offering_id", filters={"user_id": f"eq.{user_id}"} + ) or [] + + +def enrollment_id_for(user_id: str, course_id: str, *, create: bool = False) -> str | None: + """Resolve (user, abstract course) → the user's current-term enrollment id. + + Prefer the user's enrollment in the course's current-term offering, else + their only offering of the course. With ``create=True``, ensure an offering + (current term) and an enrollment row exist so a write never silently drops. + """ + if not user_id or not course_id: + return None + + offering_ids = user_offering_ids_for_course(user_id, course_id) + if offering_ids: + chosen = offering_ids[0] + cur = current_term() + cur_id = cur["id"] if cur else None + if cur_id: + for oid in offering_ids: + t = term_for_offering(oid) + if t and t.get("id") == cur_id: + chosen = oid + break + rows = table("enrollments").select( + "id", + filters={"user_id": f"eq.{user_id}", "offering_id": f"eq.{chosen}"}, + limit=1, + ) + if rows: + return rows[0]["id"] + + if not create: + return None + + offering_id = resolve_offering(course_id, create=True) + if not offering_id: + return None + existing = table("enrollments").select( + "id", + filters={"user_id": f"eq.{user_id}", "offering_id": f"eq.{offering_id}"}, + limit=1, + ) + if existing: + return existing[0]["id"] + new_id = str(uuid.uuid4()) + table("enrollments").insert( + {"id": new_id, "user_id": user_id, "offering_id": offering_id} + ) + return new_id +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `venv/bin/python -m pytest tests/test_academics_enrollment_resolver.py -q` +Expected: PASS (4 tests). + +- [ ] **Step 5: Commit** + +```bash +git add services/academics.py tests/test_academics_enrollment_resolver.py +git commit -m "feat(academics): enrollment_id_for + user_enrollment_ids resolvers" +``` + +--- + +### Task 2: Read path — `get_upcoming` + `get_all` on enrollment_id + +**Files:** +- Modify: `routes/calendar.py` (`get_upcoming` ~136-161, `get_all_assignments` ~164-188; add module helpers) +- Test: `tests/test_calendar_read_enrollment.py` (create) + +**Interfaces:** +- Consumes: `academics.user_enrollment_ids`, `academics.offering_course_id` +- Produces: module-level `_read_assignments(user_id, *, due_gte=None, limit=None) -> list[dict]` + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_calendar_read_enrollment.py +from unittest.mock import MagicMock, patch +from fastapi.testclient import TestClient +from main import app + +client = TestClient(app) + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestUpcomingEnrollmentKeyed: + def test_empty_when_no_enrollments(self): + with patch("routes.calendar.table", side_effect=_dispatch({"enrollments": _tbl(select=[])})), \ + patch("services.academics.table", side_effect=_dispatch({"enrollments": _tbl(select=[])})): + r = client.get("/api/calendar/upcoming/user_andres") + assert r.status_code == 200 + assert r.json() == {"assignments": []} + + def test_decorates_with_course_meta(self): + tables = { + "enrollments": _tbl(select=[{"id": "e1", "offering_id": "o1"}]), + "assignments": _tbl(select=[{ + "id": "a1", "enrollment_id": "e1", "title": "HW1", + "due_date": "2999-01-01", "assignment_type": "homework", + "notes": None, "google_event_id": None, "source": "manual", + }]), + "courses": _tbl(select=[{"id": "CS101", "course_code": "CS101", "course_name": "Intro"}]), + } + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + ac.offering_course_id.return_value = "CS101" + r = client.get("/api/calendar/upcoming/user_andres") + assert r.status_code == 200 + items = r.json()["assignments"] + assert len(items) == 1 + assert items[0]["course_code"] == "CS101" + assert items[0]["course_id"] == "CS101" + assert items[0]["user_id"] == "user_andres" +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `venv/bin/python -m pytest tests/test_calendar_read_enrollment.py -q` +Expected: FAIL — current `get_upcoming` selects `user_id,course_id,courses!left(...)`; mock returns no such rows / shape mismatch (the live path would 400). + +- [ ] **Step 3: Implement read path** + +In `routes/calendar.py`: add the import and helpers near the top (after the existing imports), then rewrite the two read handlers. + +```python +from services import academics # add to imports +``` + +```python +def _course_meta_cached(offering_id, cache): + if not offering_id: + return {} + if offering_id not in cache: + course_id = academics.offering_course_id(offering_id) + course = {} + if course_id: + rows = table("courses").select( + "id,course_code,course_name", + filters={"id": f"eq.{course_id}"}, limit=1, + ) + course = rows[0] if rows else {} + cache[offering_id] = { + "course_id": course_id, + "course_code": course.get("course_code"), + "course_name": course.get("course_name"), + } + return cache[offering_id] + + +def _read_assignments(user_id, *, due_gte=None, limit=None): + enrollments = academics.user_enrollment_ids(user_id) + if not enrollments: + return [] + offering_by_enrollment = {e["id"]: e.get("offering_id") for e in enrollments} + ids = ",".join(offering_by_enrollment.keys()) + filters = {"enrollment_id": f"in.({ids})"} + if due_gte: + filters["due_date"] = f"gte.{due_gte}" + rows = table("assignments").select( + "id,enrollment_id,title,due_date,assignment_type,notes,google_event_id,source", + filters=filters, order="due_date.asc", limit=limit, + ) + cache = {} + out = [] + for r in rows: + meta = _course_meta_cached(offering_by_enrollment.get(r.get("enrollment_id")), cache) + out.append({ + "id": r["id"], + "user_id": user_id, + "title": r["title"], + "due_date": r["due_date"], + "assignment_type": r.get("assignment_type"), + "notes": decrypt_if_present(r.get("notes")), + "google_event_id": r.get("google_event_id"), + "course_id": meta.get("course_id"), + "course_code": meta.get("course_code") or "", + "course_name": meta.get("course_name") or "", + }) + return out +``` + +```python +@router.get("/upcoming/{user_id}") +def get_upcoming(user_id: str, request: FastAPIRequest): + require_self(user_id, request) + today = datetime.utcnow().strftime("%Y-%m-%d") + return {"assignments": _read_assignments(user_id, due_gte=today, limit=20)} + + +@router.get("/all/{user_id}") +def get_all_assignments(user_id: str, request: FastAPIRequest): + """Return all assignments for a user (past and future) for the calendar view.""" + require_self(user_id, request) + return {"assignments": _read_assignments(user_id)} +``` + +(Confirm `db.connection.table.select` accepts `limit=None` as "no limit"; it does — `limit` is an optional kwarg appended only when truthy.) + +- [ ] **Step 4: Run test to verify it passes** + +Run: `venv/bin/python -m pytest tests/test_calendar_read_enrollment.py -q` +Expected: PASS (3 tests). + +- [ ] **Step 5: Commit** + +```bash +git add routes/calendar.py tests/test_calendar_read_enrollment.py +git commit -m "feat(calendar): read upcoming/all via enrollment_id (unblocks dashboard)" +``` + +--- + +### Task 3: Write path — `calendar_service` + `POST /save` on enrollment_id + +**Files:** +- Modify: `services/calendar_service.py` (`load_existing_assignment_keys`, `insert_new_assignments`, `save_assignments_to_db`) +- Modify: `routes/calendar.py` (`save_assignments` ~119-133 — pass raw notes + `source="manual"`) +- Test: `tests/test_calendar_write_enrollment.py` (create) + +**Interfaces:** +- Consumes: `academics.enrollment_id_for`, `academics.user_enrollment_ids` +- Produces: `insert_new_assignments(user_id, assignments, *, source="manual") -> int`; `save_assignments_to_db(user_id, assignments, *, source="syllabus") -> int` + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_calendar_write_enrollment.py +from unittest.mock import MagicMock, patch +import services.calendar_service as cs + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +class TestInsertNewAssignments: + def test_resolves_enrollment_and_inserts(self): + assignments_tbl = _tbl(select=[], insert=[]) + with patch("services.calendar_service.table", return_value=assignments_tbl), \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1") as eif: + n = cs.insert_new_assignments("user_andres", [ + {"title": "HW1", "due_date": "2026-03-01", "course_id": "CS101", "assignment_type": "homework"}, + ], source="manual") + assert n == 1 + eif.assert_called_with("user_andres", "CS101", create=True) + inserted = assignments_tbl.insert.call_args[0][0] + assert inserted[0]["enrollment_id"] == "e1" + assert inserted[0]["source"] == "manual" + assert "user_id" not in inserted[0] and "course_id" not in inserted[0] + + def test_skips_when_no_course(self): + with patch("services.calendar_service.table", return_value=_tbl(select=[], insert=[])), \ + patch("services.academics.user_enrollment_ids", return_value=[]): + n = cs.insert_new_assignments("user_andres", [ + {"title": "HW1", "due_date": "2026-03-01"}, # no course_id + ]) + assert n == 0 + + def test_dedup_against_enrollment_set(self): + # existing row in the user's enrollment has same title+day -> skip + existing = _tbl(select=[{"title": "HW1", "due_date": "2026-03-01"}], insert=[]) + with patch("services.calendar_service.table", return_value=existing), \ + patch("services.academics.user_enrollment_ids", return_value=[{"id": "e1", "offering_id": "o1"}]), \ + patch("services.academics.enrollment_id_for", return_value="e1"): + n = cs.insert_new_assignments("user_andres", [ + {"title": "HW1", "due_date": "2026-03-01", "course_id": "CS101"}, + ]) + assert n == 0 +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `venv/bin/python -m pytest tests/test_calendar_write_enrollment.py -q` +Expected: FAIL — current `insert_new_assignments` builds `{user_id, course_id}` rows and dedups by `user_id`; `source`/`enrollment_id` assertions fail. + +- [ ] **Step 3: Implement write path** + +Rewrite in `services/calendar_service.py` (keep `assignment_dedupe_key`, `uuid`, `encrypt_if_present` imports): + +```python +def load_existing_assignment_keys(user_id: str) -> set: + from services.academics import user_enrollment_ids + enrollments = user_enrollment_ids(user_id) + if not enrollments: + return set() + ids = ",".join(e["id"] for e in enrollments) + existing_rows = table("assignments").select( + "title,due_date", filters={"enrollment_id": f"in.({ids})"}, + ) + return {assignment_dedupe_key(r.get("title"), r.get("due_date")) for r in (existing_rows or [])} + + +def insert_new_assignments(user_id: str, assignments: list[dict], *, source: str = "manual") -> int: + """Insert assignments (deduped per the user's enrollment set, #16) on the + enrollment-keyed schema. Each assignment must carry a ``course_id`` — it is + resolved to the user's enrollment (created if missing). Returns rows inserted.""" + from services.academics import enrollment_id_for + existing_keys = load_existing_assignment_keys(user_id) + rows = [] + for a in assignments: + title = (a.get("title") or "").strip() + due_raw = (a.get("due_date") or "").strip() + if not title or not due_raw: + continue + key = assignment_dedupe_key(title, due_raw) + if key in existing_keys: + continue + course_id = a.get("course_id") + enrollment_id = enrollment_id_for(user_id, course_id, create=True) if course_id else None + if not enrollment_id: + continue # decision: every assignment is course-tied + existing_keys.add(key) + rows.append({ + "id": str(uuid.uuid4()), + "enrollment_id": enrollment_id, + "title": title, + "due_date": key[1], + "assignment_type": a.get("assignment_type") or "other", + "notes": encrypt_if_present(a.get("notes")), # #126: encrypt at write + "source": source, + }) + if rows: + table("assignments").insert(rows) + return len(rows) + + +def save_assignments_to_db(user_id: str, assignments: list, *, source: str = "syllabus") -> int: + """Write extracted assignment dicts (deduped via insert_new_assignments).""" + return insert_new_assignments(user_id, assignments, source=source) +``` + +In `routes/calendar.py`, fix `save_assignments` to pass **raw** notes (the service encrypts once — avoids the prior double-encryption) and tag source: + +```python +@router.post("/save") +def save_assignments(body: SaveAssignmentsBody, request: FastAPIRequest): + require_self(body.user_id, request) + payload = [ + { + "title": a.title, + "course_id": a.course_id, + "due_date": a.due_date, + "assignment_type": a.assignment_type, + "notes": a.notes, # raw; insert_new_assignments encrypts + } + for a in body.assignments + ] + saved = insert_new_assignments(body.user_id, payload, source="manual") + return {"saved_count": saved} +``` + +- [ ] **Step 4: Run tests** + +Run: `venv/bin/python -m pytest tests/test_calendar_write_enrollment.py tests/test_assignment_dedupe.py tests/test_assignment_notes_encryption.py -q` +Expected: new tests PASS. The two existing modules may reference the old schema — if they fail, fix them in Task 6 (note which here); do not delete coverage. + +- [ ] **Step 5: Commit** + +```bash +git add services/calendar_service.py routes/calendar.py tests/test_calendar_write_enrollment.py +git commit -m "feat(calendar): write assignments via resolved enrollment_id + source tag" +``` + +--- + +### Task 4: `suggest_study_blocks` + ownership scoping (`update`/`delete`) + +**Files:** +- Modify: `routes/calendar.py` (`suggest_study_blocks` ~236, `update_assignment` ~191, `delete_assignment` ~221) +- Test: `tests/test_calendar_scoping_enrollment.py` (create) + +**Interfaces:** +- Consumes: `academics.user_enrollment_ids`, `_read_assignments` (Task 2) +- Produces: module-level `_owned_enrollment_ids(user_id) -> set[str]` + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_calendar_scoping_enrollment.py +from unittest.mock import MagicMock, patch +from fastapi.testclient import TestClient +from main import app + +client = TestClient(app) + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestUpdateScoping: + def test_404_when_assignment_not_in_user_enrollments(self): + tables = { + "assignments": _tbl(select=[]), # no row owned by user's enrollments + } + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + r = client.patch("/api/calendar/assignments/a-other", + json={"user_id": "user_andres", "title": "x"}) + assert r.status_code == 404 + + def test_updates_owned_assignment(self): + tables = {"assignments": _tbl(select=[{"id": "a1"}], update=[])} + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + r = client.patch("/api/calendar/assignments/a1", + json={"user_id": "user_andres", "title": "new"}) + assert r.status_code == 200 + assert r.json() == {"updated": True} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `venv/bin/python -m pytest tests/test_calendar_scoping_enrollment.py -q` +Expected: FAIL — current handlers filter `assignments` by `user_id`, which doesn't exist. + +- [ ] **Step 3: Implement scoping + study blocks** + +Add helper to `routes/calendar.py`: + +```python +def _owned_enrollment_ids(user_id) -> set: + return {e["id"] for e in academics.user_enrollment_ids(user_id)} +``` + +Rewrite the existence/ownership checks to scope by `enrollment_id in (user's)`: + +```python +@router.patch("/assignments/{assignment_id}") +def update_assignment(assignment_id: str, body: dict, request: FastAPIRequest): + user_id = body.get("user_id") + if not user_id: + raise HTTPException(status_code=400, detail="user_id is required") + require_self(user_id, request) + + owned = _owned_enrollment_ids(user_id) + if not owned: + raise HTTPException(status_code=404, detail="Assignment not found") + existing = table("assignments").select( + "id", + filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"}, + limit=1, + ) + if not existing: + raise HTTPException(status_code=404, detail="Assignment not found") + + ALLOWED = {"title", "due_date", "assignment_type"} # course_id no longer settable here + patch = {k: v for k, v in body.items() if k in ALLOWED} + if not patch: + return {"updated": False} + table("assignments").update( + patch, filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"} + ) + return {"updated": True} + + +@router.delete("/assignments/{assignment_id}") +def delete_assignment(assignment_id: str, request: FastAPIRequest, user_id: str = Query(...)): + require_self(user_id, request) + owned = _owned_enrollment_ids(user_id) + if not owned: + raise HTTPException(status_code=404, detail="Assignment not found") + existing = table("assignments").select( + "id", + filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"}, + limit=1, + ) + if not existing: + raise HTTPException(status_code=404, detail="Assignment not found") + table("assignments").delete( + filters={"id": f"eq.{assignment_id}", "enrollment_id": f"in.({','.join(owned)})"} + ) + return {"deleted": True} +``` + +Rewrite `suggest_study_blocks` to read via `_read_assignments`: + +```python +@router.post("/suggest-study-blocks") +def suggest_study_blocks(body: StudyBlockBody, request: FastAPIRequest): + require_self(body.user_id, request) + today = datetime.utcnow().strftime("%Y-%m-%d") + assignments = _read_assignments(body.user_id, due_gte=today) + blocks = [] + for a in assignments: + cc = a.get("course_code") or "" + cn = a.get("course_name") or "" + course_label = f"[{cc}] " if cc else (f"{cn}: " if cn else "") + blocks.append({ + "topic": f"{course_label}{a['title']}" if course_label else a["title"], + "suggested_date": a["due_date"], + "duration_minutes": 60, + "reason": f"Due {a['due_date']}", + "related_assignment_id": a["id"], + }) + return {"study_blocks": blocks[:5]} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `venv/bin/python -m pytest tests/test_calendar_scoping_enrollment.py -q` +Expected: PASS (2 tests). + +- [ ] **Step 5: Commit** + +```bash +git add routes/calendar.py tests/test_calendar_scoping_enrollment.py +git commit -m "feat(calendar): enrollment-scoped update/delete + study-block reads" +``` + +--- + +### Task 5: `sync_to_google` + `export_to_google` on enrollment_id + +**Files:** +- Modify: `routes/calendar.py` (`sync_to_google` ~327, `export_to_google` ~378) +- Test: `tests/test_calendar_sync_export_enrollment.py` (create) + +**Interfaces:** +- Consumes: `_owned_enrollment_ids` (Task 4), `_read_assignments` (Task 2), `_require_google_creds` (existing) + +- [ ] **Step 1: Write the failing test** + +```python +# tests/test_calendar_sync_export_enrollment.py +from unittest.mock import MagicMock, patch +from fastapi.testclient import TestClient +from main import app + +client = TestClient(app) + +def _tbl(**rows_by_verb): + m = MagicMock() + for verb, val in rows_by_verb.items(): + getattr(m, verb).return_value = val + return m + +def _dispatch(tables): + def _table(name): + return tables.get(name) or _tbl(select=[], insert=[], update=[], delete=[]) + return _table + +class TestExportScoping: + def test_export_skips_unowned_id(self): + tables = {"assignments": _tbl(select=[], update=[])} # id not owned -> no row + creds = MagicMock() + with patch("routes.calendar.table", side_effect=_dispatch(tables)), \ + patch("routes.calendar._require_google_creds", return_value=creds), \ + patch("routes.calendar.build") as build, \ + patch("routes.calendar.academics") as ac: + ac.user_enrollment_ids.return_value = [{"id": "e1", "offering_id": "o1"}] + r = client.post("/api/calendar/export", + json={"user_id": "user_andres", "assignment_ids": ["a-other"]}) + assert r.status_code == 200 + assert r.json() == {"exported_count": 0, "skipped_count": 0} + build.return_value.events.return_value.insert.assert_not_called() +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `venv/bin/python -m pytest tests/test_calendar_sync_export_enrollment.py -q` +Expected: FAIL — current export filters by `user_id` and selects `courses!left(...)`. + +- [ ] **Step 3: Implement sync/export** + +For `sync_to_google`: select unsynced via the owned enrollment set instead of `user_id`, drop the `courses!left` embed and derive the label from `_read_assignments` data or a per-enrollment course-meta lookup. Concretely, replace the two `unsynced` selects with one scoped read of unsynced rows: + +```python +@router.post("/sync") +def sync_to_google(body: SyncBody, request: FastAPIRequest): + require_self(body.user_id, request) + creds = _require_google_creds(body.user_id) + service = build("calendar", "v3", credentials=creds) + + owned = _owned_enrollment_ids(body.user_id) + if not owned: + return {"synced_count": 0} + in_clause = f"in.({','.join(owned)})" + unsynced = table("assignments").select( + "id,enrollment_id,title,due_date,notes,google_event_id", + filters={"enrollment_id": in_clause, "google_event_id": "is.null"}, + ) + unsynced += table("assignments").select( + "id,enrollment_id,title,due_date,notes,google_event_id", + filters={"enrollment_id": in_clause, "google_event_id": "eq."}, + ) + + enr_to_offering = {e["id"]: e.get("offering_id") for e in academics.user_enrollment_ids(body.user_id)} + cache = {} + synced = 0 + for a in unsynced: + if not a.get("due_date"): + continue + meta = _course_meta_cached(enr_to_offering.get(a.get("enrollment_id")), cache) + cc = meta.get("course_code") or "" + cn = meta.get("course_name") or "" + course_label = f"[{cc}] " if cc else (f"{cn}: " if cn else "") + event = { + "summary": f"{course_label}{a['title']}" if course_label else a["title"], + "description": decrypt_if_present(a.get("notes")) or "", + "start": {"date": a["due_date"]}, + "end": {"date": a["due_date"]}, + } + created = service.events().insert(calendarId="primary", body=event).execute() + table("assignments").update( + {"google_event_id": created["id"]}, + filters={"id": f"eq.{a['id']}", "enrollment_id": in_clause}, + ) + synced += 1 + return {"synced_count": synced} +``` + +For `export_to_google`: replace the per-id `filters={"id":..., "user_id":...}` with `{"id":..., "enrollment_id": in.(owned)}`, drop `courses!left`, and build the label via `_course_meta_cached(enr_to_offering[a["enrollment_id"]], cache)`. Mirror the select column list above (`id,enrollment_id,title,due_date,notes,google_event_id`). Keep the existing skip-when-`google_event_id` and write-back-scoped-by-enrollment behavior (#123). + +- [ ] **Step 4: Run test to verify it passes** + +Run: `venv/bin/python -m pytest tests/test_calendar_sync_export_enrollment.py tests/test_calendar_export_idor.py -q` +Expected: new test PASSES. If `test_calendar_export_idor.py` asserts the old `user_id` scoping, update it to assert enrollment scoping (same IDOR guarantee, new key) — do not weaken the security assertion. + +- [ ] **Step 5: Commit** + +```bash +git add routes/calendar.py tests/test_calendar_sync_export_enrollment.py tests/test_calendar_export_idor.py +git commit -m "feat(calendar): enrollment-scoped Google sync/export" +``` + +--- + +### Task 6: Syllabus-save source tag + reconcile existing tests + full-suite + staging verify + +**Files:** +- Modify: `routes/documents.py` (the two `save_assignments_to_db` call sites ~475, ~988) +- Modify: `tests/test_calendar_routes.py`, `tests/test_assignment_dedupe.py`, `tests/test_assignment_notes_encryption.py`, `tests/test_calendar_sibling_write_scoping.py` (update any asserting the old `user_id`/`course_id`/`courses!left` schema) +- Test: full suite + +- [ ] **Step 1: Tag syllabus saves** + +In `routes/documents.py`, both call sites already attach `course_id` per assignment. Pass the source explicitly: + +```python +save_assignments_to_db(user_id, legacy, source="syllabus") +``` +```python +save_assignments_to_db(user_id, ai["assignments"], source="syllabus") +``` + +- [ ] **Step 2: Run the full calendar/assignment suite, see what the schema change broke** + +Run: `venv/bin/python -m pytest tests/test_calendar_routes.py tests/test_assignment_dedupe.py tests/test_assignment_notes_encryption.py tests/test_calendar_sibling_write_scoping.py -q` +Expected: failures in tests that mock the old single-table `select` shape or assert `user_id`/`course_id` columns. + +- [ ] **Step 3: Update those tests to the enrollment-keyed contract** + +For each failing test, switch its mock to the `_tbl`/`_dispatch` multi-table pattern and assert the new behavior: writes produce `enrollment_id`+`source` rows (no `user_id`/`course_id`); reads decorate via course-meta; scoping uses `enrollment_id`. Keep every existing behavioral guarantee (dedup by title+day, notes encryption at write, IDOR scoping) — only the key changes. Show the corrected mock per test (mirror Tasks 2-5). + +- [ ] **Step 4: Run the entire backend suite** + +Run: `venv/bin/python -m pytest tests/ -q` +Expected: PASS (no regressions). Investigate and fix any calendar/assignment-related failure; unrelated pre-existing failures (if any) are out of scope — note them. + +- [ ] **Step 5: Verify against staging DB (real reproduction)** + +Re-run the staging reproduction harness used during diagnosis (queries the live staging DB for the real user via `.env.staging`): + +Run: `PYTHONPATH=. venv/bin/python /repro_dashboard.py` +Expected: `/api/calendar/upcoming/{user}` line flips from `FAIL 400` to `OK 0 rows` (the user has no enrollments yet), and every other endpoint stays OK. + +> Note: this only exercises the read query path against staging. The deployed backend fix lands when this branch merges to `main` and Railway redeploys the staging backend. + +- [ ] **Step 6: Commit** + +```bash +git add routes/documents.py tests/ +git commit -m "feat(calendar): tag syllabus saves + migrate calendar tests to enrollment schema" +``` + +--- + +## Self-Review (completed by plan author) + +- **Spec coverage:** resolver (Task 1) ✓; read path incl. empty-enrollment dashboard unblock (Task 2) ✓; write path + dedup + source (Task 3) ✓; auto-create enrollment (`enrollment_id_for(create=True)`, Tasks 1+3) ✓; ownership scoping (Task 4) ✓; sync/export (Task 5) ✓; syllabus-save source + encryption-at-write (Tasks 3+6) ✓; tests rewritten (all tasks + Task 6) ✓; staging verification (Task 6) ✓. No schema migration (none added) ✓. +- **Placeholders:** none — every code step shows real code; Task 5 export references the exact column list and helpers from earlier tasks. +- **Type consistency:** `enrollment_id_for(user_id, course_id, *, create=False)`, `user_enrollment_ids(user_id)->list[dict]`, `_read_assignments(user_id,*,due_gte=None,limit=None)`, `_course_meta_cached(offering_id,cache)`, `_owned_enrollment_ids(user_id)->set`, `insert_new_assignments(...,*,source="manual")`, `save_assignments_to_db(...,*,source="syllabus")` — used consistently across tasks. diff --git a/docs/superpowers/specs/2026-06-28-calendar-assignments-enrollment-rewire-design.md b/docs/superpowers/specs/2026-06-28-calendar-assignments-enrollment-rewire-design.md new file mode 100644 index 00000000..54c5faaa --- /dev/null +++ b/docs/superpowers/specs/2026-06-28-calendar-assignments-enrollment-rewire-design.md @@ -0,0 +1,144 @@ +# Calendar / assignments rewire to the enrollment-keyed schema — design + +**Date:** 2026-06-28 +**Status:** Approved (pending spec review) → next: implementation plan +**Owner:** backend + +## Problem + +The DB modular redesign (migration `0021_gradebook.sql`) did +`DROP TABLE assignments CASCADE` and recreated `assignments` as the +**enrollment-keyed gradebook table**: columns are +`id, enrollment_id, category_id, title, due_date, assignment_type, notes, +points_possible, points_earned, source, google_event_id, gradescope_*, curve_*`. +There is **no `user_id`, no `course_id`, and no `assignments→courses` relationship**. + +`routes/calendar.py` and `services/calendar_service.py` were never rewired and +still speak the pre-redesign schema (`select user_id,course_id,courses!left(...)`, +`filter user_id=…`, `insert {user_id, course_id}`). Every such call now returns a +PostgREST `400`, which the route does not catch → unhandled → **HTTP 500**. The +migration itself flagged this as deferred: *"(was assignments.\*) → … See issues +filed for the code rewire."* + +### Evidence (staging, reproduced against the live DB for the real user) + +| Endpoint | Result | +|---|---| +| `/api/users` | OK | +| `/api/auth/me` | OK | +| `/api/graph/{user}` (+ `/recommendations`, `/courses`) | OK | +| **`/api/calendar/upcoming/{user}`** | **400 from PostgREST on `assignments`** | +| `/api/learn/sessions/{user}` | OK | +| `/api/profile/{user}/achievements` | OK | + +The dashboard issues these in a `Promise.all`, so the single calendar 500 tanks +the whole dashboard load. **Blast radius is the calendar/assignments domain +only**; all other redesigned domains are healthy. + +## Decisions (approved) + +1. **Every assignment is tied to a course.** No standalone/course-less items, so + no schema migration — assignments key on `enrollment_id`, reached via + `enrollment → offering → course`. +2. **Auto-create the enrollment on write.** A manual or syllabus save for a + course the user is not yet enrolled in resolves/creates the current-term + offering and an enrollment, so saves never silently drop. +3. **Full-domain rewire** (read + write + sync/export + syllabus-save + tests), + not just a read-path band-aid. + +## Design + +Mirror the already-migrated `routes/gradebook.py` helpers rather than invent a +parallel pattern or rely on fragile nested PostgREST embeds (the `academics` +module explicitly avoids embedded-filter syntax). The HTTP request/response +shapes are **unchanged**, so the frontend needs no edits. + +### 1. Shared resolver — `services/academics.py` + +``` +enrollment_id_for(user_id, course_id, *, create=False) -> str | None +``` + +Resolve `(user, abstract course)` → the user's current-term `enrollment_id`, +reusing `resolve_offering` / `user_offering_ids_for_course` / `current_term`. +With `create=True`: ensure an offering (`resolve_offering(course_id, create=True)`) +and an enrollment row exist (insert if missing), then return its id. Enrollment +resolution living in `academics.py` matches the CLAUDE.md convention. + +### 2. Read path — `get_upcoming`, `get_all`, `suggest_study_blocks` + +1. Fetch the user's enrollments: `table("enrollments").select("id,offering_id", + filters={"user_id": f"eq.{user_id}"})` → `enrollment_id`s + offering map. +2. **No enrollments → return `{"assignments": []}`** (this is what unblocks the + dashboard for new users). +3. `table("assignments").select("id,enrollment_id,title,due_date,assignment_type, + notes,google_event_id,source", filters={"enrollment_id": f"in.({ids})", …})` + (+ `due_date >= today` for `upcoming`), `order=due_date.asc`. +4. Decrypt `notes` (`decrypt_if_present`). Attach `course_id` (abstract), + `course_code`, `course_name` per assignment by mapping + `enrollment_id → offering_id → _course_meta(offering_id)` (cache per offering). +5. Response keeps the existing shape, echoing `user_id` (the path param) and the + abstract `course_id`. + +### 3. Write path — `save_assignments`, `services/calendar_service.insert_new_assignments`, syllabus-save callers + +- A `course_id` is **required** on write (consistent with decision 1). Both + syllabus-save call sites in `documents.py` already attach `course_id` per + assignment dict. +- For each assignment: `enrollment_id = academics.enrollment_id_for(user_id, + course_id, create=True)`; insert `{enrollment_id, title, due_date, + assignment_type, notes: encrypt_if_present(...), google_event_id, source}`. +- `source` = `'manual'` for `POST /save`, `'syllabus'` for syllabus extraction. +- **Dedup rewire:** `calendar_service.load_existing_assignment_keys` currently + queries `assignments` by `user_id`; it must dedup against the assignments in + the user's enrollment set (resolve the user's `enrollment_id`s first, then + query by `enrollment_id in (...)`). Dedup key stays trimmed-title + calendar-day. + +### 4. Ownership scoping — `update_assignment`, `delete_assignment`, `sync_to_google`, `export_to_google` + +Replace `user_id` filters with enrollment-ownership checks: the assignment's +`enrollment_id` must belong to one of the caller's enrollments. Concretely, +resolve the user's `enrollment_id`s once and require the target assignment's +`enrollment_id` to be in that set before read/update/delete/push. This preserves +the existing defense-in-depth guarantees (#123) under the new key. +`oauth_tokens` (Google credentials) is **unchanged** — still keyed by `user_id`. + +### 5. Encryption + +`assignments.notes` is column-encrypted: `encrypt_if_present` at write, +`decrypt_if_present` at read (both already imported in `calendar.py`). Points +columns are not touched by the calendar feature. + +## Files touched + +- `services/academics.py` — add `enrollment_id_for(...)`. +- `routes/calendar.py` — `get_upcoming`, `get_all`, `save_assignments`, + `suggest_study_blocks`, `update_assignment`, `delete_assignment`, + `sync_to_google`, `export_to_google`. +- `services/calendar_service.py` — `insert_new_assignments`, + `load_existing_assignment_keys`, `save_assignments_to_db` (thread `source`). +- `routes/documents.py` — confirm the two syllabus-save call sites pass + `course_id` (they do); thread `source='syllabus'`. +- `backend/tests/` — rewrite calendar tests against the enrollment-keyed schema. + +## Testing + +TDD per project conventions (`backend/tests/`, mock Supabase in `conftest.py`). +Cover: read with/without enrollments (empty-list path), read course-meta mapping, +write resolves an existing enrollment, write auto-creates when none exists, dedup +across the enrollment set, and ownership scoping rejects another user's +assignment id. Update any existing calendar test asserting the old schema. + +## Out of scope + +- No schema migration (decision 1). +- No frontend changes (request/response shapes preserved). +- Gradebook route (already migrated), other domains (verified healthy). +- Standalone/course-less assignments (explicitly excluded). + +## Verification / rollout + +- Backend test suite green (`python -m pytest tests/ -q`). +- Re-run the staging reproduction harness: `/api/calendar/upcoming/{user}` returns + `200 {"assignments": []}` for the new user; dashboard `Promise.all` resolves. +- Manual: add a course → upload a syllabus → assignment appears in the calendar.