From 111bdfd129bf43122ef082bfe0886db4c5692687 Mon Sep 17 00:00:00 2001 From: saa938 Date: Fri, 21 Aug 2026 09:53:02 -0700 Subject: [PATCH 1/3] Give FRQ templates real prompt and rubric data; fix authoring and test taking FRQTemplate carried neither prompt content nor a rubric, so the editor had nothing to persist and the test renderer fell back to hardcoded AP Human Geography content for every FRQ. This adds the missing data model and fixes the authoring and test-taking paths that depend on it. Data model - types/frq.ts: add criteria, prompt, answerType, status, timeLimitMinutes - add src/lib/frq/template.ts to normalize legacy and partial template docs so older documents keep loading Authoring and taking - editorRenderer/editorFooter: actually persist questions and rubric - testRenderer: render the real prompt and rubric, and persist drafts - admin subject page: surface load errors, clean up references to deleted units, and tolerate partial fetch failures - subject page, sidebar layout, and sidebar: add the missing FRQ links - remove components/global/frqFooter.tsx, which had no remaining references Rules and tooling - firestore.rules: null-guard resource in the FRQ collections, where a missing document raised an evaluation error that surfaced to the client as "insufficient permissions"; let graders read templates, since the rubric and prompt they score against live there - firestore.rules: also add the graded-frqs grades key here so the whole rules file lands in one PR. It is declarative and unused until the grading PR that builds on this branch. - package.json: add deploy:rules --- firestore.rules | 15 +- package.json | 3 +- .../subject/[slug]/[unit]/frq/[id]/page.tsx | 80 ++- src/app/admin/subject/[slug]/page.tsx | 99 +++- .../(no-sidebar)/[unit]/frq/[id]/page.tsx | 90 +-- src/app/subject/[slug]/(sidebar)/layout.tsx | 48 +- src/app/subject/[slug]/(sidebar)/page.tsx | 64 ++- src/components/frq/editorFooter.tsx | 151 ++--- src/components/frq/editorRenderer.tsx | 528 +++++++++--------- src/components/frq/testRenderer.tsx | 427 +++++++------- src/components/global/frqFooter.tsx | 98 ---- src/components/subject/subject-sidebar.tsx | 36 +- src/lib/frq/template.ts | 193 +++++++ src/types/frq.ts | 71 ++- 14 files changed, 1093 insertions(+), 810 deletions(-) delete mode 100644 src/components/global/frqFooter.tsx create mode 100644 src/lib/frq/template.ts diff --git a/firestore.rules b/firestore.rules index 9fb850bf..1edc8124 100644 --- a/firestore.rules +++ b/firestore.rules @@ -76,8 +76,11 @@ service cloud.firestore { match /ungraded-frqs/{submissionId} { + // `resource` is null when the document does not exist, and dereferencing it + // raises an evaluation error rather than denying, which surfaces to the + // client as "insufficient permissions" for what is really a missing doc. allow get, list: if isGraderOrAdmin() || - (isAuthenticated() && resource.data.studentId == request.auth.uid); + (isAuthenticated() && resource != null && resource.data.studentId == request.auth.uid); allow create: if isAuthenticated() && request.resource.data.keys().hasOnly([ @@ -102,7 +105,7 @@ service cloud.firestore { match /graded-frqs/{resultId} { allow get, list: if isGraderOrAdmin() || - (isAuthenticated() && resource.data.studentId == request.auth.uid); + (isAuthenticated() && resource != null && resource.data.studentId == request.auth.uid); allow create: if isGraderOrAdmin() && resultId == request.resource.data.sourceSubmissionId @@ -116,9 +119,11 @@ service cloud.firestore { 'submittedAt', 'score', 'feedback', + 'grades', 'graderId', 'gradedAt' ]) + && request.resource.data.grades is list && request.resource.data.sourceSubmissionId is string && request.resource.data.templateId is string && request.resource.data.subject is string @@ -156,8 +161,12 @@ service cloud.firestore { allow write: if isMemberOrAdmin(); } + // Graders read templates too: the rubric being scored against lives on + // the template, and so does the prompt shown beside a student's answer. + // Restricting reads to members left graders unable to open a private + // FRQ's submission at all. match /frqs/{frq} { - allow read: if resource.data.isPublic == true || isMemberOrAdmin(); + allow read: if (resource != null && resource.data.isPublic == true) || isGraderOrMemberOrAdmin(); allow write: if isMemberOrAdmin(); } } diff --git a/package.json b/package.json index 37713219..88b388ea 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,8 @@ "dev": "next dev", "lint": "next lint", "start": "next start", - "emulate": "firebase emulators:start --import emulator --export-on-exit" + "emulate": "firebase emulators:start --import emulator --export-on-exit", + "deploy:rules": "firebase deploy --only firestore:rules,firestore:indexes" }, "dependencies": { "@editorjs/attaches": "^1.3.0", diff --git a/src/app/admin/subject/[slug]/[unit]/frq/[id]/page.tsx b/src/app/admin/subject/[slug]/[unit]/frq/[id]/page.tsx index 990f7845..43246005 100644 --- a/src/app/admin/subject/[slug]/[unit]/frq/[id]/page.tsx +++ b/src/app/admin/subject/[slug]/[unit]/frq/[id]/page.tsx @@ -1,7 +1,9 @@ "use client"; import FRQEditorRenderer from "@/components/frq/editorRenderer"; +import { useUser } from "@/components/hooks/UserContext"; import { getFrqTemplateDocRef } from "@/lib/firestore/frqRefs"; +import { normalizeFrqTemplate } from "@/lib/frq/template"; import type { FRQTemplate } from "@/types/frq"; import { getDoc } from "firebase/firestore"; import { usePathname } from "next/navigation"; @@ -9,49 +11,67 @@ import { useEffect, useState } from "react"; const Page = () => { const pathname = usePathname() ?? ""; + const { user, loading: userLoading } = useUser(); const pathParts = pathname.split("/").slice(-4); const subject = pathParts[0] ?? ""; const unitId = pathParts[1] ?? ""; const frqId = pathParts[3] ?? ""; - const [frqTemplate, setFrqTemplate] = - useState(null); + const [frqTemplate, setFrqTemplate] = useState(null); const [isLoading, setIsLoading] = useState(true); - const [frqFound, setFrqFound] = useState(false); + const [loadError, setLoadError] = useState(null); + + // Firestore rules are the real gate, but without a UI check an unauthorized + // visitor gets the full editor and only discovers the denial when Save fails. + const canEdit = user?.access === "admin" || user?.access === "member"; useEffect(() => { + if (userLoading) { + return; + } + + if (!canEdit) { + setIsLoading(false); + return; + } + if (!subject || !unitId || !frqId) { - setFrqFound(false); + setLoadError("This FRQ address is not valid."); setIsLoading(false); return; } const loadFrq = async () => { + setIsLoading(true); + setLoadError(null); + try { - const docRef = getFrqTemplateDocRef( - subject, - unitId, - frqId, + const docSnap = await getDoc( + getFrqTemplateDocRef(subject, unitId, frqId), ); - const docSnap = await getDoc(docRef); - if (!docSnap.exists()) { - setFrqFound(false); + setLoadError("This FRQ no longer exists."); setFrqTemplate(null); return; } - const loadedFrq: FRQTemplate = { - id: docSnap.id, - ...(docSnap.data() as Omit), - }; + setFrqTemplate( + normalizeFrqTemplate(docSnap.data(), { + id: docSnap.id, + subject, + unitId, + }), + ); + } catch (error) { + console.error("Error loading FRQ template:", error); - setFrqTemplate(loadedFrq); - setFrqFound(true); - } catch { - setFrqFound(false); + setLoadError( + error instanceof Error + ? `Could not load this FRQ: ${error.message}` + : "Could not load this FRQ.", + ); setFrqTemplate(null); } finally { setIsLoading(false); @@ -59,18 +79,30 @@ const Page = () => { }; void loadFrq(); - }, [subject, unitId, frqId]); + }, [userLoading, canEdit, subject, unitId, frqId]); + + if (userLoading || isLoading) { + return
Loading...
; + } + + if (!canEdit) { + return ( +
+ You need porter or admin access to edit FRQs. +
+ ); + } - if (isLoading) { - return
Loading...
; + if (loadError) { + return
{loadError}
; } return ( ); }; -export default Page; \ No newline at end of file +export default Page; diff --git a/src/app/admin/subject/[slug]/page.tsx b/src/app/admin/subject/[slug]/page.tsx index b444a376..f3ae5cca 100644 --- a/src/app/admin/subject/[slug]/page.tsx +++ b/src/app/admin/subject/[slug]/page.tsx @@ -107,18 +107,29 @@ export default function Page({ params }: { params: { slug: string } }) { const frqSnapshots = await Promise.all( fetchedUnits.map(async (unit) => { - const snapshot = await getDocs( - getFrqTemplatesCollectionRef(params.slug, unit.id), - ); - - return snapshot.docs.map( - (frqDoc): FRQTemplate => ({ - id: frqDoc.id, - ...(frqDoc.data() as Omit), - subject: params.slug, - unitId: unit.id, - }), - ); + // One unreadable FRQ subcollection must not blank the whole + // subject editor; the units and chapters loaded fine. + try { + const snapshot = await getDocs( + getFrqTemplatesCollectionRef(params.slug, unit.id), + ); + + return snapshot.docs.map( + (frqDoc): FRQTemplate => ({ + id: frqDoc.id, + ...(frqDoc.data() as Omit), + subject: params.slug, + unitId: unit.id, + }), + ); + } catch (frqError) { + console.error( + `Unable to load FRQs for unit ${unit.id}:`, + frqError, + ); + + return []; + } }), ); @@ -251,8 +262,16 @@ const handleAddFrq = async ( ]); setUnsavedChanges(true); - } catch { - alert("Unable to add the FRQ. Please try again."); + } catch (error) { + // Discarding this error is what made a permission-denied rule look + // identical to a network blip, so the real cause never reached anyone. + console.error("Error adding FRQ:", error); + + alert( + error instanceof Error + ? `Unable to add the FRQ: ${error.message}` + : "Unable to add the FRQ. Please try again.", + ); } }; @@ -291,8 +310,14 @@ const handleRenameFrq = async ( ); setUnsavedChanges(true); - } catch { - alert("Unable to rename the FRQ. Please try again."); + } catch (error) { + console.error("Error renaming FRQ:", error); + + alert( + error instanceof Error + ? `Unable to rename the FRQ: ${error.message}` + : "Unable to rename the FRQ. Please try again.", + ); } }; @@ -325,8 +350,14 @@ const handleFrqVisibilityChange = async ( ); setUnsavedChanges(true); - } catch { - alert("Unable to update the FRQ visibility. Please try again."); + } catch (error) { + console.error("Error updating FRQ visibility:", error); + + alert( + error instanceof Error + ? `Unable to update the FRQ visibility: ${error.message}` + : "Unable to update the FRQ visibility. Please try again.", + ); } }; /**************************************************** @@ -429,6 +460,38 @@ const handleFrqVisibilityChange = async ( // 1. Save the main subject doc batch.set(doc(db, "subjects", params.slug), subjectToSave); + // 1b. Remove units that were deleted locally. The loop below only visits + // units that still exist, so a deleted unit previously kept its document + // and every chapter, test, and FRQ underneath it. The orphaned FRQs are + // the visible symptom: they stay readable at their old URL and keep + // appearing in the grading queue's template lookups. + const unitsCollectionRef = collection( + db, + "subjects", + params.slug, + "units", + ); + const existingUnitsSnap = await getDocs(unitsCollectionRef); + const localUnitIds = new Set(subjectToSave.units.map((u) => u.id)); + + for (const unitDoc of existingUnitsSnap.docs) { + if (localUnitIds.has(unitDoc.id)) { + continue; + } + + for (const subcollection of ["chapters", "tests", "frqs"]) { + const staleDocs = await getDocs( + collection(unitDoc.ref, subcollection), + ); + + staleDocs.forEach((staleDoc) => { + batch.delete(staleDoc.ref); + }); + } + + batch.delete(unitDoc.ref); + } + // 2. For each Unit, update or create the unit doc, then manage sub-collections for (const unit of subjectToSave.units) { // Set (upsert) the Unit itself diff --git a/src/app/subject/[slug]/(no-sidebar)/[unit]/frq/[id]/page.tsx b/src/app/subject/[slug]/(no-sidebar)/[unit]/frq/[id]/page.tsx index a16439c7..b0112f10 100644 --- a/src/app/subject/[slug]/(no-sidebar)/[unit]/frq/[id]/page.tsx +++ b/src/app/subject/[slug]/(no-sidebar)/[unit]/frq/[id]/page.tsx @@ -3,23 +3,31 @@ import usePathname from "@/components/client/pathname"; import FRQTestRenderer from "@/components/frq/testRenderer"; import { getFrqTemplateDocRef } from "@/lib/firestore/frqRefs"; +import { normalizeFrqTemplate } from "@/lib/frq/template"; +import type { FRQTemplate } from "@/types/frq"; import { getDoc } from "firebase/firestore"; import { useEffect, useState } from "react"; +/** + * The unit segment is built as `unit-{displayNumber}-{unitId}`, so the id is + * everything after the second dash rather than the last dash-delimited chunk — + * that keeps working if a unit id ever contains a dash. + */ +const parseUnitId = (unitSegment: string | undefined) => { + const parts = (unitSegment ?? "").split("-"); + + return parts.length > 2 ? parts.slice(2).join("-") : ""; +}; + const Page = () => { const pathname = usePathname(); - const basePath = pathname - .split("/") - .filter(Boolean) - .slice(-4) - .join("_"); + const pathParts = pathname.split("/").filter(Boolean).slice(-4); + const subject = pathParts[0] ?? ""; + const unitId = parseUnitId(pathParts[1]); + const frqId = pathParts[3] ?? ""; - const subject = basePath.split("_")[0]!; - const unitId = basePath.split("_")[1]?.split("-").at(-1); - const frqId = basePath.split("_")[3]!; - - const [frq, setFrq] = useState | null>(null); + const [template, setTemplate] = useState(null); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); @@ -27,35 +35,39 @@ const Page = () => { const fetchFRQ = async () => { setLoading(true); setError(null); - setFrq(null); + setTemplate(null); - try { - if (!subject || !unitId || !frqId) { - setError("Invalid FRQ route."); - return; - } + if (!subject || !unitId || !frqId) { + setError("This FRQ address is not valid."); + setLoading(false); + return; + } - const docRef = getFrqTemplateDocRef( - subject, - unitId, - frqId, + try { + const docSnap = await getDoc( + getFrqTemplateDocRef(subject, unitId, frqId), ); - const docSnap = await getDoc(docRef); + if (!docSnap.exists()) { + setError("FRQ not found."); + return; + } - if (docSnap.exists()) { - setFrq({ + setTemplate( + normalizeFrqTemplate(docSnap.data(), { id: docSnap.id, - ...docSnap.data(), subject, unitId, - }); - } else { - setFrq(null); - } - } catch (error: unknown) { - console.error("Error fetching FRQ data:", error); - setError("Error fetching FRQ data."); + }), + ); + } catch (fetchError: unknown) { + console.error("Error fetching FRQ data:", fetchError); + + setError( + fetchError instanceof Error + ? `Error fetching FRQ data: ${fetchError.message}` + : "Error fetching FRQ data.", + ); } finally { setLoading(false); } @@ -64,19 +76,9 @@ const Page = () => { void fetchFRQ(); }, [subject, unitId, frqId]); - if (loading) { - return
Loading...
; - } - - if (error) { - return
{error}
; - } - - if (!frq) { - return
FRQ not found.
; - } - - return ; + return ( + + ); }; export default Page; diff --git a/src/app/subject/[slug]/(sidebar)/layout.tsx b/src/app/subject/[slug]/(sidebar)/layout.tsx index dd37cfa0..3767358b 100644 --- a/src/app/subject/[slug]/(sidebar)/layout.tsx +++ b/src/app/subject/[slug]/(sidebar)/layout.tsx @@ -7,7 +7,7 @@ import { useEffect, useState } from "react"; import { type Subject } from "@/types/firestore"; import { db } from "@/lib/firebase"; -import { doc, getDoc } from "firebase/firestore"; +import { collection, doc, getDoc, getDocs, query, where } from "firebase/firestore"; import { useUser } from "@/components/hooks/UserContext"; export default function Layout({ @@ -35,7 +35,51 @@ export default function Layout({ const docRef = doc(db, "subjects", params.slug); const docSnap = await getDoc(docRef); if (docSnap.exists()) { - setSubject(docSnap.data() as Subject); + const subjectData = docSnap.data() as Subject; + + const canPreview = + user?.access === "admin" || user?.access === "member"; + + // The sidebar is the only navigation on chapter and test pages, so it + // needs the unit's FRQs too — otherwise an FRQ is reachable only from + // the subject landing page. + const unitsWithFrqs = await Promise.all( + subjectData.units.map(async (unit) => { + try { + const frqsCollectionRef = collection( + db, + "subjects", + params.slug, + "units", + unit.id, + "frqs", + ); + + const frqsSnapshot = await getDocs( + canPreview + ? frqsCollectionRef + : query(frqsCollectionRef, where("isPublic", "==", true)), + ); + + return { + ...unit, + frqs: frqsSnapshot.docs.map((frqDoc) => ({ + ...frqDoc.data(), + id: frqDoc.id, + })), + }; + } catch (frqError) { + console.error( + `Unable to load FRQs for unit ${unit.id}:`, + frqError, + ); + + return { ...unit, frqs: [] }; + } + }), + ); + + setSubject({ ...subjectData, units: unitsWithFrqs }); } } catch (error) { console.error("Error fetching subject data:", error); diff --git a/src/app/subject/[slug]/(sidebar)/page.tsx b/src/app/subject/[slug]/(sidebar)/page.tsx index 4d74a2d3..94717080 100644 --- a/src/app/subject/[slug]/(sidebar)/page.tsx +++ b/src/app/subject/[slug]/(sidebar)/page.tsx @@ -50,30 +50,46 @@ const Page = ({ params }: { params: { slug: string } }) => { const unitsWithFrqs = await Promise.all( subjectData.units.map(async (unit) => { - const frqsCollectionRef = collection( - db, - "subjects", - params.slug, - "units", - unit.id, - "frqs", - ); - - const frqsQuery = canPreview - ? frqsCollectionRef - : query(frqsCollectionRef, where("isPublic", "==", true)); - - const frqsSnapshot = await getDocs(frqsQuery); - - const frqs = frqsSnapshot.docs.map((frqDoc) => ({ - ...frqDoc.data(), - id: frqDoc.id, - })); - - return { - ...unit, - frqs, - }; + // FRQs are supplementary to the curriculum. A failure here — a + // rules change that has not been deployed, an offline read — + // must not take down the whole subject page, which is what an + // unguarded rejection inside Promise.all did for every visitor. + try { + const frqsCollectionRef = collection( + db, + "subjects", + params.slug, + "units", + unit.id, + "frqs", + ); + + const frqsQuery = canPreview + ? frqsCollectionRef + : query(frqsCollectionRef, where("isPublic", "==", true)); + + const frqsSnapshot = await getDocs(frqsQuery); + + const frqs = frqsSnapshot.docs.map((frqDoc) => ({ + ...frqDoc.data(), + id: frqDoc.id, + })); + + return { + ...unit, + frqs, + }; + } catch (frqError) { + console.error( + `Unable to load FRQs for unit ${unit.id}:`, + frqError, + ); + + return { + ...unit, + frqs: [], + }; + } }), ); diff --git a/src/components/frq/editorFooter.tsx b/src/components/frq/editorFooter.tsx index de83390e..cbd2f077 100644 --- a/src/components/frq/editorFooter.tsx +++ b/src/components/frq/editorFooter.tsx @@ -1,155 +1,104 @@ "use client"; -import { Button } from "@/components/ui/button"; import { Popover, PopoverContent, PopoverTrigger, } from "@/components/ui/popover"; -import { ChevronUp, MapPin, Plus, Trash2 } from "lucide-react"; +import { ChevronUp } from "lucide-react"; import { useState } from "react"; -type BatchVisibility = "public" | "private"; +type FRQVisibility = "public" | "private"; -interface FRQNavigationItem { +interface FRQPartSummary { id: string; - title: string; + label: string; } interface FRQEditorFooterProps { - frqs: FRQNavigationItem[]; - currentFrqIndex: number; - batchName: string; - batchVisibility: BatchVisibility; - onCreateFrq: () => void; - onDeleteFrq: () => void; - onSelectFrq: (index: number) => void; - onPrevious: () => void; - onNext: () => void; + parts: FRQPartSummary[]; + frqName: string; + visibility: FRQVisibility; + hasUnsavedChanges: boolean; } +/** + * Status bar for the FRQ editor. Each FRQ is its own Firestore document, so + * this navigates the parts of the open FRQ rather than a batch of FRQs; adding + * and removing whole FRQs belongs to the admin subject page that owns the list. + */ const FRQEditorFooter = ({ - frqs, - currentFrqIndex, - batchName, - batchVisibility, - onCreateFrq, - onDeleteFrq, - onSelectFrq, - onPrevious, - onNext, + parts, + frqName, + visibility, + hasUnsavedChanges, }: FRQEditorFooterProps) => { - // Controlled so picking an FRQ closes the panel instead of leaving it parked + // Controlled so picking a part closes the panel instead of leaving it parked // over the footer, matching frq/FRQFooter.tsx on the other FRQ pages. const [navigationOpen, setNavigationOpen] = useState(false); - const selectFrq = (index: number) => { - onSelectFrq(index); + const scrollToPart = (partId: string) => { setNavigationOpen(false); + document + .querySelector(`[data-frq-part="${partId}"]`) + ?.scrollIntoView({ behavior: "smooth", block: "start" }); }; return (
-

- {batchName} +

+ {frqName}

Visibility:{" "} - {batchVisibility} + {visibility}

- - - FRQ {currentFrqIndex + 1} of {frqs.length} + {parts.length} {parts.length === 1 ? "part" : "parts"} -

Navigate to an FRQ

- -

- Select an FRQ from this batch. -

- -
- {frqs.map((frq, index) => { - const isCurrent = index === currentFrqIndex; - - return ( +

Jump to a part

+ + {parts.length === 0 ? ( +

+ This FRQ has no parts yet. +

+ ) : ( +
+ {parts.map((part) => ( - ); - })} -
+ ))} +
+ )}
- -
-
- - - +
+ {hasUnsavedChanges ? ( + Unsaved changes + ) : ( + All changes saved + )}
); }; -export default FRQEditorFooter; \ No newline at end of file +export default FRQEditorFooter; diff --git a/src/components/frq/editorRenderer.tsx b/src/components/frq/editorRenderer.tsx index 799c1690..534b9ca7 100644 --- a/src/components/frq/editorRenderer.tsx +++ b/src/components/frq/editorRenderer.tsx @@ -23,91 +23,58 @@ import { PopoverTrigger, } from "@/components/ui/popover"; import { Textarea } from "@/components/ui/textarea"; +import { getFrqTemplateDocRef } from "@/lib/firestore/frqRefs"; +import { + DEFAULT_TIME_LIMIT_MINUTES, + getPartLabel, + getQuestionPoints, + toQuestionInput, +} from "@/lib/frq/template"; import type { QuestionFormat, QuestionInput } from "@/types/questions"; -import { Clock3, Eye, Info, Plus, Save, Trash2 } from "lucide-react"; -import type { FRQTemplate, FRQTemplateQuestion } from "@/types/frq"; -import { useMemo, useState } from "react"; - -type QuestionStatus = "public" | "legacy"; -type InputType = "text" | "equation"; -type BatchVisibility = "public" | "private"; - -interface GradingCriterion { - id: string; - description: string; - points: number; -} +import { serverTimestamp, updateDoc } from "firebase/firestore"; +import { Clock3, Info, Plus, Save, Trash2 } from "lucide-react"; +import type { + FRQAnswerType, + FRQGradingCriterion, + FRQQuestionStatus, + FRQTemplate, + FRQTemplateQuestion, +} from "@/types/frq"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; interface EditorQuestion { id: string; questionData: QuestionFormat; - status: QuestionStatus; - inputType: InputType; - criteria: GradingCriterion[]; -} - -interface EditorFRQ { - id: string; - title: string; - description: QuestionInput; - questions: EditorQuestion[]; + status: FRQQuestionStatus; + answerType: FRQAnswerType; + criteria: FRQGradingCriterion[]; } -type CompatibleFrqTemplate = FRQTemplate & { - name?: string; - isVisible?: boolean; - timeLimit?: number; - timeLimitMinutes?: number; -}; - interface FRQEditorRendererProps { frqFound: boolean; frqTemplate: FRQTemplate | null; } +type SaveState = "idle" | "saving" | "saved" | "error"; + const createQuestionInput = (value = ""): QuestionInput => ({ value, files: [], }); /** - * Unique, immutable ID built from the current time plus a short random suffix, - * per the FRQ system spec. The random half is what makes it collision-safe: - * a timestamp alone repeats when several IDs are minted in the same - * millisecond. + * Unique, immutable ID built from the current time plus a short random suffix. + * The random half is what makes it collision-safe: a timestamp alone repeats + * when several IDs are minted in the same millisecond. */ const makeId = (prefix: string) => `${prefix}-${Date.now().toString(36)}-${Math.random() .toString(36) .slice(2, 8)}`; -/** - * A question's point value is the sum of its grading criteria. Criteria are the - * single source of truth so the editor can never disagree with what the grading - * page will actually award. - */ - const formatPoints = (points: number) => `${points} ${points === 1 ? "point" : "points"}`; -const getQuestionPoints = (question: EditorQuestion) => - question.criteria.reduce((total, criterion) => total + criterion.points, 0); - -const createDescriptionQuestion = ( - description: QuestionInput, -): QuestionFormat => ({ - question: { - value: description.value, - files: [...description.files], - }, - type: "frq", - options: [], - answers: [], - explanation: createQuestionInput(), - content: createQuestionInput(), - topic: "", -}); - const createQuestionData = ( question = createQuestionInput(), ): QuestionFormat => ({ @@ -124,186 +91,142 @@ const createEditorQuestion = (): EditorQuestion => ({ id: makeId("question"), questionData: createQuestionData(), status: "public", - inputType: "text", + answerType: "text", criteria: [], }); const createEditorQuestionFromTemplate = ( templateQuestion: FRQTemplateQuestion, -): EditorQuestion => { - const trimmedPrompt = templateQuestion.prompt?.trim(); - const prompt = - trimmedPrompt && trimmedPrompt.length > 0 - ? trimmedPrompt - : templateQuestion.title; - - return { - id: templateQuestion.id, - questionData: createQuestionData(createQuestionInput(prompt)), - status: "public", - inputType: "text", - criteria: [], - }; -}; - -/** - * AP-style subquestion label: 1a, 1b, 1c. Past 26 questions it rolls over to - * two letters (1aa, 1ab) rather than walking off the end of the alphabet into - * punctuation, which is what a bare String.fromCharCode(97 + index) would do. - */ -const getSubquestionLabel = (frqIndex: number, questionIndex: number) => { - let label = ""; - let remaining = questionIndex; - - do { - label = String.fromCharCode(97 + (remaining % 26)) + label; - remaining = Math.floor(remaining / 26) - 1; - } while (remaining >= 0); +): EditorQuestion => ({ + id: templateQuestion.id, + questionData: createQuestionData( + toQuestionInput(templateQuestion.prompt, templateQuestion.promptFiles), + ), + status: templateQuestion.status ?? "public", + answerType: templateQuestion.answerType ?? "text", + criteria: templateQuestion.criteria ?? [], +}); - return `${frqIndex + 1}${label}`; -}; +interface EditorState { + title: string; + description: QuestionInput; + questions: EditorQuestion[]; + timeLimitMinutes: number; + isPublic: boolean; +} -const createEditorFrqFromTemplate = (template: FRQTemplate): EditorFRQ => ({ - id: template.id ?? makeId("frq"), - title: template.title?.trim() || "Untitled FRQ", - description: createQuestionInput(template.directions ?? ""), - questions: (template.questions ?? []).map(createEditorQuestionFromTemplate), +const buildInitialState = (template: FRQTemplate | null): EditorState => ({ + title: template?.title ?? "", + description: toQuestionInput( + template?.directions, + template?.directionsFiles, + ), + questions: (template?.questions ?? []).map(createEditorQuestionFromTemplate), + timeLimitMinutes: template?.timeLimitMinutes ?? DEFAULT_TIME_LIMIT_MINUTES, + isPublic: template?.isPublic === true, }); -const createBlankEditorFrq = (position: number): EditorFRQ => ({ - id: makeId("frq"), - title: `FRQ ${position}`, - description: createQuestionInput(), - questions: [], +/** + * The exact document body a save writes, minus the server timestamp. Unsaved + * state is detected by comparing this against the last persisted version rather + * than by watching for state updates: React StrictMode double-invokes effects + * in development, and the rich-text children re-emit equal values on mount, so + * a "something changed" listener reported unsaved work before any edit. + */ +const buildTemplatePayload = (state: EditorState) => ({ + title: state.title.trim() || "Untitled FRQ", + directions: state.description.value, + directionsFiles: state.description.files, + timeLimitMinutes: state.timeLimitMinutes, + isPublic: state.isPublic, + questions: state.questions.map((question, index) => ({ + id: question.id, + title: getPartLabel(index), + prompt: question.questionData.question.value, + promptFiles: question.questionData.question.files, + answerType: question.answerType, + status: question.status, + criteria: question.criteria, + })), }); -const createEditorFrqsFromTemplate = (template: FRQTemplate): EditorFRQ[] => [ - createEditorFrqFromTemplate(template), -]; - -const getBatchName = (template: FRQTemplate | null) => { - if (!template) { - return "Untitled FRQ"; - } - - const compatibleTemplate = template as CompatibleFrqTemplate; - const practiceName = compatibleTemplate.name?.trim(); - - if (practiceName && practiceName.length > 0) { - return practiceName; - } - - const templateTitle = template.title?.trim(); - - return templateTitle && templateTitle.length > 0 - ? templateTitle - : "Untitled FRQ"; -}; - -const getBatchVisibility = (template: FRQTemplate | null): BatchVisibility => { - if (!template) { - return "private"; - } - - const compatibleTemplate = template as CompatibleFrqTemplate; - const isVisible = - typeof compatibleTemplate.isVisible === "boolean" - ? compatibleTemplate.isVisible - : template.isPublic === true; - - return isVisible ? "public" : "private"; -}; - -const getInitialTimeLimit = (template: FRQTemplate | null) => { - if (!template) { - return 90; - } - - const compatibleTemplate = template as CompatibleFrqTemplate; - const configuredLimit = - compatibleTemplate.timeLimitMinutes ?? compatibleTemplate.timeLimit; - - return typeof configuredLimit === "number" && - Number.isFinite(configuredLimit) && - configuredLimit >= 1 - ? Math.floor(configuredLimit) - : 90; -}; - const FRQEditorRenderer = ({ frqFound, frqTemplate, }: FRQEditorRendererProps) => { - const [frqs, setFrqs] = useState(() => - frqTemplate ? createEditorFrqsFromTemplate(frqTemplate) : [], + const initialState = useRef(buildInitialState(frqTemplate)).current; + + const [title, setTitle] = useState(initialState.title); + const [description, setDescription] = useState( + initialState.description, ); - const [currentFrqIndex, setCurrentFrqIndex] = useState(0); - const batchName = getBatchName(frqTemplate); - const batchVisibility = getBatchVisibility(frqTemplate); - const [timeLimitMinutes, setTimeLimitMinutes] = useState(() => - getInitialTimeLimit(frqTemplate), + const [questions, setQuestions] = useState( + initialState.questions, ); + const [timeLimitMinutes, setTimeLimitMinutes] = useState( + initialState.timeLimitMinutes, + ); + const [isPublic, setIsPublic] = useState(initialState.isPublic); - const goToPreviousFrq = () => { - setCurrentFrqIndex((index) => Math.max(index - 1, 0)); - }; - - const goToNextFrq = () => { - setCurrentFrqIndex((index) => Math.min(index + 1, frqs.length - 1)); - }; - - const updateCurrentFrq = (updater: (frq: EditorFRQ) => EditorFRQ) => { - setFrqs((currentFrqs) => - currentFrqs.map((frq, index) => - index === currentFrqIndex ? updater(frq) : frq, - ), - ); - }; + const [saveState, setSaveState] = useState("idle"); + const [saveError, setSaveError] = useState(null); + const [savedSignature, setSavedSignature] = useState(() => + JSON.stringify(buildTemplatePayload(initialState)), + ); - const createFrq = () => { - const newFrq = createBlankEditorFrq(frqs.length + 1); + const currentPayload = useMemo( + () => + buildTemplatePayload({ + title, + description, + questions, + timeLimitMinutes, + isPublic, + }), + [title, description, questions, timeLimitMinutes, isPublic], + ); - setFrqs((currentFrqs) => [...currentFrqs, newFrq]); - setCurrentFrqIndex(frqs.length); - }; + const hasUnsavedChanges = JSON.stringify(currentPayload) !== savedSignature; - const deleteCurrentFrq = () => { - if (frqs.length <= 1) { + useEffect(() => { + if (!hasUnsavedChanges) { return; } - const remainingFrqs = frqs.filter( - (_frq, index) => index !== currentFrqIndex, - ); + const warnBeforeLeaving = (event: BeforeUnloadEvent) => { + event.preventDefault(); + event.returnValue = ""; + }; - setFrqs(remainingFrqs); - setCurrentFrqIndex(Math.min(currentFrqIndex, remainingFrqs.length - 1)); - }; + window.addEventListener("beforeunload", warnBeforeLeaving); - const addQuestion = () => { - updateCurrentFrq((frq) => ({ - ...frq, - questions: [...frq.questions, createEditorQuestion()], - })); - }; + return () => { + window.removeEventListener("beforeunload", warnBeforeLeaving); + }; + }, [hasUnsavedChanges]); const updateQuestion = ( questionId: string, updater: (question: EditorQuestion) => EditorQuestion, ) => { - updateCurrentFrq((frq) => ({ - ...frq, - questions: frq.questions.map((question) => + setQuestions((currentQuestions) => + currentQuestions.map((question) => question.id === questionId ? updater(question) : question, ), - })); + ); + }; + + const addQuestion = () => { + setQuestions((currentQuestions) => [ + ...currentQuestions, + createEditorQuestion(), + ]); }; const deleteQuestion = (questionId: string) => { - updateCurrentFrq((frq) => ({ - ...frq, - questions: frq.questions.filter((question) => question.id !== questionId), - })); + setQuestions((currentQuestions) => + currentQuestions.filter((question) => question.id !== questionId), + ); }; const addCriterion = (questionId: string) => { @@ -311,11 +234,7 @@ const FRQEditorRenderer = ({ ...question, criteria: [ ...question.criteria, - { - id: makeId("criterion"), - description: "", - points: 1, - }, + { id: makeId("criterion"), description: "", points: 1 }, ], })); }; @@ -323,7 +242,7 @@ const FRQEditorRenderer = ({ const updateCriterion = ( questionId: string, criterionId: string, - changes: Partial>, + changes: Partial>, ) => { updateQuestion(questionId, (question) => ({ ...question, @@ -342,44 +261,80 @@ const FRQEditorRenderer = ({ })); }; - const currentFrq = frqs[currentFrqIndex]; + const saveTemplate = useCallback(async () => { + if (!frqTemplate?.id) { + setSaveState("error"); + setSaveError("This FRQ has no document to save to."); + return; + } - // Read through to the fields the memos actually depend on. AdvancedTextbox - // treats its `questions[qIndex]` entry as a stable reference, so these must - // keep their identity across unrelated re-renders instead of being rebuilt - // fresh every time the parent renders. Both memos sit above the early returns - // so the hooks stay unconditional. - const currentDescription = currentFrq?.description; - const currentQuestions = currentFrq?.questions; + setSaveState("saving"); + setSaveError(null); + + try { + await updateDoc( + getFrqTemplateDocRef( + frqTemplate.subject, + frqTemplate.unitId, + frqTemplate.id, + ), + { ...currentPayload, updatedAt: serverTimestamp() }, + ); + + // Re-baseline against exactly what was written, so an edit made while the + // save was in flight still registers as unsaved. + setSavedSignature(JSON.stringify(currentPayload)); + setSaveState("saved"); + } catch (error) { + // The previous version swallowed this entirely, which is why a + // permission-denied rule looked identical to a successful save. + console.error("Error saving FRQ template:", error); + + setSaveState("error"); + setSaveError( + error instanceof Error + ? error.message + : "Unknown error while saving this FRQ.", + ); + } + }, [currentPayload, frqTemplate]); + // AdvancedTextbox treats its `questions[qIndex]` entry as a stable reference, + // so these must keep identity across unrelated re-renders instead of being + // rebuilt fresh. Both memos sit above the early return so hooks stay + // unconditional. const descriptionQuestions = useMemo( - () => - currentDescription ? [createDescriptionQuestion(currentDescription)] : [], - [currentDescription], + () => [createQuestionData(description)], + [description], ); const questionFormats = useMemo( - () => currentQuestions?.map((question) => question.questionData) ?? [], - [currentQuestions], + () => questions.map((question) => question.questionData), + [questions], ); - if (!frqFound || !currentFrq) { + const totalPoints = questions.reduce( + (total, question) => + total + + getQuestionPoints({ + id: question.id, + title: "", + criteria: question.criteria, + }), + 0, + ); + + if (!frqFound || !frqTemplate) { return
Failed to load FRQ.
; } return (
-
- +
+ + {formatPoints(totalPoints)} total +
@@ -400,14 +355,32 @@ const FRQEditorRenderer = ({ minutes
-
+
+ {saveState === "saved" && !hasUnsavedChanges && ( + Saved + )} + + {saveState === "error" && ( + + {saveError} + + )} +
@@ -416,10 +389,6 @@ const FRQEditorRenderer = ({
-

- FRQ {currentFrqIndex + 1} of {frqs.length} -

-

FRQ Description

Add the source material and directions students need for this - FRQ. + FRQ. This is what students see beside the response box.

{ const updatedDescription = updatedQuestions[0]?.question; @@ -457,10 +433,7 @@ const FRQEditorRenderer = ({ return; } - updateCurrentFrq((frq) => ({ - ...frq, - description: updatedDescription, - })); + setDescription(updatedDescription); }} origin="question" qIndex={0} @@ -475,7 +448,7 @@ const FRQEditorRenderer = ({

Questions

- {currentFrq.questions.length} questions in this FRQ + {questions.length} questions in this FRQ

@@ -490,15 +463,15 @@ const FRQEditorRenderer = ({
question.id)} + defaultValue={questions.map((question) => question.id)} className="space-y-4" > - {currentFrq.questions.map((question, questionIndex) => ( + {questions.map((question, questionIndex) => ( - {getSubquestionLabel(currentFrqIndex, questionIndex)} + {getPartLabel(questionIndex)} {question.status === "legacy" && ( @@ -518,7 +491,13 @@ const FRQEditorRenderer = ({ )} - {formatPoints(getQuestionPoints(question))} + {formatPoints( + getQuestionPoints({ + id: question.id, + title: "", + criteria: question.criteria, + }), + )}
@@ -574,14 +553,14 @@ const FRQEditorRenderer = ({ variant="outline" className="mt-2 w-full justify-between" > - {question.inputType === "text" + {question.answerType === "text" ? "Text" : "Equation"} { if (value !== "text" && value !== "equation") { return; @@ -591,7 +570,7 @@ const FRQEditorRenderer = ({ question.id, (currentQuestion) => ({ ...currentQuestion, - inputType: value, + answerType: value, }), ); }} @@ -674,8 +653,8 @@ const FRQEditorRenderer = ({ A question's point total is calculated from its - grading criteria. Set its response type and status - above. + grading criteria. Graders award points against these + exact lines. @@ -698,7 +677,8 @@ const FRQEditorRenderer = ({ {question.criteria.length === 0 ? (

- No grading criteria have been added. + No grading criteria have been added. A question with + no criteria is worth zero points.

) : ( question.criteria.map((criterion, criterionIndex) => ( @@ -770,18 +750,16 @@ const FRQEditorRenderer = ({ ({ + id: question.id, + label: getPartLabel(index), + }))} + frqName={title.trim() || "Untitled FRQ"} + visibility={isPublic ? "public" : "private"} + hasUnsavedChanges={hasUnsavedChanges} />
); }; -export default FRQEditorRenderer; \ No newline at end of file +export default FRQEditorRenderer; diff --git a/src/components/frq/testRenderer.tsx b/src/components/frq/testRenderer.tsx index 1e169094..404a4e0d 100644 --- a/src/components/frq/testRenderer.tsx +++ b/src/components/frq/testRenderer.tsx @@ -1,82 +1,132 @@ "use client"; +import { RenderContent } from "@/components/article-creator/custom_questions/RenderAdvancedTextbox"; import { useUser } from "@/components/hooks/UserContext"; import FRQFooter from "@/components/frq/FRQFooter"; import FRQResponseEditor from "@/components/frq/responseEditor"; import { getUngradedFrqsCollectionRef } from "@/lib/firestore/frqRefs"; +import { + DEFAULT_TIME_LIMIT_MINUTES, + getPartLabel, + getStudentFacingQuestions, + hasResponseText, + toQuestionInput, +} from "@/lib/frq/template"; +import type { FRQTemplate } from "@/types/frq"; import { addDoc, serverTimestamp } from "firebase/firestore"; import { Bookmark, LogOut } from "lucide-react"; import { useRouter } from "next/navigation"; -import { useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; type FRQTestRendererProps = { - frq: Record | null; + template: FRQTemplate | null; loading?: boolean; error?: string | null; }; -type FRQQuestion = { - id: string; - title: string; -}; +/** + * Answers live in localStorage until they are submitted. A refresh, a closed + * laptop, or a stray back-navigation used to lose the whole attempt, and there + * is no server-side draft store to write to. + */ +const getDraftKey = (templateId: string, studentId: string) => + `frq-draft:${templateId}:${studentId}`; + +const readDraft = (draftKey: string): Record => { + try { + const stored = window.localStorage.getItem(draftKey); -const mockFRQs: FRQQuestion[] = [ - { id: "mock-demographic-transition", title: "Demographic Transition Model" }, - { id: "mock-urban-land-use", title: "Urban Land Use" }, - { id: "mock-agricultural-regions", title: "Agricultural Regions" }, -]; -const emptyQuestions: FRQQuestion[] = []; + if (!stored) { + return {}; + } -const isTemplateQuestion = (value: unknown): value is FRQQuestion => - typeof value === "object" && - value !== null && - typeof (value as FRQQuestion).id === "string" && - typeof (value as FRQQuestion).title === "string"; + const parsed: unknown = JSON.parse(stored); + + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + return {}; + } + + return Object.fromEntries( + Object.entries(parsed).filter( + (entry): entry is [string, string] => typeof entry[1] === "string", + ), + ); + } catch { + // A corrupt or unreadable draft must not block the student from starting. + return {}; + } +}; const FRQTestRenderer = ({ - frq, + template, loading = false, error = null, }: FRQTestRendererProps) => { const router = useRouter(); const { user } = useUser(); + + const questions = useMemo( + () => (template ? getStudentFacingQuestions(template) : []), + [template], + ); + const [submitting, setSubmitting] = useState(false); + const [hasSubmitted, setHasSubmitted] = useState(false); const [currentFRQIndex, setCurrentFRQIndex] = useState(0); - const templateQuestions = useMemo( - () => (Array.isArray(frq?.questions) ? frq.questions : null), - [frq], - ); - const hasInvalidTemplateQuestions = - templateQuestions !== null && - (templateQuestions.length === 0 || - !templateQuestions.every(isTemplateQuestion)); - const questions = hasInvalidTemplateQuestions - ? emptyQuestions - : (templateQuestions ?? mockFRQs); const [responses, setResponses] = useState>({}); - - useEffect(() => { - setResponses((currentResponses) => - Object.fromEntries( - questions.map((question) => [ - question.id, - currentResponses[question.id] ?? "", - ]), - ), - ); - setCurrentFRQIndex(0); - }, [questions]); const [markedForReview, setMarkedForReview] = useState< Record >({}); - const [timeRemaining, setTimeRemaining] = useState(1 * 60 * 60 + 30 * 60); + const [timeRemaining, setTimeRemaining] = useState( + () => (template?.timeLimitMinutes ?? DEFAULT_TIME_LIMIT_MINUTES) * 60, + ); const [timerHidden, setTimerHidden] = useState(false); const [showTimeUpPopup, setShowTimeUpPopup] = useState(false); const [showReviewPage, setShowReviewPage] = useState(false); const [showSubmissionModal, setShowSubmissionModal] = useState(false); + const templateId = template?.id ?? ""; + const studentId = user?.uid ?? ""; + const draftKey = + templateId && studentId ? getDraftKey(templateId, studentId) : ""; + + // Seed responses from the saved draft, then keep every question id present so + // the review grid and submission payload never have holes. + useEffect(() => { + const draft = draftKey ? readDraft(draftKey) : {}; + + setResponses( + Object.fromEntries( + questions.map((question) => [question.id, draft[question.id] ?? ""]), + ), + ); + setCurrentFRQIndex(0); + }, [questions, draftKey]); + useEffect(() => { + if (!draftKey || hasSubmitted) { + return; + } + + try { + window.localStorage.setItem(draftKey, JSON.stringify(responses)); + } catch { + // A full or disabled localStorage should not interrupt the attempt. + } + }, [draftKey, responses, hasSubmitted]); + + useEffect(() => { + setTimeRemaining( + (template?.timeLimitMinutes ?? DEFAULT_TIME_LIMIT_MINUTES) * 60, + ); + }, [template?.timeLimitMinutes]); + + useEffect(() => { + if (hasSubmitted) { + return; + } + const timer = window.setInterval(() => { setTimeRemaining((currentTime) => currentTime > 0 ? currentTime - 1 : 0, @@ -86,20 +136,66 @@ const FRQTestRenderer = ({ return () => { window.clearInterval(timer); }; - }, []); + }, [hasSubmitted]); useEffect(() => { - if (timeRemaining === 0) { + if (timeRemaining === 0 && !hasSubmitted) { setShowTimeUpPopup(true); } - }, [timeRemaining]); + }, [timeRemaining, hasSubmitted]); + + const submitForGrading = useCallback(async () => { + if (!template?.id || !user) { + window.alert("Please sign in before submitting this FRQ for grading."); + return; + } + + setSubmitting(true); + + try { + await addDoc(getUngradedFrqsCollectionRef(), { + templateId: template.id, + subject: template.subject, + unitId: template.unitId, + studentId: user.uid, + responses, + submittedAt: serverTimestamp(), + }); + + setHasSubmitted(true); + setShowSubmissionModal(false); + setShowTimeUpPopup(false); + + // Only clear the draft once the write has actually landed, so a failed + // submission still leaves the student's work recoverable. + if (draftKey) { + try { + window.localStorage.removeItem(draftKey); + } catch { + // Nothing to do: the submission already succeeded. + } + } + + window.alert("Your FRQ was submitted for grading."); + } catch (submissionError) { + console.error("Error submitting FRQ for grading:", submissionError); + + window.alert( + submissionError instanceof Error + ? `We could not submit your FRQ: ${submissionError.message}` + : "We could not submit your FRQ. Please try again.", + ); + } finally { + setSubmitting(false); + } + }, [draftKey, responses, template, user]); const formattedTime = `${Math.floor(timeRemaining / 3600)}:${String( Math.floor((timeRemaining % 3600) / 60), ).padStart(2, "0")}:${String(timeRemaining % 60).padStart(2, "0")}`; const currentFRQ = questions[currentFRQIndex]; - const hasBackendData = Boolean(frq); + const testName = template?.title ?? "FRQ"; const handleNext = () => { if (currentFRQIndex === questions.length - 1) { @@ -111,19 +207,28 @@ const FRQTestRenderer = ({ }; if (loading) { - return

Loading FRQ test...

; + return

Loading FRQ test...

; } if (error) { - return

Failed to load FRQ test.

; + return

{error}

; + } + + if (!template) { + return

FRQ test not found.

; } - if (hasInvalidTemplateQuestions) { - return

This FRQ template has invalid question identifiers.

; + if (questions.length === 0) { + return ( +

+ This FRQ has no questions yet. Check back once a porter has finished + writing it. +

+ ); } if (!currentFRQ) { - return

FRQ test not found.

; + return

FRQ test not found.

; } const timeUpModal = showTimeUpPopup ? ( @@ -153,18 +258,17 @@ const FRQTestRenderer = ({
) : null; + const downloadResponsesAsPdf = () => { const printWindow = window.open("", "_blank", "width=900,height=700"); @@ -183,7 +287,7 @@ const FRQTestRenderer = ({ const printDocument = printWindow.document; - printDocument.title = "FRQ Responses"; + printDocument.title = `${testName} Responses`; printDocument.head.replaceChildren(); printDocument.body.replaceChildren(); @@ -221,7 +325,7 @@ const FRQTestRenderer = ({ printDocument.head.appendChild(styleElement); const pageTitle = printDocument.createElement("h1"); - pageTitle.textContent = "AP Human Geography FRQ Responses"; + pageTitle.textContent = `${testName} Responses`; printDocument.body.appendChild(pageTitle); questions.forEach((question, index) => { @@ -229,7 +333,7 @@ const FRQTestRenderer = ({ section.className = "response"; const heading = printDocument.createElement("h2"); - heading.textContent = `FRQ ${index + 1}: ${question.title}`; + heading.textContent = `Part ${getPartLabel(index)}`; const responseText = printDocument.createElement("p"); responseText.textContent = getPlainText(responses[question.id] ?? ""); @@ -245,38 +349,6 @@ const FRQTestRenderer = ({ }, 250); }; - const submitForGrading = async () => { - const templateId = typeof frq?.id === "string" ? frq.id : null; - const subject = typeof frq?.subject === "string" ? frq.subject : null; - const unitId = typeof frq?.unitId === "string" ? frq.unitId : null; - - if (!user || !templateId || !subject || !unitId) { - window.alert("Please sign in before submitting this FRQ for grading."); - return; - } - - setSubmitting(true); - - try { - await addDoc(getUngradedFrqsCollectionRef(), { - templateId, - subject, - unitId, - studentId: user.uid, - responses, - submittedAt: serverTimestamp(), - }); - - setShowSubmissionModal(false); - window.alert("Your FRQ was submitted for grading."); - } catch (submissionError) { - console.error("Error submitting FRQ for grading:", submissionError); - window.alert("We could not submit your FRQ. Please try again."); - } finally { - setSubmitting(false); - } - }; - const submissionModal = showSubmissionModal ? (
void submitForGrading()} disabled={submitting} > @@ -332,6 +404,27 @@ const FRQTestRenderer = ({
) : null; + if (hasSubmitted) { + return ( +
+

Test submitted

+ +

+ Your responses for {testName} are in the grading queue. You will be + able to see your score and feedback once a grader has reviewed them. +

+ + +
+ ); + } + if (showReviewPage) { return (
@@ -342,7 +435,7 @@ const FRQTestRenderer = ({

Review your responses before submitting your test.

-

Select an FRQ number to return to that question.

+

Select a part to return to that question.

@@ -370,14 +463,7 @@ const FRQTestRenderer = ({
{questions.map((question, index) => { - const response = responses[question.id] ?? ""; - - const isAnswered = - response - .replace(/<[^>]*>/g, "") - .replace(/ /g, " ") - .trim().length > 0; - + const isAnswered = hasResponseText(responses[question.id]); const isMarked = markedForReview[index] ?? false; return ( @@ -394,7 +480,7 @@ const FRQTestRenderer = ({ setShowReviewPage(false); }} > - {index + 1} + {getPartLabel(index)} {isMarked && ( {timeUpModal} + {submissionModal}
-

Section I

-

Directions ▾

+

Section II

+

Free response

@@ -475,65 +562,21 @@ const FRQTestRenderer = ({
-
-
- {hasBackendData ? "FRQ data loaded" : "Using mock FRQ content"} -
- -
-
- {Array.from({ length: 17 }).map((_, index) => ( -
- ))} - {Array.from({ length: 17 }).map((_, index) => ( -
- ))} -
-

Figure 1

-

- Source: populationpyramid.net -

-
- -
-

- DEMOGRAPHIC TRANSITION MODEL -

-
-
-
-
-
- Stage 1 - Stage 2 - Stage 3 - Stage 4 - Stage 5 -
-
-

Figure 2

-
+
+
- {currentFRQIndex + 1} + {getPartLabel(currentFRQIndex)}
- -
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-

- The {currentFRQ.title} can be used to theorize - changes in a country's total population over time. -

- -
    -
  1. - Identify the stage of the model that this country is most likely - in. -
  2. -
  3. - Explain one social cause of the transition between two stages. -
  4. -
  5. Define the term post-industrial society.
  6. -
  7. - Explain one change in the birth rate or death rate shown in the - model. -
  8. -
  9. - Describe how economic development can affect population growth. -
  10. -
  11. - Explain one factor that may contribute to a country's aging - population. -
  12. -
  13. Explain how migration may influence population trends.
  14. -
+
+ +
{ - setResponses((currentResponses) => { - return { - ...currentResponses, - [currentFRQ.id]: newResponse, - }; - }); + setResponses((currentResponses) => ({ + ...currentResponses, + [currentFRQ.id]: newResponse, + })); }} />
@@ -631,7 +636,7 @@ const FRQTestRenderer = ({
{ diff --git a/src/components/global/frqFooter.tsx b/src/components/global/frqFooter.tsx deleted file mode 100644 index 906f4849..00000000 --- a/src/components/global/frqFooter.tsx +++ /dev/null @@ -1,98 +0,0 @@ -"use client"; - -import { ChevronLeft, ChevronRight, ChevronUp, MapPin, X } from "lucide-react"; -import { Popover, PopoverContent, PopoverTrigger} from "@/components/ui/popover"; -import { useState } from "react"; - - -interface FooterProps { - testName: string; - currentFrqIndex: number; - totalFrqs: number; - onPrevious: () => void; - onNext: () => void; - onJumpToFrq: (index: number) => void; -} - -export default function Footer({testName, currentFrqIndex, totalFrqs, onPrevious, onNext, onJumpToFrq,}: FooterProps) { - const [navigationOpen, setNavigationOpen] = useState(false); - - return ( -
-

{testName}

- -
- - - - - Question {currentFrqIndex + 1} of {totalFrqs} - - - - -
-
-

{testName} Questions

- - -
- -
- -
-
- {Array.from({ length: totalFrqs }).map((_, index) => ( - - ))} -
-
-
- - - - -
- -
- - - -
-
- ); -} diff --git a/src/components/subject/subject-sidebar.tsx b/src/components/subject/subject-sidebar.tsx index 80ec007e..1e0c341a 100644 --- a/src/components/subject/subject-sidebar.tsx +++ b/src/components/subject/subject-sidebar.tsx @@ -1,5 +1,5 @@ import { useState } from "react"; -import { BookDashed, BookOpenCheck, ChevronsLeft } from "lucide-react"; +import { BookDashed, BookOpenCheck, ChevronsLeft, PenLine } from "lucide-react"; import { Accordion, AccordionContent, @@ -142,6 +142,40 @@ const SubjectSidebar = (props: Props) => {

))} + {unit.frqs?.map((frq, frqIndex) => ( + + + + {frq.title + ? frq.title + : `Unit ${uNum} FRQ ${unit.frqs && unit.frqs.length > 1 ? ` ${frqIndex + 1}` : ""}`} + +

+ WIP +

+ + ))}
diff --git a/src/lib/frq/template.ts b/src/lib/frq/template.ts new file mode 100644 index 00000000..648bbbda --- /dev/null +++ b/src/lib/frq/template.ts @@ -0,0 +1,193 @@ +import type { + FRQAnswerType, + FRQGradingCriterion, + FRQQuestionStatus, + FRQTemplate, + FRQTemplateQuestion, +} from "@/types/frq"; +import type { QuestionFile, QuestionInput } from "@/types/questions"; + +/** Default minutes on the clock when a template predates the time-limit field. */ +export const DEFAULT_TIME_LIMIT_MINUTES = 90; + +const asString = (value: unknown): string => + typeof value === "string" ? value : ""; + +const asRecord = (value: unknown): Record | null => + typeof value === "object" && value !== null && !Array.isArray(value) + ? (value as Record) + : null; + +/** + * Firestore hands back whatever was written, and FRQ documents predate several + * schema passes. Every field is therefore re-checked rather than cast, so one + * malformed document degrades to an empty prompt instead of crashing the page + * that renders it. + */ +const normalizeFiles = (value: unknown): QuestionFile[] => { + if (!Array.isArray(value)) { + return []; + } + + return value.flatMap((entry): QuestionFile[] => { + const record = asRecord(entry); + + if (!record || typeof record.key !== "string") { + return []; + } + + const file: QuestionFile = { + key: record.key, + name: asString(record.name) || record.key, + }; + + if (typeof record.url === "string") file.url = record.url; + if (typeof record.id === "string") file.id = record.id; + if (typeof record.alt === "string") file.alt = record.alt; + if (typeof record.order === "number") file.order = record.order; + + return [file]; + }); +}; + +const normalizeCriteria = (value: unknown): FRQGradingCriterion[] => { + if (!Array.isArray(value)) { + return []; + } + + return value.flatMap((entry, index): FRQGradingCriterion[] => { + const record = asRecord(entry); + + if (!record) { + return []; + } + + const points = Number(record.points); + + return [ + { + id: asString(record.id) || `criterion-${index}`, + description: asString(record.description), + // A criterion worth a fraction of a point would make the "x/y points" + // summaries on three separate pages disagree, so clamp to whole points. + points: Number.isFinite(points) ? Math.max(0, Math.round(points)) : 0, + }, + ]; + }); +}; + +const normalizeAnswerType = (value: unknown): FRQAnswerType => + value === "equation" ? "equation" : "text"; + +const normalizeStatus = (value: unknown): FRQQuestionStatus => + value === "legacy" ? "legacy" : "public"; + +const normalizeQuestion = ( + value: unknown, + index: number, +): FRQTemplateQuestion[] => { + const record = asRecord(value); + + if (!record) { + return []; + } + + const id = asString(record.id); + + // A part with no stable id cannot be scored or matched to a response, so it + // is dropped rather than given a positional id that would silently rebind to + // a different part the next time the author reorders the list. + if (!id) { + return []; + } + + return [ + { + id, + title: asString(record.title) || `Part ${index + 1}`, + prompt: asString(record.prompt), + promptFiles: normalizeFiles(record.promptFiles), + answerType: normalizeAnswerType(record.answerType), + status: normalizeStatus(record.status), + criteria: normalizeCriteria(record.criteria), + }, + ]; +}; + +/** + * Turn a raw Firestore FRQ document into a template every page can trust. + * `identity` supplies the values that live in the document path rather than the + * document body, so a template still knows where it came from when the stored + * `subject`/`unitId` fields are missing. + */ +export const normalizeFrqTemplate = ( + raw: unknown, + identity: { id: string; subject: string; unitId: string }, +): FRQTemplate => { + const record = asRecord(raw) ?? {}; + const timeLimit = Number(record.timeLimitMinutes); + + return { + id: identity.id, + subject: asString(record.subject) || identity.subject, + unitId: asString(record.unitId) || identity.unitId, + title: asString(record.title) || "Untitled FRQ", + directions: asString(record.directions), + directionsFiles: normalizeFiles(record.directionsFiles), + questions: Array.isArray(record.questions) + ? record.questions.flatMap(normalizeQuestion) + : [], + isPublic: record.isPublic === true, + timeLimitMinutes: + Number.isFinite(timeLimit) && timeLimit >= 1 + ? Math.floor(timeLimit) + : DEFAULT_TIME_LIMIT_MINUTES, + }; +}; + +export const toQuestionInput = ( + value: string | undefined, + files: QuestionFile[] | undefined, +): QuestionInput => ({ + value: value ?? "", + files: files ? [...files] : [], +}); + +/** Parts a student actually sits. Legacy parts stay readable but unassigned. */ +export const getStudentFacingQuestions = (template: FRQTemplate) => + template.questions.filter((question) => question.status !== "legacy"); + +export const getQuestionPoints = (question: FRQTemplateQuestion) => + (question.criteria ?? []).reduce( + (total, criterion) => total + criterion.points, + 0, + ); + +export const getTemplatePoints = (questions: FRQTemplateQuestion[]) => + questions.reduce( + (total, question) => total + getQuestionPoints(question), + 0, + ); + +/** + * AP-style part label: A, B, C ... then AA, AB past 26 parts rather than + * walking off the end of the alphabet into punctuation. + */ +export const getPartLabel = (index: number) => { + let label = ""; + let remaining = index; + + do { + label = String.fromCharCode(65 + (remaining % 26)) + label; + remaining = Math.floor(remaining / 26) - 1; + } while (remaining >= 0); + + return label; +}; + +/** Strips markup so "did the student write anything" is not fooled by `

`. */ +export const hasResponseText = (response: string | undefined) => + (response ?? "") + .replace(/<[^>]*>/g, "") + .replace(/ /g, " ") + .trim().length > 0; diff --git a/src/types/frq.ts b/src/types/frq.ts index 3a76c78a..025d1c83 100644 --- a/src/types/frq.ts +++ b/src/types/frq.ts @@ -1,4 +1,38 @@ import type { Timestamp } from "firebase/firestore"; +import type { QuestionFile } from "@/types/questions"; + +/** How a student is expected to answer a single FRQ part. */ +export type FRQAnswerType = "text" | "equation"; + +/** + * Whether a part is shown to students. "legacy" parts stay attached to the + * template so old submissions still resolve their prompts, but new test takers + * never see them. + */ +export type FRQQuestionStatus = "public" | "legacy"; + +/** + * One line of a part's rubric. Points are the single source of truth for what a + * part is worth: the editor, the grading page, and the student's feedback page + * all derive totals by summing these, so none of them can disagree. + */ +export interface FRQGradingCriterion { + id: string; + description: string; + points: number; +} + +export interface FRQTemplateQuestion { + /** Stable template-local identifier; never derived from display order. */ + id: string; + title: string; + /** Authored prompt text. Files referenced by it live in `promptFiles`. */ + prompt?: string; + promptFiles?: QuestionFile[]; + answerType?: FRQAnswerType; + status?: FRQQuestionStatus; + criteria?: FRQGradingCriterion[]; +} /** An admin-authored prompt used by the digital FRQ testing experience. */ export interface FRQTemplate { @@ -6,20 +40,21 @@ export interface FRQTemplate { subject: string; unitId: string; title: string; + /** + * Stimulus and directions shown in the left-hand pane of the test. Stored as + * text plus a file list rather than a nested object so that documents written + * before the editor could save still load without migration. + */ directions: string; + directionsFiles?: QuestionFile[]; questions: FRQTemplateQuestion[]; isPublic?: boolean; + /** Minutes on the test clock. Absent on templates authored before timing. */ + timeLimitMinutes?: number; createdAt?: Timestamp; updatedAt?: Timestamp; } -export interface FRQTemplateQuestion { - /** Stable template-local identifier; never derived from display order. */ - id: string; - title: string; - prompt?: string; -} - /** A completed digital-test response awaiting staff grading. */ export interface GradableFRQSubmission { id?: string; @@ -32,10 +67,30 @@ export interface GradableFRQSubmission { submittedAt: Timestamp; } +/** Points a grader awarded for one rubric line. */ +export interface FRQCriterionScore { + criterionId: string; + points: number; +} + +/** + * A grader's verdict on a single part. Stored per part rather than as one + * aggregate blob so the student's feedback page can show which rubric lines + * were earned and what the grader said about each part. + */ +export interface FRQQuestionGrade { + questionId: string; + feedback: string; + criteria: FRQCriterionScore[]; +} + /** The immutable grading result presented on a student's dashboard. */ export interface GradedFRQSubmission extends GradableFRQSubmission { + /** Human-readable aggregate, e.g. "4/6". Derived from `grades`. */ score: string; + /** Overall comment. Per-part comments live in `grades`. */ feedback: string; + grades: FRQQuestionGrade[]; graderId: string; gradedAt: Timestamp; sourceSubmissionId: string; @@ -58,4 +113,4 @@ export interface FRQSubmission { }; } -export type GradingStatus = "ungraded" | "graded" | "flagged" | "rejected"; \ No newline at end of file +export type GradingStatus = "ungraded" | "graded" | "flagged" | "rejected"; From 72d16470ed0da45fa1686254097038454809b190 Mon Sep 17 00:00:00 2001 From: saa938 Date: Fri, 21 Aug 2026 09:53:55 -0700 Subject: [PATCH 2/3] Point FRQ grading at the collection submissions are actually written to The grading queue read `gradableFrqSubmissions`, a collection name from an earlier schema pass that nothing writes to, so the queue was permanently empty no matter how many tests students submitted. The grader view itself rendered mock data rather than the submission being graded. - frq-grading list: read `ungraded-frqs`, the collection the student test writes to, and resolve templates through their real nested location instead of a flat `frqTemplates` collection - frq-grading list: gate on grader/admin access, so a non-grader sees why the queue is empty rather than an unexplained blank page - frq-grading/[id]: load the real submission and its template - gradingRenderer: score against the template's rubric and write grades back, replacing the mock content Builds on the data model and rules from the previous PR. --- src/app/frq-grading/[id]/page.tsx | 110 +++- src/app/frq-grading/page.tsx | 90 ++- src/components/frq/gradingRenderer.tsx | 880 +++++++++++-------------- 3 files changed, 547 insertions(+), 533 deletions(-) diff --git a/src/app/frq-grading/[id]/page.tsx b/src/app/frq-grading/[id]/page.tsx index 8ccc8a9f..c0be8e77 100644 --- a/src/app/frq-grading/[id]/page.tsx +++ b/src/app/frq-grading/[id]/page.tsx @@ -1,8 +1,13 @@ "use client"; import FRQGradingRenderer from "@/components/frq/gradingRenderer"; -import { getUngradedFrqDocRef } from "@/lib/firestore/frqRefs"; -import type { GradableFRQSubmission } from "@/types/frq"; +import { useUser } from "@/components/hooks/UserContext"; +import { + getFrqTemplateDocRef, + getUngradedFrqDocRef, +} from "@/lib/firestore/frqRefs"; +import { normalizeFrqTemplate } from "@/lib/frq/template"; +import type { FRQTemplate, GradableFRQSubmission } from "@/types/frq"; import { getDoc } from "firebase/firestore"; import { useEffect, useState } from "react"; @@ -13,39 +18,100 @@ type PageProps = { }; const Page = ({ params }: PageProps) => { - const [frq, setFrq] = useState(null); + const { user, loading: userLoading } = useUser(); + + const [submission, setSubmission] = useState( + null, + ); + const [template, setTemplate] = useState(null); const [isLoading, setIsLoading] = useState(true); + const [loadError, setLoadError] = useState(null); + + const canGrade = user?.access === "admin" || user?.access === "grader"; useEffect(() => { - const fetchFrq = async () => { + if (userLoading || !canGrade) { + if (!userLoading) { + setIsLoading(false); + } + return; + } + + const fetchSubmissionAndTemplate = async () => { + setIsLoading(true); + setLoadError(null); + try { - const docRef = getUngradedFrqDocRef(params.id); - const docSnap = await getDoc(docRef); - - if (docSnap.exists()) { - setFrq({ - id: docSnap.id, - ...(docSnap.data() as GradableFRQSubmission), - }); - } else { - setFrq(null); + const submissionSnapshot = await getDoc( + getUngradedFrqDocRef(params.id), + ); + + if (!submissionSnapshot.exists()) { + setLoadError( + "This submission is no longer in the queue. It may already have been graded.", + ); + return; } + + const loadedSubmission = { + id: submissionSnapshot.id, + ...(submissionSnapshot.data() as Omit), + }; + + setSubmission(loadedSubmission); + + // The rubric being graded against lives on the template, not on the + // submission, so both have to be in hand before grading can start. + const templateSnapshot = await getDoc( + getFrqTemplateDocRef( + loadedSubmission.subject, + loadedSubmission.unitId, + loadedSubmission.templateId, + ), + ); + + setTemplate( + templateSnapshot.exists() + ? normalizeFrqTemplate(templateSnapshot.data(), { + id: templateSnapshot.id, + subject: loadedSubmission.subject, + unitId: loadedSubmission.unitId, + }) + : null, + ); } catch (error) { - console.error("Error fetching FRQ:", error); - setFrq(null); + console.error("Error loading FRQ submission:", error); + + setLoadError( + error instanceof Error + ? `Could not load this submission: ${error.message}` + : "Could not load this submission.", + ); } finally { setIsLoading(false); } }; - void fetchFrq(); - }, [params.id]); + void fetchSubmissionAndTemplate(); + }, [params.id, userLoading, canGrade]); + + if (userLoading || isLoading) { + return
Loading...
; + } + + if (!canGrade) { + return ( +
+ You need grader or admin access to grade FRQ submissions. +
+ ); + } - if (isLoading) { - return
Loading...
; + if (loadError) { + return
{loadError}
; } - return ; + return ; }; -export default Page; \ No newline at end of file +export default Page; diff --git a/src/app/frq-grading/page.tsx b/src/app/frq-grading/page.tsx index 6de4f474..287cc688 100644 --- a/src/app/frq-grading/page.tsx +++ b/src/app/frq-grading/page.tsx @@ -3,8 +3,13 @@ import Navbar from "@/components/global/navbar"; import Footer from "@/components/global/footer"; import Link from "next/link"; -import { db } from "@/lib/firebase"; -import { deleteDoc, collection, doc, getDoc, getCountFromServer, getDocs, limit, orderBy, query, startAfter, type DocumentData, type QueryDocumentSnapshot, Timestamp } from "firebase/firestore"; +import { useUser } from "@/components/hooks/UserContext"; +import { + getFrqTemplateDocRef, + getUngradedFrqDocRef, + getUngradedFrqsCollectionRef, +} from "@/lib/firestore/frqRefs"; +import { deleteDoc, getDoc, getCountFromServer, getDocs, limit, orderBy, query, startAfter, type DocumentData, type QueryDocumentSnapshot, Timestamp } from "firebase/firestore"; import { useCallback, useEffect, useState } from "react"; import { Button } from "@/components/ui/button"; import { Trash2, ChevronLeft, ChevronRight } from "lucide-react"; @@ -28,6 +33,12 @@ type PageCursor = QueryDocumentSnapshot | null; const Page = () => { + const { user, loading: userLoading } = useUser(); + + // Firestore rules already reject a non-grader's reads, but without this check + // the page renders a full, permanently empty queue and gives no hint why. + const canGrade = user?.access === "admin" || user?.access === "grader"; + const [frqs, setFrqs] = useState(null); const [frqToDelete, setFrqToDelete] = useState(null); @@ -54,10 +65,11 @@ const Page = () => { const fetchFrqs = useCallback(async (cursor: PageCursor) => { setIsPageLoading(true); try { - const collectionRef = collection( - db, - "gradableFrqSubmissions", - ); + // `ungraded-frqs` is the collection the student test actually writes to. + // This page previously read `gradableFrqSubmissions`, a name from an + // earlier schema pass that nothing has written to since, so the queue + // read as permanently empty no matter how many tests were submitted. + const collectionRef = getUngradedFrqsCollectionRef(); const pageQuery = cursor ? query( @@ -107,20 +119,27 @@ const Page = () => { ) : {}; + // A submission records where its template lives, so the location no + // longer has to be recovered from a flat template collection. + const subject = + typeof rawData.subject === "string" ? rawData.subject : ""; + const unitId = + typeof rawData.unitId === "string" ? rawData.unitId : ""; + let isMalformed = templateId === "" || studentId === "" || + subject === "" || + unitId === "" || submittedAt === null || !hasValidResponses; - + let frqTitle = "Unknown FRQ"; - let subject = "Unknown subject"; - let unitId = "Unknown unit"; - if (templateId !== "") { + if (!isMalformed) { try { const templateSnapshot = await getDoc( - doc(db, "frqTemplates", templateId), + getFrqTemplateDocRef(subject, unitId, templateId), ); if (templateSnapshot.exists()) { @@ -131,18 +150,6 @@ const Page = () => { } else { isMalformed = true; } - - if (typeof templateData.subject === "string") { - subject = templateData.subject; - } else { - isMalformed = true; - } - - if (typeof templateData.unitId === "string") { - unitId = templateData.unitId; - } else { - isMalformed = true; - } } else { isMalformed = true; } @@ -163,8 +170,8 @@ const Page = () => { responses, isMalformed, frqTitle, - subject, - unitId + subject: subject || "Unknown subject", + unitId: unitId || "Unknown unit", }; }), ); @@ -179,9 +186,13 @@ const Page = () => { useEffect(() => { + if (userLoading || !canGrade) { + return; + } + const initializePage = async () => { const countSnapshot = await getCountFromServer( - collection(db, "gradableFrqSubmissions"), + getUngradedFrqsCollectionRef(), ); setTotalCount(countSnapshot.data().count); @@ -195,7 +206,7 @@ const Page = () => { ); setFrqs([]); }); - }, [fetchFrqs]); + }, [fetchFrqs, userLoading, canGrade]); const handleNextPage = async () => { @@ -239,8 +250,7 @@ const handlePreviousPage = async () => { setIsDeleting(true); try { - await deleteDoc( - doc(db, "gradableFrqSubmissions", frqToDelete.id)); + await deleteDoc(getUngradedFrqDocRef(frqToDelete.id)); setFrqs((currentFrqs) => currentFrqs @@ -270,8 +280,28 @@ const handlePreviousPage = async () => { } }; + if (userLoading) { + return
Loading...
; + } + + if (!canGrade) { + return ( +
+ +
+

Grader access required

+

+ Ask an admin to grant your account grader access to review FRQ + submissions. +

+
+
+
+ ); + } + if (frqs === null) { - return
Loading...
; + return
Loading...
; } return ( diff --git a/src/components/frq/gradingRenderer.tsx b/src/components/frq/gradingRenderer.tsx index b5458d24..580bf65d 100644 --- a/src/components/frq/gradingRenderer.tsx +++ b/src/components/frq/gradingRenderer.tsx @@ -1,15 +1,10 @@ "use client"; +import { RenderContent } from "@/components/article-creator/custom_questions/RenderAdvancedTextbox"; import { db } from "@/lib/firebase"; -import { useState, type ReactNode } from "react"; +import { useMemo, useState } from "react"; import Link from "next/link"; -import { - ChevronDown, - ChevronLeft, - ChevronRight, - ChevronUp, - LogOut, -} from "lucide-react"; +import { ChevronLeft, ChevronRight, ChevronUp, LogOut } from "lucide-react"; import { runTransaction, serverTimestamp } from "firebase/firestore"; import { Popover, @@ -20,75 +15,127 @@ import { getGradedFrqDocRef, getUngradedFrqDocRef, } from "@/lib/firestore/frqRefs"; +import { + getPartLabel, + getQuestionPoints, + getTemplatePoints, + toQuestionInput, +} from "@/lib/frq/template"; import { useUser } from "@/components/hooks/UserContext"; -import type { GradableFRQSubmission } from "@/types/frq"; +import type { + FRQTemplate, + FRQTemplateQuestion, + GradableFRQSubmission, +} from "@/types/frq"; type FRQGradingRendererProps = { - frq: GradableFRQSubmission | null; + submission: GradableFRQSubmission | null; + template: FRQTemplate | null; }; -type MockFrq = { title: string; questionCount: number }; -type RubricItem = { - description: string; - earnedPoints: number; - possiblePoints: number; +/** Points awarded per criterion, and the grader's note, for one part. */ +type PartGrade = { + feedback: string; + criteria: Record; }; -const MOCK_FRQS: MockFrq[] = [ - { title: "FRQ 1", questionCount: 3 }, - { title: "FRQ 2", questionCount: 4 }, - { title: "FRQ 3", questionCount: 2 }, -]; - -const RUBRIC_ITEMS: RubricItem[] = [ - { - description: "Answer draws a connection to the Great Demographic Shift", - earnedPoints: 0, - possiblePoints: 2, - }, - { - description: "Answer mentions John Smith’s contributions", - earnedPoints: 0, - possiblePoints: 1, - }, - { - description: "Answer is at least 27 paragraphs long", - earnedPoints: 1, - possiblePoints: 1, - }, - { - description: - "Answer contains correct punctuation, capitalization, and supporting details", - earnedPoints: 2, - possiblePoints: 2, - }, -]; - -const FRQGradingRenderer = ({ frq }: FRQGradingRendererProps) => { +const createEmptyGrade = (question: FRQTemplateQuestion): PartGrade => ({ + feedback: "", + criteria: Object.fromEntries( + (question.criteria ?? []).map((criterion) => [criterion.id, 0]), + ), +}); + +const FRQGradingRenderer = ({ + submission, + template, +}: FRQGradingRendererProps) => { const { user } = useUser(); - const [currentFrqIndex, setCurrentFrqIndex] = useState(0); + + // Grading covers every part the template defines, including ones marked + // legacy: an older submission may still hold a response to them. + const questions = useMemo(() => template?.questions ?? [], [template]); + + const [currentIndex, setCurrentIndex] = useState(0); const [isNavigationOpen, setIsNavigationOpen] = useState(false); - const [feedback, setFeedback] = useState(""); - const [savedFeedback, setSavedFeedback] = useState(""); + const [overallFeedback, setOverallFeedback] = useState(""); const [isSubmitting, setIsSubmitting] = useState(false); const [showPrompt, setShowPrompt] = useState(true); + const [grades, setGrades] = useState>(() => + Object.fromEntries( + questions.map((question) => [question.id, createEmptyGrade(question)]), + ), + ); - if (!frq) return
FRQ not found.
; + const possiblePoints = getTemplatePoints(questions); - const currentFrq = MOCK_FRQS[currentFrqIndex]; - if (!currentFrq) return
No FRQs available.
; + const earnedPoints = questions.reduce((total, question) => { + const partGrade = grades[question.id]; + + if (!partGrade) { + return total; + } + + return ( + total + + (question.criteria ?? []).reduce( + (partTotal, criterion) => + partTotal + (partGrade.criteria[criterion.id] ?? 0), + 0, + ) + ); + }, 0); + + // A part counts as graded once the grader has written a note for it, which is + // the only signal that distinguishes "looked at and awarded zero" from + // "not looked at yet". + const gradedPartCount = questions.filter((question) => + (grades[question.id]?.feedback ?? "").trim(), + ).length; + + const setCriterionPoints = ( + questionId: string, + criterionId: string, + rawPoints: number, + maximumPoints: number, + ) => { + const points = Math.min( + Math.max(Number.isFinite(rawPoints) ? Math.round(rawPoints) : 0, 0), + maximumPoints, + ); + + setGrades((currentGrades) => ({ + ...currentGrades, + [questionId]: { + feedback: currentGrades[questionId]?.feedback ?? "", + criteria: { + ...(currentGrades[questionId]?.criteria ?? {}), + [criterionId]: points, + }, + }, + })); + }; + + const setPartFeedback = (questionId: string, feedback: string) => { + setGrades((currentGrades) => ({ + ...currentGrades, + [questionId]: { + feedback, + criteria: currentGrades[questionId]?.criteria ?? {}, + }, + })); + }; - const earnedPoints = RUBRIC_ITEMS.reduce( - (total, item) => total + item.earnedPoints, - 0, - ); - const possiblePoints = RUBRIC_ITEMS.reduce( - (total, item) => total + item.possiblePoints, - 0, - ); const submitGradeReport = async () => { - if (!frq.id || !user || !feedback.trim()) return; + if (!submission?.id || !user || !template) { + return; + } + + if (!overallFeedback.trim()) { + window.alert("Add overall feedback before submitting the grade report."); + return; + } setIsSubmitting(true); @@ -96,8 +143,8 @@ const FRQGradingRenderer = ({ frq }: FRQGradingRendererProps) => { // Use the submission ID as the result ID and atomically claim the queue // item. Firestore retries concurrent transactions, so only one grader // can successfully issue a report for a submission. - const queueRef = getUngradedFrqDocRef(frq.id); - const resultRef = getGradedFrqDocRef(frq.id); + const queueRef = getUngradedFrqDocRef(submission.id); + const resultRef = getGradedFrqDocRef(submission.id); await runTransaction(db, async (transaction) => { const [queueSnapshot, resultSnapshot] = await Promise.all([ @@ -109,22 +156,31 @@ const FRQGradingRenderer = ({ frq }: FRQGradingRendererProps) => { throw new Error("This submission has already been graded."); } - const queuedSubmission = - queueSnapshot.data() as GradableFRQSubmission; - - transaction.set(resultRef, { - sourceSubmissionId: frq.id, - templateId: queuedSubmission.templateId, - subject: queuedSubmission.subject, - unitId: queuedSubmission.unitId, - studentId: queuedSubmission.studentId, - responses: queuedSubmission.responses, - submittedAt: queuedSubmission.submittedAt, - score: `${earnedPoints}/${possiblePoints}`, - feedback: feedback.trim(), - graderId: user.uid, - gradedAt: serverTimestamp(), - }); + const queuedSubmission = queueSnapshot.data() as GradableFRQSubmission; + + transaction.set(resultRef, { + sourceSubmissionId: submission.id, + templateId: queuedSubmission.templateId, + subject: queuedSubmission.subject, + unitId: queuedSubmission.unitId, + studentId: queuedSubmission.studentId, + responses: queuedSubmission.responses, + submittedAt: queuedSubmission.submittedAt, + score: `${earnedPoints}/${possiblePoints}`, + feedback: overallFeedback.trim(), + // Per-part detail is what lets the student's feedback page show which + // rubric lines were earned instead of a bare aggregate. + grades: questions.map((question) => ({ + questionId: question.id, + feedback: grades[question.id]?.feedback?.trim() ?? "", + criteria: (question.criteria ?? []).map((criterion) => ({ + criterionId: criterion.id, + points: grades[question.id]?.criteria[criterion.id] ?? 0, + })), + })), + graderId: user.uid, + gradedAt: serverTimestamp(), + }); transaction.delete(queueRef); }); @@ -144,427 +200,289 @@ const FRQGradingRenderer = ({ frq }: FRQGradingRendererProps) => { } }; - return ( -
- setSavedFeedback(feedback)} - onSubmitGradeReport={() => void submitGradeReport()} - /> - -
- - setShowPrompt((visible) => !visible)} - /> -
+ if (!submission) return
FRQ submission not found.
; - setCurrentFrqIndex((index) => Math.max(index - 1, 0))} - onNext={() => - setCurrentFrqIndex((index) => - Math.min(index + 1, MOCK_FRQS.length - 1), - ) - } - onJumpToFrq={(index) => { - if (index >= 0 && index < MOCK_FRQS.length) { - setCurrentFrqIndex(index); - setIsNavigationOpen(false); - } - }} - /> -
- ); -}; - -function GradingHeader({ - gradedQuestionCount, - totalQuestionCount, - isSubmitting, - onSaveChanges, - onSubmitGradeReport, -}: { - gradedQuestionCount: number; - totalQuestionCount: number; - isSubmitting: boolean; - onSaveChanges: () => void; - onSubmitGradeReport: () => void; -}) { - return ( -
-
- - -
-
-

- {gradedQuestionCount}/{totalQuestionCount} Questions Graded -

-
-
- -
-
- ); -} - -function GraderResponsePanel({ - frqTitle, - feedback, - savedFeedback, - onFeedbackChange, -}: { - frqTitle: string; - feedback: string; - savedFeedback: string; - onFeedbackChange: (feedback: string) => void; -}) { - return ( -
-

- {frqTitle} | Question A | Grader Response -

- -
- {RUBRIC_ITEMS.map((item) => ( - - ))} + if (!template) { + return ( +
+ The FRQ this submission came from no longer exists, so it cannot be + graded.
-
-
- - - - Ω -
-