From f35512786117e51515a50f3a769c45de9714dfc2 Mon Sep 17 00:00:00 2001 From: Jeremy Wiebe Date: Wed, 4 Dec 2024 15:38:50 -0800 Subject: [PATCH] SSS: Move group scoring into group scorer (#1946) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: This PR refactors the scoring around `group` so that it follows the same pattern as all the other widgets. This means that our main `scoreWidgetsFunctional` can be simplified to remove the special case for the `group` widget. All tests still pass! Issue: LEMS-2561 ## Test plan: `yarn test` `yarn typecheck` Author: jeremywiebe Reviewers: Myranae, handeyeco, jeremywiebe Required Reviewers: Approved By: handeyeco Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1946 --- .changeset/metal-readers-impress.md | 5 ++ .changeset/stupid-apricots-retire.md | 5 ++ .../src/components/graph-settings.tsx | 1 - packages/perseus/src/renderer-util.ts | 50 ++++++------------- packages/perseus/src/validation.types.ts | 1 + packages/perseus/src/widgets/group/group.tsx | 5 ++ .../perseus/src/widgets/group/score-group.ts | 30 +++++++++++ 7 files changed, 60 insertions(+), 37 deletions(-) create mode 100644 .changeset/metal-readers-impress.md create mode 100644 .changeset/stupid-apricots-retire.md create mode 100644 packages/perseus/src/widgets/group/score-group.ts diff --git a/.changeset/metal-readers-impress.md b/.changeset/metal-readers-impress.md new file mode 100644 index 0000000000..5303398c8f --- /dev/null +++ b/.changeset/metal-readers-impress.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Refactor scoring for `group` widget to follow the same pattern as all other widgets diff --git a/.changeset/stupid-apricots-retire.md b/.changeset/stupid-apricots-retire.md new file mode 100644 index 0000000000..5011192a2e --- /dev/null +++ b/.changeset/stupid-apricots-retire.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus-editor": patch +--- + +Remove debugging call in GraphSettings component diff --git a/packages/perseus-editor/src/components/graph-settings.tsx b/packages/perseus-editor/src/components/graph-settings.tsx index c7ceda7499..483033dba5 100644 --- a/packages/perseus-editor/src/components/graph-settings.tsx +++ b/packages/perseus-editor/src/components/graph-settings.tsx @@ -160,7 +160,6 @@ const GraphSettings = createReactClass({ // @ts-expect-error - TS2531 - Object is possibly 'null'. | TS2339 - Property 'value' does not exist on type 'Element | Text'. const url = ReactDOM.findDOMNode(this.refs["bg-url"]).value; // eslint-disable-line react/no-string-refs - url; // ? if (url) { Util.getImageSize(url, (width, height) => { if (this._isMounted) { diff --git a/packages/perseus/src/renderer-util.ts b/packages/perseus/src/renderer-util.ts index cc332f8713..065e68af95 100644 --- a/packages/perseus/src/renderer-util.ts +++ b/packages/perseus/src/renderer-util.ts @@ -53,27 +53,13 @@ export function emptyWidgetsFunctional( return false; } - let score: PerseusScore | null = null; - const userInput = userInputMap[id]; const scorer = getWidgetScorer(widget.type); - - if (widget.type === "group") { - const scores = scoreWidgetsFunctional( - widget.options.widgets, - Object.keys(widget.options.widgets), - userInputMap[id] as UserInputMap, - strings, - locale, - ); - score = Util.flattenScores(scores); - } else if (scorer) { - score = scorer( - userInput as UserInput, - widget.options, - strings, - locale, - ); - } + const score = scorer?.( + userInputMap[id] as UserInput, + widget.options, + strings, + locale, + ); if (score) { return Util.scoreIsEmpty(score); @@ -133,22 +119,14 @@ export function scoreWidgetsFunctional( const userInput = userInputMap[id]; const scorer = getWidgetScorer(widget.type); - if (widget.type === "group") { - const scores = scoreWidgetsFunctional( - widget.options.widgets, - getWidgetIdsFromContent(widget.options.content), - userInputMap[id] as UserInputMap, - strings, - locale, - ); - widgetScores[id] = Util.flattenScores(scores); - } else if (scorer) { - widgetScores[id] = scorer( - userInput as UserInput, - widget.options, - strings, - locale, - ); + const score = scorer?.( + userInput as UserInput, + widget.options, + strings, + locale, + ); + if (score != null) { + widgetScores[id] = score; } }); diff --git a/packages/perseus/src/validation.types.ts b/packages/perseus/src/validation.types.ts index f0fde91d66..03f7139c7c 100644 --- a/packages/perseus/src/validation.types.ts +++ b/packages/perseus/src/validation.types.ts @@ -90,6 +90,7 @@ export type PerseusExpressionRubric = { export type PerseusExpressionUserInput = string; export type PerseusGroupRubric = PerseusGroupWidgetOptions; +export type PerseusGroupUserInput = UserInputMap; export type PerseusGradedGroupRubric = PerseusGradedGroupWidgetOptions; diff --git a/packages/perseus/src/widgets/group/group.tsx b/packages/perseus/src/widgets/group/group.tsx index 6df7646007..ec5ae760d0 100644 --- a/packages/perseus/src/widgets/group/group.tsx +++ b/packages/perseus/src/widgets/group/group.tsx @@ -8,6 +8,8 @@ import {ApiOptions} from "../../perseus-api"; import Renderer from "../../renderer"; import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/group/group-ai-utils"; +import scoreGroup from "./score-group"; + import type {PerseusGroupWidgetOptions} from "../../perseus-types"; import type { APIOptions, @@ -202,6 +204,9 @@ export default { displayName: "Group (SAT only)", widget: Group, traverseChildWidgets: traverseChildWidgets, + // TODO(LEMS-2656): remove TS suppression + // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusCSProgramUserInput'. + scorer: scoreGroup, hidden: true, isLintable: true, } satisfies WidgetExports; diff --git a/packages/perseus/src/widgets/group/score-group.ts b/packages/perseus/src/widgets/group/score-group.ts new file mode 100644 index 0000000000..4db4058118 --- /dev/null +++ b/packages/perseus/src/widgets/group/score-group.ts @@ -0,0 +1,30 @@ +import {scoreWidgetsFunctional} from "../../renderer-util"; +import Util from "../../util"; + +import type {PerseusStrings} from "../../strings"; +import type {PerseusScore} from "../../types"; +import type { + PerseusGroupRubric, + PerseusGroupUserInput, +} from "../../validation.types"; + +// The `group` widget is basically a widget hosting a full Perseus system in +// it. As such, scoring a group means scoring all widgets it contains. +function scoreGroup( + userInput: PerseusGroupUserInput, + options: PerseusGroupRubric, + strings: PerseusStrings, + locale: string, +): PerseusScore { + const scores = scoreWidgetsFunctional( + options.widgets, + Object.keys(options.widgets), + userInput, + strings, + locale, + ); + + return Util.flattenScores(scores); +} + +export default scoreGroup;