Skip to content

Commit

Permalink
Migrate expression widget to latest version in PerseusItem parser (#1908
Browse files Browse the repository at this point in the history
)

This demonstrates how we can upgrade/migrate widget options in the parsing
code, obviating the need for `propUpgrades`.

The fact that `version` is an object makes this really annoying because
TypeScript can't discriminate unions based on nested properties like
`version.major`. Given that constraint, I think the approach I came up with is
reasonable, but I'm certainly open to feedback on how to improve it.

Issue: LEMS-2582

## Test plan:

`yarn test`

Author: benchristel

Reviewers: jeremywiebe, anakaren-rojas, catandthemachines, nishasy

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1908
  • Loading branch information
benchristel authored Nov 26, 2024
1 parent 3dbca96 commit 7f2866c
Show file tree
Hide file tree
Showing 5 changed files with 172 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/yellow-ducks-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Internal: Migrate expression widget options to the latest version in parseAndTypecheckPerseusItem (not yet used in production).
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import {parse} from "../parse";
import {failure, success} from "../result";

import {parseExpressionWidget} from "./expression-widget";

describe("parseExpressionWidget", () => {
it("migrates v0 options to v1", () => {
const widget = {
type: "expression",
graded: true,
options: {
value: "the value",
form: false,
simplify: false,
times: false,
buttonsVisible: "never",
buttonSets: ["basic"],
functions: ["f", "g", "h"],
},
version: {
major: 0,
minor: 1,
},
};

expect(parse(widget, parseExpressionWidget)).toEqual(
success({
type: "expression",
graded: true,
static: undefined,
key: undefined,
alignment: undefined,
options: {
times: false,
ariaLabel: undefined,
visibleLabel: undefined,
buttonsVisible: "never",
buttonSets: ["basic"],
functions: ["f", "g", "h"],
answerForms: [
{
considered: "correct",
form: false,
simplify: false,
value: "the value",
},
],
},
version: {
major: 1,
minor: 0,
},
}),
);
});

it("rejects a widget with unrecognized version", () => {
const widget = {
type: "expression",
version: {
major: -1,
minor: 0,
},
graded: true,
options: {},
};

expect(parse(widget, parseExpressionWidget)).toEqual(
failure(
expect.stringContaining(
"At (root).version.major -- expected 0, but got -1",
),
),
);
});
});
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import ExpressionWidgetModule from "../../../widgets/expression/expression";
import {
array,
boolean,
Expand All @@ -11,13 +12,18 @@ import {
union,
} from "../general-purpose-parsers";

import {parseWidget} from "./widget";
import {parseWidgetWithVersion} from "./widget";

import type {
ExpressionWidget,
PerseusExpressionAnswerForm,
} from "../../../perseus-types";
import type {Parser} from "../parser-types";
import type {
ParseContext,
ParsedValue,
Parser,
ParseResult,
} from "../parser-types";

const parseAnswerForm: Parser<PerseusExpressionAnswerForm> = object({
value: string,
Expand All @@ -29,14 +35,42 @@ const parseAnswerForm: Parser<PerseusExpressionAnswerForm> = object({
).parser,
});

export const parseExpressionWidget: Parser<ExpressionWidget> = parseWidget(
const parseExpressionWidgetV1: Parser<ExpressionWidget> =
parseWidgetWithVersion(
object({major: constant(1), minor: number}),
constant("expression"),
object({
answerForms: array(parseAnswerForm),
functions: array(string),
times: boolean,
visibleLabel: optional(string),
ariaLabel: optional(string),
buttonSets: array(
enumeration(
"basic",
"basic+div",
"trig",
"prealgebra",
"logarithms",
"basic relations",
"advanced relations",
),
),
buttonsVisible: optional(enumeration("always", "never", "focused")),
}),
);

const parseExpressionWidgetV0 = parseWidgetWithVersion(
optional(object({major: constant(0), minor: number})),
constant("expression"),
object({
answerForms: array(parseAnswerForm),
functions: array(string),
times: boolean,
visibleLabel: optional(string),
ariaLabel: optional(string),
form: boolean,
simplify: boolean,
value: string,
buttonSets: array(
enumeration(
"basic",
Expand All @@ -51,3 +85,35 @@ export const parseExpressionWidget: Parser<ExpressionWidget> = parseWidget(
buttonsVisible: optional(enumeration("always", "never", "focused")),
}),
);

function migrateV0ToV1(
widget: ParsedValue<typeof parseExpressionWidgetV0>,
ctx: ParseContext,
): ParseResult<ExpressionWidget> {
const {options} = widget;
return ctx.success({
...widget,
version: ExpressionWidgetModule.version,
options: {
times: options.times,
buttonSets: options.buttonSets,
functions: options.functions,
buttonsVisible: options.buttonsVisible,
visibleLabel: options.visibleLabel,
ariaLabel: options.ariaLabel,

answerForms: [
{
considered: "correct",
form: options.form,
simplify: options.simplify,
value: options.value,
},
],
},
});
}

export const parseExpressionWidget: Parser<ExpressionWidget> = union(
parseExpressionWidgetV1,
).or(pipeParsers(parseExpressionWidgetV0).then(migrateV0ToV1).parser).parser;
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,23 @@ export function parseWidget<Type extends string, Options>(
),
});
}

export function parseWidgetWithVersion<
Version extends {major: number; minor: number} | undefined,
Type extends string,
Options,
>(
parseVersion: Parser<Version>,
parseType: Parser<Type>,
parseOptions: Parser<Options>,
): Parser<WidgetOptions<Type, Options>> {
return object({
type: parseType,
static: optional(boolean),
graded: optional(boolean),
alignment: optional(string),
options: parseOptions,
key: optional(number),
version: parseVersion,
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ describe("parseWidgetsMap", () => {
const widgetsMap: unknown = {
"expression 1": {
type: "expression",
version: {major: 0, minor: 0},
version: {major: 1, minor: 0},
options: {
answerForms: [],
buttonSets: [],
Expand Down

0 comments on commit 7f2866c

Please sign in to comment.