Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensuring UserInput and Rubric widget keys match #1884

Merged
merged 3 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/proud-horses-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Ensuring UserInput and Rubric widget keys match for edge cases
18 changes: 10 additions & 8 deletions packages/perseus/src/renderer-util.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import Util from "./util";
import {
conversionRequired,
convertDeprecatedWidgets,
convertUserInputData,
convertDeprecatedWidgetsForScoring,
} from "./util/deprecated-widgets/modernize-widgets-utils";
import {getWidgetIdsFromContent} from "./widget-type-utils";
import {getWidgetScorer} from "./widgets";
Expand Down Expand Up @@ -66,14 +65,17 @@ export function scorePerseusItem(
strings: PerseusStrings,
locale: string,
): PerseusScore {
let convertedRenderData = perseusRenderData;
let convertedUserInputMap = userInputMap;

// Check if the PerseusRenderer object contains any deprecated widgets that need to be converted
const mustConvertData = conversionRequired(perseusRenderData);
const convertedRenderData = mustConvertData
? convertDeprecatedWidgets(perseusRenderData) // Convert deprecated widgets to their modern equivalents
: perseusRenderData;
const convertedUserInputMap = mustConvertData
? convertUserInputData(userInputMap) // Convert deprecated user input data keys to their modern equivalents
: userInputMap;
if (mustConvertData) {
const {convertedRubric, convertedUserData} =
convertDeprecatedWidgetsForScoring(perseusRenderData, userInputMap);
convertedRenderData = convertedRubric;
convertedUserInputMap = convertedUserData;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do a ternary destruction here, but it looked really messy, imo. I preferred the simple if statement for simple legibility - but I'd be happy hear any alternative opinions/suggestions!

Suggested change
const {convertedRubric, convertedUserData} = conversionRequired(perseusRenderData)
? convertDeprecatedWidgetsForScoring(perseusRenderData, userInputMap)
: {convertedRubric: perseusRenderData, convertedUserData: userInputMap};
// There seems to be a chance that PerseusRenderer.widgets might include
// widget data for widgets that are not in PerseusRenderer.content,
// so this checks that the widgets are being used before scoring them
const usedWidgetIds = getWidgetIdsFromContent(convertedRubric.content);
const scores = scoreWidgetsFunctional(
convertedRubric.widgets,
usedWidgetIds,
convertedUserData,
strings,
locale,
);

// There seems to be a chance that PerseusRenderer.widgets might include
// widget data for widgets that are not in PerseusRenderer.content,
Expand Down
21 changes: 19 additions & 2 deletions packages/perseus/src/util/deprecated-widgets/input-number.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Methods to convert input-number widgets to numeric-input widgets

import type {
NumericInputWidget,
PerseusRenderer,
Expand Down Expand Up @@ -162,16 +161,34 @@ export const getInputNumberRenameMap = (
// Convert the user input data keys from input-number to numeric-input
export const convertUserInputNumberData = (
userInputMap: UserInputMap,
renameMap: WidgetRenameMap,
): UserInputMap => {
const updatedUserInputMap = {...userInputMap};

for (const key of Object.keys(userInputMap)) {
if (key.includes("input-number")) {
const updatedKey = key.replace("input-number", "numeric-input");
const updatedKey = renameMap[key];
updatedUserInputMap[updatedKey] = userInputMap[key];
delete updatedUserInputMap[key];
}
}

return updatedUserInputMap;
};

// Used to convert InputNumber widgets to NumericInput widgets for scoring
export const convertInputNumberForScoring = (
rubric: PerseusRenderer,
userInputMap: UserInputMap,
): {convertedRubric: PerseusRenderer; convertedUserData: UserInputMap} => {
// First we need to create a map of the old input-number keys to the new numeric-input keys
// so that we can ensure we update the content, widgets, AND userInput accordingly
const renameMap = getInputNumberRenameMap(rubric);
const convertedRubric = convertInputNumberJson(rubric, renameMap);
const convertedUserData = convertUserInputNumberData(
userInputMap,
renameMap,
);

return {convertedRubric, convertedUserData};
};
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {
inputNumberToNumericInput,
convertUserInputNumberData,
convertInputNumberForScoring,
} from "./input-number";

import type {PerseusRenderer, UserInputMap} from "@khanacademy/perseus";
Expand All @@ -12,6 +12,7 @@ const widgetRegExes = [/input-number \d+/]; // We can add more regexes here in t
// content creators use the Editor Page to update content containing these widgets.
// Currently, we're only converting input-number to numeric-input,
// but we can add more conversions here in the future.

// Modernize the json content of a PerseusRenderer object
// by converting deprecated widgets to their modern equivalents
export const convertDeprecatedWidgets = (
Expand Down Expand Up @@ -45,11 +46,14 @@ export const conversionRequired = (json: PerseusRenderer): boolean => {
return false;
};

// Convert the user input data keys for deprecated widgets to their modern equivalents
export const convertUserInputData = (
/**
* Convert the deprecated widgets in the rubric and user data to their
* modern equivalents for scoring. These need to be updated together
* in order to ensure that the widgetKeys match between the rubric and user data.
*/
export const convertDeprecatedWidgetsForScoring = (
rubric: PerseusRenderer,
userInputMap: UserInputMap,
): UserInputMap => {
// Currently we're only converting input-number to numeric-input,
// But we can add more conversions here in the future
return convertUserInputNumberData(userInputMap);
): {convertedRubric: PerseusRenderer; convertedUserData: UserInputMap} => {
return convertInputNumberForScoring(rubric, userInputMap);
};
Loading