[Bugfix] Interactive Graph crashes in editor when setting domain for …

…locked function (#2223)

## Summary:
When editing an Interactive Graph widget and specifying only one of the domain values for a locked function, the exercise is then unable to be loaded in the editor once it has been saved and re-opened. The problem was that the logic handling the domain values (an array) wasn't set up to handle null values within the array.

Issue: CP-8932

## Test plan:
1. Open an exercise editor
2. Add an interactive graph widget
3. Add a locked function
4. Set the value of one of the domain options to some number (leave the other one blank)
5. Click the "Save All" button for the editor
6. When the page reloads, the page should render (previously, it was showing the "Oops. Something went wrong" message.

Author: mark-fitzgerald

Reviewers: benchristel, mark-fitzgerald, nishasy, #perseus

Required Reviewers:

Approved By: nishasy

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2223

.changeset/afraid-dots-provide.md

@@ -0,0 +1,7 @@
+---
+"@khanacademy/perseus": patch
+"@khanacademy/perseus-core": patch
+"@khanacademy/perseus-editor": patch
+---
+
+[Bugfix] Interactive Graph crashes in editor when setting domain for locked function

packages/perseus-core/src/data-schema.ts

@@ -871,7 +871,7 @@ export type LockedFunctionType = {
     strokeStyle: LockedLineStyle;
     equation: string; // This is the user-defined equation (as it was typed)
     directionalAxis: "x" | "y";
-    domain?: Interval;
+    domain?: [min: number | null, max: number | null];
     labels?: LockedLabelType[];
     ariaLabel?: string;
 };

packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.test.tsx

@@ -1,5 +1,6 @@
 import {RenderStateRoot} from "@khanacademy/wonder-blocks-core";
-import {render, screen} from "@testing-library/react";
+// eslint-disable-next-line testing-library/no-manual-cleanup
+import {render, screen, cleanup} from "@testing-library/react";
 import {userEvent as userEventLib} from "@testing-library/user-event";
 import * as React from "react";
 
@@ -57,6 +58,74 @@ describe("Locked Function Settings", () => {
         expect(titleText).toBeInTheDocument();
     });
 
+    test("renders as expected with partial domain information", () => {
+        // Act (max domain is Infinity)
+        render(
+            <LockedFunctionSettings {...defaultProps} domain={[0, Infinity]} />,
+            {
+                wrapper: RenderStateRoot,
+            },
+        );
+
+        // Assert
+        let inputField = screen
+            .getByLabelText("domain max")
+            // eslint-disable-next-line testing-library/no-node-access
+            .querySelector("input");
+        expect(inputField?.value).toEqual("");
+
+        // Act (min domain is -Infinity)
+        cleanup();
+        render(
+            <LockedFunctionSettings
+                {...defaultProps}
+                domain={[-Infinity, 0]}
+            />,
+            {
+                wrapper: RenderStateRoot,
+            },
+        );
+
+        // Assert
+        inputField = screen
+            .getByText("domain min")
+            // eslint-disable-next-line testing-library/no-node-access
+            .querySelector("input");
+        expect(inputField?.value).toEqual("");
+
+        // Act (max domain not defined)
+        cleanup();
+        render(
+            <LockedFunctionSettings {...defaultProps} domain={[0, null]} />,
+            {
+                wrapper: RenderStateRoot,
+            },
+        );
+
+        // Assert
+        inputField = screen
+            .getByLabelText("domain max")
+            // eslint-disable-next-line testing-library/no-node-access
+            .querySelector("input");
+        expect(inputField?.value).toEqual("");
+
+        // Act (min domain not defined)
+        cleanup();
+        render(
+            <LockedFunctionSettings {...defaultProps} domain={[null, 0]} />,
+            {
+                wrapper: RenderStateRoot,
+            },
+        );
+
+        // Assert
+        inputField = screen
+            .getByText("domain min")
+            // eslint-disable-next-line testing-library/no-node-access
+            .querySelector("input");
+        expect(inputField?.value).toEqual("");
+    });
+
     describe("Header interactions", () => {
         test("should show the function's color and stroke by default", () => {
             // Arrange
@@ -189,6 +258,7 @@ describe("Locked Function Settings", () => {
             // Assert
             expect(onChangeProps).toHaveBeenCalledWith({
                 color: "green",
+                labels: [],
             });
         });
 
@@ -766,6 +836,66 @@ describe("Locked Function Settings", () => {
                 });
             });
 
+            test("aria label does not auto-generate with domain when both values are null", async () => {
+                // Arrange
+                const onChangeProps = jest.fn();
+                render(
+                    <LockedFunctionSettings
+                        {...defaultProps}
+                        ariaLabel={undefined}
+                        onChangeProps={onChangeProps}
+                        domain={[null, null]}
+                    />,
+                    {wrapper: RenderStateRoot},
+                );
+
+                // Act
+                const autoGenButton = screen.getByRole("button", {
+                    name: "Auto-generate",
+                });
+                await userEvent.click(autoGenButton);
+
+                // Assert
+                expect(onChangeProps).toHaveBeenCalledWith({
+                    ariaLabel:
+                        "Function with equation y=x^2. Appearance solid gray.",
+                });
+            });
+
+            test.each`
+                domainValue       | domainAria
+                ${[1, Infinity]}  | ${"1 to Infinity"}
+                ${[-Infinity, 2]} | ${"-Infinity to 2"}
+                ${[1, null]}      | ${"1 to Infinity"}
+                ${[null, 2]}      | ${"-Infinity to 2"}
+            `(
+                "aria label auto-generates with partial domain info: $domainValue",
+                async ({domainValue, domainAria}) => {
+                    // Arrange
+                    const onChangeProps = jest.fn();
+                    render(
+                        <LockedFunctionSettings
+                            {...defaultProps}
+                            ariaLabel={undefined}
+                            onChangeProps={onChangeProps}
+                            domain={domainValue}
+                        />,
+                        {wrapper: RenderStateRoot},
+                    );
+
+                    // Act
+                    const autoGenButton = screen.getByRole("button", {
+                        name: "Auto-generate",
+                    });
+                    await userEvent.click(autoGenButton);
+
+                    // Assert
+                    expect(onChangeProps).toHaveBeenCalledWith({
+                        ariaLabel: `Function with equation y=x^2, domain from ${domainAria}. Appearance solid gray.`,
+                    });
+                },
+            );
+
             test("aria label auto-generates (one label)", async () => {
                 // Arrange
                 const onChangeProps = jest.fn();

packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/locked-function-settings.tsx

@@ -40,7 +40,6 @@ import type {
     LockedFunctionType,
     LockedLabelType,
 } from "@khanacademy/perseus-core";
-import type {Interval} from "mafs";
 
 export type Props = LockedFunctionType &
     LockedFigureSettingsCommonProps & {
@@ -57,35 +56,38 @@ const LockedFunctionSettings = (props: Props) => {
         equation,
         directionalAxis,
         domain,
-        labels,
         ariaLabel,
         onChangeProps,
         onMove,
         onRemove,
     } = props;
+    const labels = props.labels ?? [];
     const equationPrefix = directionalAxis === "x" ? "y=" : "x=";
     const lineLabel = `Function (${equationPrefix}${equation})`;
 
     // Tracking the string value of domain/range constraints to handle interim state of
     //     entering a negative value, as well as representing Infinity as an empty string.
     // This variable is used when specifying the values of the input fields.
-    const [domainEntries, setDomainEntries] = useState([
-        domain && domain[0] !== -Infinity ? domain[0].toString() : "",
-        domain && domain[1] !== Infinity ? domain[1].toString() : "",
-    ]);
+    const getDomainStringValues = (domain): [string, string] => {
+        return [
+            domain && Number.isFinite(domain[0]) ? domain[0].toString() : "",
+            domain && Number.isFinite(domain[1]) ? domain[1].toString() : "",
+        ];
+    };
+
+    const [domainEntries, setDomainEntries] = useState(
+        getDomainStringValues(domain),
+    );
 
     const [exampleCategory, setExampleCategory] = useState("");
 
     useEffect(() => {
         // "useEffect" used to maintain parity between domain/range constraints and their string representation.
-        setDomainEntries([
-            domain && domain[0] !== -Infinity ? domain[0].toString() : "",
-            domain && domain[1] !== Infinity ? domain[1].toString() : "",
-        ]);
+        setDomainEntries(getDomainStringValues(domain));
     }, [domain]);
 
     /**
-     * Generate the prepopulated aria label for the polygon,
+     * Generate the prepopulated aria label for the function,
      * with the math details converted into spoken words.
      */
     async function getPrepopulatedAriaLabel() {
@@ -95,8 +97,15 @@ const LockedFunctionSettings = (props: Props) => {
 
         // Add the domain/range constraints to the aria label
         // if they are not the default values.
-        if (domain && !(domain[0] === -Infinity && domain[1] === Infinity)) {
-            str += `, domain from ${domain[0]} to ${domain[1]}`;
+        const domainMin =
+            domain && Number.isFinite(domain[0]) ? domain[0] : "-Infinity";
+        const domainMax =
+            domain && Number.isFinite(domain[1]) ? domain[1] : "Infinity";
+        if (
+            domain &&
+            (Number.isFinite(domain[0]) || Number.isFinite(domain[1]))
+        ) {
+            str += `, domain from ${domainMin} to ${domainMax}`;
         }
 
         const functionAppearance = generateLockedFigureAppearanceDescription(
@@ -122,10 +131,10 @@ const LockedFunctionSettings = (props: Props) => {
             dedicated code to convert empty to Infinity.
      */
     function handleDomainChange(limitIndex: number, newValueString: string) {
-        const newDomainEntries = [...domainEntries];
+        const newDomainEntries: [string, string] = [...domainEntries];
         newDomainEntries[limitIndex] = newValueString;
         setDomainEntries(newDomainEntries);
-        const newDomain: Interval = domain
+        const newDomain: [min: number | null, max: number | null] = domain
             ? [...domain]
             : [-Infinity, Infinity];
 
@@ -150,8 +159,8 @@ const LockedFunctionSettings = (props: Props) => {
             color: newValue,
         };
 
-        // Update the color of the all labels to match the point
-        newProps.labels = labels?.map((label) => ({
+        // Update the color of all the labels to match the point
+        newProps.labels = labels.map((label) => ({
             ...label,
             color: newValue,
         }));
@@ -163,10 +172,6 @@ const LockedFunctionSettings = (props: Props) => {
         updatedLabel: Partial<LockedLabelType>,
         labelIndex: number,
     ) {
-        if (!labels) {
-            return;
-        }
-
         const updatedLabels = [...labels];
         updatedLabels[labelIndex] = {
             ...labels[labelIndex],
@@ -177,10 +182,6 @@ const LockedFunctionSettings = (props: Props) => {
     }
 
     function handleLabelRemove(labelIndex: number) {
-        if (!labels) {
-            return;
-        }
-
         const updatedLabels = labels.filter((_, index) => index !== labelIndex);
 
         onChangeProps({labels: updatedLabels});
@@ -340,7 +341,7 @@ const LockedFunctionSettings = (props: Props) => {
             <View style={styles.horizontalRule} />
             <Strut size={spacing.small_12} />
             <LabelMedium>Visible labels</LabelMedium>
-            {labels?.map((label, labelIndex) => (
+            {labels.map((label, labelIndex, allLabels) => (
                 <LockedLabelSettings
                     key={labelIndex}
                     {...label}
@@ -362,13 +363,13 @@ const LockedFunctionSettings = (props: Props) => {
                         ...getDefaultFigureForType("label"),
                         // Vertical offset for each label so they
                         // don't overlap.
-                        coord: [0, -(labels?.length ?? 0)],
+                        coord: [0, -labels.length],
                         // Default to the same color as the function
                         color: lineColor,
                     } satisfies LockedLabelType;
 
                     onChangeProps({
-                        labels: [...(labels ?? []), newLabel],
+                        labels: [...labels, newLabel],
                     });
                 }}
                 style={styles.addButton}

packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts

@@ -24,7 +24,7 @@ export type LockedFunctionOptions = {
     color?: LockedFigureColor;
     strokeStyle?: LockedLineStyle;
     directionalAxis?: "x" | "y";
-    domain?: Interval;
+    domain?: [min: number | null, max: number | null];
     labels?: LockedFigureLabelOptions[];
     ariaLabel?: string;
 };