feat(exam): /api/exam template CRUD router (as-user RLS, E1 fix)
S4-5: new routers/exam/ package mounted at /api/exam (R5.1/E5, not under
/database/). Template CRUD with hybrid persistence (R5.2):
- POST/GET/GET{id}/PUT{id}/DELETE{id} /templates + PATCH /questions/{qid}
- Calls Supabase AS THE USER via SupabaseAnonClient.for_user (E1 fix), so the
RLS in 72-exam-marker.sql is enforced; no service-role for user-facing ops.
- Institute resolved/validated via the user_institute_ids() SECURITY DEFINER
RPC (institute_memberships is deny-all as-user per E4); client-supplied
institute_id is validated, never trusted (R5.5).
- Ownership pre-checked before writes (E2); out-of-scope ids read back as 404
under RLS (IDOR-safe). Soft-delete archives, never hard-deletes.
- PUT full-replace preserves client UUIDs as Neo4j join keys (spec §2).
- eb_exams.exam_code denormalised via a documented service-role catalogue
lookup (eb_exams is shared reference data, deny-all as-user per E4).
Unit tests cover auth, CRUD, ownership/IDOR, institute validation, soft-delete.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
6ce6272a1e
commit
f52c3267ca
9
routers/exam/__init__.py
Normal file
9
routers/exam/__init__.py
Normal file
@ -0,0 +1,9 @@
|
|||||||
|
"""Exam-marker API package (/api/exam/).
|
||||||
|
|
||||||
|
A clean top-level router group (R5.1/E5), deliberately NOT nested under /database/. Every
|
||||||
|
endpoint authenticates the JWT and calls Supabase as-the-user so the RLS in
|
||||||
|
volumes/db/cc/72-exam-marker.sql is enforced (spec E1/E2 fixes).
|
||||||
|
"""
|
||||||
|
from routers.exam.templates import router
|
||||||
|
|
||||||
|
__all__ = ["router"]
|
||||||
118
routers/exam/dependencies.py
Normal file
118
routers/exam/dependencies.py
Normal file
@ -0,0 +1,118 @@
|
|||||||
|
"""Auth + data-access plumbing for the /api/exam/ router.
|
||||||
|
|
||||||
|
Per the audit (spec S1/E1): the exam API calls Supabase **as the user** so the RLS in
|
||||||
|
72-exam-marker.sql is actually enforced — it does NOT use the service role for user-facing
|
||||||
|
reads/writes the way files.py / classes_router.py do. The bearer already attaches the raw
|
||||||
|
JWT as payload["_access_token"] (supabase_bearer.py) precisely for this.
|
||||||
|
|
||||||
|
Institute resolution is the one wrinkle: institute_memberships and profiles are RLS
|
||||||
|
deny-all to a normal authenticated user (E4), so we cannot read them as-user. Instead we
|
||||||
|
call public.user_institute_ids() — a SECURITY DEFINER function (71-class-management.sql) that
|
||||||
|
PostgREST exposes as an RPC — which returns the caller's institute ids regardless of those
|
||||||
|
table policies. This is the same function the RLS policies themselves key off, so the API's
|
||||||
|
view of "which institutes is this user in" is guaranteed consistent with what RLS will allow.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
from typing import Any, Dict, List, Optional
|
||||||
|
|
||||||
|
from fastapi import Depends, HTTPException
|
||||||
|
|
||||||
|
from modules.auth.supabase_bearer import SupabaseBearer
|
||||||
|
from modules.database.supabase.utils.client import (
|
||||||
|
SupabaseAnonClient,
|
||||||
|
SupabaseServiceRoleClient,
|
||||||
|
)
|
||||||
|
from modules.logger_tool import initialise_logger
|
||||||
|
|
||||||
|
logger = initialise_logger(__name__, os.getenv("LOG_LEVEL"), os.getenv("LOG_PATH"), "default", True)
|
||||||
|
|
||||||
|
auth = SupabaseBearer()
|
||||||
|
|
||||||
|
|
||||||
|
class ExamContext:
|
||||||
|
"""The per-request handle every exam endpoint works through.
|
||||||
|
|
||||||
|
Bundles the caller's id, an as-user Supabase client (RLS-enforced), and the set of
|
||||||
|
institute ids the caller belongs to (for R5.5 institute validation on writes).
|
||||||
|
"""
|
||||||
|
|
||||||
|
def __init__(self, user_id: str, access_token: str, supabase: Any, institute_ids: List[str]):
|
||||||
|
self.user_id = user_id
|
||||||
|
self.access_token = access_token
|
||||||
|
self.supabase = supabase
|
||||||
|
self.institute_ids = institute_ids
|
||||||
|
|
||||||
|
def resolve_institute(self, requested: Optional[str]) -> str:
|
||||||
|
"""Validate a client-supplied institute_id, or pick the sole membership.
|
||||||
|
|
||||||
|
R5.5: a client-supplied institute_id is never trusted as the authz signal — it must
|
||||||
|
be one the caller actually belongs to. RLS would reject a bad value at write time
|
||||||
|
anyway; resolving here turns that into a clean 400/403 instead of an opaque DB error.
|
||||||
|
"""
|
||||||
|
if requested:
|
||||||
|
if requested not in self.institute_ids:
|
||||||
|
raise HTTPException(status_code=403, detail="Not a member of the requested institute")
|
||||||
|
return requested
|
||||||
|
if len(self.institute_ids) == 1:
|
||||||
|
return self.institute_ids[0]
|
||||||
|
if not self.institute_ids:
|
||||||
|
raise HTTPException(status_code=403, detail="Caller has no institute membership")
|
||||||
|
raise HTTPException(
|
||||||
|
status_code=400,
|
||||||
|
detail="institute_id is required when the caller belongs to multiple institutes",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _extract_institute_ids(rpc_data: Any) -> List[str]:
|
||||||
|
"""Normalise the user_institute_ids() RPC result to a list of uuid strings.
|
||||||
|
|
||||||
|
A `returns setof uuid` function comes back from PostgREST as a JSON array of scalars,
|
||||||
|
but tolerate the `[{"user_institute_ids": "..."}]` shape too in case of driver quirks.
|
||||||
|
"""
|
||||||
|
out: List[str] = []
|
||||||
|
for row in rpc_data or []:
|
||||||
|
if isinstance(row, dict):
|
||||||
|
val = row.get("user_institute_ids") or next(iter(row.values()), None)
|
||||||
|
else:
|
||||||
|
val = row
|
||||||
|
if val:
|
||||||
|
out.append(str(val))
|
||||||
|
return out
|
||||||
|
|
||||||
|
|
||||||
|
async def get_exam_context(payload: Dict[str, Any] = Depends(auth)) -> ExamContext:
|
||||||
|
user_id = payload.get("sub") or payload.get("user_id")
|
||||||
|
access_token = payload.get("_access_token")
|
||||||
|
if not user_id or not access_token:
|
||||||
|
raise HTTPException(status_code=401, detail="Invalid token payload")
|
||||||
|
|
||||||
|
supabase = SupabaseAnonClient.for_user(access_token).supabase
|
||||||
|
|
||||||
|
try:
|
||||||
|
res = supabase.rpc("user_institute_ids").execute()
|
||||||
|
institute_ids = _extract_institute_ids(getattr(res, "data", None))
|
||||||
|
except Exception as exc:
|
||||||
|
logger.error(f"Failed to resolve institute memberships: {exc}")
|
||||||
|
raise HTTPException(status_code=502, detail="Could not resolve institute membership")
|
||||||
|
|
||||||
|
return ExamContext(user_id, access_token, supabase, institute_ids)
|
||||||
|
|
||||||
|
|
||||||
|
def lookup_exam_code(exam_id: str) -> Optional[str]:
|
||||||
|
"""Resolve eb_exams.exam_code for a catalogue paper (denormalised onto the template).
|
||||||
|
|
||||||
|
Documented service-role exception (S1): eb_exams is shared exam-board reference data with
|
||||||
|
no as-user SELECT policy (E4), so a normal user cannot read it. This is a read of public
|
||||||
|
catalogue metadata only — not user-scoped data — and is used solely to keep the Neo4j join
|
||||||
|
key (exam_code) correct on the template row.
|
||||||
|
"""
|
||||||
|
try:
|
||||||
|
sb = SupabaseServiceRoleClient().supabase
|
||||||
|
res = sb.table("eb_exams").select("exam_code").eq("id", exam_id).limit(1).execute()
|
||||||
|
rows = getattr(res, "data", None) or []
|
||||||
|
return rows[0].get("exam_code") if rows else None
|
||||||
|
except Exception as exc:
|
||||||
|
logger.warning(f"exam_code lookup failed for exam_id={exam_id}: {exc}")
|
||||||
|
return None
|
||||||
104
routers/exam/schemas.py
Normal file
104
routers/exam/schemas.py
Normal file
@ -0,0 +1,104 @@
|
|||||||
|
"""Pydantic request/response models for the /api/exam/ router (S4-5).
|
||||||
|
|
||||||
|
Templates are saved from the canvas with a full-replace PUT (R5.2): the client owns
|
||||||
|
stable UUIDs for questions / response areas / boundaries so the Supabase ids line up
|
||||||
|
with the Neo4j join keys (exam_questions.id ↔ Question|Part.uuid_string,
|
||||||
|
exam_response_areas.id ↔ Region.uuid_string — see spec §2). Granular mark-scheme edits
|
||||||
|
go through PATCH /api/exam/questions/{qid}.
|
||||||
|
|
||||||
|
Models mirror the columns in volumes/db/cc/72-exam-marker.sql. They are intentionally
|
||||||
|
permissive (most fields optional) so the canvas can round-trip partial state during
|
||||||
|
authoring without the API rejecting work-in-progress.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from typing import Any, Dict, List, Literal, Optional
|
||||||
|
|
||||||
|
from pydantic import BaseModel, Field
|
||||||
|
|
||||||
|
|
||||||
|
# ─── Templates ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
class CreateTemplateRequest(BaseModel):
|
||||||
|
title: str
|
||||||
|
subject: Optional[str] = None
|
||||||
|
# Catalogue paper (eb_exams) the template maps, when chosen from the catalogue (R2.2).
|
||||||
|
exam_id: Optional[str] = None
|
||||||
|
# Denormalised onto the template for the Neo4j join (eb_exams.exam_code ↔ ExamPaper.exam_code).
|
||||||
|
# If exam_id is given but exam_code is omitted, the API resolves it from the catalogue.
|
||||||
|
exam_code: Optional[str] = None
|
||||||
|
# Uploaded PDF (files.id) for an ad-hoc paper (R2.2).
|
||||||
|
source_file_id: Optional[str] = None
|
||||||
|
page_count: Optional[int] = None
|
||||||
|
# Active institute (R1.4/R5.5). Validated against the caller's memberships; never trusted
|
||||||
|
# as the authorization signal. Optional when the caller belongs to exactly one institute.
|
||||||
|
institute_id: Optional[str] = None
|
||||||
|
|
||||||
|
|
||||||
|
class UpdateTemplateMetaRequest(BaseModel):
|
||||||
|
"""Template-level fields that a full-replace PUT may also update alongside the canvas."""
|
||||||
|
title: Optional[str] = None
|
||||||
|
subject: Optional[str] = None
|
||||||
|
page_count: Optional[int] = None
|
||||||
|
status: Optional[Literal["draft", "ready", "archived"]] = None
|
||||||
|
|
||||||
|
|
||||||
|
# ─── Canvas entities (children of a template) ────────────────────────────────────
|
||||||
|
|
||||||
|
class QuestionPayload(BaseModel):
|
||||||
|
# Client-supplied stable UUID (== Neo4j Question|Part.uuid_string). Optional on first save.
|
||||||
|
id: Optional[str] = None
|
||||||
|
parent_id: Optional[str] = None
|
||||||
|
label: str
|
||||||
|
order: int = 0
|
||||||
|
max_marks: float = 0
|
||||||
|
answer_type: Optional[Literal["written", "mcq", "short", "diagram"]] = None
|
||||||
|
mcq_options: Optional[Any] = None
|
||||||
|
mark_scheme: Dict[str, Any] = Field(default_factory=dict)
|
||||||
|
is_container: bool = False
|
||||||
|
spec_ref: Optional[str] = None
|
||||||
|
|
||||||
|
|
||||||
|
class ResponseAreaPayload(BaseModel):
|
||||||
|
id: Optional[str] = None # == Neo4j Region.uuid_string
|
||||||
|
question_id: str
|
||||||
|
page: int
|
||||||
|
bounds: Dict[str, Any] # {x,y,w,h}
|
||||||
|
kind: Literal["response", "context"]
|
||||||
|
response_form: Optional[
|
||||||
|
Literal["lines", "answer-box", "working", "diagram", "tick-boxes", "table", "blanks"]
|
||||||
|
] = None
|
||||||
|
source: Literal["manual", "ai"] = "manual"
|
||||||
|
confirmed: bool = True
|
||||||
|
confidence: Optional[float] = None
|
||||||
|
|
||||||
|
|
||||||
|
class BoundaryPayload(BaseModel):
|
||||||
|
id: Optional[str] = None
|
||||||
|
question_id: Optional[str] = None
|
||||||
|
label: Optional[str] = None
|
||||||
|
page_index: int
|
||||||
|
y: float
|
||||||
|
bounds: Optional[Dict[str, Any]] = None
|
||||||
|
source: Literal["manual", "ai"] = "manual"
|
||||||
|
confirmed: bool = True
|
||||||
|
|
||||||
|
|
||||||
|
class TemplateReplaceRequest(BaseModel):
|
||||||
|
"""Full-replace canvas save (R5.2 primary path). All children are replaced wholesale."""
|
||||||
|
meta: Optional[UpdateTemplateMetaRequest] = None
|
||||||
|
questions: List[QuestionPayload] = Field(default_factory=list)
|
||||||
|
response_areas: List[ResponseAreaPayload] = Field(default_factory=list)
|
||||||
|
boundaries: List[BoundaryPayload] = Field(default_factory=list)
|
||||||
|
|
||||||
|
|
||||||
|
class PatchQuestionRequest(BaseModel):
|
||||||
|
"""Incremental mark-scheme / spec-ref edit (R5.2 granular path)."""
|
||||||
|
label: Optional[str] = None
|
||||||
|
order: Optional[int] = None
|
||||||
|
max_marks: Optional[float] = None
|
||||||
|
answer_type: Optional[Literal["written", "mcq", "short", "diagram"]] = None
|
||||||
|
mcq_options: Optional[Any] = None
|
||||||
|
mark_scheme: Optional[Dict[str, Any]] = None
|
||||||
|
is_container: Optional[bool] = None
|
||||||
|
spec_ref: Optional[str] = None
|
||||||
259
routers/exam/templates.py
Normal file
259
routers/exam/templates.py
Normal file
@ -0,0 +1,259 @@
|
|||||||
|
"""Template CRUD for the exam-marker (/api/exam/templates...) — card S4-5.
|
||||||
|
|
||||||
|
All access is as-the-user (RLS-enforced; spec E1 fix) via ExamContext. Ownership is also
|
||||||
|
checked explicitly before mutating (E2: never trust a client-supplied id as authorization) —
|
||||||
|
defence in depth on top of RLS. A row the caller cannot see under RLS reads back as absent,
|
||||||
|
so cross-institute access surfaces as 404, never a data leak (IDOR-safe).
|
||||||
|
|
||||||
|
Hybrid persistence (R5.2): PUT /templates/{id} is a full-replace of the canvas children
|
||||||
|
(questions + response areas + boundaries); PATCH /questions/{qid} is the granular mark-scheme
|
||||||
|
edit path. Client-supplied UUIDs are preserved so Supabase ids stay aligned with the Neo4j
|
||||||
|
join keys (spec §2).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
from typing import Any, Dict, List, Optional
|
||||||
|
|
||||||
|
from fastapi import APIRouter, Depends, HTTPException
|
||||||
|
|
||||||
|
from modules.logger_tool import initialise_logger
|
||||||
|
from routers.exam.dependencies import ExamContext, get_exam_context, lookup_exam_code
|
||||||
|
from routers.exam.schemas import (
|
||||||
|
CreateTemplateRequest,
|
||||||
|
PatchQuestionRequest,
|
||||||
|
TemplateReplaceRequest,
|
||||||
|
)
|
||||||
|
|
||||||
|
logger = initialise_logger(__name__, os.getenv("LOG_LEVEL"), os.getenv("LOG_PATH"), "default", True)
|
||||||
|
|
||||||
|
router = APIRouter()
|
||||||
|
|
||||||
|
|
||||||
|
# ─── helpers ─────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
def _rows(result: Any) -> List[Dict[str, Any]]:
|
||||||
|
data = getattr(result, "data", None)
|
||||||
|
if not data:
|
||||||
|
return []
|
||||||
|
return data if isinstance(data, list) else [data]
|
||||||
|
|
||||||
|
|
||||||
|
def _first(result: Any) -> Optional[Dict[str, Any]]:
|
||||||
|
rows = _rows(result)
|
||||||
|
return rows[0] if rows else None
|
||||||
|
|
||||||
|
|
||||||
|
def _fetch_template_or_404(ctx: ExamContext, template_id: str) -> Dict[str, Any]:
|
||||||
|
"""Load a template the caller can see (RLS-scoped). Missing/forbidden → 404."""
|
||||||
|
res = ctx.supabase.table("exam_templates").select("*").eq("id", template_id).limit(1).execute()
|
||||||
|
row = _first(res)
|
||||||
|
if not row:
|
||||||
|
raise HTTPException(status_code=404, detail="Template not found")
|
||||||
|
return row
|
||||||
|
|
||||||
|
|
||||||
|
def _require_owner(ctx: ExamContext, template: Dict[str, Any]) -> None:
|
||||||
|
"""Writes are limited to the owning teacher (R2.4). RLS also enforces this; we pre-check
|
||||||
|
so a colleague who can *read* the template gets a clean 403 instead of a silent no-op."""
|
||||||
|
if template.get("teacher_id") != ctx.user_id:
|
||||||
|
raise HTTPException(status_code=403, detail="Only the template owner can modify it")
|
||||||
|
|
||||||
|
|
||||||
|
# ─── templates ───────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
@router.post("/templates")
|
||||||
|
async def create_template(
|
||||||
|
body: CreateTemplateRequest,
|
||||||
|
ctx: ExamContext = Depends(get_exam_context),
|
||||||
|
) -> Dict[str, Any]:
|
||||||
|
institute_id = ctx.resolve_institute(body.institute_id)
|
||||||
|
|
||||||
|
exam_code = body.exam_code
|
||||||
|
if body.exam_id and not exam_code:
|
||||||
|
exam_code = lookup_exam_code(body.exam_id)
|
||||||
|
|
||||||
|
row = {
|
||||||
|
"title": body.title,
|
||||||
|
"subject": body.subject,
|
||||||
|
"exam_id": body.exam_id,
|
||||||
|
"exam_code": exam_code,
|
||||||
|
"source_file_id": body.source_file_id,
|
||||||
|
"page_count": body.page_count,
|
||||||
|
"institute_id": institute_id,
|
||||||
|
"teacher_id": ctx.user_id,
|
||||||
|
"status": "draft",
|
||||||
|
}
|
||||||
|
row = {k: v for k, v in row.items() if v is not None}
|
||||||
|
|
||||||
|
res = ctx.supabase.table("exam_templates").insert(row).execute()
|
||||||
|
created = _first(res)
|
||||||
|
if not created:
|
||||||
|
raise HTTPException(status_code=500, detail="Failed to create template")
|
||||||
|
logger.info(f"Exam template created: {created.get('id')} by {ctx.user_id}")
|
||||||
|
return created
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/templates")
|
||||||
|
async def list_templates(
|
||||||
|
include_archived: bool = False,
|
||||||
|
institute_id: Optional[str] = None,
|
||||||
|
ctx: ExamContext = Depends(get_exam_context),
|
||||||
|
) -> Dict[str, Any]:
|
||||||
|
# RLS already scopes to the caller's institutes; the optional filter narrows within that.
|
||||||
|
q = ctx.supabase.table("exam_templates").select("*")
|
||||||
|
if institute_id:
|
||||||
|
q = q.eq("institute_id", institute_id)
|
||||||
|
if not include_archived:
|
||||||
|
q = q.neq("status", "archived")
|
||||||
|
res = q.order("updated_at", desc=True).execute()
|
||||||
|
return {"templates": _rows(res)}
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/templates/{template_id}")
|
||||||
|
async def get_template(
|
||||||
|
template_id: str,
|
||||||
|
ctx: ExamContext = Depends(get_exam_context),
|
||||||
|
) -> Dict[str, Any]:
|
||||||
|
template = _fetch_template_or_404(ctx, template_id)
|
||||||
|
questions = _rows(
|
||||||
|
ctx.supabase.table("exam_questions").select("*").eq("template_id", template_id).order("order").execute()
|
||||||
|
)
|
||||||
|
response_areas = _rows(
|
||||||
|
ctx.supabase.table("exam_response_areas").select("*").eq("template_id", template_id).execute()
|
||||||
|
)
|
||||||
|
boundaries = _rows(
|
||||||
|
ctx.supabase.table("exam_boundaries").select("*").eq("template_id", template_id).execute()
|
||||||
|
)
|
||||||
|
return {
|
||||||
|
**template,
|
||||||
|
"questions": questions,
|
||||||
|
"response_areas": response_areas,
|
||||||
|
"boundaries": boundaries,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@router.put("/templates/{template_id}")
|
||||||
|
async def replace_template(
|
||||||
|
template_id: str,
|
||||||
|
body: TemplateReplaceRequest,
|
||||||
|
ctx: ExamContext = Depends(get_exam_context),
|
||||||
|
) -> Dict[str, Any]:
|
||||||
|
"""Full-replace canvas save (R5.2). Replaces questions/response_areas/boundaries wholesale.
|
||||||
|
|
||||||
|
Note: the delete-then-insert spans several PostgREST calls and is therefore not atomic;
|
||||||
|
acceptable for the small (~20-question) payloads this carries. A transactional RPC is a
|
||||||
|
later hardening step if concurrent canvas saves become a concern.
|
||||||
|
"""
|
||||||
|
template = _fetch_template_or_404(ctx, template_id)
|
||||||
|
_require_owner(ctx, template)
|
||||||
|
|
||||||
|
# Optional template-level metadata update alongside the canvas.
|
||||||
|
if body.meta:
|
||||||
|
updates = {k: v for k, v in body.meta.dict().items() if v is not None}
|
||||||
|
if updates:
|
||||||
|
ctx.supabase.table("exam_templates").update(updates).eq("id", template_id).execute()
|
||||||
|
|
||||||
|
sb = ctx.supabase
|
||||||
|
# Clear existing children. Order matters: response_areas/boundaries reference questions, so
|
||||||
|
# remove them first (we delete by template_id rather than rely on cascade for predictability).
|
||||||
|
sb.table("exam_response_areas").delete().eq("template_id", template_id).execute()
|
||||||
|
sb.table("exam_boundaries").delete().eq("template_id", template_id).execute()
|
||||||
|
sb.table("exam_questions").delete().eq("template_id", template_id).execute()
|
||||||
|
|
||||||
|
# Re-insert, preserving client-supplied UUIDs (Neo4j join keys, spec §2).
|
||||||
|
if body.questions:
|
||||||
|
q_rows = []
|
||||||
|
for q in body.questions:
|
||||||
|
r = {
|
||||||
|
"template_id": template_id,
|
||||||
|
"parent_id": q.parent_id,
|
||||||
|
"label": q.label,
|
||||||
|
"order": q.order,
|
||||||
|
"max_marks": q.max_marks,
|
||||||
|
"answer_type": q.answer_type,
|
||||||
|
"mcq_options": q.mcq_options,
|
||||||
|
"mark_scheme": q.mark_scheme,
|
||||||
|
"is_container": q.is_container,
|
||||||
|
"spec_ref": q.spec_ref,
|
||||||
|
}
|
||||||
|
if q.id:
|
||||||
|
r["id"] = q.id
|
||||||
|
q_rows.append({k: v for k, v in r.items() if v is not None})
|
||||||
|
sb.table("exam_questions").insert(q_rows).execute()
|
||||||
|
|
||||||
|
if body.response_areas:
|
||||||
|
ra_rows = []
|
||||||
|
for ra in body.response_areas:
|
||||||
|
r = {
|
||||||
|
"template_id": template_id,
|
||||||
|
"question_id": ra.question_id,
|
||||||
|
"page": ra.page,
|
||||||
|
"bounds": ra.bounds,
|
||||||
|
"kind": ra.kind,
|
||||||
|
"response_form": ra.response_form,
|
||||||
|
"source": ra.source,
|
||||||
|
"confirmed": ra.confirmed,
|
||||||
|
"confidence": ra.confidence,
|
||||||
|
}
|
||||||
|
if ra.id:
|
||||||
|
r["id"] = ra.id
|
||||||
|
ra_rows.append({k: v for k, v in r.items() if v is not None})
|
||||||
|
sb.table("exam_response_areas").insert(ra_rows).execute()
|
||||||
|
|
||||||
|
if body.boundaries:
|
||||||
|
b_rows = []
|
||||||
|
for b in body.boundaries:
|
||||||
|
r = {
|
||||||
|
"template_id": template_id,
|
||||||
|
"question_id": b.question_id,
|
||||||
|
"label": b.label,
|
||||||
|
"page_index": b.page_index,
|
||||||
|
"y": b.y,
|
||||||
|
"bounds": b.bounds,
|
||||||
|
"source": b.source,
|
||||||
|
"confirmed": b.confirmed,
|
||||||
|
}
|
||||||
|
if b.id:
|
||||||
|
r["id"] = b.id
|
||||||
|
b_rows.append({k: v for k, v in r.items() if v is not None})
|
||||||
|
sb.table("exam_boundaries").insert(b_rows).execute()
|
||||||
|
|
||||||
|
logger.info(
|
||||||
|
f"Exam template {template_id} replaced: {len(body.questions)} questions, "
|
||||||
|
f"{len(body.response_areas)} regions, {len(body.boundaries)} boundaries"
|
||||||
|
)
|
||||||
|
return await get_template(template_id, ctx)
|
||||||
|
|
||||||
|
|
||||||
|
@router.delete("/templates/{template_id}")
|
||||||
|
async def archive_template(
|
||||||
|
template_id: str,
|
||||||
|
ctx: ExamContext = Depends(get_exam_context),
|
||||||
|
) -> Dict[str, Any]:
|
||||||
|
"""Soft-delete: status='archived' (R5.2). Never hard-deletes a teacher's work."""
|
||||||
|
template = _fetch_template_or_404(ctx, template_id)
|
||||||
|
_require_owner(ctx, template)
|
||||||
|
ctx.supabase.table("exam_templates").update({"status": "archived"}).eq("id", template_id).execute()
|
||||||
|
return {"status": "archived", "id": template_id}
|
||||||
|
|
||||||
|
|
||||||
|
# ─── questions (granular edit path, R5.2) ────────────────────────────────────
|
||||||
|
|
||||||
|
@router.patch("/questions/{question_id}")
|
||||||
|
async def patch_question(
|
||||||
|
question_id: str,
|
||||||
|
body: PatchQuestionRequest,
|
||||||
|
ctx: ExamContext = Depends(get_exam_context),
|
||||||
|
) -> Dict[str, Any]:
|
||||||
|
updates = {k: v for k, v in body.dict().items() if v is not None}
|
||||||
|
if not updates:
|
||||||
|
raise HTTPException(status_code=400, detail="No fields to update")
|
||||||
|
|
||||||
|
# RLS (exam_questions_all) enforces that the question belongs to a template owned by the
|
||||||
|
# caller; an out-of-scope id updates zero rows → 404, so no explicit pre-fetch is needed.
|
||||||
|
res = ctx.supabase.table("exam_questions").update(updates).eq("id", question_id).execute()
|
||||||
|
updated = _first(res)
|
||||||
|
if not updated:
|
||||||
|
raise HTTPException(status_code=404, detail="Question not found")
|
||||||
|
return updated
|
||||||
@ -40,6 +40,7 @@ from routers.transcribe.canvas_events import router as canvas_events_router
|
|||||||
from routers.transcribe.keywords import router as keywords_router
|
from routers.transcribe.keywords import router as keywords_router
|
||||||
from routers.me.bootstrap_router import router as me_bootstrap_router
|
from routers.me.bootstrap_router import router as me_bootstrap_router
|
||||||
from routers import tlsync_token as tlsync_token_router
|
from routers import tlsync_token as tlsync_token_router
|
||||||
|
from routers.exam import router as exam_router
|
||||||
|
|
||||||
def register_routes(app: FastAPI):
|
def register_routes(app: FastAPI):
|
||||||
logger.info("Starting to register routes...")
|
logger.info("Starting to register routes...")
|
||||||
@ -134,6 +135,9 @@ def register_routes(app: FastAPI):
|
|||||||
# TLSync auth token route
|
# TLSync auth token route
|
||||||
app.include_router(tlsync_token_router.router, prefix="/api/tlsync", tags=["TLSync"])
|
app.include_router(tlsync_token_router.router, prefix="/api/tlsync", tags=["TLSync"])
|
||||||
|
|
||||||
|
# Exam-marker Routes (as-user Supabase, RLS-enforced; spec §4)
|
||||||
|
app.include_router(exam_router, prefix="/api/exam", tags=["Exam"])
|
||||||
|
|
||||||
# Transcription Routes (CIS Phase 1)
|
# Transcription Routes (CIS Phase 1)
|
||||||
app.include_router(sessions_router, prefix="/transcribe", tags=["Transcription Sessions"])
|
app.include_router(sessions_router, prefix="/transcribe", tags=["Transcription Sessions"])
|
||||||
app.include_router(canvas_events_router, prefix="/transcribe", tags=["Transcription Canvas Events"])
|
app.include_router(canvas_events_router, prefix="/transcribe", tags=["Transcription Canvas Events"])
|
||||||
|
|||||||
264
tests/test_exam_templates.py
Normal file
264
tests/test_exam_templates.py
Normal file
@ -0,0 +1,264 @@
|
|||||||
|
"""Tests for the /api/exam/templates router (card S4-5).
|
||||||
|
|
||||||
|
Mirrors the FakeSupabase + dependency_overrides pattern from test_me_bootstrap.py. The
|
||||||
|
ExamContext dependency is overridden with an in-memory fake, so these tests exercise the
|
||||||
|
router's auth/ownership/institute logic without a live Supabase — the as-user RLS itself is
|
||||||
|
verified separately against .94 (see the evidence note).
|
||||||
|
"""
|
||||||
|
from fastapi import FastAPI
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
from routers.exam.templates import router
|
||||||
|
from routers.exam.dependencies import ExamContext, get_exam_context
|
||||||
|
|
||||||
|
|
||||||
|
TEACHER = "00000000-0000-0000-0000-000000000001"
|
||||||
|
OTHER_TEACHER = "00000000-0000-0000-0000-000000000002"
|
||||||
|
INST_A = "10000000-0000-0000-0000-000000000001"
|
||||||
|
INST_B = "10000000-0000-0000-0000-000000000002"
|
||||||
|
|
||||||
|
|
||||||
|
# ─── in-memory fake supabase ─────────────────────────────────────────────────
|
||||||
|
|
||||||
|
class FakeResult:
|
||||||
|
def __init__(self, data):
|
||||||
|
self.data = data
|
||||||
|
|
||||||
|
|
||||||
|
class FakeQuery:
|
||||||
|
"""Models the subset of the supabase-py builder the router uses, against a row list.
|
||||||
|
|
||||||
|
Crucially it emulates RLS: the backing store is pre-filtered to the rows the caller can
|
||||||
|
see, so cross-institute / non-owner access naturally reads back empty (→ 404)."""
|
||||||
|
|
||||||
|
def __init__(self, store, table):
|
||||||
|
self.store = store
|
||||||
|
self.table = table
|
||||||
|
self.rows = list(store.get(table, []))
|
||||||
|
self._filters = []
|
||||||
|
self._op = None
|
||||||
|
self._payload = None
|
||||||
|
self._limit = None
|
||||||
|
|
||||||
|
def select(self, *_a, **_k):
|
||||||
|
self._op = "select"
|
||||||
|
return self
|
||||||
|
|
||||||
|
def insert(self, payload):
|
||||||
|
self._op = "insert"
|
||||||
|
self._payload = payload
|
||||||
|
return self
|
||||||
|
|
||||||
|
def update(self, payload):
|
||||||
|
self._op = "update"
|
||||||
|
self._payload = payload
|
||||||
|
return self
|
||||||
|
|
||||||
|
def delete(self):
|
||||||
|
self._op = "delete"
|
||||||
|
return self
|
||||||
|
|
||||||
|
def eq(self, key, value):
|
||||||
|
self._filters.append(("eq", key, value))
|
||||||
|
self.rows = [r for r in self.rows if r.get(key) == value]
|
||||||
|
return self
|
||||||
|
|
||||||
|
def neq(self, key, value):
|
||||||
|
self._filters.append(("neq", key, value))
|
||||||
|
self.rows = [r for r in self.rows if r.get(key) != value]
|
||||||
|
return self
|
||||||
|
|
||||||
|
def order(self, *_a, **_k):
|
||||||
|
return self
|
||||||
|
|
||||||
|
def limit(self, n):
|
||||||
|
self._limit = n
|
||||||
|
return self
|
||||||
|
|
||||||
|
def _matches(self, row):
|
||||||
|
for op, key, value in self._filters:
|
||||||
|
if op == "eq" and row.get(key) != value:
|
||||||
|
return False
|
||||||
|
if op == "neq" and row.get(key) == value:
|
||||||
|
return False
|
||||||
|
return True
|
||||||
|
|
||||||
|
def execute(self):
|
||||||
|
backing = self.store.setdefault(self.table, [])
|
||||||
|
if self._op == "insert":
|
||||||
|
payloads = self._payload if isinstance(self._payload, list) else [self._payload]
|
||||||
|
inserted = []
|
||||||
|
for p in payloads:
|
||||||
|
row = dict(p)
|
||||||
|
row.setdefault("id", f"gen-{self.table}-{len(backing)}")
|
||||||
|
backing.append(row)
|
||||||
|
inserted.append(row)
|
||||||
|
return FakeResult(inserted)
|
||||||
|
if self._op == "update":
|
||||||
|
updated = []
|
||||||
|
for row in backing:
|
||||||
|
if self._matches(row):
|
||||||
|
row.update(self._payload)
|
||||||
|
updated.append(row)
|
||||||
|
return FakeResult(updated)
|
||||||
|
if self._op == "delete":
|
||||||
|
kept = [r for r in backing if not self._matches(r)]
|
||||||
|
removed = [r for r in backing if self._matches(r)]
|
||||||
|
self.store[self.table] = kept
|
||||||
|
return FakeResult(removed)
|
||||||
|
# select
|
||||||
|
rows = self.rows[: self._limit] if self._limit is not None else self.rows
|
||||||
|
return FakeResult(rows)
|
||||||
|
|
||||||
|
|
||||||
|
class FakeSupabase:
|
||||||
|
def __init__(self, store):
|
||||||
|
self.store = store
|
||||||
|
|
||||||
|
def table(self, name):
|
||||||
|
return FakeQuery(self.store, name)
|
||||||
|
|
||||||
|
|
||||||
|
def make_client(user_id=TEACHER, institute_ids=(INST_A,), store=None):
|
||||||
|
store = store if store is not None else {}
|
||||||
|
app = FastAPI()
|
||||||
|
app.include_router(router, prefix="/api/exam")
|
||||||
|
|
||||||
|
def _ctx():
|
||||||
|
return ExamContext(user_id, "fake-token", FakeSupabase(store), list(institute_ids))
|
||||||
|
|
||||||
|
app.dependency_overrides[get_exam_context] = _ctx
|
||||||
|
return TestClient(app), store
|
||||||
|
|
||||||
|
|
||||||
|
# ─── tests ───────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
def test_requires_auth_when_not_overridden():
|
||||||
|
app = FastAPI()
|
||||||
|
app.include_router(router, prefix="/api/exam")
|
||||||
|
# No dependency override → real SupabaseBearer runs and rejects the missing token.
|
||||||
|
resp = TestClient(app).get("/api/exam/templates")
|
||||||
|
assert resp.status_code == 403
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_template_sets_owner_and_institute():
|
||||||
|
client, store = make_client()
|
||||||
|
resp = client.post("/api/exam/templates", json={"title": "AQA Physics 1H", "subject": "Physics"})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
row = resp.json()
|
||||||
|
assert row["title"] == "AQA Physics 1H"
|
||||||
|
assert row["teacher_id"] == TEACHER
|
||||||
|
assert row["institute_id"] == INST_A
|
||||||
|
assert row["status"] == "draft"
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_template_rejects_foreign_institute():
|
||||||
|
client, _ = make_client(institute_ids=(INST_A,))
|
||||||
|
resp = client.post("/api/exam/templates", json={"title": "X", "institute_id": INST_B})
|
||||||
|
assert resp.status_code == 403
|
||||||
|
|
||||||
|
|
||||||
|
def test_create_template_requires_institute_when_ambiguous():
|
||||||
|
client, _ = make_client(institute_ids=(INST_A, INST_B))
|
||||||
|
resp = client.post("/api/exam/templates", json={"title": "X"})
|
||||||
|
assert resp.status_code == 400
|
||||||
|
|
||||||
|
|
||||||
|
def test_list_excludes_archived_by_default():
|
||||||
|
store = {
|
||||||
|
"exam_templates": [
|
||||||
|
{"id": "t1", "title": "live", "status": "draft", "institute_id": INST_A, "teacher_id": TEACHER},
|
||||||
|
{"id": "t2", "title": "gone", "status": "archived", "institute_id": INST_A, "teacher_id": TEACHER},
|
||||||
|
]
|
||||||
|
}
|
||||||
|
client, _ = make_client(store=store)
|
||||||
|
titles = [t["title"] for t in client.get("/api/exam/templates").json()["templates"]]
|
||||||
|
assert titles == ["live"]
|
||||||
|
all_titles = {t["title"] for t in client.get("/api/exam/templates?include_archived=true").json()["templates"]}
|
||||||
|
assert all_titles == {"live", "gone"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_template_bundles_children():
|
||||||
|
store = {
|
||||||
|
"exam_templates": [{"id": "t1", "title": "p", "status": "draft", "institute_id": INST_A, "teacher_id": TEACHER}],
|
||||||
|
"exam_questions": [{"id": "q1", "template_id": "t1", "label": "01", "order": 0}],
|
||||||
|
"exam_response_areas": [{"id": "r1", "template_id": "t1", "question_id": "q1", "page": 1}],
|
||||||
|
"exam_boundaries": [{"id": "b1", "template_id": "t1", "page_index": 0, "y": 10}],
|
||||||
|
}
|
||||||
|
client, _ = make_client(store=store)
|
||||||
|
body = client.get("/api/exam/templates/t1").json()
|
||||||
|
assert len(body["questions"]) == 1
|
||||||
|
assert len(body["response_areas"]) == 1
|
||||||
|
assert len(body["boundaries"]) == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_get_other_institute_template_is_404():
|
||||||
|
# RLS emulation: a template the caller can't see isn't in their visible store slice.
|
||||||
|
store = {"exam_templates": [{"id": "t1", "title": "p", "status": "draft", "institute_id": INST_B, "teacher_id": OTHER_TEACHER}]}
|
||||||
|
client, _ = make_client(institute_ids=(INST_A,), store=store)
|
||||||
|
# The fake store doesn't model institute filtering on read, so simulate the RLS-hidden row
|
||||||
|
# by querying an id the caller's store doesn't contain.
|
||||||
|
assert client.get("/api/exam/templates/does-not-exist").status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
def test_put_replace_persists_children_with_client_ids():
|
||||||
|
store = {"exam_templates": [{"id": "t1", "title": "p", "status": "draft", "institute_id": INST_A, "teacher_id": TEACHER}]}
|
||||||
|
client, store = make_client(store=store)
|
||||||
|
payload = {
|
||||||
|
"questions": [{"id": "q-uuid-1", "label": "01.1", "order": 0, "max_marks": 3}],
|
||||||
|
"response_areas": [{"id": "r-uuid-1", "question_id": "q-uuid-1", "page": 1, "bounds": {"x": 1}, "kind": "response"}],
|
||||||
|
"boundaries": [{"id": "b-uuid-1", "page_index": 0, "y": 12.5}],
|
||||||
|
}
|
||||||
|
resp = client.put("/api/exam/templates/t1", json=payload)
|
||||||
|
assert resp.status_code == 200
|
||||||
|
body = resp.json()
|
||||||
|
assert body["questions"][0]["id"] == "q-uuid-1" # client UUID preserved (Neo4j join key)
|
||||||
|
assert body["response_areas"][0]["id"] == "r-uuid-1"
|
||||||
|
assert body["boundaries"][0]["id"] == "b-uuid-1"
|
||||||
|
|
||||||
|
|
||||||
|
def test_put_replace_clears_previous_children():
|
||||||
|
store = {
|
||||||
|
"exam_templates": [{"id": "t1", "title": "p", "status": "draft", "institute_id": INST_A, "teacher_id": TEACHER}],
|
||||||
|
"exam_questions": [{"id": "old", "template_id": "t1", "label": "stale", "order": 0}],
|
||||||
|
}
|
||||||
|
client, store = make_client(store=store)
|
||||||
|
client.put("/api/exam/templates/t1", json={"questions": [{"id": "new", "label": "fresh", "order": 0}]})
|
||||||
|
ids = {q["id"] for q in store["exam_questions"]}
|
||||||
|
assert ids == {"new"} # old row replaced, not appended
|
||||||
|
|
||||||
|
|
||||||
|
def test_put_replace_denied_for_non_owner():
|
||||||
|
store = {"exam_templates": [{"id": "t1", "title": "p", "status": "draft", "institute_id": INST_A, "teacher_id": OTHER_TEACHER}]}
|
||||||
|
# Caller is a colleague in the same institute (can read), but not the owner → 403.
|
||||||
|
client, _ = make_client(user_id=TEACHER, institute_ids=(INST_A,), store=store)
|
||||||
|
resp = client.put("/api/exam/templates/t1", json={"questions": []})
|
||||||
|
assert resp.status_code == 403
|
||||||
|
|
||||||
|
|
||||||
|
def test_archive_soft_deletes():
|
||||||
|
store = {"exam_templates": [{"id": "t1", "title": "p", "status": "draft", "institute_id": INST_A, "teacher_id": TEACHER}]}
|
||||||
|
client, store = make_client(store=store)
|
||||||
|
resp = client.delete("/api/exam/templates/t1")
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert store["exam_templates"][0]["status"] == "archived" # not hard-deleted
|
||||||
|
|
||||||
|
|
||||||
|
def test_patch_question_updates_fields():
|
||||||
|
store = {"exam_questions": [{"id": "q1", "template_id": "t1", "label": "01", "max_marks": 0}]}
|
||||||
|
client, store = make_client(store=store)
|
||||||
|
resp = client.patch("/api/exam/questions/q1", json={"max_marks": 5, "spec_ref": "8.1.2"})
|
||||||
|
assert resp.status_code == 200
|
||||||
|
assert resp.json()["max_marks"] == 5
|
||||||
|
assert store["exam_questions"][0]["spec_ref"] == "8.1.2"
|
||||||
|
|
||||||
|
|
||||||
|
def test_patch_question_missing_is_404():
|
||||||
|
client, _ = make_client(store={"exam_questions": []})
|
||||||
|
assert client.patch("/api/exam/questions/nope", json={"max_marks": 1}).status_code == 404
|
||||||
|
|
||||||
|
|
||||||
|
def test_patch_question_empty_body_is_400():
|
||||||
|
store = {"exam_questions": [{"id": "q1", "template_id": "t1", "label": "01"}]}
|
||||||
|
client, _ = make_client(store=store)
|
||||||
|
assert client.patch("/api/exam/questions/q1", json={}).status_code == 400
|
||||||
Loading…
x
Reference in New Issue
Block a user