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

Sorter: Extract validation out of scoring #1876

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/sweet-trainers-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Split out validation function for the `sorter` widget. This can be used to check if the user has made any changes to the sorting order, confirming whether or not the question can be scored.
47 changes: 46 additions & 1 deletion packages/perseus/src/widgets/sorter/score-sorter.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import scoreSorter from "./score-sorter";
import * as SorterValidator from "./validate-sorter";

import type {
PerseusSorterRubric,
Expand All @@ -7,38 +8,82 @@ import type {

describe("scoreSorter", () => {
it("is correct when the user input values are in the order defined in the rubric", () => {
// Arrange
const userInput: PerseusSorterUserInput = {
options: ["$0.005$ kilograms", "$15$ grams", "$55$ grams"],
changed: true,
};
const rubric: PerseusSorterRubric = {
correct: ["$0.005$ kilograms", "$15$ grams", "$55$ grams"],
};

// Act
const result = scoreSorter(userInput, rubric);

// Assert
expect(result).toHaveBeenAnsweredCorrectly();
});

it("is incorrect when the user input values are not in the order defined in the rubric", () => {
// Arrange
const userInput: PerseusSorterUserInput = {
options: ["$15$ grams", "$55$ grams", "$0.005$ kilograms"],
changed: true,
};
const rubric: PerseusSorterRubric = {
correct: ["$0.005$ kilograms", "$15$ grams", "$55$ grams"],
};

// Act
const result = scoreSorter(userInput, rubric);

// Assert
expect(result).toHaveBeenAnsweredIncorrectly();
});

it("is invalid when the user has not made any changes", () => {
it("should abort if validator returns invalid", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeremywiebe Do the below added tests look as expected? Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, this looks correct! Thanks for adding these!

Matthew has an idea to refactor the scoring so that we don't have to guarantee the scorer calls validation first: https://khanacademy.atlassian.net/browse/LEMS-2658

But for now, let's leave these tests and logic in place.

// Arrange
// Mock validator saying input is invalid
const mockValidate = jest
.spyOn(SorterValidator, "default")
.mockReturnValue({type: "invalid", message: null});

const userInput: PerseusSorterUserInput = {
options: ["$15$ grams", "$55$ grams", "$0.005$ kilograms"],
changed: false,
};
const rubric: PerseusSorterRubric = {
correct: ["$0.005$ kilograms", "$15$ grams", "$55$ grams"],
};

// Act
const result = scoreSorter(userInput, rubric);

// Assert
expect(mockValidate).toHaveBeenCalledWith(userInput);
expect(result).toHaveInvalidInput();
});

it("should score if validator passes", () => {
// Arrange
// Mock validator saying "all good"
const mockValidate = jest
.spyOn(SorterValidator, "default")
.mockReturnValue(null);

const userInput: PerseusSorterUserInput = {
options: ["$0.005$ kilograms", "$15$ grams", "$55$ grams"],
changed: true,
};
const rubric: PerseusSorterRubric = {
correct: ["$0.005$ kilograms", "$15$ grams", "$55$ grams"],
};

// Act
const result = scoreSorter(userInput, rubric);

// Assert
expect(mockValidate).toHaveBeenCalledWith(userInput);
expect(result).toHaveBeenAnsweredCorrectly();
});
});
16 changes: 5 additions & 11 deletions packages/perseus/src/widgets/sorter/score-sorter.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import Util from "../../util";

import validateSorter from "./validate-sorter";

import type {PerseusScore} from "../../types";
import type {
PerseusSorterRubric,
Expand All @@ -10,17 +12,9 @@ function scoreSorter(
userInput: PerseusSorterUserInput,
rubric: PerseusSorterRubric,
): PerseusScore {
// If the sorter widget hasn't been changed yet, we treat it as "empty" which
// prevents the "Check" button from becoming active. We want the user
// to make a change before trying to move forward. This makes an
// assumption that the initial order isn't the correct order! However,
// this should be rare if it happens, and interacting with the list
// will enable the button, so they won't be locked out of progressing.
if (!userInput.changed) {
return {
type: "invalid",
message: null,
};
const validationError = validateSorter(userInput);
if (validationError) {
return validationError;
}

const correct = Util.deepEq(userInput.options, rubric.correct);
Expand Down
33 changes: 33 additions & 0 deletions packages/perseus/src/widgets/sorter/validate-sorter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import validateSorter from "./validate-sorter";

import type {PerseusSorterUserInput} from "../../validation.types";

describe("validateSorter", () => {
it("is invalid when the user has not made any changes", () => {
// Arrange
const userInput: PerseusSorterUserInput = {
options: ["$15$ grams", "$55$ grams", "$0.005$ kilograms"],
changed: false,
};

// Act
const result = validateSorter(userInput);

// Assert
expect(result).toHaveInvalidInput();
});

it("returns null when the user has made any changes", () => {
// Arrange
const userInput: PerseusSorterUserInput = {
options: ["$55$ grams", "$0.005$ kilograms", "$15$ grams"],
changed: true,
};

// Act
const result = validateSorter(userInput);

// Assert
expect(result).toBeNull();
});
});
29 changes: 29 additions & 0 deletions packages/perseus/src/widgets/sorter/validate-sorter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type {PerseusScore} from "../../types";
import type {PerseusSorterUserInput} from "../../validation.types";

/**
* Checks user input for the sorter widget to ensure that the user has made
* changes before attempting to score the widget.
* @param userInput
* @see 'scoreSorter' in 'packages/perseus/src/widgets/sorter/score-sorter.ts'
* for more details on how the sorter widget is scored.
*/
function validateSorter(
userInput: PerseusSorterUserInput,
): Extract<PerseusScore, {type: "invalid"}> | null {
// If the sorter widget hasn't been changed yet, we treat it as "empty" which
// prevents the "Check" button from becoming active. We want the user
// to make a change before trying to move forward. This makes an
// assumption that the initial order isn't the correct order! However,
// this should be rare if it happens, and interacting with the list
// will enable the button, so they won't be locked out of progressing.
if (!userInput.changed) {
return {
type: "invalid",
message: null,
};
}
return null;
}

export default validateSorter;