Matrix: Extract validation from scorer (#1883)

## Summary:

This PR extracts validation from the `matrix`'s scorer function. In reality, it's an empty function, but it follows our conventions for having the scorer call a validator first. I've created the standard tests to ensure that scorer calls the validator.

Issue: LEMS-2602

## Test plan:

`yarn test`
`yarn typecheck`

Author: jeremywiebe

Reviewers: Myranae, handeyeco, jeremywiebe

Required Reviewers:

Approved By: Myranae, handeyeco

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1883

.changeset/giant-vans-notice.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": minor
+---
+
+Introduces a validation function for the matrix widget (extracted from matrix scoring function).

packages/perseus/src/validation.types.ts

@@ -9,10 +9,10 @@
  * entered. This is referred to as the 'guess' in some older parts of Perseus.
  *
  * `Perseus<Widget>ValidationData`: the data needed to do validation of the
- * user input. Validation is the checks we can do both on the client-side,
- * before submitting user input for scoring, and on the server-side when we
- * score it. As such, it cannot contain any of the sensitive scoring data
- * that would reveal the answer.
+ * user input. Validation refers to the different checks that we can do both on
+ * the client-side (before submitting user input for scoring) and on the
+ * server-side (when we score it). As such, it cannot contain any of the
+ * sensitive scoring data that would reveal the answer.
  *
  * `Perseus<Widget>ScoringData` (nee `Perseus<Widget>Rubric`): the data needed
  * to score the user input. By convention, this type is defined as the set of
@@ -22,7 +22,8 @@
  * For example:
  * ```
  * type Perseus<Widget>ScoringData = {
- *     correct: string;
+ *     correct: string;  // Used _only_ for scoring
+ *     size: number;     // Used _only_ for scoring
  * } & Perseus<Widget>ValidationData;
  * ```
  */
@@ -150,7 +151,9 @@ export type PerseusMatcherUserInput = {
 export type PerseusMatrixRubric = {
     // A data matrix representing the "correct" answers to be entered into the matrix
     answers: PerseusMatrixWidgetAnswers;
-};
+} & PerseusMatrixValidationData;
+
+export type PerseusMatrixValidationData = Empty;
 
 export type PerseusMatrixUserInput = {
     answers: PerseusMatrixRubric["answers"];

packages/perseus/src/widgets/matrix/score-matrix.test.ts

@@ -1,13 +1,74 @@
 import {mockStrings} from "../../strings";
 
 import scoreMatrix from "./score-matrix";
+import * as MatrixValidator from "./validate-matrix";
 
 import type {
     PerseusMatrixRubric,
     PerseusMatrixUserInput,
 } from "../../validation.types";
 
 describe("scoreMatrix", () => {
+    it("should be correctly answerable if validation passes", function () {
+        // Arrange
+        const mockValidator = jest
+            .spyOn(MatrixValidator, "default")
+            .mockReturnValue(null);
+
+        const rubric: PerseusMatrixRubric = {
+            answers: [
+                [0, 1, 2],
+                [3, 4, 5],
+                [6, 7, 8],
+            ],
+        };
+
+        const userInput: PerseusMatrixUserInput = {
+            answers: rubric.answers,
+        };
+
+        // Act
+        const score = scoreMatrix(userInput, rubric, mockStrings);
+
+        // Assert
+        expect(mockValidator).toHaveBeenCalledWith(
+            userInput,
+            rubric,
+            mockStrings,
+        );
+        expect(score).toHaveBeenAnsweredCorrectly();
+    });
+
+    it("should return 'empty' result if validation fails", function () {
+        // Arrange
+        const mockValidator = jest
+            .spyOn(MatrixValidator, "default")
+            .mockReturnValue({type: "invalid", message: null});
+
+        const rubric: PerseusMatrixRubric = {
+            answers: [
+                [0, 1, 2],
+                [3, 4, 5],
+                [6, 7, 8],
+            ],
+        };
+
+        const userInput: PerseusMatrixUserInput = {
+            answers: rubric.answers,
+        };
+
+        // Act
+        const score = scoreMatrix(userInput, rubric, mockStrings);
+
+        // Assert
+        expect(mockValidator).toHaveBeenCalledWith(
+            userInput,
+            rubric,
+            mockStrings,
+        );
+        expect(score).toHaveInvalidInput();
+    });
+
     it("can be answered correctly", () => {
         // Arrange
         const rubric: PerseusMatrixRubric = {

packages/perseus/src/widgets/matrix/score-matrix.ts

@@ -3,6 +3,7 @@ import _ from "underscore";
 import KhanAnswerTypes from "../../util/answer-types";
 
 import {getMatrixSize} from "./matrix";
+import validateMatrix from "./validate-matrix";
 
 import type {PerseusStrings} from "../../strings";
 import type {PerseusScore} from "../../types";
@@ -16,6 +17,11 @@ function scoreMatrix(
     rubric: PerseusMatrixRubric,
     strings: PerseusStrings,
 ): PerseusScore {
+    const validationResult = validateMatrix(userInput, rubric, strings);
+    if (validationResult != null) {
+        return validationResult;
+    }
+
     const solution = rubric.answers;
     const supplied = userInput.answers;
     const solutionSize = getMatrixSize(solution);
@@ -27,16 +33,9 @@ function scoreMatrix(
 
     const createValidator = KhanAnswerTypes.number.createValidatorFunctional;
     let message = null;
-    let hasEmptyCell = false;
     let incorrect = false;
     _(suppliedSize[0]).times((row) => {
         _(suppliedSize[1]).times((col) => {
-            if (
-                supplied[row][col] == null ||
-                supplied[row][col].toString().length === 0
-            ) {
-                hasEmptyCell = true;
-            }
             if (!incorrectSize) {
                 const validator = createValidator(
                     // @ts-expect-error - TS2345 - Argument of type 'number' is not assignable to parameter of type 'string'.
@@ -58,13 +57,6 @@ function scoreMatrix(
         });
     });
 
-    if (hasEmptyCell) {
-        return {
-            type: "invalid",
-            message: strings.fillAllCells,
-        };
-    }
-
     if (incorrectSize) {
         return {
             type: "points",

packages/perseus/src/widgets/matrix/validate-matrix.test.ts

@@ -0,0 +1,50 @@
+import {mockStrings} from "../../strings";
+
+import validateMatrix from "./validate-matrix";
+
+import type {PerseusMatrixUserInput} from "../../validation.types";
+
+describe("matrixValidator", () => {
+    it("should return invalid when answers is completely empty", () => {
+        // Arrange
+        const userInput: PerseusMatrixUserInput = {
+            answers: [[]],
+        };
+
+        // Act
+        const result = validateMatrix(userInput, {}, mockStrings);
+
+        // Assert
+        expect(result).toHaveInvalidInput();
+    });
+
+    it("should return invalid when any answer row is empty", () => {
+        // Arrange
+        const userInput: PerseusMatrixUserInput = {
+            answers: [[1, 2, 3], [], [7, 8, 9]],
+        };
+
+        // Act
+        const result = validateMatrix(userInput, {}, mockStrings);
+
+        // Assert
+        expect(result).toHaveInvalidInput();
+    });
+
+    it("should return null for non-empty user input", () => {
+        // Arrange
+        const userInput: PerseusMatrixUserInput = {
+            answers: [
+                [1, 2, 3],
+                [4, 5, 6],
+                [7, 8, 9],
+            ],
+        };
+
+        // Act
+        const result = validateMatrix(userInput, {}, mockStrings);
+
+        // Assert
+        expect(result).toBeNull();
+    });
+});

packages/perseus/src/widgets/matrix/validate-matrix.ts

@@ -0,0 +1,45 @@
+import _ from "underscore";
+
+import {getMatrixSize} from "./matrix";
+
+import type {PerseusStrings} from "../../strings";
+import type {PerseusScore} from "../../types";
+import type {
+    PerseusMatrixUserInput,
+    PerseusMatrixValidationData,
+} from "../../validation.types";
+
+/**
+ * Checks user input from the matrix widget to see if it is scorable.
+ *
+ * Note: The matrix widget cannot do much validation without the Scoring
+ * Data because of its use of KhanAnswerTypes as a core part of scoring.
+ *
+ * @see `scoreMatrix()` for more details.
+ */
+function validateMatrix(
+    userInput: PerseusMatrixUserInput,
+    validationData: PerseusMatrixValidationData,
+    strings: PerseusStrings,
+): Extract<PerseusScore, {type: "invalid"}> | null {
+    const supplied = userInput.answers;
+    const suppliedSize = getMatrixSize(supplied);
+
+    for (let row = 0; row < suppliedSize[0]; row++) {
+        for (let col = 0; col < suppliedSize[1]; col++) {
+            if (
+                supplied[row][col] == null ||
+                supplied[row][col].toString().length === 0
+            ) {
+                return {
+                    type: "invalid",
+                    message: strings.fillAllCells,
+                };
+            }
+        }
+    }
+
+    return null;
+}
+
+export default validateMatrix;