Skip to content

Commit

Permalink
Ensuring UserInput and Rubric widget keys match (#1884)
Browse files Browse the repository at this point in the history
## Summary:
While performing regression testing on the latest changes related to a conflict between our Server Side Scoring and Input Number Conversion projects, I found an edge case bug that needed to be handled. 

This bug was only reproducible on a test course that had an `input-number 2` and no `input-number 1`. As a result of the key mismatch between the `perseusRenderData` and `userInputMap`, the exercise became blocked / locked and unable to be continued after attempting to score. (If you would like a link to this exercise, please reach out to me!)

This PR updates the conversion logic in `renderer-utils.ts` to update both the `perseusRenderData` and `userInputMap` at the same time, so that the widget keys are identical between the two formats. This should cover any additional edge cases (such as the presence of other numeric-inputs that might adjust the IDs of the convertedWidgets). 

Some additional updates were required to create the helper functions that can perform this work in tandem. 

Issue: LEMS-2624

## Test plan:
- manual testing 
- Snapshot testing in Webapp (fixes issue)

Author: SonicScrewdriver

Reviewers: SonicScrewdriver, handeyeco

Required Reviewers:

Approved By: handeyeco

Checks: ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald, ✅ Publish npm snapshot (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), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ gerald, ✅ gerald, 🚫 Publish npm snapshot (ubuntu-latest, 20.x), 🚫 Check builds for changes in size (ubuntu-latest, 20.x), 🚫 Cypress (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), ✅ gerald, ✅ 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), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ⌛ undefined

Pull Request URL: #1884
  • Loading branch information
SonicScrewdriver authored Nov 19, 2024
1 parent 6c640c7 commit b4cf444
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 17 deletions.
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;
}

// 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);
};

0 comments on commit b4cf444

Please sign in to comment.