Skip to content

Commit e269c7a

Browse files
authored
Score widgets with missing user input as 'invalid' (#2752)
This fixes a bug observed while testing the Perseus service with production traffic. Issue: https://khanacademy.atlassian.net/browse/LEMS-3341 ## Test plan: `pnpm test` Author: benchristel Reviewers: handeyeco, benchristel, ivyolamit, mark-fitzgerald, SonicScrewdriver, anakaren-rojas, nishasy Required Reviewers: Approved By: handeyeco Checks: ✅ 10 checks were successful Pull Request URL: #2752
1 parent 2b925d5 commit e269c7a

File tree

62 files changed

+640
-45
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

62 files changed

+640
-45
lines changed

.changeset/loud-schools-end.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@khanacademy/perseus-score": patch
3+
---
4+
5+
Fix a bug where `scorePerseusItem` would throw an exception if some scorable widgets in the question did not have a corresponding entry in the `UserInputMap`. Such inputs are now scored as "invalid," meaning the learner did not fully answer the question.

packages/perseus-core/src/validation.types.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,14 @@ import type {
4848
import type {Relationship} from "./types";
4949

5050
export type WidgetValidatorFunction = (
51-
userInput: UserInput,
51+
userInput: UserInput | undefined,
5252
validationData: ValidationData,
5353
locale: string,
5454
) => ValidationResult;
5555

5656
export type WidgetScorerFunction = (
5757
// The user data needed to score
58-
userInput: UserInput,
58+
userInput: UserInput | undefined,
5959
// The scoring criteria to score against
6060
rubric: Rubric,
6161
// Locale, for math evaluation

packages/perseus-score/src/score.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,22 @@ function generateDropdown(): DropdownWidget {
419419
}
420420

421421
describe("scorePerseusItem", () => {
422+
it("returns a score of 'invalid' if some widgets are missing from the user input", () => {
423+
// Arrange:
424+
const item = {
425+
content: "[[☃ dropdown 1]]",
426+
widgets: {"dropdown 1": generateDropdown()},
427+
images: {},
428+
};
429+
const userInputMap = {};
430+
431+
// Act:
432+
const score = scorePerseusItem(item, userInputMap, "en");
433+
434+
// Assert:
435+
expect(score).toEqual({type: "invalid", message: null});
436+
});
437+
422438
it("should return empty if any validator returns empty", () => {
423439
// Act
424440
const score = scorePerseusItem(

packages/perseus-score/src/score.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type {
1212
PerseusScore,
1313
PerseusWidgetsMap,
1414
UserInputMap,
15+
UserInput,
1516
} from "@khanacademy/perseus-core";
1617

1718
const noScore: PerseusScore = {
@@ -174,7 +175,11 @@ export function scoreWidgetsFunctional(
174175
return;
175176
}
176177

177-
const userInput = userInputMap[id];
178+
// TODO(benchristel): Without the explicit type annotation, the type of
179+
// userInput would be inferred as `any`. This is because the keys of
180+
// userInputMap are strings with a specific format, but `id` is any old
181+
// string. Find a way to make this more typesafe.
182+
const userInput: UserInput | undefined = userInputMap[id];
178183
const validator = getWidgetValidator(widget.type);
179184
const scorer = getWidgetScorer(widget.type);
180185

packages/perseus-score/src/widgets/categorizer/score-categorizer.test.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,19 @@ import scoreCategorizer from "./score-categorizer";
33
import type {PerseusCategorizerRubric} from "@khanacademy/perseus-core";
44

55
describe("scoreCategorizer", () => {
6+
it("returns a score of 'invalid' when the user input is undefined", () => {
7+
const rubric: PerseusCategorizerRubric = {
8+
values: [1, 3],
9+
items: ["apples", "oranges"],
10+
};
11+
12+
const userInput = undefined;
13+
14+
const score = scoreCategorizer(userInput, rubric);
15+
16+
expect(score).toHaveInvalidInput();
17+
});
18+
619
it("gives points when the answer is correct", () => {
720
const rubric: PerseusCategorizerRubric = {
821
values: [1, 3],

packages/perseus-score/src/widgets/categorizer/score-categorizer.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,14 @@ import type {
55
} from "@khanacademy/perseus-core";
66

77
function scoreCategorizer(
8-
userInput: PerseusCategorizerUserInput,
8+
// NOTE(benchristel): userInput can be undefined if the widget has never
9+
// been interacted with.
10+
userInput: PerseusCategorizerUserInput | undefined,
911
rubric: PerseusCategorizerRubric,
1012
): PerseusScore {
13+
if (userInput == null) {
14+
return {type: "invalid", message: null};
15+
}
1116
let allCorrect = true;
1217
rubric.values.forEach((value, i) => {
1318
if (userInput.values[i] !== value) {

packages/perseus-score/src/widgets/categorizer/validate-categorizer.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,18 @@ import validateCategorizer from "./validate-categorizer";
33
import type {PerseusCategorizerValidationData} from "@khanacademy/perseus-core";
44

55
describe("validateCategorizer", () => {
6+
it("returns a score of 'invalid' when the user input is undefined", () => {
7+
const rubric: PerseusCategorizerValidationData = {
8+
items: ["apples", "oranges"],
9+
};
10+
11+
const userInput = undefined;
12+
13+
const score = validateCategorizer(userInput, rubric);
14+
15+
expect(score).toHaveInvalidInput();
16+
});
17+
618
it("tells the learner its not complete if not selected", () => {
719
const validationData: PerseusCategorizerValidationData = {
820
items: ["apples", "oranges"],

packages/perseus-score/src/widgets/categorizer/validate-categorizer.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,15 @@ import type {
1515
* @param strings - Used to provide a validation message
1616
*/
1717
function validateCategorizer(
18-
userInput: PerseusCategorizerUserInput,
18+
// NOTE(benchristel): userInput can be undefined if the widget has never
19+
// been interacted with.
20+
userInput: PerseusCategorizerUserInput | undefined,
1921
validationData: PerseusCategorizerValidationData,
2022
): ValidationResult {
23+
if (userInput == null) {
24+
return {type: "invalid", message: null};
25+
}
26+
2127
const incomplete = validationData.items.some(
2228
(_, i) => userInput.values[i] == null,
2329
);

packages/perseus-score/src/widgets/cs-program/score-cs-program.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,18 @@ import scoreCSProgram from "./score-cs-program";
33
import type {PerseusCSProgramUserInput} from "@khanacademy/perseus-core";
44

55
describe("scoreCSProgram", () => {
6+
it("is 'invalid' when the state is undefined", () => {
7+
// Arrange
8+
const state = undefined;
9+
10+
// Act
11+
const result = scoreCSProgram(state);
12+
13+
// Assert
14+
15+
expect(result).toHaveInvalidInput();
16+
});
17+
618
it("is correct when the state from the iframe shows the status is correct", () => {
719
// Arrange
820
const state: PerseusCSProgramUserInput = {

packages/perseus-score/src/widgets/cs-program/score-cs-program.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,15 @@ import type {
33
PerseusScore,
44
} from "@khanacademy/perseus-core";
55

6-
function scoreCSProgram(userInput: PerseusCSProgramUserInput): PerseusScore {
6+
function scoreCSProgram(
7+
// NOTE(benchristel): userInput can be undefined if the widget has never
8+
// been interacted with.
9+
userInput: PerseusCSProgramUserInput | undefined,
10+
): PerseusScore {
11+
if (userInput == null) {
12+
return {type: "invalid", message: null};
13+
}
14+
715
// The CS program can tell us whether it's correct or incorrect,
816
// and pass an optional message
917
if (userInput.status === "correct") {

0 commit comments

Comments
 (0)