[Client Validation] Use emptyWidgetsFunctional in scoring

.changeset/pink-pumas-hug.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": minor
+---
+
+Use empty widgets check in scoring function

packages/perseus/src/renderer-util.test.ts

@@ -1,11 +1,12 @@
-import {screen} from "@testing-library/react";
+import {act, screen} from "@testing-library/react";
 import {userEvent as userEventLib} from "@testing-library/user-event";
 
 import {
     testDependencies,
     testDependenciesV2,
 } from "../../../testing/test-dependencies";
 
+import {question1} from "./__testdata__/renderer.testdata";
 import * as Dependencies from "./dependencies";
 import {
     emptyWidgetsFunctional,
@@ -15,7 +16,7 @@ import {
 import {mockStrings} from "./strings";
 import {registerAllWidgetsForTesting} from "./util/register-all-widgets-for-testing";
 import {renderQuestion} from "./widgets/__testutils__/renderQuestion";
-import {question1} from "./widgets/group/group.testdata";
+import DropdownWidgetExport from "./widgets/dropdown";
 
 import type {UserInputMap} from "./validation.types";
 import type {
@@ -78,6 +79,7 @@ function getLegacyExpressionWidget() {
         },
     };
 }
+
 describe("renderer utils", () => {
     beforeAll(() => {
         registerAllWidgetsForTesting();
@@ -743,23 +745,151 @@ describe("renderer utils", () => {
             });
         });
 
+        it("should return empty if any validator returns empty", () => {
+            // Act
+            const validatorSpy = jest
+                .spyOn(DropdownWidgetExport, "validator")
+                // 1st call - Empty
+                .mockReturnValueOnce({
+                    type: "invalid",
+                    message: null,
+                })
+                // 2nd call - Not empty
+                .mockReturnValueOnce(null);
+            const scoringSpy = jest
+                .spyOn(DropdownWidgetExport, "scorer")
+                .mockReturnValueOnce({type: "points", total: 1, earned: 1});
+
+            // Act
+            const score = scorePerseusItem(
+                {
+                    content:
+                        question1.content +
+                        question1.content.replace("dropdown 1", "dropdown 2"),
+                    widgets: {
+                        "dropdown 1": question1.widgets["dropdown 1"],
+                        "dropdown 2": question1.widgets["dropdown 1"],
+                    },
+                    images: {},
+                },
+                {},
+                mockStrings,
+                "en",
+            );
+
+            // Assert
+            expect(validatorSpy).toHaveBeenCalledTimes(2);
+            // Scoring is only called if validation passes
+            expect(scoringSpy).toHaveBeenCalledTimes(1);
+            expect(score).toEqual({type: "invalid", message: null});
+        });
+
+        it("should score item if all validators return null", () => {
+            // Arrange
+            const validatorSpy = jest
+                .spyOn(DropdownWidgetExport, "validator")
+                .mockReturnValue(null);
+            const scoreSpy = jest
+                .spyOn(DropdownWidgetExport, "scorer")
+                .mockReturnValue({
+                    type: "points",
+                    total: 1,
+                    earned: 1,
+                    message: null,
+                });
+
+            // Act
+            const score = scorePerseusItem(
+                {
+                    content:
+                        question1.content +
+                        question1.content.replace("dropdown 1", "dropdown 2"),
+                    widgets: {
+                        "dropdown 1": question1.widgets["dropdown 1"],
+                        "dropdown 2": question1.widgets["dropdown 1"],
+                    },
+                    images: {},
+                },
+                {"dropdown 1": {value: 0}},
+                mockStrings,
+                "en",
+            );
+
+            // Assert
+            expect(validatorSpy).toHaveBeenCalledTimes(2);
+            expect(scoreSpy).toHaveBeenCalledTimes(2);
+            expect(score).toEqual({
+                type: "points",
+                total: 2,
+                earned: 2,
+                message: null,
+            });
+        });
+
+        it("should return correct, with no points earned, if widget is static", () => {
+            const validatorSpy = jest.spyOn(DropdownWidgetExport, "validator");
+
+            const score = scorePerseusItem(
+                {
+                    ...question1,
+                    widgets: {
+                        "dropdown 1": {
+                            ...question1.widgets["dropdown 1"],
+                            static: true,
+                        },
+                    },
+                },
+                {"dropdown 1": {value: 1}},
+                mockStrings,
+                "en",
+            );
+
+            expect(validatorSpy).not.toHaveBeenCalled();
+            expect(score).toHaveBeenAnsweredCorrectly({
+                shouldHavePoints: false,
+            });
+        });
+
+        it("should ignore widgets that aren't referenced in content", () => {
+            const validatorSpy = jest.spyOn(DropdownWidgetExport, "validator");
+            const score = scorePerseusItem(
+                {
+                    content:
+                        "This content references [[☃ dropdown 1]] but not dropdown 2!",
+                    widgets: {
+                        ...question1.widgets,
+                        "dropdown 2": {
+                            ...question1.widgets["dropdown 1"],
+                        },
+                    },
+                    images: {},
+                },
+                {"dropdown 1": {value: 2}},
+                mockStrings,
+                "en",
+            );
+
+            expect(validatorSpy).toHaveBeenCalledTimes(1);
+            expect(score).toHaveBeenAnsweredCorrectly({
+                shouldHavePoints: true,
+            });
+        });
+
         it("should return score from contained Renderer", async () => {
             // Arrange
             const {renderer} = renderQuestion(question1);
-            // Answer all widgets correctly
-            await userEvent.click(screen.getAllByRole("radio")[4]);
-            // Note(jeremy): If we don't tab away from the radio button in this
-            // test, it seems like the userEvent typing doesn't land in the first
-            // text field.
-            await userEvent.tab();
-            await userEvent.type(
-                screen.getByRole("textbox", {name: /nearest ten/}),
-                "230",
-            );
-            await userEvent.type(
-                screen.getByRole("textbox", {name: /nearest hundred/}),
-                "200",
+
+            // Answer correctly
+            await userEvent.click(screen.getByRole("combobox"));
+            await act(() => jest.runOnlyPendingTimers());
+
+            await userEvent.click(
+                screen.getByRole("option", {
+                    name: "less than or equal to",
+                }),
             );
+
+            // Act
             const userInput = renderer.getUserInputMap();
             const score = scorePerseusItem(
                 question1,
@@ -770,12 +900,6 @@ describe("renderer utils", () => {
 
             // Assert
             expect(score).toHaveBeenAnsweredCorrectly();
-            expect(score).toEqual({
-                earned: 3,
-                message: null,
-                total: 3,
-                type: "points",
-            });
         });
     });
 });

packages/perseus/src/renderer-util.ts

@@ -1,4 +1,8 @@
-import {mapObject} from "@khanacademy/perseus-core";
+import {
+    mapObject,
+    type PerseusRenderer,
+    type PerseusWidgetsMap,
+} from "@khanacademy/perseus-core";
 
 import {scoreIsEmpty, flattenScores} from "./util/scoring";
 import {getWidgetIdsFromContent} from "./widget-type-utils";
@@ -10,15 +14,7 @@ import {
 
 import type {PerseusStrings} from "./strings";
 import type {PerseusScore} from "./types";
-import type {
-    UserInput,
-    UserInputMap,
-    ValidationDataMap,
-} from "./validation.types";
-import type {
-    PerseusRenderer,
-    PerseusWidgetsMap,
-} from "@khanacademy/perseus-core";
+import type {ValidationDataMap, UserInputMap} from "./validation.types";
 
 export function getUpgradedWidgetOptions(
     oldWidgetOptions: PerseusWidgetsMap,
@@ -54,7 +50,7 @@ export function emptyWidgetsFunctional(
     widgets: ValidationDataMap,
     // This is a port of old code, I'm not sure why
     // we need widgetIds vs the keys of the widgets object
-    widgetIds: Array<string>,
+    widgetIds: ReadonlyArray<string>,
     userInputMap: UserInputMap,
     strings: PerseusStrings,
     locale: string,
@@ -128,13 +124,14 @@ export function scoreWidgetsFunctional(
         }
 
         const userInput = userInputMap[id];
+        const validator = getWidgetValidator(widget.type);
         const scorer = getWidgetScorer(widget.type);
-        const score = scorer?.(
-            userInput as UserInput,
-            widget.options,
-            strings,
-            locale,
-        );
+
+        // We do validation (empty checks) first and then scoring. If
+        // validation fails, it's result is itself a PerseusScore.
+        const score =
+            validator?.(userInput, widget.options, strings, locale) ??
+            scorer?.(userInput, widget.options, strings, locale);
         if (score != null) {
             widgetScores[id] = score;
         }