Migrate PerseusItem.answerArea, removing unused fields (#1895)

`answerArea` sometimes includes `options` and `type` fields. `type` always
seems to be set to `multiple` if it's present. I couldn't find evidence in
Perseus or webapp that this data is used anywhere, so I am removing it.
This is necessary to make parsePerseusItem return a valid PerseusItem while
still being able to handle existing data that has the extra fields.

Issue: https://khanacademy.atlassian.net/browse/LEMS-2582

## Test plan:

`yarn test`

Author: benchristel

Reviewers: jeremywiebe, catandthemachines, benchristel, anakaren-rojas, nishasy

Required Reviewers:

Approved By: jeremywiebe

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

Pull Request URL: #1895

.changeset/rotten-peaches-move.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": patch
+---
+
+Internal: remove unused fields from `answerArea` when parsing `PerseusItem`s.

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";

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]));
     });

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<string, number> = (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);
+    });
+});

packages/perseus/src/util/parse-perseus-json/general-purpose-parsers/compose-parsers.ts → 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<T>(p: Parser<T>): ParserPipeline<T> {
+    return new ParserPipeline(p);
+}
+
+export class ParserPipeline<T> {
+    constructor(public readonly parser: Parser<T>) {}
+
+    then<U>(nextParser: PartialParser<T, U>): ParserPipeline<U> {
+        return new ParserPipeline<U>(composeParsers(this.parser, nextParser));
+    }
+}
+
+function composeParsers<
     A extends Parser<any>,
     B extends PartialParser<ParsedValue<A>, any>,
 >(parserA: A, parserB: B): Parser<ParsedValue<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<PartialParser<string, number>>();
+const numberToBoolean = summon<PartialParser<number, boolean>>();
+
+{
+    pipeParsers(string).then(stringToNumber).then(numberToBoolean)
+        .parser satisfies Parser<boolean>;
+}
+
+{
+    // @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<string>
+    p satisfies Parser<string>;
+}
+
+function summon<T>(): T {
+    return "fake summoned value" as any;
+}

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]));
     });
 });

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<unknown> = (rawValue, ctx) =>
+    ctx.success(rawValue);

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});
+    });
 });

packages/perseus/src/util/parse-perseus-json/perseus-parsers/perseus-item.ts

@@ -6,23 +6,47 @@ import {
     enumeration,
     number,
     object,
+    pipeParsers,
     record,
 } from "../general-purpose-parsers";
 
 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<PerseusItem> = 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,
     }),
     // 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<unknown> {
+    const {type: _, options: __, ...rest} = rawValue;
+    return ctx.success(rest);
+}

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<PerseusRenderer> = 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,
+            }),
+        ),
+        () => ({}),
     ),
 });