Skip to content

Commit

Permalink
[Client Validation] Fixes after merging main (#2122)
Browse files Browse the repository at this point in the history
## Summary:

I recently merged `main` and the introduction of `MockWidget` caused some tests to break. I've fixed them in this PR and adjusted the MockWidget to be simpler (ie. not use KhanAnswerTypes) as I don't _think_ that's needed.

I also fixed up some related types so that the MockWidget does not exist in our production Widget types, only in tests. The new `MockWidget` illustrates this "test-only" expansion of the widget types.

Issue: LEMS-2561

## Test plan:

`yarn test`
`yarn typecheck`

Author: jeremywiebe

Reviewers: jeremywiebe, handeyeco, benchristel, Myranae

Required Reviewers:

Approved By: handeyeco

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x)

Pull Request URL: #2122
  • Loading branch information
jeremywiebe authored Jan 21, 2025
1 parent 5f79a67 commit 1a75ca6
Show file tree
Hide file tree
Showing 14 changed files with 120 additions and 89 deletions.
7 changes: 7 additions & 0 deletions .changeset/eight-squids-repair.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-core": patch
"@khanacademy/perseus-editor": patch
---

Type and test fixes for new MockWidget (isolating to be seen only in tests)
59 changes: 13 additions & 46 deletions packages/perseus-core/src/data-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export type MakeWidgetMap<TRegistry> = {
* `PerseusWidgets` with the one defined below.
*
* ```typescript
* declare module "@khanacademy/perseus" {
* declare module "@khanacademy/perseus-core" {
* interface PerseusWidgetTypes {
* // A new widget
* "new-awesomeness": MyAwesomeNewWidget;
Expand Down Expand Up @@ -144,7 +144,6 @@ export interface PerseusWidgetTypes {
matcher: MatcherWidget;
matrix: MatrixWidget;
measurer: MeasurerWidget;
"mock-widget": MockWidget;
"molecule-renderer": MoleculeRendererWidget;
"number-line": NumberLineWidget;
"numeric-input": NumericInputWidget;
Expand Down Expand Up @@ -181,6 +180,18 @@ export interface PerseusWidgetTypes {
*/
export type PerseusWidgetsMap = MakeWidgetMap<PerseusWidgetTypes>;

/**
* PerseusWidget is a union of all the different types of widget options that
* Perseus knows about.
*
* Thanks to it being based on PerseusWidgetTypes interface, this union is
* automatically extended to include widgets used in tests without those widget
* option types seeping into our production types.
*
* @see MockWidget for an example
*/
export type PerseusWidget = PerseusWidgetTypes[keyof PerseusWidgetTypes];

/**
* A "PerseusItem" is a classic Perseus item. It is rendered by the
* `ServerItemRenderer` and the layout is pre-set.
Expand Down Expand Up @@ -346,8 +357,6 @@ export type MatrixWidget = WidgetOptions<'matrix', PerseusMatrixWidgetOptions>;
// prettier-ignore
export type MeasurerWidget = WidgetOptions<'measurer', PerseusMeasurerWidgetOptions>;
// prettier-ignore
export type MockWidget = WidgetOptions<'mock-widget', MockWidgetOptions>;
// prettier-ignore
export type NumberLineWidget = WidgetOptions<'number-line', PerseusNumberLineWidgetOptions>;
// prettier-ignore
export type NumericInputWidget = WidgetOptions<'numeric-input', PerseusNumericInputWidgetOptions>;
Expand Down Expand Up @@ -380,43 +389,6 @@ export type VideoWidget = WidgetOptions<'video', PerseusVideoWidgetOptions>;
//prettier-ignore
export type DeprecatedStandinWidget = WidgetOptions<'deprecated-standin', object>;

export type PerseusWidget =
| CategorizerWidget
| CSProgramWidget
| DefinitionWidget
| DropdownWidget
| ExplanationWidget
| ExpressionWidget
| GradedGroupSetWidget
| GradedGroupWidget
| GrapherWidget
| GroupWidget
| IFrameWidget
| ImageWidget
| InputNumberWidget
| InteractionWidget
| InteractiveGraphWidget
| LabelImageWidget
| MatcherWidget
| MatrixWidget
| MeasurerWidget
| MockWidget
| MoleculeRendererWidget
| NumberLineWidget
| NumericInputWidget
| OrdererWidget
| PassageRefWidget
| PassageWidget
| PhetSimulationWidget
| PlotterWidget
| PythonProgramWidget
| RadioWidget
| RefTargetWidget
| SorterWidget
| TableWidget
| VideoWidget
| DeprecatedStandinWidget;

/**
* A background image applied to various widgets.
*/
Expand Down Expand Up @@ -1720,11 +1692,6 @@ export type PerseusVideoWidgetOptions = {
static?: boolean;
};

export type MockWidgetOptions = {
static?: boolean;
value: string;
};

export type PerseusInputNumberWidgetOptions = {
answerType?:
| "number"
Expand Down
3 changes: 1 addition & 2 deletions packages/perseus/src/__testdata__/renderer.testdata.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import type {MockWidget} from "../widgets/mock-widgets/mock-widget-types";
import type {RenderProps} from "../widgets/radio";
import type {
DropdownWidget,
ExpressionWidget,
ImageWidget,
NumericInputWidget,
MockWidget,
PerseusRenderer,
} from "@khanacademy/perseus-core";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import {
type ExpressionWidget,
type RadioWidget,
type NumericInputWidget,
type MockWidget,
} from "@khanacademy/perseus-core";

import type {MockWidget} from "../widgets/mock-widgets/mock-widget-types";

export const itemWithNumericInput: PerseusItem = {
question: {
content:
Expand Down Expand Up @@ -40,7 +41,7 @@ export const itemWithNumericInput: PerseusItem = {
labelText: "What's the answer?",
size: "normal",
},
} as NumericInputWidget,
} satisfies NumericInputWidget,
},
},
hints: [
Expand All @@ -64,7 +65,7 @@ export const itemWithMockWidget: PerseusItem = {
options: {
value: "3",
},
} as MockWidget,
} satisfies MockWidget,
},
},
hints: [
Expand Down Expand Up @@ -158,14 +159,14 @@ export const itemWithTwoMockWidgets: PerseusItem = {
options: {
value: "3",
},
} as MockWidget,
} satisfies MockWidget,
"mock-widget 2": {
type: "mock-widget",
graded: true,
options: {
value: "3",
},
} as MockWidget,
} satisfies MockWidget,
},
},
hints: [
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/__tests__/renderer-api.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import mockWidget1Item from "./test-items/mock-widget-1-item";
import mockWidget2Item from "./test-items/mock-widget-2-item";
import tableItem from "./test-items/table-item";

import type {PerseusMockWidgetUserInput} from "../widgets/mock-widgets/mock-widget";
import type {PerseusMockWidgetUserInput} from "../widgets/mock-widgets/mock-widget-types";
import type {UserEvent} from "@testing-library/user-event";

const itemWidget = mockWidget1Item;
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export const MafsGraphTypeFlags = [
/**
* APIOptions provides different ways to customize the behaviour of Perseus.
*
* @see APIOptionsWithDefaults
* @see {@link APIOptionsWithDefaults}
*/
export type APIOptions = Readonly<{
isArticle?: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {registerWidget} from "../../widgets";
import {renderQuestion} from "../../widgets/__testutils__/renderQuestion";
import MockWidgetExport from "../../widgets/mock-widgets/mock-widget";

import type {PerseusRenderer, MockWidget} from "@khanacademy/perseus-core";
import type {MockWidget} from "../../widgets/mock-widgets/mock-widget-types";
import type {PerseusRenderer} from "@khanacademy/perseus-core";
import type {UserEvent} from "@testing-library/user-event";

const question: PerseusRenderer = {
Expand All @@ -25,7 +26,7 @@ const question: PerseusRenderer = {
value: "42",
},
alignment: "default",
} as MockWidget,
} satisfies MockWidget,
},
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {getPromptJSON} from "./prompt-utils";

import type {PerseusMockWidgetUserInput} from "../../widgets/mock-widgets/mock-widget";
import type {PerseusMockWidgetUserInput} from "../../widgets/mock-widgets/mock-widget-types";

describe("InputNumber getPromptJSON", () => {
it("it returns JSON with the expected format and fields", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {PerseusMockWidgetUserInput} from "../../widgets/mock-widgets/mock-widget";
import type mockWidget from "../../widgets/mock-widgets/mock-widget";
import type {PerseusMockWidgetUserInput} from "../../widgets/mock-widgets/mock-widget-types";
import type React from "react";

export type MockWidgetPromptJSON = {
Expand Down
27 changes: 27 additions & 0 deletions packages/perseus/src/widgets/mock-widgets/mock-widget-types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import type {WidgetOptions} from "@khanacademy/perseus-core";

export type MockWidget = WidgetOptions<"mock-widget", MockWidgetOptions>;

export type MockWidgetOptions = {
static?: boolean;
value: string;
};

export type PerseusMockWidgetRubric = {
value: string;
};

export type PerseusMockWidgetUserInput = {
currentValue: string;
};

// Extend the widget registries for testing
// See @khanacademy/perseus-core's PerseusWidgetTypes for a full explanation.
// Basically, we're extending the interface from that package so that our
// testing code knows of the MockWidget. In production code, there's no
// knowledge of the mock widget.
declare module "@khanacademy/perseus-core" {
export interface PerseusWidgetTypes {
"mock-widget": MockWidget;
}
}
26 changes: 13 additions & 13 deletions packages/perseus/src/widgets/mock-widgets/mock-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,15 @@ import * as React from "react";
import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/mock-widget/prompt-utils";

import scoreMockWidget from "./score-mock-widget";
import validateMockWidget from "./validate-mock-widget";

import type {
MockWidgetOptions,
PerseusMockWidgetRubric,
PerseusMockWidgetUserInput,
} from "./mock-widget-types";
import type {WidgetExports, WidgetProps, Widget, FocusPath} from "../../types";
import type {MockWidgetPromptJSON} from "../../widget-ai-utils/mock-widget/prompt-utils";
import type {MockWidgetOptions} from "@khanacademy/perseus-core";

export type PerseusMockWidgetRubric = {
value: string;
};

export type PerseusMockWidgetUserInput = {
currentValue: string;
};

type ExternalProps = WidgetProps<MockWidgetOptions, PerseusMockWidgetRubric>;

Expand All @@ -41,7 +38,7 @@ type Props = ExternalProps & {
*
* You can register this widget for your tests by calling `registerWidget("mock-widget", MockWidget);`
*/
export class MockWidget extends React.Component<Props> implements Widget {
class MockWidgetComponent extends React.Component<Props> implements Widget {
static defaultProps: DefaultProps = {
currentValue: "",
};
Expand Down Expand Up @@ -93,7 +90,7 @@ export class MockWidget extends React.Component<Props> implements Widget {
};

getUserInput(): PerseusMockWidgetUserInput {
return MockWidget.getUserInputFromProps(this.props);
return MockWidgetComponent.getUserInputFromProps(this.props);
}

handleChange: (
Expand Down Expand Up @@ -131,9 +128,12 @@ const styles = StyleSheet.create({
export default {
name: "mock-widget",
displayName: "Mock Widget",
widget: MockWidget,
widget: MockWidgetComponent,
isLintable: true,
// TODO(LEMS-2656): remove TS suppression
// @ts-expect-error: Type 'UserInput' is not assignable to type 'MockWidget'.
scorer: scoreMockWidget,
} satisfies WidgetExports<typeof MockWidget>;
// TODO(LEMS-2656): remove TS suppression
// @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusMockWidgetUserInput'.
validator: validateMockWidget,
} satisfies WidgetExports<typeof MockWidgetComponent>;
25 changes: 8 additions & 17 deletions packages/perseus/src/widgets/mock-widgets/score-mock-widget.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {KhanAnswerTypes} from "@khanacademy/perseus-score";
import validateMockWidget from "./validate-mock-widget";

import type {
PerseusMockWidgetUserInput,
PerseusMockWidgetRubric,
} from "./mock-widget";
} from "./mock-widget-types";
import type {PerseusStrings} from "../../strings";
import type {PerseusScore} from "@khanacademy/perseus";

Expand All @@ -12,25 +12,16 @@ function scoreMockWidget(
rubric: PerseusMockWidgetRubric,
strings: PerseusStrings,
): PerseusScore {
const stringValue = `${rubric.value}`;
const val = KhanAnswerTypes.number.createValidatorFunctional(
stringValue,
strings,
);

const result = val(userInput.currentValue);

if (result.empty) {
return {
type: "invalid",
message: result.message,
};
const validationResult = validateMockWidget(userInput);
if (validationResult != null) {
return validationResult;
}

return {
type: "points",
earned: result.correct ? 1 : 0,
earned: userInput.currentValue === rubric.value ? 1 : 0,
total: 1,
message: result.message,
message: "",
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import validateMockWidget from "./validate-mock-widget";

import type {PerseusMockWidgetUserInput} from "./mock-widget-types";

describe("mock-widget", () => {
it("should be invalid if no value provided", () => {
const input: PerseusMockWidgetUserInput = {currentValue: ""};

const result = validateMockWidget(input);

expect(result).toHaveInvalidInput();
});

it("should be valid if a value provided", () => {
const input: PerseusMockWidgetUserInput = {currentValue: "a"};

const result = validateMockWidget(input);

expect(result).toBeNull();
});
});
17 changes: 17 additions & 0 deletions packages/perseus/src/widgets/mock-widgets/validate-mock-widget.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import type {PerseusMockWidgetUserInput} from "./mock-widget-types";
import type {ValidationResult} from "../../types";

function validateMockWidget(
userInput: PerseusMockWidgetUserInput,
): ValidationResult {
if (userInput.currentValue == null || userInput.currentValue === "") {
return {
type: "invalid",
message: "",
};
}

return null;
}

export default validateMockWidget;

0 comments on commit 1a75ca6

Please sign in to comment.