Move some more validation logic out of matrix scorer into validator

Khan · Nov 20, 2024 · 718fc62 · 718fc62
1 parent eaa1492
commit 718fc62
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 29 deletions.
diff --git a/packages/perseus/src/widgets/matrix/score-matrix.test.ts b/packages/perseus/src/widgets/matrix/score-matrix.test.ts
@@ -31,7 +31,11 @@ describe("scoreMatrix", () => {
         const score = scoreMatrix(userInput, rubric, mockStrings);
 
         // Assert
-        expect(mockValidator).toHaveBeenCalledWith(userInput, rubric);
+        expect(mockValidator).toHaveBeenCalledWith(
+            userInput,
+            rubric,
+            mockStrings,
+        );
         expect(score).toHaveBeenAnsweredCorrectly();
     });
 
@@ -57,7 +61,11 @@ describe("scoreMatrix", () => {
         const score = scoreMatrix(userInput, rubric, mockStrings);
 
         // Assert
-        expect(mockValidator).toHaveBeenCalledWith(userInput, rubric);
+        expect(mockValidator).toHaveBeenCalledWith(
+            userInput,
+            rubric,
+            mockStrings,
+        );
         expect(score).toHaveInvalidInput();
     });
 

diff --git a/packages/perseus/src/widgets/matrix/score-matrix.ts b/packages/perseus/src/widgets/matrix/score-matrix.ts
@@ -17,7 +17,7 @@ function scoreMatrix(
     rubric: PerseusMatrixRubric,
     strings: PerseusStrings,
 ): PerseusScore {
-    const validationResult = validateMatrix(userInput, rubric);
+    const validationResult = validateMatrix(userInput, rubric, strings);
     if (validationResult != null) {
         return validationResult;
     }
@@ -33,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'.
@@ -64,13 +57,6 @@ function scoreMatrix(
         });
     });
 
-    if (hasEmptyCell) {
-        return {
-            type: "invalid",
-            message: strings.fillAllCells,
-        };
-    }
-
     if (incorrectSize) {
         return {
             type: "points",

diff --git a/packages/perseus/src/widgets/matrix/validate-matrix.test.ts b/packages/perseus/src/widgets/matrix/validate-matrix.test.ts
@@ -1,3 +1,5 @@
+import {mockStrings} from "../../strings";
+
 import validateMatrix from "./validate-matrix";
 
 import type {PerseusMatrixUserInput} from "../../validation.types";
@@ -6,11 +8,11 @@ describe("matrixValidator", () => {
     it("should return invalid when answers is completely empty", () => {
         // Arrange
         const userInput: PerseusMatrixUserInput = {
-            answers: [],
+            answers: [[]],
         };
 
         // Act
-        const result = validateMatrix(userInput, {});
+        const result = validateMatrix(userInput, {}, mockStrings);
 
         // Assert
         expect(result).toHaveInvalidInput();
@@ -23,7 +25,7 @@ describe("matrixValidator", () => {
         };
 
         // Act
-        const result = validateMatrix(userInput, {});
+        const result = validateMatrix(userInput, {}, mockStrings);
 
         // Assert
         expect(result).toHaveInvalidInput();
@@ -40,7 +42,7 @@ describe("matrixValidator", () => {
         };
 
         // Act
-        const result = validateMatrix(userInput, {});
+        const result = validateMatrix(userInput, {}, mockStrings);
 
         // Assert
         expect(result).toBeNull();

diff --git a/packages/perseus/src/widgets/matrix/validate-matrix.ts b/packages/perseus/src/widgets/matrix/validate-matrix.ts
@@ -1,3 +1,8 @@
+import _ from "underscore";
+
+import {getMatrixSize} from "./matrix";
+
+import type {PerseusStrings} from "../../strings";
 import type {PerseusScore} from "../../types";
 import type {
     PerseusMatrixUserInput,
@@ -14,16 +19,26 @@ import type {
  */
 function validateMatrix(
     userInput: PerseusMatrixUserInput,
-    rubric: PerseusMatrixValidationData,
+    validationData: PerseusMatrixValidationData,
+    strings: PerseusStrings,
 ): Extract<PerseusScore, {type: "invalid"}> | null {
-    // Very basic check: did we get 0 rows or any rows with 0 answers? If so,
-    // then the user input is not gradable.
-    if (
-        userInput.answers.length === 0 ||
-        userInput.answers.some((row) => row.length === 0)
-    ) {
-        return {type: "invalid", message: 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;
 }