diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 08142ee..9f100b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,4 +39,6 @@ jobs: - name: Upload coverage reports to Codecov uses: codecov/codecov-action@v7 with: - token: ${{ secrets.CODECOV_TOKEN }} \ No newline at end of file + token: ${{ secrets.CODECOV_TOKEN }} + files: packages/models/coverage/lcov.info,packages/backend/coverage/lcov.info,packages/frontend/coverage/lcov.info + fail_ci_if_error: true \ No newline at end of file diff --git a/NEWS.md b/NEWS.md index d50142d..efce3d0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,24 @@ +## [Release] v1.7.2 – 2026-08-26 + +Patch release fixing a client-side crash in running quizzes: advancing from a +question with more options to one with fewer could throw a full-page error +("undefined is not an object (question.answers[i].correct)"). Students could +dismiss and refresh to continue; no data was affected. No operator action — +frontend-only change, deploy stack and config unchanged since v1.7.1. + +Versions: frontend `1.7.1 → 1.7.2`, backend `1.1.0`, models `1.1.0`. + +### Highlights + +- **Quiz reliability** + - Fixed a render crash when advancing to a question with fewer answer options + than the previous one. Stale local selection state (sized to the previous + question) briefly outlived the question change and was indexed out of bounds. + - The answer-correctness check now ignores a selection that doesn't match the + current question's option count, and per-question state is reset in step with + the question during render — without reintroducing the earlier double-submit + regression. + ## [Release] v1.7.1 – 2026-08-20 Patch release fixing a quiz-answer race: an answer selected during the brief diff --git a/packages/frontend/package.json b/packages/frontend/package.json index 569be0b..3e94720 100644 --- a/packages/frontend/package.json +++ b/packages/frontend/package.json @@ -1,7 +1,7 @@ { "name": "@recapp/frontend", "private": true, - "version": "1.7.1", + "version": "1.7.2", "type": "module", "scripts": { "dev": "vite", diff --git a/packages/frontend/src/components/quiz-tabs/RunningQuizTab.tsx b/packages/frontend/src/components/quiz-tabs/RunningQuizTab.tsx index 9960f74..c81c818 100644 --- a/packages/frontend/src/components/quiz-tabs/RunningQuizTab.tsx +++ b/packages/frontend/src/components/quiz-tabs/RunningQuizTab.tsx @@ -51,6 +51,20 @@ export const RunningQuizTab: React.FC<{ const questionText = questions.at(run?.counter ?? 0)?.text; const { rendered, isStale } = useRendered({ value: questionText ?? "" }); + // Reset per-question local state in step with the current question, before it is + // read during render. questionId only changes once the counter advances (after the + // backend round-trip), so this does NOT reintroduce the synchronous pre-round-trip + // reset removed in 47bef5d (which reopened the repetition-glitch double submission). + // Without this, stale `answers` (sized to a previous, larger question) outlives the + // counter change for one render and crashes isMultiChoiceAnsweredCorrectly. + const [prevQuestionId, setPrevQuestionId] = useState(questionId); + if (questionId !== prevQuestionId) { + setPrevQuestionId(questionId); + setAnswered(false); + setTextAnswer(""); + setAnswers([]); + } + if (!quizState.run || !quizState.questions) { return null; } diff --git a/packages/frontend/src/utils.test.ts b/packages/frontend/src/utils.test.ts new file mode 100644 index 0000000..7eeaf5f --- /dev/null +++ b/packages/frontend/src/utils.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from "vitest"; +import { Question } from "@recapp/models"; +import { isMultiChoiceAnsweredCorrectly } from "./utils"; + +// The function only reads `question.answers`, so a minimal shape is enough. +const questionWith = (correct: boolean[]): Question => + ({ answers: correct.map(c => ({ text: "", correct: c })) }) as unknown as Question; + +describe("isMultiChoiceAnsweredCorrectly", () => { + it("returns false for an empty selection", () => { + expect(isMultiChoiceAnsweredCorrectly([], questionWith([true, false]))).toBe(false); + }); + + it("returns false when question is undefined", () => { + expect(isMultiChoiceAnsweredCorrectly([true], undefined)).toBe(false); + }); + + it("returns true when the selection matches the correct answers", () => { + const question = questionWith([true, false, true]); + expect(isMultiChoiceAnsweredCorrectly([true, false, true], question)).toBe(true); + }); + + it("returns false when the selection does not match", () => { + const question = questionWith([true, false, true]); + expect(isMultiChoiceAnsweredCorrectly([true, true, false], question)).toBe(false); + }); + + it("pads a too-short selection with false and still evaluates correctly", () => { + // Trailing unselected options are equivalent to `false` entries. + const question = questionWith([true, false, false]); + expect(isMultiChoiceAnsweredCorrectly([true], question)).toBe(true); + }); + + // Regression: answers-length render crash. A stale selection sized to a previous, + // larger question must NOT index question.answers out of bounds — it returns false. + it("returns false (no crash) when the selection is longer than the question's options", () => { + const question = questionWith([true, false]); + expect(() => isMultiChoiceAnsweredCorrectly([true, false, true], question)).not.toThrow(); + expect(isMultiChoiceAnsweredCorrectly([true, false, true], question)).toBe(false); + }); +}); diff --git a/packages/frontend/src/utils.ts b/packages/frontend/src/utils.ts index a4edf76..afb8616 100644 --- a/packages/frontend/src/utils.ts +++ b/packages/frontend/src/utils.ts @@ -49,6 +49,14 @@ export const isMultiChoiceAnsweredCorrectly = (answers2: boolean[], question: Qu return false; } + // A selection longer than the current question's option count belongs to a + // previous (larger) question whose local state hasn't been reset yet. Treat it + // as not-yet-answered instead of indexing question.answers out of bounds, which + // throws "question.answers[i].correct" during render (answers-length crash). + if (answers.length > (question?.answers.length ?? 0)) { + return false; + } + while (answers.length < (question?.answers.length ?? 0)) { answers.push(false); } diff --git a/packages/frontend/test/components/RunningQuizTab.test.tsx b/packages/frontend/test/components/RunningQuizTab.test.tsx new file mode 100644 index 0000000..584a1fd --- /dev/null +++ b/packages/frontend/test/components/RunningQuizTab.test.tsx @@ -0,0 +1,89 @@ +import { describe, it, expect, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { toId, Question, QuizRun } from "@recapp/models"; +import { toTimestamp } from "itu-utils"; +import { RunningQuizTab } from "../../src/components/quiz-tabs/RunningQuizTab"; +import type { CurrentQuizState } from "../../src/actors/CurrentQuizActor"; + +vi.mock("@lingui/react", async importOriginal => { + const actual = await importOriginal(); + return { ...actual, Trans: ({ id }: { id: string }) => {id} }; +}); + +// Render markdown synchronously — this test is about the reset/guard logic, not the +// async unified pipeline (whose late setState would otherwise trigger act() warnings). +vi.mock("../../src/hooks/useRendered", () => ({ + useRendered: ({ value }: { value: string }) => ({ rendered: value, isStale: false }), +})); + +function makeQuestion(uid: string, optionCount: number): Question { + return { + uid: toId(uid), + text: `Question ${uid}`, + type: "MULTIPLE", + authorId: toId("author-1"), + quiz: toId("quiz-1"), + answers: Array.from({ length: optionCount }, (_, i) => ({ text: `opt-${i}`, correct: i === 0 })), + approved: true, + editMode: false, + created: toTimestamp(), + updated: toTimestamp(), + } as unknown as Question; +} + +function makeState(counter: number): CurrentQuizState { + const run: QuizRun = { + uid: toId("run-1"), + studentId: toId("student-1"), + quizId: toId("quiz-1"), + questions: [toId("q1"), toId("q2")], + counter, + answers: [], + correct: [], + wrong: [], + created: toTimestamp(), + updated: toTimestamp(), + } as unknown as QuizRun; + return { + quiz: { uid: toId("quiz-1") }, + comments: [], + // q1 has 4 options, q2 has 2 — the shrinking adjacency that triggered the crash. + questions: [makeQuestion("q1", 4), makeQuestion("q2", 2)], + teacherNames: [], + run, + } as unknown as CurrentQuizState; +} + +const renderTab = (quizState: CurrentQuizState) => ( + +); + +describe("RunningQuizTab — advance to a lower-option question", () => { + // Regression for answers-length-render-crash: selecting an option on a 4-option + // question then advancing to a 2-option one left stale `answers` (length 4) that + // isMultiChoiceAnsweredCorrectly indexed out of bounds during render. The in-render + // reset keys on questionId and the utils guard clamps the mismatch. + it("does not crash and resets the selection when the next question has fewer options", () => { + const { rerender } = render(renderTab(makeState(0))); + + // Q1: four options — select the first so `answers` is sized to 4. + const q1Checkboxes = screen.getAllByRole("checkbox"); + expect(q1Checkboxes).toHaveLength(4); + fireEvent.click(q1Checkboxes[0]); + expect(q1Checkboxes[0]).toBeChecked(); + + // Advance to Q2 (two options). The stale 4-length selection reaches the render + // where the current question already has 2 options — must not throw. + expect(() => rerender(renderTab(makeState(1)))).not.toThrow(); + + // Q2 is shown with its two options and the stale selection has been cleared. + const q2Checkboxes = screen.getAllByRole("checkbox"); + expect(q2Checkboxes).toHaveLength(2); + q2Checkboxes.forEach(cb => expect(cb).not.toBeChecked()); + }); +});