Skip to content

Commit

Permalink
[Feature] Client Validation (#2031)
Browse files Browse the repository at this point in the history
## Summary:
This PR includes the following commits:
- Swap out deprecated input-number with numeric-input in some tests (#1995)
- SSS: Hook emptyWidgets() up to validation functions (#2000)
- Add test to document empty expression can be a correct answer (#2003)
- Remove unused rubric type for CS Program (#1997)
- Remove unused rubric type for iFrame (#1996)
- Refactor LabelImage to separate out answers from userInput into scoringData (#1965)
- Label-image: Extract validation out of scoring (#2016)
- Rename usages of rubric to scoringData (#2006)
- SSS: Improve types for validation (#2002)

## Test plan:
- Confirm all checks pass
- Manual test widgets to confirm they act as expected by creating a webapp testing branch/PR
- Confirm widgets all grade as expected

Author: Myranae

Reviewers: jeremywiebe, handeyeco

Required Reviewers:

Approved By: jeremywiebe

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

Pull Request URL: #2031
  • Loading branch information
jeremywiebe authored Jan 23, 2025
2 parents 19332bb + cd0e130 commit 36198bf
Show file tree
Hide file tree
Showing 102 changed files with 1,644 additions and 951 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)
5 changes: 5 additions & 0 deletions .changeset/few-rings-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Add and improve types for scoring and validation
5 changes: 5 additions & 0 deletions .changeset/fifty-laws-hear.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Remove unused CS Program rubric type
6 changes: 6 additions & 0 deletions .changeset/many-penguins-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": major
"@khanacademy/perseus-editor": patch
---

Refactor the LabelImage widget to separate out answers from userInput into scoringData
5 changes: 5 additions & 0 deletions .changeset/nine-planes-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Fix some naming discrepancies related to validation and simplify Matcher ScoringData type
5 changes: 5 additions & 0 deletions .changeset/pink-pumas-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Use empty widgets check in scoring function
5 changes: 5 additions & 0 deletions .changeset/proud-ghosts-learn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Remove unused iframe rubric type
5 changes: 5 additions & 0 deletions .changeset/quiet-adults-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Change empty widgets check in Renderer to depend only on data available (and not on scoring data)
5 changes: 5 additions & 0 deletions .changeset/smooth-cheetahs-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Rename usages of rubric to scoringData
5 changes: 5 additions & 0 deletions .changeset/spicy-cups-join.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

TESTS: swap input-number out of renderer tests as it is deprecated
5 changes: 5 additions & 0 deletions .changeset/thirty-hornets-punch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Introduces a validation function for the label-image widget (extracted from label-image scoring function).
12 changes: 6 additions & 6 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ base Markdown syntax:

1. Widgets - Perseus can render custom widgets (in the form of React
components) which conform to a special API that enables the user to
interact with the widget and for the widget to check taht input for
correctness against a rubric. Widgets are denoted using the following
Markdown syntax: `[[☃️ widget-id ]]` (where `widget-id` represents a
generated ID that is unique within the Perseus instance.
interact with the widget and for the widget to check that input for
correctness against a set of scoring data. Widgets are denoted using the
following Markdown syntax: `[[☃️ widget-id ]]` (where `widget-id`
represents a generated ID that is unique within the Perseus instance.
1. Math - Perseus can also render beautiful math using MathJax. Math is
denoted using an opening and close dollar sign (eg. `$y = mx + b$`).

Expand Down Expand Up @@ -181,15 +181,15 @@ the widgets options type (ie. the type `T` wrapped in `WidgetOptions<T>` from
In a few rare cases, this type is defined as the sum of RenderProps wrapped in
`WidgetOptions`.

### `Rubric`
### `Scoring Data`

This type defines the data that the scoring function needs in order to score
the learner's guess (aka user input).

### `Props`

Finally, `Props` form the entire set of props that widget's component supports.
Typically it is defined as `type Props = WidgetProps<RenderProps, Rubric>`. In
Typically it is defined as `type Props = WidgetProps<RenderProps, ScoringData>`. In
cases where there are `RenderProps` that are optional that are provided via
`DefaultProps`, this `Props` type "redefines" these props as `myProp:
NonNullable<ExternalProps["myProps"]>;`.
Expand Down
107 changes: 58 additions & 49 deletions packages/perseus-core/src/data-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,50 @@ export type Size = [width: number, height: number];
export type CollinearTuple = [Vector2, Vector2];
export type ShowSolutions = "all" | "selected" | "none";

/**
* A utility type that constructs a widget map from a "registry interface".
* The keys of the registry should be the widget type (aka, "categorizer" or
* "radio", etc) and the value should be the option type stored in the value
* of the map.
*
* You can think of this as a type that generates another type. We use
* "registry interfaces" as a way to keep a set of widget types to their data
* type in several places in Perseus. This type then allows us to generate a
* map type that maps a widget id to its data type and keep strong typing by
* widget id.
*
* For example, given a fictitious registry such as this:
*
* ```
* interface DummyRegistry {
* categorizer: { categories: ReadonlyArray<string> };
* dropdown: { choices: ReadonlyArray<string> }:
* }
* ```
*
* If we create a DummyMap using this helper:
*
* ```
* type DummyMap = MakeWidgetMap<DummyRegistry>;
* ```
*
* We'll get a map that looks like this:
*
* ```
* type DummyMap = {
* `categorizer ${number}`: { categories: ReadonlyArray<string> };
* `dropdown ${number}`: { choices: ReadonlyArray<string> };
* }
* ```
*
* We use interfaces for the registries so that they can be extended in cases
* where the consuming app brings along their own widgets. Interfaces in
* TypeScript are always open (ie. you can extend them) whereas types aren't.
*/
export type MakeWidgetMap<TRegistry> = {
[Property in keyof TRegistry as `${Property & string} ${number}`]: TRegistry[Property];
};

/**
* Our core set of Perseus widgets.
*
Expand All @@ -58,7 +102,7 @@ export type ShowSolutions = "all" | "selected" | "none";
* `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 @@ -100,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 @@ -135,9 +178,19 @@ export interface PerseusWidgetTypes {
* @see {@link PerseusWidgetTypes} additional widgets can be added to this map type
* by augmenting the PerseusWidgetTypes with new widget types!
*/
export type PerseusWidgetsMap = {
[Property in keyof PerseusWidgetTypes as `${Property} ${number}`]: PerseusWidgetTypes[Property];
};
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
Expand Down Expand Up @@ -304,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 @@ -338,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 @@ -1678,11 +1692,6 @@ export type PerseusVideoWidgetOptions = {
static?: boolean;
};

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

export type PerseusInputNumberWidgetOptions = {
answerType?:
| "number"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as React from "react";

import LabelImageEditor from "../label-image-editor";

import type {MarkerType} from "@khanacademy/perseus-core";
import type {PerseusLabelImageWidgetOptions} from "@khanacademy/perseus-core";

type StoryArgs = Record<any, any>;

Expand All @@ -29,7 +29,7 @@ type State = {
imageUrl: string;
imageWidth: number;
imageHeight: number;
markers: ReadonlyArray<MarkerType>;
markers: PerseusLabelImageWidgetOptions["markers"];
};

class WithState extends React.Component<Empty, State> {
Expand Down
10 changes: 5 additions & 5 deletions packages/perseus-editor/src/widgets/label-image-editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import Behavior from "./label-image/behavior";
import QuestionMarkers from "./label-image/question-markers";
import SelectImage from "./label-image/select-image";

import type {MarkerType} from "@khanacademy/perseus-core";
import type {PerseusLabelImageWidgetOptions} from "@khanacademy/perseus-core";

type Props = {
// List of answer choices to label question image with.
Expand All @@ -28,7 +28,7 @@ type Props = {
imageWidth: number;
imageHeight: number;
// The list of label markers on the question image.
markers: ReadonlyArray<MarkerType>;
markers: PerseusLabelImageWidgetOptions["markers"];
// Whether multiple answer choices may be selected for markers.
multipleAnswers: boolean;
// Whether to hide answer choices from user instructions.
Expand Down Expand Up @@ -176,9 +176,9 @@ class LabelImageEditor extends React.Component<Props> {
this.props.onChange({choices});
};

handleMarkersChange: (markers: ReadonlyArray<MarkerType>) => void = (
markers: ReadonlyArray<MarkerType>,
) => {
handleMarkersChange: (
markers: PerseusLabelImageWidgetOptions["markers"],
) => void = (markers: PerseusLabelImageWidgetOptions["markers"]) => {
this.props.onChange({markers});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import * as React from "react";

import QuestionMarkers from "../question-markers";

import type {MarkerType} from "@khanacademy/perseus-core";
import type {PerseusLabelImageWidgetOptions} from "@khanacademy/perseus-core";

type StoryArgs = Record<any, any>;

Expand Down Expand Up @@ -31,7 +31,7 @@ const Wrapper = (props) => (
class WithState extends React.Component<
Record<any, any>,
{
markers: ReadonlyArray<MarkerType>;
markers: PerseusLabelImageWidgetOptions["markers"];
}
> {
state = {
Expand Down
10 changes: 6 additions & 4 deletions packages/perseus-editor/src/widgets/label-image/marker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ import Option, {OptionGroup} from "../../components/dropdown-option";
import FormWrappedTextField from "../../components/form-wrapped-text-field";
import {gray17, gray85, gray98} from "../../styles/global-colors";

import type {MarkerType} from "@khanacademy/perseus-core";
import type {PerseusLabelImageWidgetOptions} from "@khanacademy/perseus-core";

type Props = MarkerType & {
type Props = PerseusLabelImageWidgetOptions["markers"][number] & {
// The list of possible answer choices.
choices: ReadonlyArray<string>;
choices: PerseusLabelImageWidgetOptions["choices"];
// Callback for when any of the marker props are changed.
onChange: (marker: MarkerType) => void;
onChange: (
marker: PerseusLabelImageWidgetOptions["markers"][number],
) => void;
// Callback to remove marker from the question image.
onRemove: () => void;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {gray17, gray68} from "../../styles/global-colors";

import Marker from "./marker";

import type {MarkerType} from "@khanacademy/perseus-core";
import type {PerseusLabelImageWidgetOptions} from "@khanacademy/perseus-core";

type Props = {
// The list of possible answers in a specific order.
Expand All @@ -21,9 +21,9 @@ type Props = {
imageWidth: number;
imageHeight: number;
// The list of markers placed on the question image.
markers: ReadonlyArray<MarkerType>;
markers: PerseusLabelImageWidgetOptions["markers"];
// Callback for when any of markers change.
onChange: (markers: ReadonlyArray<MarkerType>) => void;
onChange: (markers: PerseusLabelImageWidgetOptions["markers"]) => void;
};

export default class QuestionMarkers extends React.Component<Props> {
Expand Down
10 changes: 10 additions & 0 deletions packages/perseus-score/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ export type {Score} from "./util/answer-types";
export {default as ErrorCodes} from "./error-codes";
export type * from "./validation.types";
export {default as scoreCategorizer} from "./widgets/categorizer/score-categorizer";
export {default as validateCategorizer} from "./widgets/categorizer/validate-categorizer";
export {default as scoreCSProgram} from "./widgets/cs-program/score-cs-program";
export {default as scoreDropdown} from "./widgets/dropdown/score-dropdown";
export {default as validateDropdown} from "./widgets/dropdown/validate-dropdown";
export {default as scoreExpression} from "./widgets/expression/score-expression";
export {default as validateExpression} from "./widgets/expression/validate-expression";
export {default as scoreGrapher} from "./widgets/grapher/score-grapher";
export {default as scoreIframe} from "./widgets/iframe/score-iframe";
export {default as scoreInteractiveGraph} from "./widgets/interactive-graph/score-interactive-graph";
Expand All @@ -15,13 +18,20 @@ export {
} from "./widgets/label-image/score-label-image";
export {default as scoreMatcher} from "./widgets/matcher/score-matcher";
export {default as scoreMatrix} from "./widgets/matrix/score-matrix";
export {default as validateMatrix} from "./widgets/matrix/validate-matrix";
export {default as scoreNumberLine} from "./widgets/number-line/score-number-line";
export {default as validateNumberLine} from "./widgets/number-line/validate-number-line";
export {default as scoreNumericInput} from "./widgets/numeric-input/score-numeric-input";
export {default as scoreOrderer} from "./widgets/orderer/score-orderer";
export {default as validateOrderer} from "./widgets/orderer/validate-orderer";
export {default as scorePlotter} from "./widgets/plotter/score-plotter";
export {default as validatePlotter} from "./widgets/plotter/validate-plotter";
export {default as scoreRadio} from "./widgets/radio/score-radio";
export {default as validateRadio} from "./widgets/radio/validate-radio";
export {default as scoreSorter} from "./widgets/sorter/score-sorter";
export {default as validateSorter} from "./widgets/sorter/validate-sorter";
export {default as scoreTable} from "./widgets/table/score-table";
export {default as validateTable} from "./widgets/table/validate-table";
export {
default as scoreInputNumber,
inputNumberAnswerTypes,
Expand Down
Loading

0 comments on commit 36198bf

Please sign in to comment.