Skip to content
Merged

Frq #383

Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions firestore.rules
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
80 changes: 56 additions & 24 deletions src/app/admin/subject/[slug]/[unit]/frq/[id]/page.tsx
Original file line number Diff line number Diff line change
@@ -1,76 +1,108 @@
"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";
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<FRQTemplate | null>(null);
const [frqTemplate, setFrqTemplate] = useState<FRQTemplate | null>(null);
const [isLoading, setIsLoading] = useState(true);
const [frqFound, setFrqFound] = useState(false);
const [loadError, setLoadError] = useState<string | null>(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<FRQTemplate, "id">),
};
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);
}
};

void loadFrq();
}, [subject, unitId, frqId]);
}, [userLoading, canEdit, subject, unitId, frqId]);

if (userLoading || isLoading) {
return <div className="p-8">Loading...</div>;
}

if (!canEdit) {
return (
<div className="p-8">
You need porter or admin access to edit FRQs.
</div>
);
}

if (isLoading) {
return <div>Loading...</div>;
if (loadError) {
return <div className="p-8">{loadError}</div>;
}

return (
<FRQEditorRenderer
frqFound={frqFound}
frqFound={frqTemplate !== null}
frqTemplate={frqTemplate}
/>
);
};

export default Page;
export default Page;
99 changes: 81 additions & 18 deletions src/app/admin/subject/[slug]/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<FRQTemplate, "id">),
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<FRQTemplate, "id">),
subject: params.slug,
unitId: unit.id,
}),
);
} catch (frqError) {
console.error(
`Unable to load FRQs for unit ${unit.id}:`,
frqError,
);

return [];
}
}),
);

Expand Down Expand Up @@ -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.",
);
}
};

Expand Down Expand Up @@ -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.",
);
}
};

Expand Down Expand Up @@ -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.",
);
}
};
/****************************************************
Expand Down Expand Up @@ -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
Expand Down
Loading