Skip to content

Commit c1ba55f

Browse files
authored
Expose a way to get user input from ServerItemRenderer (#1753)
## Summary: [Webapp PR](Khan/webapp#27079) I'm trying to tread lightly, maybe being overly cautious at the expense of keeping too much old code around. I tried to make everything we can get rid of after the move with `@deprecated`. Basically this should expose everything we need to move the actual scoring process out of ServerItemRenderer and the React tree. We now have `getUserInput` on `ServerItemRenderer` and `scorePerseusItem` which is a non-React, pure function that returns a score. Next step is to replace uses of `scoreInput` in Webapp with `scorePerseusItem`; then we can come back and delete a lot of this legacy code 🤞 Issue: [LEMS-XXXX](https://khanacademy.atlassian.net/browse/LEMS-XXXX) ## Test plan: After the swap in Webapp, we should be able to complete an exercise with `scorePerseusItem` and everything else will work the same. [LEMS-2389]: https://khanacademy.atlassian.net/browse/LEMS-2389?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Author: handeyeco Reviewers: jeremywiebe Required Reviewers: Approved By: jeremywiebe Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1753
1 parent def4630 commit c1ba55f

File tree

19 files changed

+240
-76
lines changed

19 files changed

+240
-76
lines changed
+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@khanacademy/perseus": major
3+
"@khanacademy/perseus-dev-ui": patch
4+
---
5+
6+
Change ServerItemRenderer scoring APIs to externalize scoring

dev/flipbook.tsx

+10-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {useEffect, useMemo, useReducer, useRef, useState} from "react";
1818

1919
import {Renderer} from "../packages/perseus/src";
2020
import {SvgImage} from "../packages/perseus/src/components";
21+
import {scorePerseusItem} from "../packages/perseus/src/renderer-util";
2122
import {mockStrings} from "../packages/perseus/src/strings";
2223
import {isCorrect} from "../packages/perseus/src/util";
2324
import {trueForAllMafsSupportedGraphTypes} from "../packages/perseus/src/widgets/interactive-graphs/mafs-supported-graph-types";
@@ -319,7 +320,15 @@ function GradableRenderer(props: QuestionRendererProps) {
319320
leftContent={
320321
<Button
321322
onClick={() => {
322-
setScore(rendererRef.current?.score());
323+
if (rendererRef.current) {
324+
const score = scorePerseusItem(
325+
question,
326+
rendererRef.current.getUserInputMap(),
327+
mockStrings,
328+
"en",
329+
);
330+
setScore(score);
331+
}
323332
clearScoreTimeout.set();
324333
}}
325334
>

packages/perseus/src/components/__tests__/sorter.test.tsx

+4-8
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,13 @@ describe("sorter widget", () => {
7676
const {renderer} = renderQuestion(question1, apiOptions);
7777
const sorter = renderer.findWidgets("sorter 1")[0];
7878

79+
// Act
7980
// Put the options in the correct order
80-
8181
["$0.005$ kilograms", "$15$ grams", "$55$ grams"].forEach((option) => {
8282
act(() => sorter.moveOptionToIndex(option, 3));
8383
});
84-
// Act
85-
renderer.guessAndScore();
8684

87-
// assert
85+
// Assert
8886
expect(renderer).toHaveBeenAnsweredCorrectly();
8987
});
9088
it("can be answered incorrectly", () => {
@@ -95,17 +93,15 @@ describe("sorter widget", () => {
9593
const {renderer} = renderQuestion(question1, apiOptions);
9694
const sorter = renderer.findWidgets("sorter 1")[0];
9795

96+
// Act
9897
// Put the options in the reverse order
9998
["$0.005$ kilograms", "$15$ grams", "$55$ grams"].forEach(
10099
(option, index) => {
101100
act(() => sorter.moveOptionToIndex(option, 0));
102101
},
103102
);
104103

105-
// Act
106-
renderer.guessAndScore();
107-
108-
// assert
104+
// Assert
109105
expect(renderer).toHaveBeenAnsweredIncorrectly();
110106
});
111107
});

packages/perseus/src/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export {default as ServerItemRenderer} from "./server-item-renderer";
1818
export {default as HintsRenderer} from "./hints-renderer";
1919
export {default as HintRenderer} from "./hint-renderer";
2020
export {default as Renderer} from "./renderer";
21+
export {scorePerseusItem} from "./renderer-util";
2122

2223
/**
2324
* Widgets
@@ -226,6 +227,7 @@ export type {
226227
PerseusWidgetsMap,
227228
MultiItem,
228229
} from "./perseus-types";
230+
export type {UserInputMap} from "./validation.types";
229231
export type {Coord} from "./interactive2/types";
230232
export type {MarkerType} from "./widgets/label-image/types";
231233

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

+50-1
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,19 @@
1-
import {emptyWidgetsFunctional, scoreWidgetsFunctional} from "./renderer-util";
1+
import {screen} from "@testing-library/react";
2+
import {userEvent as userEventLib} from "@testing-library/user-event";
3+
4+
import {
5+
emptyWidgetsFunctional,
6+
scorePerseusItem,
7+
scoreWidgetsFunctional,
8+
} from "./renderer-util";
29
import {mockStrings} from "./strings";
310
import {registerAllWidgetsForTesting} from "./util/register-all-widgets-for-testing";
11+
import {renderQuestion} from "./widgets/__testutils__/renderQuestion";
12+
import {question1} from "./widgets/group/group.testdata";
413

514
import type {DropdownWidget, PerseusWidgetsMap} from "./perseus-types";
615
import type {UserInputMap} from "./validation.types";
16+
import type {UserEvent} from "@testing-library/user-event";
717

818
const testDropdownWidget: DropdownWidget = {
919
type: "dropdown",
@@ -469,3 +479,42 @@ describe("scoreWidgetsFunctional", () => {
469479
expect(result["group 1"]).toHaveBeenAnsweredIncorrectly();
470480
});
471481
});
482+
483+
describe("scorePerseusItem", () => {
484+
let userEvent: UserEvent;
485+
beforeEach(() => {
486+
userEvent = userEventLib.setup({
487+
advanceTimers: jest.advanceTimersByTime,
488+
});
489+
});
490+
491+
it("should return score from contained Renderer", async () => {
492+
// Arrange
493+
const {renderer} = renderQuestion(question1);
494+
// Answer all widgets correctly
495+
await userEvent.click(screen.getAllByRole("radio")[4]);
496+
// Note(jeremy): If we don't tab away from the radio button in this
497+
// test, it seems like the userEvent typing doesn't land in the first
498+
// text field.
499+
await userEvent.tab();
500+
await userEvent.type(
501+
screen.getByRole("textbox", {name: /nearest ten/}),
502+
"230",
503+
);
504+
await userEvent.type(
505+
screen.getByRole("textbox", {name: /nearest hundred/}),
506+
"200",
507+
);
508+
const userInput = renderer.getUserInputMap();
509+
const score = scorePerseusItem(question1, userInput, mockStrings, "en");
510+
511+
// Assert
512+
expect(score).toHaveBeenAnsweredCorrectly();
513+
expect(score).toEqual({
514+
earned: 3,
515+
message: null,
516+
total: 3,
517+
type: "points",
518+
});
519+
});
520+
});

packages/perseus/src/renderer-util.ts

+28-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import Util from "./util";
2+
import {getWidgetIdsFromContent} from "./widget-type-utils";
23
import {getWidgetScorer} from "./widgets";
34

4-
import type {PerseusWidgetsMap} from "./perseus-types";
5+
import type {PerseusRenderer, PerseusWidgetsMap} from "./perseus-types";
56
import type {PerseusStrings} from "./strings";
67
import type {PerseusScore} from "./types";
78
import type {UserInput, UserInputMap} from "./validation.types";
@@ -50,11 +51,35 @@ export function emptyWidgetsFunctional(
5051
});
5152
}
5253

54+
// TODO: combine scorePerseusItem with scoreWidgetsFunctional
55+
// once scorePerseusItem is the only one calling scoreWidgetsFunctional
56+
export function scorePerseusItem(
57+
perseusRenderData: PerseusRenderer,
58+
userInputMap: UserInputMap,
59+
// TODO(LEMS-2461,LEMS-2391): these probably
60+
// need to be removed before we move this to the server
61+
strings: PerseusStrings,
62+
locale: string,
63+
): PerseusScore {
64+
// There seems to be a chance that PerseusRenderer.widgets might include
65+
// widget data for widgets that are not in PerseusRenderer.content,
66+
// so this checks that the widgets are being used before scoring them
67+
const usedWidgetIds = getWidgetIdsFromContent(perseusRenderData.content);
68+
const scores = scoreWidgetsFunctional(
69+
perseusRenderData.widgets,
70+
usedWidgetIds,
71+
userInputMap,
72+
strings,
73+
locale,
74+
);
75+
return Util.flattenScores(scores);
76+
}
77+
5378
export function scoreWidgetsFunctional(
5479
widgets: PerseusWidgetsMap,
5580
// This is a port of old code, I'm not sure why
5681
// we need widgetIds vs the keys of the widgets object
57-
widgetIds: Array<string>,
82+
widgetIds: ReadonlyArray<string>,
5883
userInputMap: UserInputMap,
5984
strings: PerseusStrings,
6085
locale: string,
@@ -79,7 +104,7 @@ export function scoreWidgetsFunctional(
79104
if (widget.type === "group") {
80105
const scores = scoreWidgetsFunctional(
81106
widget.options.widgets,
82-
Object.keys(widget.options.widgets),
107+
getWidgetIdsFromContent(widget.options.content),
83108
userInputMap[id] as UserInputMap,
84109
strings,
85110
locale,

packages/perseus/src/renderer.tsx

+17-21
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,7 @@ class Renderer
11241124
// /cry(aria)
11251125
this._foundTextNodes = true;
11261126

1127-
if (_.contains(this.widgetIds, node.id)) {
1127+
if (this.widgetIds.includes(node.id)) {
11281128
// We don't want to render a duplicate widget key/ref,
11291129
// as this causes problems with react (for obvious
11301130
// reasons). Instead we just notify the
@@ -1510,15 +1510,15 @@ class Renderer
15101510

15111511
getInputPaths: () => ReadonlyArray<FocusPath> = () => {
15121512
const inputPaths: Array<FocusPath> = [];
1513-
_.each(this.widgetIds, (widgetId: string) => {
1513+
this.widgetIds.forEach((widgetId: string) => {
15141514
const widget = this.getWidgetInstance(widgetId);
15151515
if (widget && widget.getInputPaths) {
15161516
// Grab all input paths and add widgetID to the front
15171517
const widgetInputPaths = widget.getInputPaths();
15181518
// Prefix paths with their widgetID and add to collective
15191519
// list of paths.
15201520
// @ts-expect-error - TS2345 - Argument of type '(inputPath: string) => void' is not assignable to parameter of type 'CollectionIterator<FocusPath, void, readonly FocusPath[]>'.
1521-
_.each(widgetInputPaths, (inputPath: string) => {
1521+
widgetInputPaths.forEach((inputPath: string) => {
15221522
const relativeInputPath = [widgetId].concat(inputPath);
15231523
inputPaths.push(relativeInputPath);
15241524
});
@@ -1742,46 +1742,42 @@ class Renderer
17421742
}
17431743

17441744
/**
1745-
* Returns an object mapping from widget ID to perseus-style score.
1746-
* The keys of this object are the values of the array returned
1747-
* from `getWidgetIds`.
1745+
* Scores the content.
1746+
*
1747+
* @deprecated use scorePerseusItem
17481748
*/
1749-
scoreWidgets(): {[widgetId: string]: PerseusScore} {
1750-
return scoreWidgetsFunctional(
1749+
score(): PerseusScore {
1750+
const scores = scoreWidgetsFunctional(
17511751
this.state.widgetInfo,
17521752
this.widgetIds,
17531753
this.getUserInputMap(),
17541754
this.props.strings,
17551755
this.context.locale,
17561756
);
1757-
}
1758-
1759-
/**
1760-
* Grades the content.
1761-
*/
1762-
score(): PerseusScore {
1763-
const scores = this.scoreWidgets();
17641757
const combinedScore = Util.flattenScores(scores);
17651758
return combinedScore;
17661759
}
17671760

1768-
guessAndScore(): [UserInputArray, PerseusScore] {
1761+
/**
1762+
* @deprecated use scorePerseusItem
1763+
*/
1764+
guessAndScore: () => [UserInputArray, PerseusScore] = () => {
17691765
const totalGuess = this.getUserInput();
17701766
const totalScore = this.score();
17711767

17721768
return [totalGuess, totalScore];
1773-
}
1769+
};
17741770

17751771
examples: () => ReadonlyArray<string> | null | undefined = () => {
17761772
const widgetIds = this.widgetIds;
1777-
const examples = _.compact(
1778-
_.map(widgetIds, (widgetId) => {
1773+
const examples = widgetIds
1774+
.map((widgetId) => {
17791775
const widget = this.getWidgetInstance(widgetId);
17801776
return widget != null && widget.examples
17811777
? widget.examples()
17821778
: null;
1783-
}),
1784-
);
1779+
})
1780+
.filter(Boolean);
17851781

17861782
// no widgets with examples
17871783
if (!examples.length) {

0 commit comments

Comments
 (0)