Skip to content

Commit dd8d25c

Browse files
benchristelbenchristel
authored
Make WidgetLogic.version the source of truth for widget versions (#3784)
## Summary: Previously, widget versions were encoded in three different places: - In the `perseus` package, [on the WidgetExports type]. This version used to be used for widget prop upgrades (the data migration strategy that preceded the parsers, now removed). It is now used by the editors to determine the version number that gets stamped on newly-created widgets. - In `perseus-core`, [in the parser code]. - In `perseus-core`, [on the WidgetLogic type]. These three sources of truth could get out of sync. Also, not every package can import `perseus` (for example, in the backend perseus service, we can't import React code at all, so we can't use the `perseus` package) which means that some code could not get a runtime representation of the current version of a widget if the `WidgetLogic` didn't have the version. This PR makes `WidgetLogic.version` the single source of truth for widget versions, and adds tests for the parsers to make sure that they output the correct version number. `WidgetExports.version` has been deleted, and the editors now use `WidgetLogic.version` instead. In the process of making this change, I noticed a bug in the Measurer widget parser where it did not parse version 0 options correctly: it zeroed out the image properties. This PR also fixes that bug. Only one widget in our content corpus was affected. [on the WidgetExports type]: https://github.com/Khan/perseus/blob/4a130c0367d9e83953c4b37c19881f81437bca39/packages/perseus/src/types.ts#L436 [in the parser code]: https://github.com/Khan/perseus/blob/449a085f7d7136fba5ba198e291b2d44f1f2cd74/packages/perseus-core/src/parse-perseus-json/perseus-parsers/radio-widget.ts#L54 [on the WidgetLogic type]: https://github.com/Khan/perseus/blob/135eaf6c330bd3b5092bda458d2b90d450e7d9a5/packages/perseus-core/src/widgets/logic-export.types.ts#L62 Issue: none ## Test plan: CI checks should pass. Author: benchristel Reviewers: benchristel, jeremywiebe, handeyeco Required Reviewers: Approved By: jeremywiebe Checks: ✅ 11 checks were successful, ⏭️ 1 check has been skipped Pull Request URL: #3784
1 parent be10fbb commit dd8d25c

20 files changed

Lines changed: 535 additions & 98 deletions

File tree

‎.changeset/friendly-gorillas-serve.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@khanacademy/perseus": patch
3+
"@khanacademy/perseus-core": patch
4+
"@khanacademy/perseus-editor": patch
5+
---
6+
7+
Internal: `WidgetLogic.version` is now the source of truth for widget versions.

‎packages/perseus-core/src/parse-perseus-json/perseus-parsers/measurer-widget.ts‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
import {
22
boolean,
33
constant,
4+
nullable,
45
number,
56
object,
7+
optional,
68
pair,
79
string,
810
} from "../general-purpose-parsers";
911
import {defaulted} from "../general-purpose-parsers/defaulted";
1012

1113
import {parsePerseusImageBackground} from "./perseus-image-background";
12-
import {parseWidget} from "./widget";
14+
import {versionedWidgetOptions} from "./versioned-widget-options";
15+
import {parseWidget, parseWidgetWithVersion} from "./widget";
1316

14-
export const parseMeasurerWidget = parseWidget(
17+
import type {ParsedValue} from "../parser-types";
18+
19+
const parseMeasurerWidgetV1 = parseWidgetWithVersion(
20+
object({major: constant(1), minor: number}),
1521
constant("measurer"),
1622
object({
1723
// The default value for image comes from measurer.tsx.
@@ -31,3 +37,42 @@ export const parseMeasurerWidget = parseWidget(
3137
box: pair(number, number),
3238
}),
3339
);
40+
41+
const parseMeasurerWidgetV0 = parseWidget(
42+
constant("measurer"),
43+
object({
44+
imageTop: number,
45+
imageLeft: number,
46+
imageUrl: optional(nullable(string)),
47+
showProtractor: boolean,
48+
showRuler: boolean,
49+
rulerLabel: string,
50+
rulerTicks: number,
51+
rulerPixels: number,
52+
rulerLength: number,
53+
box: pair(number, number),
54+
}),
55+
);
56+
57+
function migrateV0ToV1(
58+
v0: ParsedValue<typeof parseMeasurerWidgetV0>,
59+
): ParsedValue<typeof parseMeasurerWidgetV1> {
60+
const {imageTop, imageLeft, imageUrl, ...v1Options} = v0.options;
61+
return {
62+
...v0,
63+
version: {major: 1, minor: 0},
64+
options: {
65+
image: {
66+
top: imageTop,
67+
left: imageLeft,
68+
url: imageUrl,
69+
},
70+
...v1Options,
71+
},
72+
};
73+
}
74+
75+
export const parseMeasurerWidget = versionedWidgetOptions(
76+
1,
77+
parseMeasurerWidgetV1,
78+
).withMigrationFrom(0, parseMeasurerWidgetV0, migrateV0ToV1).parser;

‎packages/perseus-core/src/parse-perseus-json/perseus-parsers/widgets-map.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ describe("parseWidgetsMap", () => {
501501
const widgetsMap: unknown = {
502502
"measurer 1": {
503503
type: "measurer",
504-
version: {major: 0, minor: 0},
504+
version: {major: 1, minor: 0},
505505
options: {
506506
image: {},
507507
showProtractor: false,

‎packages/perseus-core/src/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-regression.test.ts.snap‎

Lines changed: 231 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13007,7 +13007,7 @@ exports[`parseAndMigratePerseusItem given measurer-missing-image.ts returns the
1300713007
"image": {
1300813008
"left": 0,
1300913009
"top": 0,
13010-
"url": null,
13010+
"url": "crwdns6514084:0crwdne6514084:0",
1301113011
},
1301213012
"rulerLabel": "",
1301313013
"rulerLength": 10,
@@ -13017,6 +13017,10 @@ exports[`parseAndMigratePerseusItem given measurer-missing-image.ts returns the
1301713017
"showRuler": false,
1301813018
},
1301913019
"type": "measurer",
13020+
"version": {
13021+
"major": 1,
13022+
"minor": 0,
13023+
},
1302013024
},
1302113025
},
1302213026
},
@@ -13114,7 +13118,7 @@ exports[`parseAndMigratePerseusItem given measurer-missing-image.ts returns the
1311413118
"image": {
1311513119
"left": 0,
1311613120
"top": 0,
13117-
"url": null,
13121+
"url": "crwdns6514084:0crwdne6514084:0",
1311813122
},
1311913123
"rulerLabel": "",
1312013124
"rulerLength": 10,
@@ -13223,6 +13227,231 @@ exports[`parseAndMigratePerseusItem given measurer-missing-static.ts returns the
1322313227
}
1322413228
`;
1322513229

13230+
exports[`parseAndMigratePerseusItem given measurer-missing-version.ts returns the same result as before 1`] = `
13231+
{
13232+
"answerArea": {
13233+
"calculator": false,
13234+
"financialCalculatorMonthlyPayment": false,
13235+
"financialCalculatorTimeToPayOff": false,
13236+
"financialCalculatorTotalAmount": false,
13237+
"periodicTable": false,
13238+
"periodicTableWithKey": false,
13239+
},
13240+
"hints": [
13241+
{
13242+
"content": "We should be able to approach the base of the mountain. Measure it angle of the base using the protractor.",
13243+
"images": {},
13244+
"widgets": {},
13245+
},
13246+
{
13247+
"content": "In the training course built for the earthbound version of Curiosity, they had a number obstacles and hills set out. For example, the **smallest** of these hills had an incline of 5 degrees.",
13248+
"images": {},
13249+
"widgets": {},
13250+
},
13251+
{
13252+
"content": "Curiosity is expected to have the ability to ascend inclines as large as 20 degrees.",
13253+
"images": {},
13254+
"widgets": {},
13255+
},
13256+
],
13257+
"question": {
13258+
"content": "Curiosity's landing site is Gale Crater; a location notable for having a large mountain of layered rock in the center known as Aeolis Mons (*Mount Sharp*). This prevents Curiosity from traveling **directly across** due to the steep incline towards the top. Instead, it was designed to drive up and along the shallower base.
13259+
13260+
**How steep of an incline was Curiosity designed for?**
13261+
13262+
*Click and drag the protractor to help you!*
13263+
[[☃ measurer 1]]
13264+
13265+
[[☃ dropdown 1]]",
13266+
"images": {},
13267+
"widgets": {
13268+
"dropdown 1": {
13269+
"graded": true,
13270+
"options": {
13271+
"choices": [
13272+
{
13273+
"content": "5",
13274+
"correct": false,
13275+
},
13276+
{
13277+
"content": "10",
13278+
"correct": false,
13279+
},
13280+
{
13281+
"content": "20",
13282+
"correct": true,
13283+
},
13284+
{
13285+
"content": "30",
13286+
"correct": false,
13287+
},
13288+
{
13289+
"content": "45",
13290+
"correct": false,
13291+
},
13292+
{
13293+
"content": "60",
13294+
"correct": false,
13295+
},
13296+
{
13297+
"content": "90",
13298+
"correct": false,
13299+
},
13300+
],
13301+
"placeholder": "",
13302+
"static": false,
13303+
},
13304+
"type": "dropdown",
13305+
},
13306+
"measurer 1": {
13307+
"graded": true,
13308+
"options": {
13309+
"box": [
13310+
480,
13311+
480,
13312+
],
13313+
"image": {
13314+
"left": 0,
13315+
"top": 0,
13316+
"url": "https://ka-perseus-images.s3.amazonaws.com/32d7ebe969c7ce5b241873ac172d50992ca79de3.jpeg",
13317+
},
13318+
"rulerLabel": "",
13319+
"rulerLength": 10,
13320+
"rulerPixels": 40,
13321+
"rulerTicks": 10,
13322+
"showProtractor": true,
13323+
"showRuler": false,
13324+
},
13325+
"type": "measurer",
13326+
"version": {
13327+
"major": 1,
13328+
"minor": 0,
13329+
},
13330+
},
13331+
},
13332+
},
13333+
}
13334+
`;
13335+
13336+
exports[`parseAndMigratePerseusItem given measurer-missing-version.ts returns the same result as before with answer information removed 1`] = `
13337+
{
13338+
"answerArea": {
13339+
"calculator": false,
13340+
"financialCalculatorMonthlyPayment": false,
13341+
"financialCalculatorTimeToPayOff": false,
13342+
"financialCalculatorTotalAmount": false,
13343+
"periodicTable": false,
13344+
"periodicTableWithKey": false,
13345+
},
13346+
"hints": [
13347+
{
13348+
"content": "",
13349+
"images": {},
13350+
"placeholder": true,
13351+
"widgets": {},
13352+
},
13353+
{
13354+
"content": "",
13355+
"images": {},
13356+
"placeholder": true,
13357+
"widgets": {},
13358+
},
13359+
{
13360+
"content": "",
13361+
"images": {},
13362+
"placeholder": true,
13363+
"widgets": {},
13364+
},
13365+
],
13366+
"question": {
13367+
"content": "Curiosity's landing site is Gale Crater; a location notable for having a large mountain of layered rock in the center known as Aeolis Mons (*Mount Sharp*). This prevents Curiosity from traveling **directly across** due to the steep incline towards the top. Instead, it was designed to drive up and along the shallower base.
13368+
13369+
**How steep of an incline was Curiosity designed for?**
13370+
13371+
*Click and drag the protractor to help you!*
13372+
[[☃ measurer 1]]
13373+
13374+
[[☃ dropdown 1]]",
13375+
"images": {},
13376+
"widgets": {
13377+
"dropdown 1": {
13378+
"alignment": "default",
13379+
"graded": true,
13380+
"options": {
13381+
"ariaLabel": undefined,
13382+
"choices": [
13383+
{
13384+
"content": "5",
13385+
"correct": false,
13386+
},
13387+
{
13388+
"content": "10",
13389+
"correct": false,
13390+
},
13391+
{
13392+
"content": "20",
13393+
"correct": false,
13394+
},
13395+
{
13396+
"content": "30",
13397+
"correct": false,
13398+
},
13399+
{
13400+
"content": "45",
13401+
"correct": false,
13402+
},
13403+
{
13404+
"content": "60",
13405+
"correct": false,
13406+
},
13407+
{
13408+
"content": "90",
13409+
"correct": false,
13410+
},
13411+
],
13412+
"placeholder": "",
13413+
"static": false,
13414+
"visibleLabel": undefined,
13415+
},
13416+
"static": false,
13417+
"type": "dropdown",
13418+
"version": {
13419+
"major": 0,
13420+
"minor": 0,
13421+
},
13422+
},
13423+
"measurer 1": {
13424+
"alignment": "default",
13425+
"graded": true,
13426+
"options": {
13427+
"box": [
13428+
480,
13429+
480,
13430+
],
13431+
"image": {
13432+
"left": 0,
13433+
"top": 0,
13434+
"url": "https://ka-perseus-images.s3.amazonaws.com/32d7ebe969c7ce5b241873ac172d50992ca79de3.jpeg",
13435+
},
13436+
"rulerLabel": "",
13437+
"rulerLength": 10,
13438+
"rulerPixels": 40,
13439+
"rulerTicks": 10,
13440+
"showProtractor": true,
13441+
"showRuler": false,
13442+
},
13443+
"static": false,
13444+
"type": "measurer",
13445+
"version": {
13446+
"major": 1,
13447+
"minor": 0,
13448+
},
13449+
},
13450+
},
13451+
},
13452+
}
13453+
`;
13454+
1322613455
exports[`parseAndMigratePerseusItem given number-line-missing-snapDivisions.ts returns the same result as before 1`] = `
1322713456
{
1322813457
"answerArea": {

0 commit comments

Comments
 (0)