Skip to content

Commit

Permalink
Radio: Extract validation out of scoring (#1902)
Browse files Browse the repository at this point in the history
## 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: #1902
  • Loading branch information
Myranae authored Nov 26, 2024
1 parent 2437ce6 commit 0cec762
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 42 deletions.
5 changes: 5 additions & 0 deletions .changeset/spotty-moles-reply.md
Original file line number Diff line number Diff line change
@@ -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.
51 changes: 16 additions & 35 deletions packages/perseus/src/widgets/radio/score-radio.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -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", () => {
Expand All @@ -181,8 +162,8 @@ describe("scoreRadio", () => {
],
};

const result = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric, mockStrings);

expect(result).toHaveBeenAnsweredIncorrectly();
expect(score).toHaveBeenAnsweredIncorrectly();
});
});
14 changes: 7 additions & 7 deletions packages/perseus/src/widgets/radio/score-radio.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import validateRadio from "./validate-radio";

import type {PerseusStrings} from "../../strings";
import type {PerseusScore} from "../../types";
import type {
Expand All @@ -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);
Expand Down
25 changes: 25 additions & 0 deletions packages/perseus/src/widgets/radio/validate-radio.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
29 changes: 29 additions & 0 deletions packages/perseus/src/widgets/radio/validate-radio.ts
Original file line number Diff line number Diff line change
@@ -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<PerseusScore, {type: "invalid"}> | 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;

0 comments on commit 0cec762

Please sign in to comment.