diff --git a/.changeset/pink-pumas-hug.md b/.changeset/pink-pumas-hug.md new file mode 100644 index 0000000000..d37b81ea10 --- /dev/null +++ b/.changeset/pink-pumas-hug.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Use empty widgets check in scoring function diff --git a/packages/perseus/src/renderer-util.test.ts b/packages/perseus/src/renderer-util.test.ts index 2bdfc26fca..0a5fe26905 100644 --- a/packages/perseus/src/renderer-util.test.ts +++ b/packages/perseus/src/renderer-util.test.ts @@ -1,4 +1,4 @@ -import {screen} from "@testing-library/react"; +import {act, screen} from "@testing-library/react"; import {userEvent as userEventLib} from "@testing-library/user-event"; import { @@ -6,6 +6,7 @@ import { 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", - }); }); }); }); diff --git a/packages/perseus/src/renderer-util.ts b/packages/perseus/src/renderer-util.ts index 9b7826a6aa..b0a059042c 100644 --- a/packages/perseus/src/renderer-util.ts +++ b/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, + widgetIds: ReadonlyArray, 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; }