From 0cec7628c4a061f14b126fd1e3dab6df45fc0178 Mon Sep 17 00:00:00 2001 From: Tamara <60857422+Myranae@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:47:05 -0600 Subject: [PATCH] Radio: Extract validation out of scoring (#1902) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: To complete server-side scoring, we are separating out validation logic from scoring logic. This PR separates validation logic that does not depend on answer information and also separates associated tests for the radio widget. Issue: LEMS-2594 ## Test plan: - Confirm checks pass - Confirm widget still works as expected - Confirm the validation logic remaining in the scoring function cannot be removed from scoring Author: Myranae Reviewers: Myranae, jeremywiebe, handeyeco Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald, ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1902 --- .changeset/spotty-moles-reply.md | 5 ++ .../src/widgets/radio/score-radio.test.ts | 51 ++++++------------- .../perseus/src/widgets/radio/score-radio.ts | 14 ++--- .../src/widgets/radio/validate-radio.test.ts | 25 +++++++++ .../src/widgets/radio/validate-radio.ts | 29 +++++++++++ 5 files changed, 82 insertions(+), 42 deletions(-) create mode 100644 .changeset/spotty-moles-reply.md create mode 100644 packages/perseus/src/widgets/radio/validate-radio.test.ts create mode 100644 packages/perseus/src/widgets/radio/validate-radio.ts diff --git a/.changeset/spotty-moles-reply.md b/.changeset/spotty-moles-reply.md new file mode 100644 index 0000000000..8ab9761a4d --- /dev/null +++ b/.changeset/spotty-moles-reply.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Introduces a validation function for the radio widget (extracted from the scoring function), though not all validation logic can be extracted. diff --git a/packages/perseus/src/widgets/radio/score-radio.test.ts b/packages/perseus/src/widgets/radio/score-radio.test.ts index 4a13196603..0ad4c50b1c 100644 --- a/packages/perseus/src/widgets/radio/score-radio.test.ts +++ b/packages/perseus/src/widgets/radio/score-radio.test.ts @@ -8,25 +8,6 @@ import type { } from "../../validation.types"; describe("scoreRadio", () => { - it("is invalid when no options are selected", () => { - const userInput: PerseusRadioUserInput = { - choicesSelected: [false, false, false, false], - }; - - const rubric: PerseusRadioRubric = { - choices: [ - {content: "Choice 1"}, - {content: "Choice 2"}, - {content: "Choice 3"}, - {content: "Choice 4"}, - ], - }; - - const result = scoreRadio(userInput, rubric, mockStrings); - - expect(result).toHaveInvalidInput(); - }); - it("is invalid when number selected does not match number correct", () => { const userInput: PerseusRadioUserInput = { choicesSelected: [true, false, false, false], @@ -41,9 +22,9 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveInvalidInput(); + expect(score).toHaveInvalidInput(); }); it("is invalid when none of the above and an answer are both selected", () => { @@ -65,9 +46,9 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveInvalidInput(); + expect(score).toHaveInvalidInput(); }); it("can handle single correct answer", () => { @@ -84,9 +65,9 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveBeenAnsweredCorrectly(); + expect(score).toHaveBeenAnsweredCorrectly(); }); it("can handle single incorrect answer", () => { @@ -103,9 +84,9 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveBeenAnsweredIncorrectly(); + expect(score).toHaveBeenAnsweredIncorrectly(); }); it("can handle multiple correct answer", () => { @@ -122,9 +103,9 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveBeenAnsweredCorrectly(); + expect(score).toHaveBeenAnsweredCorrectly(); }); it("can handle multiple incorrect answer", () => { @@ -141,9 +122,9 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveBeenAnsweredIncorrectly(); + expect(score).toHaveBeenAnsweredIncorrectly(); }); it("can handle none of the above correct answer", () => { @@ -161,9 +142,9 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveBeenAnsweredCorrectly(); + expect(score).toHaveBeenAnsweredCorrectly(); }); it("can handle none of the above incorrect answer", () => { @@ -181,8 +162,8 @@ describe("scoreRadio", () => { ], }; - const result = scoreRadio(userInput, rubric, mockStrings); + const score = scoreRadio(userInput, rubric, mockStrings); - expect(result).toHaveBeenAnsweredIncorrectly(); + expect(score).toHaveBeenAnsweredIncorrectly(); }); }); diff --git a/packages/perseus/src/widgets/radio/score-radio.ts b/packages/perseus/src/widgets/radio/score-radio.ts index 4b907e7965..51586d3c41 100644 --- a/packages/perseus/src/widgets/radio/score-radio.ts +++ b/packages/perseus/src/widgets/radio/score-radio.ts @@ -1,3 +1,5 @@ +import validateRadio from "./validate-radio"; + import type {PerseusStrings} from "../../strings"; import type {PerseusScore} from "../../types"; import type { @@ -10,17 +12,15 @@ function scoreRadio( rubric: PerseusRadioRubric, strings: PerseusStrings, ): PerseusScore { + const validationError = validateRadio(userInput); + if (validationError) { + return validationError; + } + const numSelected = userInput.choicesSelected.reduce((sum, selected) => { return sum + (selected ? 1 : 0); }, 0); - if (numSelected === 0) { - return { - type: "invalid", - message: null, - }; - } - const numCorrect: number = rubric.choices.reduce((sum, currentChoice) => { return currentChoice.correct ? sum + 1 : sum; }, 0); diff --git a/packages/perseus/src/widgets/radio/validate-radio.test.ts b/packages/perseus/src/widgets/radio/validate-radio.test.ts new file mode 100644 index 0000000000..e1b6dd41c2 --- /dev/null +++ b/packages/perseus/src/widgets/radio/validate-radio.test.ts @@ -0,0 +1,25 @@ +import validateRadio from "./validate-radio"; + +import type {PerseusRadioUserInput} from "../../validation.types"; + +describe("validateRadio", () => { + it("is invalid when no options are selected", () => { + const userInput: PerseusRadioUserInput = { + choicesSelected: [false, false, false, false], + }; + + const validationError = validateRadio(userInput); + + expect(validationError).toHaveInvalidInput(); + }); + + it("returns null when validation passes", () => { + const userInput: PerseusRadioUserInput = { + choicesSelected: [true, false, false, false], + }; + + const validationError = validateRadio(userInput); + + expect(validationError).toBeNull(); + }); +}); diff --git a/packages/perseus/src/widgets/radio/validate-radio.ts b/packages/perseus/src/widgets/radio/validate-radio.ts new file mode 100644 index 0000000000..6ca069b852 --- /dev/null +++ b/packages/perseus/src/widgets/radio/validate-radio.ts @@ -0,0 +1,29 @@ +import type {PerseusScore} from "../../types"; +import type {PerseusRadioUserInput} from "../../validation.types"; + +/** + * Checks if the user has selected at least one option. Additional validation + * is done in scoreRadio to check if the number of selected options is correct + * and if the user has selected both a correct option and the "none of the above" + * option. + * @param userInput + * @see `scoreRadio` for the additional validation logic and the scoring logic. + */ +function validateRadio( + userInput: PerseusRadioUserInput, +): Extract | null { + const numSelected = userInput.choicesSelected.reduce((sum, selected) => { + return sum + (selected ? 1 : 0); + }, 0); + + if (numSelected === 0) { + return { + type: "invalid", + message: null, + }; + } + + return null; +} + +export default validateRadio;