From 7d5bef7ad82632afb756688981a023bbb112c9f2 Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Mon, 18 Nov 2024 17:06:30 -0800 Subject: [PATCH 1/5] Create numeric-input validator and tests even though its empty --- .../numeric-input/score-numeric-input.test.ts | 79 ++++++++++++++++--- .../numeric-input/score-numeric-input.ts | 7 ++ .../validate-numeric-input.test.ts | 0 .../numeric-input/validate-numeric-input.ts | 18 +++++ 4 files changed, 94 insertions(+), 10 deletions(-) create mode 100644 packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts create mode 100644 packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts diff --git a/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts index c6ee90ec2c..c096a09736 100644 --- a/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts @@ -3,6 +3,7 @@ import * as Dependencies from "../../dependencies"; import {mockStrings} from "../../strings"; import scoreNumericInput, {maybeParsePercentInput} from "./score-numeric-input"; +import * as NumericInputValidator from "./validate-numeric-input"; import type {PerseusNumericInputRubric} from "../../validation.types"; @@ -13,6 +14,64 @@ describe("static function validate", () => { ); }); + it("should be correctly answerable if validation passes", function () { + // Arrange + const mockValidator = jest + .spyOn(NumericInputValidator, "default") + .mockReturnValue(null); + const rubric: PerseusNumericInputRubric = { + answers: [ + { + value: 1, + status: "correct", + maxError: 0, + simplify: "", + strict: false, + message: "", + }, + ], + coefficient: true, + }; + + const userInput = {currentValue: "1"} as const; + + // Act + const score = scoreNumericInput(userInput, rubric, mockStrings); + + // Assert + expect(mockValidator).toHaveBeenCalledWith(userInput); + expect(score).toHaveBeenAnsweredCorrectly(); + }); + + it("should return 'empty' result if validation fails", function () { + // Arrange + const mockValidator = jest + .spyOn(NumericInputValidator, "default") + .mockReturnValue({type: "invalid", message: null}); + const rubric: PerseusNumericInputRubric = { + answers: [ + { + value: 1, + status: "correct", + maxError: 0, + simplify: "", + strict: false, + message: "", + }, + ], + coefficient: true, + }; + + const userInput = {currentValue: "1"} as const; + + // Act + const score = scoreNumericInput(userInput, rubric, mockStrings); + + // Assert + expect(mockValidator).toHaveBeenCalled(); + expect(score).toHaveInvalidInput(); + }); + it("with a simple value", () => { const rubric: PerseusNumericInputRubric = { answers: [ @@ -28,11 +87,11 @@ describe("static function validate", () => { coefficient: true, }; - const useInput = { + const userInput = { currentValue: "1", } as const; - const score = scoreNumericInput(useInput, rubric, mockStrings); + const score = scoreNumericInput(userInput, rubric, mockStrings); expect(score).toHaveBeenAnsweredCorrectly(); }); @@ -52,11 +111,11 @@ describe("static function validate", () => { coefficient: true, }; - const useInput = { + const userInput = { currentValue: "sadasdfas", } as const; - const score = scoreNumericInput(useInput, rubric, mockStrings); + const score = scoreNumericInput(userInput, rubric, mockStrings); expect(score).toHaveInvalidInput( "We could not understand your answer. Please check your answer for extra text or symbols.", @@ -143,11 +202,11 @@ describe("static function validate", () => { coefficient: true, }; - const useInput = { + const userInput = { currentValue: "1.0", } as const; - const score = scoreNumericInput(useInput, rubric, mockStrings); + const score = scoreNumericInput(userInput, rubric, mockStrings); expect(score).toHaveBeenAnsweredCorrectly(); }); @@ -167,11 +226,11 @@ describe("static function validate", () => { coefficient: true, }; - const useInput = { + const userInput = { currentValue: "1.3", } as const; - const score = scoreNumericInput(useInput, rubric, mockStrings); + const score = scoreNumericInput(userInput, rubric, mockStrings); expect(score).toHaveBeenAnsweredIncorrectly(); }); @@ -191,11 +250,11 @@ describe("static function validate", () => { coefficient: true, }; - const useInput = { + const userInput = { currentValue: "1.12", } as const; - const score = scoreNumericInput(useInput, rubric, mockStrings); + const score = scoreNumericInput(userInput, rubric, mockStrings); expect(score).toHaveBeenAnsweredCorrectly(); }); diff --git a/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts index 46a98eaf12..2cca4ef0c9 100644 --- a/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts +++ b/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts @@ -1,6 +1,8 @@ import TexWrangler from "../../tex-wrangler"; import KhanAnswerTypes from "../../util/answer-types"; +import validateNumericInput from "./validate-numeric-input"; + import type {MathFormat, PerseusNumericInputAnswer} from "../../perseus-types"; import type {PerseusStrings} from "../../strings"; import type {PerseusScore} from "../../types"; @@ -70,6 +72,11 @@ function scoreNumericInput( rubric: PerseusNumericInputRubric, strings: PerseusStrings, ): PerseusScore { + const validationResult = validateNumericInput(userInput); + if (validationResult != null) { + return validationResult; + } + const defaultAnswerForms = answerFormButtons .map((e) => e["value"]) // Don't default to validating the answer as a pi answer diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts new file mode 100644 index 0000000000..d51b324f77 --- /dev/null +++ b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts @@ -0,0 +1,18 @@ +import type {PerseusScore} from "../../types"; +import type {PerseusNumericInputUserInput} from "../../validation.types"; + +/** + * Checks user input from the numeric-input widget to see if it is scorable. + * + * Note: The numeric-input widget cannot do any validation without the Scoring + * Data because of its use of KhanAnswerTypes as a core part of scoring. + * + * @see `scoreNumericInput()` for more details. + */ +function validateNumericInput( + userInput: PerseusNumericInputUserInput, +): Extract | null { + return null; +} + +export default validateNumericInput; From 82cfce61a52666d6cce8af0886a8d9e1e8d19367 Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Mon, 18 Nov 2024 17:09:54 -0800 Subject: [PATCH 2/5] Changeset --- .changeset/honest-avocados-accept.md | 5 +++++ packages/perseus/src/validation.types.ts | 4 +++- .../src/widgets/numeric-input/score-numeric-input.test.ts | 4 ++-- .../src/widgets/numeric-input/score-numeric-input.ts | 2 +- .../widgets/numeric-input/validate-numeric-input.test.ts | 7 +++++++ .../src/widgets/numeric-input/validate-numeric-input.ts | 6 +++++- 6 files changed, 23 insertions(+), 5 deletions(-) create mode 100644 .changeset/honest-avocados-accept.md diff --git a/.changeset/honest-avocados-accept.md b/.changeset/honest-avocados-accept.md new file mode 100644 index 0000000000..4d7d3129cf --- /dev/null +++ b/.changeset/honest-avocados-accept.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Introduces a validation function for the numeric-input widget (extracted from numeric-input scoring function). diff --git a/packages/perseus/src/validation.types.ts b/packages/perseus/src/validation.types.ts index 68674223c7..ffa7023c42 100644 --- a/packages/perseus/src/validation.types.ts +++ b/packages/perseus/src/validation.types.ts @@ -173,12 +173,14 @@ export type PerseusNumericInputRubric = { answers: ReadonlyArray; // A coefficient style number allows the student to use - for -1 and an empty string to mean 1. coefficient: boolean; -}; +} & PerseusNumericInputValidationData; export type PerseusNumericInputUserInput = { currentValue: string; }; +export type PerseusNumericInputValidationData = Empty; + export type PerseusOrdererRubric = PerseusOrdererWidgetOptions; export type PerseusOrdererUserInput = { diff --git a/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts index c096a09736..af041ce1ef 100644 --- a/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts @@ -39,7 +39,7 @@ describe("static function validate", () => { const score = scoreNumericInput(userInput, rubric, mockStrings); // Assert - expect(mockValidator).toHaveBeenCalledWith(userInput); + expect(mockValidator).toHaveBeenCalledWith(userInput, rubric); expect(score).toHaveBeenAnsweredCorrectly(); }); @@ -68,7 +68,7 @@ describe("static function validate", () => { const score = scoreNumericInput(userInput, rubric, mockStrings); // Assert - expect(mockValidator).toHaveBeenCalled(); + expect(mockValidator).toHaveBeenCalledWith(userInput, rubric); expect(score).toHaveInvalidInput(); }); diff --git a/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts index 2cca4ef0c9..c611e34fff 100644 --- a/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts +++ b/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts @@ -72,7 +72,7 @@ function scoreNumericInput( rubric: PerseusNumericInputRubric, strings: PerseusStrings, ): PerseusScore { - const validationResult = validateNumericInput(userInput); + const validationResult = validateNumericInput(userInput, rubric); if (validationResult != null) { return validationResult; } diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts index e69de29bb2..13f6cedb28 100644 --- a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts @@ -0,0 +1,7 @@ +import validateNumericInput from "./validate-numeric-input"; + +describe("validateNumericInput", () => { + it("should return null always", () => { + expect(validateNumericInput({currentValue: "1"}, {})).toBeNull(); + }); +}); diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts index d51b324f77..bc303bbcbd 100644 --- a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts +++ b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts @@ -1,5 +1,8 @@ import type {PerseusScore} from "../../types"; -import type {PerseusNumericInputUserInput} from "../../validation.types"; +import type { + PerseusNumericInputUserInput, + PerseusNumericInputValidationData, +} from "../../validation.types"; /** * Checks user input from the numeric-input widget to see if it is scorable. @@ -11,6 +14,7 @@ import type {PerseusNumericInputUserInput} from "../../validation.types"; */ function validateNumericInput( userInput: PerseusNumericInputUserInput, + validationData: PerseusNumericInputValidationData, ): Extract | null { return null; } From 7992189c167f92ac73ca245acaa7598f512620fb Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Tue, 19 Nov 2024 12:05:08 -0800 Subject: [PATCH 3/5] Add validation for empty user input --- .../numeric-input/validate-numeric-input.test.ts | 10 ++++++++-- .../widgets/numeric-input/validate-numeric-input.ts | 8 ++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts index 13f6cedb28..b8a69e0a37 100644 --- a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts @@ -1,7 +1,13 @@ import validateNumericInput from "./validate-numeric-input"; describe("validateNumericInput", () => { - it("should return null always", () => { - expect(validateNumericInput({currentValue: "1"}, {})).toBeNull(); + it("should return invalid for empty user input", () => { + const result = validateNumericInput({currentValue: ""}, {}); + expect(result).toHaveInvalidInput(); + }); + + it("should return null for non-empty user input", () => { + const result = validateNumericInput({currentValue: "1"}, {}); + expect(result).toBeNull(); }); }); diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts index bc303bbcbd..6ccd16b8ce 100644 --- a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts +++ b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts @@ -7,8 +7,8 @@ import type { /** * Checks user input from the numeric-input widget to see if it is scorable. * - * Note: The numeric-input widget cannot do any validation without the Scoring - * Data because of its use of KhanAnswerTypes as a core part of scoring. + * Note: Most of the expression widget's validation requires the Rubric because + * of its use of KhanAnswerTypes as a core part of scoring. * * @see `scoreNumericInput()` for more details. */ @@ -16,6 +16,10 @@ function validateNumericInput( userInput: PerseusNumericInputUserInput, validationData: PerseusNumericInputValidationData, ): Extract | null { + if (userInput.currentValue === "") { + return {type: "invalid", message: null}; + } + return null; } From 33a71ff3d104003b814f3d2791ce9b53463d593a Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Wed, 20 Nov 2024 11:56:00 -0800 Subject: [PATCH 4/5] Update packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts Co-authored-by: Matthew --- .../perseus/src/widgets/numeric-input/validate-numeric-input.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts index 6ccd16b8ce..d0aff7722b 100644 --- a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts +++ b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts @@ -7,7 +7,7 @@ import type { /** * Checks user input from the numeric-input widget to see if it is scorable. * - * Note: Most of the expression widget's validation requires the Rubric because + * Note: Most of the numeric-input widget's validation requires the Rubric because * of its use of KhanAnswerTypes as a core part of scoring. * * @see `scoreNumericInput()` for more details. From 6a26500c0832a388c96915bdead6e11a5ac315fd Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Wed, 20 Nov 2024 13:44:56 -0800 Subject: [PATCH 5/5] Remove empty numeric-input validator --- packages/perseus/src/validation.types.ts | 4 +- .../numeric-input/score-numeric-input.test.ts | 59 ------------------- .../numeric-input/score-numeric-input.ts | 9 +-- .../validate-numeric-input.test.ts | 13 ---- .../numeric-input/validate-numeric-input.ts | 26 -------- 5 files changed, 2 insertions(+), 109 deletions(-) delete mode 100644 packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts delete mode 100644 packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts diff --git a/packages/perseus/src/validation.types.ts b/packages/perseus/src/validation.types.ts index ffa7023c42..68674223c7 100644 --- a/packages/perseus/src/validation.types.ts +++ b/packages/perseus/src/validation.types.ts @@ -173,14 +173,12 @@ export type PerseusNumericInputRubric = { answers: ReadonlyArray; // A coefficient style number allows the student to use - for -1 and an empty string to mean 1. coefficient: boolean; -} & PerseusNumericInputValidationData; +}; export type PerseusNumericInputUserInput = { currentValue: string; }; -export type PerseusNumericInputValidationData = Empty; - export type PerseusOrdererRubric = PerseusOrdererWidgetOptions; export type PerseusOrdererUserInput = { diff --git a/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts index af041ce1ef..de2a226656 100644 --- a/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts +++ b/packages/perseus/src/widgets/numeric-input/score-numeric-input.test.ts @@ -3,7 +3,6 @@ import * as Dependencies from "../../dependencies"; import {mockStrings} from "../../strings"; import scoreNumericInput, {maybeParsePercentInput} from "./score-numeric-input"; -import * as NumericInputValidator from "./validate-numeric-input"; import type {PerseusNumericInputRubric} from "../../validation.types"; @@ -14,64 +13,6 @@ describe("static function validate", () => { ); }); - it("should be correctly answerable if validation passes", function () { - // Arrange - const mockValidator = jest - .spyOn(NumericInputValidator, "default") - .mockReturnValue(null); - const rubric: PerseusNumericInputRubric = { - answers: [ - { - value: 1, - status: "correct", - maxError: 0, - simplify: "", - strict: false, - message: "", - }, - ], - coefficient: true, - }; - - const userInput = {currentValue: "1"} as const; - - // Act - const score = scoreNumericInput(userInput, rubric, mockStrings); - - // Assert - expect(mockValidator).toHaveBeenCalledWith(userInput, rubric); - expect(score).toHaveBeenAnsweredCorrectly(); - }); - - it("should return 'empty' result if validation fails", function () { - // Arrange - const mockValidator = jest - .spyOn(NumericInputValidator, "default") - .mockReturnValue({type: "invalid", message: null}); - const rubric: PerseusNumericInputRubric = { - answers: [ - { - value: 1, - status: "correct", - maxError: 0, - simplify: "", - strict: false, - message: "", - }, - ], - coefficient: true, - }; - - const userInput = {currentValue: "1"} as const; - - // Act - const score = scoreNumericInput(userInput, rubric, mockStrings); - - // Assert - expect(mockValidator).toHaveBeenCalledWith(userInput, rubric); - expect(score).toHaveInvalidInput(); - }); - it("with a simple value", () => { const rubric: PerseusNumericInputRubric = { answers: [ diff --git a/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts index c611e34fff..d4637185ec 100644 --- a/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts +++ b/packages/perseus/src/widgets/numeric-input/score-numeric-input.ts @@ -1,8 +1,6 @@ import TexWrangler from "../../tex-wrangler"; import KhanAnswerTypes from "../../util/answer-types"; -import validateNumericInput from "./validate-numeric-input"; - import type {MathFormat, PerseusNumericInputAnswer} from "../../perseus-types"; import type {PerseusStrings} from "../../strings"; import type {PerseusScore} from "../../types"; @@ -63,7 +61,7 @@ export function maybeParsePercentInput( return value / 100; } - // Otherwise, we return input valuÄe (number) stripped of the "%". + // Otherwise, we return input value (number) stripped of the "%". return value; } @@ -72,11 +70,6 @@ function scoreNumericInput( rubric: PerseusNumericInputRubric, strings: PerseusStrings, ): PerseusScore { - const validationResult = validateNumericInput(userInput, rubric); - if (validationResult != null) { - return validationResult; - } - const defaultAnswerForms = answerFormButtons .map((e) => e["value"]) // Don't default to validating the answer as a pi answer diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts deleted file mode 100644 index b8a69e0a37..0000000000 --- a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.test.ts +++ /dev/null @@ -1,13 +0,0 @@ -import validateNumericInput from "./validate-numeric-input"; - -describe("validateNumericInput", () => { - it("should return invalid for empty user input", () => { - const result = validateNumericInput({currentValue: ""}, {}); - expect(result).toHaveInvalidInput(); - }); - - it("should return null for non-empty user input", () => { - const result = validateNumericInput({currentValue: "1"}, {}); - expect(result).toBeNull(); - }); -}); diff --git a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts b/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts deleted file mode 100644 index d0aff7722b..0000000000 --- a/packages/perseus/src/widgets/numeric-input/validate-numeric-input.ts +++ /dev/null @@ -1,26 +0,0 @@ -import type {PerseusScore} from "../../types"; -import type { - PerseusNumericInputUserInput, - PerseusNumericInputValidationData, -} from "../../validation.types"; - -/** - * Checks user input from the numeric-input widget to see if it is scorable. - * - * Note: Most of the numeric-input widget's validation requires the Rubric because - * of its use of KhanAnswerTypes as a core part of scoring. - * - * @see `scoreNumericInput()` for more details. - */ -function validateNumericInput( - userInput: PerseusNumericInputUserInput, - validationData: PerseusNumericInputValidationData, -): Extract | null { - if (userInput.currentValue === "") { - return {type: "invalid", message: null}; - } - - return null; -} - -export default validateNumericInput;