Skip to content

Commit

Permalink
Move scoring out of general Util into scoring util file (#1962)
Browse files Browse the repository at this point in the history
## Summary:

Like #1948, I was working in `utils.ts` and found it annoying how there were so many different utils in one file (graphing support, scoring, word/text manipulation). So this just pulls scoring-related behaviour out into a new file. This means these symbols are no longer on the `Utils` object, but I checked and they aren't used in Perseus nor in consuming apps.

Issue: LEMS-2561

## Test plan:

`yarn test`
`yarn typecheck`

Author: jeremywiebe

Reviewers: jeremywiebe, Myranae, handeyeco, benchristel, SonicScrewdriver

Required Reviewers:

Approved By: Myranae, handeyeco

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1962
  • Loading branch information
jeremywiebe authored Dec 6, 2024
1 parent d93e3ec commit 435280a
Show file tree
Hide file tree
Showing 12 changed files with 317 additions and 307 deletions.
5 changes: 5 additions & 0 deletions .changeset/clever-cars-eat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": major
---

Move scoring utility functions out of `Util` object into their own file and only export externally used function (`keScoreFromPerseusScore`)
2 changes: 1 addition & 1 deletion dev/flipbook.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {Renderer} from "../packages/perseus/src";
import {SvgImage} from "../packages/perseus/src/components";
import {scorePerseusItem} from "../packages/perseus/src/renderer-util";
import {mockStrings} from "../packages/perseus/src/strings";
import {isCorrect} from "../packages/perseus/src/util";
import {isCorrect} from "../packages/perseus/src/util/scoring";
import {trueForAllMafsSupportedGraphTypes} from "../packages/perseus/src/widgets/interactive-graphs/mafs-supported-graph-types";

import {EditableControlledInput} from "./editable-controlled-input";
Expand Down
134 changes: 1 addition & 133 deletions packages/perseus/src/__tests__/util.test.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,4 @@
import Util, {isCorrect} from "../util";

describe("isCorrect", () => {
it("is true given a score with all points earned", () => {
const score = {type: "points", earned: 3, total: 3} as const;
expect(isCorrect(score)).toBe(true);
});

it("is false given a score with some points unearned", () => {
const score = {type: "points", earned: 2, total: 3} as const;
expect(isCorrect(score)).toBe(false);
});

it("is false given an unanswered / invalid score", () => {
const score = {type: "invalid"} as const;
expect(isCorrect(score)).toBe(false);
});
});
import Util from "../util";

describe("#constrainedTickStepsFromTickSteps", () => {
it("should not changes the tick steps if there are fewer than (or exactly) 10 steps", () => {
Expand Down Expand Up @@ -79,118 +62,3 @@ describe("deepClone", () => {
expect(result[0]).not.toBe(input[0]);
});
});

describe("flattenScores", () => {
it("defaults to an empty score", () => {
const result = Util.flattenScores({});

expect(result).toHaveBeenAnsweredCorrectly({shouldHavePoints: false});
expect(result).toEqual({
type: "points",
total: 0,
earned: 0,
message: null,
});
});

it("defaults to single score if there is only one", () => {
const result = Util.flattenScores({
"radio 1": {
type: "points",
total: 1,
earned: 1,
message: null,
},
});

expect(result).toHaveBeenAnsweredCorrectly();
expect(result).toEqual({
type: "points",
total: 1,
earned: 1,
message: null,
});
});

it("returns an invalid score if any are invalid", () => {
const result = Util.flattenScores({
"radio 1": {
type: "points",
total: 1,
earned: 1,
message: null,
},
"radio 2": {
type: "invalid",
message: null,
},
});

expect(result).toHaveInvalidInput();
expect(result).toEqual({
type: "invalid",
message: null,
});
});

it("tallies scores if multiple widgets have points", () => {
const result = Util.flattenScores({
"radio 1": {
type: "points",
total: 1,
earned: 1,
message: null,
},
"radio 2": {
type: "points",
total: 1,
earned: 1,
message: null,
},
"radio 3": {
type: "points",
total: 1,
earned: 1,
message: null,
},
});

expect(result).toHaveBeenAnsweredCorrectly();
expect(result).toEqual({
type: "points",
total: 3,
earned: 3,
message: null,
});
});

it("doesn't count incorrect widgets", () => {
const result = Util.flattenScores({
"radio 1": {
type: "points",
total: 1,
earned: 1,
message: null,
},
"radio 2": {
type: "points",
total: 1,
earned: 1,
message: null,
},
"radio 3": {
type: "points",
total: 1,
earned: 0,
message: null,
},
});

expect(result).toEqual({
type: "points",
total: 3,
earned: 2,
message: null,
});
});
});
9 changes: 7 additions & 2 deletions packages/perseus/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,9 @@ export {default as TableWidget} from "./widgets/table";
export {default as PlotterWidget} from "./widgets/plotter";
export {default as GrapherWidget} from "./widgets/grapher";

// Some utils in grapher/utils don't need to be used outside of `perseus`,
// so only export the stuff that does need to be exposed
// Some utils in grapher/utils and scoring don't need to be used outside of
// `perseus`, so only export the stuff that does need to be exposed
import {keScoreFromPerseusScore} from "./util/scoring";
import {
allTypes,
DEFAULT_GRAPHER_PROPS,
Expand All @@ -57,6 +58,10 @@ export const GrapherUtil = {
typeToButton,
};

export const ScoringUtil = {
keScoreFromPerseusScore,
};

/**
* Misc
*/
Expand Down
8 changes: 4 additions & 4 deletions packages/perseus/src/multi-items/multi-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import {DependenciesContext} from "../dependencies";
import HintsRenderer from "../hints-renderer";
import {Log} from "../logging/log";
import Renderer from "../renderer";
import Util from "../util";
import {combineScores, keScoreFromPerseusScore} from "../util/scoring";

import {itemToTree} from "./items";
import {buildMapper} from "./trees";
Expand Down Expand Up @@ -404,7 +404,7 @@ class MultiRenderer extends React.Component<Props, State> {
if (ref.getSerializedState) {
state = ref.getSerializedState();
}
return Util.keScoreFromPerseusScore(score, guess, state);
return keScoreFromPerseusScore(score, guess, state);
}

/**
Expand Down Expand Up @@ -441,9 +441,9 @@ class MultiRenderer extends React.Component<Props, State> {
return data.ref?.getUserInput();
});

const combinedScore = scores.reduce(Util.combineScores);
const combinedScore = scores.reduce(combineScores);

return Util.keScoreFromPerseusScore(combinedScore, guess, state);
return keScoreFromPerseusScore(combinedScore, guess, state);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/perseus/src/renderer-util.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {mapObject} from "./interactive2/objective_";
import Util from "./util";
import {scoreIsEmpty, flattenScores} from "./util/scoring";
import {getWidgetIdsFromContent} from "./widget-type-utils";
import {getWidgetScorer, upgradeWidgetInfoToLatestVersion} from "./widgets";

Expand Down Expand Up @@ -60,7 +60,7 @@ export function emptyWidgetsFunctional(
);

if (score) {
return Util.scoreIsEmpty(score);
return scoreIsEmpty(score);
}
});
}
Expand All @@ -86,7 +86,7 @@ export function scorePerseusItem(
strings,
locale,
);
return Util.flattenScores(scores);
return flattenScores(scores);
}

export function scoreWidgetsFunctional(
Expand Down
3 changes: 2 additions & 1 deletion packages/perseus/src/renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
} from "./renderer-util";
import TranslationLinter from "./translation-linter";
import Util from "./util";
import {flattenScores} from "./util/scoring";
import preprocessTex from "./util/tex-preprocess";
import WidgetContainer from "./widget-container";
import * as Widgets from "./widgets";
Expand Down Expand Up @@ -1737,7 +1738,7 @@ class Renderer
this.props.strings,
this.context.locale,
);
const combinedScore = Util.flattenScores(scores);
const combinedScore = flattenScores(scores);
return combinedScore;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/perseus/src/server-item-renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {ApiOptions} from "./perseus-api";
import Renderer from "./renderer";
import {scorePerseusItem} from "./renderer-util";
import Util from "./util";
import {keScoreFromPerseusScore} from "./util/scoring";

import type {PerseusItem, ShowSolutions} from "./perseus-types";
import type {
Expand Down Expand Up @@ -366,7 +367,7 @@ export class ServerItemRenderer
// analyzing ProblemLogs. If not, remove this layer.
const maxCompatGuess = [this.questionRenderer.getUserInput(), []];

const keScore = Util.keScoreFromPerseusScore(
const keScore = keScoreFromPerseusScore(
score,
maxCompatGuess,
this.questionRenderer.getSerializedState(),
Expand Down
Loading

0 comments on commit 435280a

Please sign in to comment.