Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Radio: Extract validation out of scoring #1902

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
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,
};
}

Comment on lines -17 to -23
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remaining validation checks rely on data in the scoringData object, so I don't think we can move those to the validation function. If it's possible, would love to know so we can move all that over!

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;