diff --git a/.changeset/rotten-peaches-move.md b/.changeset/rotten-peaches-move.md new file mode 100644 index 0000000000..695cab47de --- /dev/null +++ b/.changeset/rotten-peaches-move.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Internal: remove unused fields from `answerArea` when parsing `PerseusItem`s. diff --git a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/index.ts b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/index.ts index 22b58df322..1b664a0d40 100644 --- a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/index.ts +++ b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/index.ts @@ -9,7 +9,9 @@ export * from "./number"; export * from "./object"; export * from "./optional"; export * from "./pair"; +export * from "./pipe-parsers"; export * from "./record"; export * from "./string"; export * from "./trio"; export * from "./union"; +export * from "./unknown"; diff --git a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pair.test.ts b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pair.test.ts index e9c567cf74..c9fe8b5e52 100644 --- a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pair.test.ts +++ b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pair.test.ts @@ -1,8 +1,8 @@ import {success} from "../result"; -import {composeParsers} from "./compose-parsers"; import {number} from "./number"; import {pair} from "./pair"; +import {pipeParsers} from "./pipe-parsers"; import {string} from "./string"; import {anyFailure, ctx, parseFailureWith} from "./test-helpers"; @@ -72,9 +72,9 @@ describe("pair", () => { }); it("returns the parsed values from each of its sub-parsers", () => { - const increment = composeParsers(number, (x, ctx) => + const increment = pipeParsers(number).then((x, ctx) => ctx.success(x + 1), - ); + ).parser; const incrementBoth = pair(increment, increment); expect(incrementBoth([1, 5], ctx())).toEqual(success([2, 6])); }); diff --git a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.test.ts b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.test.ts new file mode 100644 index 0000000000..b383ccd4b1 --- /dev/null +++ b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.test.ts @@ -0,0 +1,41 @@ +import {success} from "../result"; + +import {pipeParsers} from "./pipe-parsers"; +import {string} from "./string"; +import {anyFailure, ctx} from "./test-helpers"; + +import type {PartialParser} from "../parser-types"; + +describe("pipeParsers given a single parser", () => { + const string2 = pipeParsers(string).parser; + it("accepts a valid value", () => { + expect(string2("abc", ctx())).toEqual(success("abc")); + }); + + it("rejects an invalid value", () => { + expect(string2(99, ctx())).toEqual(anyFailure); + }); +}); + +describe("pipeParsers given a chain of parsers", () => { + const stringToNumber: PartialParser = (rawVal, ctx) => { + if (/^\d+$/.test(rawVal)) { + return ctx.success(parseInt(rawVal, 10)); + } + return ctx.failure("a numeric string", rawVal); + }; + + const numericString = pipeParsers(string).then(stringToNumber).parser; + + it("accepts a valid value", () => { + expect(numericString("7", ctx())).toEqual(success(7)); + }); + + it("rejects a value that fails the first parser", () => { + expect(numericString(99, ctx())).toEqual(anyFailure); + }); + + it("rejects a value that fails the second parser", () => { + expect(numericString("abc", ctx())).toEqual(anyFailure); + }); +}); diff --git a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/compose-parsers.ts b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.ts similarity index 58% rename from packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/compose-parsers.ts rename to packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.ts index f50d867f1b..0eb6b9a8da 100644 --- a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/compose-parsers.ts +++ b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.ts @@ -1,13 +1,25 @@ import {isFailure} from "../result"; import type { - ParsedValue, - PartialParser, ParseContext, + ParsedValue, Parser, + PartialParser, } from "../parser-types"; -export function composeParsers< +export function pipeParsers(p: Parser): ParserPipeline { + return new ParserPipeline(p); +} + +export class ParserPipeline { + constructor(public readonly parser: Parser) {} + + then(nextParser: PartialParser): ParserPipeline { + return new ParserPipeline(composeParsers(this.parser, nextParser)); + } +} + +function composeParsers< A extends Parser, B extends PartialParser, any>, >(parserA: A, parserB: B): Parser> { diff --git a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.typetest.ts b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.typetest.ts new file mode 100644 index 0000000000..55ccce8d38 --- /dev/null +++ b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/pipe-parsers.typetest.ts @@ -0,0 +1,31 @@ +// Test: pipeParsers()...then().parser returns the expected type + +import {pipeParsers} from "./pipe-parsers"; +import {string} from "./string"; + +import type {Parser, PartialParser} from "../parser-types"; + +const stringToNumber = summon>(); +const numberToBoolean = summon>(); + +{ + pipeParsers(string).then(stringToNumber).then(numberToBoolean) + .parser satisfies Parser; +} + +{ + // @ts-expect-error - partial parser types don't match + pipeParsers(string).then(stringToNumber).then(stringToNumber).parser; +} + +{ + const p = pipeParsers(string) + .then(stringToNumber) + .then(numberToBoolean).parser; + // @ts-expect-error - return value is not assignable to Parser + p satisfies Parser; +} + +function summon(): T { + return "fake summoned value" as any; +} diff --git a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/trio.test.ts b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/trio.test.ts index 0df070566b..a4a87349de 100644 --- a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/trio.test.ts +++ b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/trio.test.ts @@ -1,8 +1,8 @@ import {success} from "../result"; import {boolean} from "./boolean"; -import {composeParsers} from "./compose-parsers"; import {number} from "./number"; +import {pipeParsers} from "./pipe-parsers"; import {string} from "./string"; import {anyFailure, ctx, parseFailureWith} from "./test-helpers"; import {trio} from "./trio"; @@ -86,10 +86,10 @@ describe("trio()", () => { }); it("returns the parsed values from each of its sub-parsers", () => { - const increment = composeParsers(number, (x, ctx) => + const increment = pipeParsers(number).then((x, ctx) => ctx.success(x + 1), - ); - const incrementBoth = trio(increment, increment, increment); - expect(incrementBoth([1, 5, 10], ctx())).toEqual(success([2, 6, 11])); + ).parser; + const incrementAll = trio(increment, increment, increment); + expect(incrementAll([1, 5, 10], ctx())).toEqual(success([2, 6, 11])); }); }); diff --git a/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/unknown.ts b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/unknown.ts new file mode 100644 index 0000000000..01fcd45d56 --- /dev/null +++ b/packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/unknown.ts @@ -0,0 +1,4 @@ +import type {Parser} from "../parser-types"; + +export const unknown: Parser = (rawValue, ctx) => + ctx.success(rawValue); diff --git a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.test.ts b/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.test.ts index 51ae3c4507..03fed86b9a 100644 --- a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.test.ts +++ b/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.test.ts @@ -40,4 +40,20 @@ describe("parsePerseusItem", () => { `At (root).answerArea.bork -- expected "calculator", "chi2Table", "financialCalculatorMonthlyPayment", "financialCalculatorTotalAmount", "financialCalculatorTimeToPayOff", "periodicTable", "periodicTableWithKey", "tTable", or "zTable", but got "bork"`, ); }); + + it("removes 'type' and 'options' keys from answerArea", () => { + const item = { + ...baseItem, + answerArea: { + calculator: true, + type: "should get removed", + options: {}, + }, + }; + + const result = parse(item, parsePerseusItem); + + assertSuccess(result); + expect(result.value.answerArea).toEqual({calculator: true}); + }); }); diff --git a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.ts b/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.ts index adfd6d3625..008cb133e3 100644 --- a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.ts +++ b/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.ts @@ -6,6 +6,7 @@ import { enumeration, number, object, + pipeParsers, record, } from "../general-purpose-parsers"; @@ -13,12 +14,14 @@ import {parseHint} from "./hint"; import {parsePerseusRenderer} from "./perseus-renderer"; import type {PerseusItem} from "../../../perseus-types"; -import type {Parser} from "../parser-types"; +import type {ParseContext, Parser, ParseResult} from "../parser-types"; export const parsePerseusItem: Parser = object({ question: parsePerseusRenderer, hints: array(parseHint), - answerArea: record(enumeration(...ItemExtras), boolean), + answerArea: pipeParsers(object({})) + .then(migrateAnswerArea) + .then(record(enumeration(...ItemExtras), boolean)).parser, itemDataVersion: object({ major: number, minor: number, @@ -26,3 +29,24 @@ export const parsePerseusItem: Parser = object({ // Deprecated field answer: any, }); + +// Some answerAreas have extra fields, like: +// +// "answerArea": { +// "type": "multiple", +// "options": { +// "content": "", +// "images": {}, +// "widgets": {} +// } +// } +// +// The "type" and "options" fields don't seem to be used anywhere. This +// migration function removes them. +function migrateAnswerArea( + rawValue: {type?: unknown; options?: unknown}, + ctx: ParseContext, +): ParseResult { + const {type: _, options: __, ...rest} = rawValue; + return ctx.success(rest); +} diff --git a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-renderer.ts b/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-renderer.ts index 573f67cfa7..385827434e 100644 --- a/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-renderer.ts +++ b/packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-renderer.ts @@ -6,6 +6,7 @@ import { record, string, } from "../general-purpose-parsers"; +import {defaulted} from "../general-purpose-parsers/defaulted"; import {parseWidgetsMap} from "./widgets-map"; @@ -18,13 +19,19 @@ export const parsePerseusRenderer: Parser = object({ // `group` widget can contain another renderer. // The anonymous function below ensures that we don't try to access // parseWidgetsMap before it's defined. - widgets: (rawVal, ctx) => parseWidgetsMap(rawVal, ctx), + widgets: defaulted( + (rawVal, ctx) => parseWidgetsMap(rawVal, ctx), + () => ({}), + ), metadata: optional(array(string)), - images: record( - string, - object({ - width: number, - height: number, - }), + images: defaulted( + record( + string, + object({ + width: number, + height: number, + }), + ), + () => ({}), ), }); diff --git a/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap b/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap index c1b40da162..3a58119d93 100644 --- a/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap +++ b/packages/perseus/src/util/parse-perseus-json/regression-tests/__snapshots__/parse-perseus-json-snapshot.test.ts.snap @@ -1,5 +1,148 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`parseAndTypecheckPerseusItem correctly parses data/orderer-option-missing-widgets.json 1`] = ` +{ + "answer": undefined, + "answerArea": { + "calculator": false, + }, + "hints": [ + { + "content": "Extinction is a natural process, and the rate of extinction can be determined by studying changes in biodiversity during the history of life on Earth. +", + "images": {}, + "metadata": undefined, + "replace": undefined, + "widgets": { + "orderer 1": { + "alignment": undefined, + "graded": true, + "key": undefined, + "options": { + "correctOptions": [ + { + "content": "$x$", + "images": {}, + "metadata": undefined, + "widgets": {}, + }, + ], + "height": "normal", + "layout": "horizontal", + "options": [ + { + "content": "$x$", + "images": {}, + "metadata": undefined, + "widgets": {}, + }, + { + "content": "$y$", + "images": {}, + "metadata": undefined, + "widgets": {}, + }, + ], + "otherOptions": [ + { + "content": "$y$", + "images": {}, + "metadata": undefined, + "widgets": {}, + }, + ], + }, + "static": undefined, + "type": "orderer", + "version": { + "major": 0, + "minor": 0, + }, + }, + }, + }, + { + "content": "Humans are causing a current sixth mass extinction event, but are not the only reason species go extinct.", + "images": {}, + "metadata": undefined, + "replace": undefined, + "widgets": {}, + }, + ], + "itemDataVersion": { + "major": 0, + "minor": 1, + }, + "question": { + "content": "What is the definition of the natural background rate of extinction? + +![](https://ka-perseus-images.s3.amazonaws.com/1f68a48ffbc7b137e99fbcbb11e9747ae799fe01.jpg) + +[[☃ radio 1]]", + "images": { + "https://ka-perseus-images.s3.amazonaws.com/1f68a48ffbc7b137e99fbcbb11e9747ae799fe01.jpg": { + "height": 253, + "width": 396, + }, + }, + "metadata": undefined, + "widgets": { + "radio 1": { + "alignment": undefined, + "graded": true, + "key": undefined, + "options": { + "choices": [ + { + "clue": undefined, + "content": "a. the rate of extinction that balances the rate of speciation", + "correct": false, + "isNoneOfTheAbove": undefined, + "widgets": undefined, + }, + { + "clue": undefined, + "content": "b. the average rate at which extinctions have occurred naturally in the geologic past, without human involvement", + "correct": true, + "isNoneOfTheAbove": undefined, + "widgets": undefined, + }, + { + "clue": undefined, + "content": "c. the rate of extinction that is currently occurring in protected areas like national parks and marine sanctuaries", + "correct": false, + "isNoneOfTheAbove": undefined, + "widgets": undefined, + }, + { + "clue": undefined, + "content": "d. the rate of extinction that can be determined by using Geiger counters and detecting radiation", + "correct": false, + "isNoneOfTheAbove": undefined, + "widgets": undefined, + }, + ], + "countChoices": undefined, + "deselectEnabled": false, + "displayCount": null, + "hasNoneOfTheAbove": undefined, + "multipleSelect": false, + "noneOfTheAbove": false, + "onePerLine": true, + "randomize": false, + }, + "static": undefined, + "type": "radio", + "version": { + "major": 0, + "minor": 0, + }, + }, + }, + }, +} +`; + exports[`parseAndTypecheckPerseusItem correctly parses data/radio-missing-noneOfTheAbove.json 1`] = ` { "answer": undefined, diff --git a/packages/perseus/src/util/parse-perseus-json/regression-tests/data/orderer-option-missing-widgets.json b/packages/perseus/src/util/parse-perseus-json/regression-tests/data/orderer-option-missing-widgets.json new file mode 100644 index 0000000000..bba1e462b5 --- /dev/null +++ b/packages/perseus/src/util/parse-perseus-json/regression-tests/data/orderer-option-missing-widgets.json @@ -0,0 +1,103 @@ +{ + "answerArea": { + "calculator": false, + "options": { + "content": "", + "images": {}, + "widgets": {} + }, + "type": "multiple" + }, + "hints": [ + { + "content": "Extinction is a natural process, and the rate of extinction can be determined by studying changes in biodiversity during the history of life on Earth.\n", + "images": {}, + "widgets": { + "orderer 1": { + "graded": true, + "options": { + "correctOptions": [ + { + "content": "$x$" + } + ], + "height": "normal", + "layout": "horizontal", + "options": [ + { + "content": "$x$" + }, + { + "content": "$y$" + } + ], + "otherOptions": [ + { + "content": "$y$" + } + ] + }, + "type": "orderer", + "version": { + "major": 0, + "minor": 0 + } + } + } + }, + { + "content": "Humans are causing a current sixth mass extinction event, but are not the only reason species go extinct.", + "images": {}, + "widgets": {} + } + ], + "itemDataVersion": { + "major": 0, + "minor": 1 + }, + "question": { + "content": "What is the definition of the natural background rate of extinction? \n\n![](https://ka-perseus-images.s3.amazonaws.com/1f68a48ffbc7b137e99fbcbb11e9747ae799fe01.jpg)\n\n[[☃ radio 1]]", + "images": { + "https://ka-perseus-images.s3.amazonaws.com/1f68a48ffbc7b137e99fbcbb11e9747ae799fe01.jpg": { + "height": 253, + "width": 396 + } + }, + "widgets": { + "radio 1": { + "graded": true, + "options": { + "choices": [ + { + "content": "a.\tthe rate of extinction that balances the rate of speciation", + "correct": false + }, + { + "content": "b.\tthe average rate at which extinctions have occurred naturally in the geologic past, without human involvement", + "correct": true + }, + { + "content": "c.\tthe rate of extinction that is currently occurring in protected areas like national parks and marine sanctuaries", + "correct": false + }, + { + "content": "d.\tthe rate of extinction that can be determined by using Geiger counters and detecting radiation", + "correct": false + } + ], + "deselectEnabled": false, + "displayCount": null, + "multipleSelect": false, + "noneOfTheAbove": false, + "onePerLine": true, + "randomize": false + }, + "type": "radio", + "version": { + "major": 0, + "minor": 0 + } + } + } + } +}