Skip to content

Commit

Permalink
Move getOneCorrect... to WidgetExport (#1742)
Browse files Browse the repository at this point in the history
## Summary:
Rather than calling a static method on a React component, move `getOneCorrectAnswerFromRubric` to the `WidgetExport` object.

Issue: Prework for [LEMS-2387](https://khanacademy.atlassian.net/browse/LEMS-2387)

## Test plan:
Just moving some code. There were already tests and they should keep passing.

[LEMS-2387]: https://khanacademy.atlassian.net/browse/LEMS-2387?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

Author: handeyeco

Reviewers: mark-fitzgerald

Required Reviewers:

Approved By: mark-fitzgerald

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), ✅ 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), ✅ gerald

Pull Request URL: #1742
  • Loading branch information
handeyeco authored Oct 15, 2024
1 parent 5e30fcd commit f383d43
Show file tree
Hide file tree
Showing 11 changed files with 108 additions and 106 deletions.
5 changes: 5 additions & 0 deletions .changeset/mighty-students-pay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Move getOneCorrectAnswerFromRubric from React components to WidgetExports
8 changes: 0 additions & 8 deletions packages/perseus/src/__tests__/widgets.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,6 @@ describe("Widget API support", () => {
expect(Widget).toHaveProperty("getUserInputFromProps");
},
);

it.each(["expression", "input-number", "numeric-input"])(
"%s widget should provide static getOneCorrectAnswerFromRubric function",
(widgetType) => {
const Widget = Widgets.getWidget(widgetType);
expect(Widget).toHaveProperty("getOneCorrectAnswerFromRubric");
},
);
});

describe("replaceWidget", () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,9 @@ export type WidgetExports<
staticTransform?: WidgetTransform; // this is a function of some sort,

validator?: WidgetValidatorFunction;
getOneCorrectAnswerFromRubric?: (
rubric: Rubric,
) => string | null | undefined;

/**
A map of major version numbers (as a string, eg "1") to a function that
Expand Down
5 changes: 2 additions & 3 deletions packages/perseus/src/util/extract-perseus-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -678,14 +678,13 @@ export const getAnswerFromUserInput = (widgetType: string, userInput: any) => {
export const getCorrectAnswerForWidgetId = (
widgetId: string,
itemData: PerseusItem,
): string | undefined => {
): string | null | undefined => {
const rubric = itemData.question.widgets[widgetId].options;
const widgetMap = getWidgetsMapFromItemData(itemData);
const widgetType = getWidgetTypeByWidgetId(widgetId, widgetMap) as string;

const widget = Widgets.getWidget(widgetType);
const widget = Widgets.getWidgetExport(widgetType);

// @ts-expect-error - TS2339 - Property 'getOneCorrectAnswerFromRubric' does not exist on type 'ComponentType<any>'.
return widget?.getOneCorrectAnswerFromRubric?.(rubric);
};

Expand Down
4 changes: 4 additions & 0 deletions packages/perseus/src/widgets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ export const getWidget = (
return widgets[name].widget;
};

export const getWidgetExport = (name: string): WidgetExports | null => {
return widgets[name] ?? null;
};

export const getWidgetValidator = (
name: string,
): WidgetValidatorFunction | null => {
Expand Down
11 changes: 7 additions & 4 deletions packages/perseus/src/widgets/expression/expression.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import * as Dependencies from "../../dependencies";
import {renderQuestion} from "../__testutils__/renderQuestion";

import {Expression} from "./expression";
import ExpressionWidgetExport from "./expression";
import {
expressionItem2,
expressionItem3,
Expand Down Expand Up @@ -264,7 +264,8 @@ describe("Expression Widget", function () {
} as const;

// Act
const result = Expression.getOneCorrectAnswerFromRubric(rubric);
const result =
ExpressionWidgetExport.getOneCorrectAnswerFromRubric?.(rubric);

// Assert
expect(result).toBeUndefined();
Expand All @@ -287,7 +288,8 @@ describe("Expression Widget", function () {
} as const;

// Act
const result = Expression.getOneCorrectAnswerFromRubric(rubric);
const result =
ExpressionWidgetExport.getOneCorrectAnswerFromRubric?.(rubric);

// Assert
expect(result).toEqual("123");
Expand Down Expand Up @@ -316,7 +318,8 @@ describe("Expression Widget", function () {
} as const;

// Act
const result = Expression.getOneCorrectAnswerFromRubric(rubric);
const result =
ExpressionWidgetExport.getOneCorrectAnswerFromRubric?.(rubric);

// Assert
expect(result).toEqual("123");
Expand Down
30 changes: 12 additions & 18 deletions packages/perseus/src/widgets/expression/expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,19 +115,6 @@ export class Expression
return normalizeTex(props.value);
}

static getOneCorrectAnswerFromRubric(
rubric: PerseusExpressionRubric,
): string | null | undefined {
const correctAnswers = (rubric.answerForms || []).filter(
(answerForm) => answerForm.considered === "correct",
);
if (correctAnswers.length === 0) {
return;
}
return correctAnswers[0].value;
}
//#endregion

static defaultProps: DefaultProps = {
value: "",
times: false,
Expand Down Expand Up @@ -532,13 +519,8 @@ const ExpressionWithDependencies = React.forwardRef<
// methods and instead adjust Perseus to provide these facilities through
// instance methods on our Renderers.
// @ts-expect-error - TS2339 - Property 'validate' does not exist on type
ExpressionWithDependencies.validate = expressionValidator;
// @ts-expect-error - TS2339 - Property 'validate' does not exist on type
ExpressionWithDependencies.getUserInputFromProps =
Expression.getUserInputFromProps;
// @ts-expect-error - TS2339 - Property 'validate' does not exist on type
ExpressionWithDependencies.getOneCorrectAnswerFromRubric =
Expression.getOneCorrectAnswerFromRubric;

export default {
name: "expression",
Expand Down Expand Up @@ -571,4 +553,16 @@ export default {
// For use by the editor
isLintable: true,
validator: expressionValidator,

getOneCorrectAnswerFromRubric(
rubric: PerseusExpressionRubric,
): string | null | undefined {
const correctAnswers = (rubric.answerForms || []).filter(
(answerForm) => answerForm.considered === "correct",
);
if (correctAnswers.length === 0) {
return;
}
return correctAnswers[0].value;
},
} as WidgetExports<typeof ExpressionWithDependencies>;
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ describe("getOneCorrectAnswerFromRubric", () => {
const rubric: Record<string, any> = {};

// Act
const result = InputNumber.widget.getOneCorrectAnswerFromRubric(rubric);
const result = InputNumber.getOneCorrectAnswerFromRubric?.(rubric);

// Assert
expect(result).toBeUndefined();
Expand All @@ -291,7 +291,7 @@ describe("getOneCorrectAnswerFromRubric", () => {
} as const;

// Act
const result = InputNumber.widget.getOneCorrectAnswerFromRubric(rubric);
const result = InputNumber.getOneCorrectAnswerFromRubric?.(rubric);

// Assert
expect(result).toEqual("0");
Expand All @@ -306,7 +306,7 @@ describe("getOneCorrectAnswerFromRubric", () => {
} as const;

// Act
const result = InputNumber.widget.getOneCorrectAnswerFromRubric(rubric);
const result = InputNumber.getOneCorrectAnswerFromRubric?.(rubric);

// Assert
expect(result).toEqual("0 ± 0.1");
Expand Down
24 changes: 11 additions & 13 deletions packages/perseus/src/widgets/input-number/input-number.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,19 +100,6 @@ class InputNumber extends React.Component<Props> implements Widget {
};
}

static getOneCorrectAnswerFromRubric(
rubric: any,
): string | null | undefined {
if (rubric.value == null) {
return;
}
let answerString = String(rubric.value);
if (rubric.inexact && rubric.maxError) {
answerString += " \u00B1 " + rubric.maxError;
}
return answerString;
}

shouldShowExamples: () => boolean = () => {
return this.props.answerType !== "number";
};
Expand Down Expand Up @@ -297,4 +284,15 @@ export default {
transform: propTransform,
isLintable: true,
validator: inputNumberValidator,

getOneCorrectAnswerFromRubric(rubric: any): string | null | undefined {
if (rubric.value == null) {
return;
}
let answerString = String(rubric.value);
if (rubric.inexact && rubric.maxError) {
answerString += " \u00B1 " + rubric.maxError;
}
return answerString;
},
} as WidgetExports<typeof InputNumber>;
50 changes: 27 additions & 23 deletions packages/perseus/src/widgets/numeric-input/numeric-input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {testDependencies} from "../../../../../testing/test-dependencies";
import * as Dependencies from "../../dependencies";
import {renderQuestion} from "../__testutils__/renderQuestion";

import {NumericInput, unionAnswerForms} from "./numeric-input";
import NumericInputWidgetExport, {unionAnswerForms} from "./numeric-input";
import {
question1AndAnswer,
multipleAnswers,
Expand Down Expand Up @@ -90,13 +90,14 @@ describe("static function getOneCorrectAnswerFromRubric", () => {
const answers: ReadonlyArray<any> =
(widgetOptions && widgetOptions.answers) || [];

const singleAnswer = NumericInput.getOneCorrectAnswerFromRubric({
answers,
labelText: "",
size: "medium",
static: false,
coefficient: false,
});
const singleAnswer =
NumericInputWidgetExport.getOneCorrectAnswerFromRubric?.({
answers,
labelText: "",
size: "medium",
static: false,
coefficient: false,
});
expect(singleAnswer).toBe("12.2");
});

Expand All @@ -105,25 +106,27 @@ describe("static function getOneCorrectAnswerFromRubric", () => {
const widgetOptions = widget && widget.options;
const answers: ReadonlyArray<any> =
(widgetOptions && widgetOptions.answers) || [];
const singleAnswer = NumericInput.getOneCorrectAnswerFromRubric({
answers,
labelText: "",
size: "medium",
static: false,
coefficient: false,
});
const singleAnswer =
NumericInputWidgetExport.getOneCorrectAnswerFromRubric?.({
answers,
labelText: "",
size: "medium",
static: false,
coefficient: false,
});
expect(singleAnswer).toBe("1252");
});

it("can not get a correct answer from a rubric with no answer", () => {
const answers: Array<never> = [];
const singleAnswer = NumericInput.getOneCorrectAnswerFromRubric({
answers,
labelText: "",
size: "medium",
static: false,
coefficient: false,
});
const singleAnswer =
NumericInputWidgetExport.getOneCorrectAnswerFromRubric?.({
answers,
labelText: "",
size: "medium",
static: false,
coefficient: false,
});
expect(singleAnswer).toBeUndefined();
});

Expand All @@ -145,7 +148,8 @@ describe("static function getOneCorrectAnswerFromRubric", () => {
static: false,
coefficient: true,
};
const singleAnswer = NumericInput.getOneCorrectAnswerFromRubric(rubric);
const singleAnswer =
NumericInputWidgetExport.getOneCorrectAnswerFromRubric?.(rubric);
expect(singleAnswer).toBe("1 ± 0.2");
});
});
Expand Down
68 changes: 34 additions & 34 deletions packages/perseus/src/widgets/numeric-input/numeric-input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -101,40 +101,6 @@ export class NumericInput
};
}

static getOneCorrectAnswerFromRubric(
rubric: PerseusNumericInputRubric,
): string | null | undefined {
const correctAnswers = rubric.answers.filter(
(answer) => answer.status === "correct",
);
const answerStrings = correctAnswers.map((answer) => {
// Figure out how this answer is supposed to be
// displayed
let format = "decimal";
if (answer.answerForms && answer.answerForms[0]) {
// NOTE(johnsullivan): This isn't exactly ideal, but
// it does behave well for all the currently known
// problems. See D14742 for some discussion on
// alternate strategies.
format = answer.answerForms[0];
}

// @ts-expect-error - TS2345 - Argument of type 'string' is not assignable to parameter of type 'MathFormat | undefined'.
let answerString = KhanMath.toNumericString(answer.value, format);
if (answer.maxError) {
answerString +=
" \u00B1 " +
// @ts-expect-error - TS2345 - Argument of type 'string' is not assignable to parameter of type 'MathFormat | undefined'.
KhanMath.toNumericString(answer.maxError, format);
}
return answerString;
});
if (answerStrings.length === 0) {
return;
}
return answerStrings[0];
}

state: State = {
// keeps track of the other set of values when switching
// between 0 and finite solutions
Expand Down Expand Up @@ -384,4 +350,38 @@ export default {
transform: propsTransform,
isLintable: true,
validator: numericInputValidator,

getOneCorrectAnswerFromRubric(
rubric: PerseusNumericInputRubric,
): string | null | undefined {
const correctAnswers = rubric.answers.filter(
(answer) => answer.status === "correct",
);
const answerStrings = correctAnswers.map((answer) => {
// Figure out how this answer is supposed to be
// displayed
let format = "decimal";
if (answer.answerForms && answer.answerForms[0]) {
// NOTE(johnsullivan): This isn't exactly ideal, but
// it does behave well for all the currently known
// problems. See D14742 for some discussion on
// alternate strategies.
format = answer.answerForms[0];
}

// @ts-expect-error - TS2345 - Argument of type 'string' is not assignable to parameter of type 'MathFormat | undefined'.
let answerString = KhanMath.toNumericString(answer.value, format);
if (answer.maxError) {
answerString +=
" \u00B1 " +
// @ts-expect-error - TS2345 - Argument of type 'string' is not assignable to parameter of type 'MathFormat | undefined'.
KhanMath.toNumericString(answer.maxError, format);
}
return answerString;
});
if (answerStrings.length === 0) {
return;
}
return answerStrings[0];
},
} as WidgetExports<typeof NumericInput>;

0 comments on commit f383d43

Please sign in to comment.