Skip to content

Commit

Permalink
Number Line: Extract validation out of scoring (#1901)
Browse files Browse the repository at this point in the history
## Summary:
To complete server-side scoring, we are separating out validation logic from scoring logic. This PR separates that logic and associated tests for the number line widget.

Issue: LEMS-2608

## Test plan:
- Confirm checks pass
- Confirm widget still works as expected
- Confirm changing the flow of validation and scoring does not affect user experience

Author: Myranae

Reviewers: Myranae, handeyeco, jeremywiebe

Required Reviewers:

Approved By: handeyeco

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

Pull Request URL: #1901
  • Loading branch information
Myranae authored Dec 4, 2024
1 parent d052726 commit 051c50c
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 96 deletions.
5 changes: 5 additions & 0 deletions .changeset/sweet-ligers-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": minor
---

Introduces a validation function for the number line widget (extracted from the scoring function).
9 changes: 6 additions & 3 deletions packages/perseus/src/validation.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import type {
PerseusGroupWidgetOptions,
PerseusMatcherWidgetOptions,
PerseusMatrixWidgetAnswers,
PerseusNumberLineWidgetOptions,
PerseusNumericInputAnswer,
PerseusOrdererWidgetOptions,
PerseusRadioChoice,
Expand Down Expand Up @@ -164,7 +163,11 @@ export type PerseusMatrixUserInput = {
answers: PerseusMatrixRubric["answers"];
};

export type PerseusNumberLineRubric = PerseusNumberLineWidgetOptions & {
export type PerseusNumberLineScoringData = {
correctRel: string | null | undefined;
correctX: number;
range: ReadonlyArray<number>;
initialX: number | null | undefined;
isInequality: boolean;
};

Expand Down Expand Up @@ -246,7 +249,7 @@ export type Rubric =
| PerseusLabelImageRubric
| PerseusMatcherRubric
| PerseusMatrixRubric
| PerseusNumberLineRubric
| PerseusNumberLineScoringData
| PerseusNumericInputRubric
| PerseusOrdererRubric
| PerseusPlotterScoringData
Expand Down
116 changes: 45 additions & 71 deletions packages/perseus/src/widgets/number-line/score-number-line.test.ts
Original file line number Diff line number Diff line change
@@ -1,109 +1,83 @@
import scoreNumberLine from "./score-number-line";

import type {
PerseusNumberLineRubric,
PerseusNumberLineScoringData,
PerseusNumberLineUserInput,
} from "../../validation.types";

const baseInput: PerseusNumberLineUserInput = {
isTickCrtl: true,
numLinePosition: 1,
rel: "eq",
numDivisions: 10,
divisionRange: [-10, 10],
};

const baseRubric: PerseusNumberLineRubric = {
correctRel: "eq",
correctX: -1.5,
divisionRange: [1, 12],
initialX: -1,
labelRange: [null, null],
labelStyle: "decimal",
labelTicks: true,
numDivisions: null,
range: [-1.5, 1.5],
snapDivisions: 2,
static: false,
tickStep: 0.5,
isInequality: false,
};

function generateInput(
extend?: Partial<PerseusNumberLineUserInput>,
): PerseusNumberLineUserInput {
return {...baseInput, ...extend};
}

function generateRubric(
extend?: Partial<PerseusNumberLineRubric>,
): PerseusNumberLineRubric {
return {...baseRubric, ...extend};
}

describe("scoreNumberLine", () => {
it("is invalid when outside allowed range", () => {
// Arrange
const userInput = generateInput({
divisionRange: [-1, 1],
numLinePosition: 10,
});

const rubric = generateRubric();

// Act
const result = scoreNumberLine(userInput, rubric);

// Assert
expect(result).toHaveInvalidInput(
"Number of divisions is outside the allowed range.",
);
});

it("is invalid when end state is the same as beginning state", () => {
// Arrange
const userInput = generateInput({
const userInput: PerseusNumberLineUserInput = {
isTickCrtl: true,
rel: "eq",
numDivisions: 10,
divisionRange: [-10, 10],
numLinePosition: 0,
});
};

const rubric = generateRubric({
const scoringData: PerseusNumberLineScoringData = {
correctRel: "eq",
correctX: -1.5,
initialX: 0,
});
range: [-1.5, 1.5],
isInequality: false,
};

// Act
const result = scoreNumberLine(userInput, rubric);
const score = scoreNumberLine(userInput, scoringData);

// Assert
expect(result).toHaveInvalidInput();
expect(score).toHaveInvalidInput();
});

it("can be answered correctly", () => {
// Arrange
const userInput = generateInput({
const userInput: PerseusNumberLineUserInput = {
isTickCrtl: true,
rel: "eq",
numDivisions: 10,
divisionRange: [-10, 10],
numLinePosition: -1.5,
});
};

const rubric = generateRubric();
const scoringData: PerseusNumberLineScoringData = {
correctRel: "eq",
correctX: -1.5,
initialX: -1,
range: [-1.5, 1.5],
isInequality: false,
};

// Act
const result = scoreNumberLine(userInput, rubric);
const score = scoreNumberLine(userInput, scoringData);

// Assert
expect(result).toHaveBeenAnsweredCorrectly();
expect(score).toHaveBeenAnsweredCorrectly();
});

it("can be answered incorrectly", () => {
// Arrange
const userInput = generateInput({
const userInput: PerseusNumberLineUserInput = {
isTickCrtl: true,
rel: "eq",
numDivisions: 10,
divisionRange: [-10, 10],
numLinePosition: 1.5,
});
};

const rubric = generateRubric();
const scoringData: PerseusNumberLineScoringData = {
correctRel: "eq",
correctX: -1.5,
initialX: -1,
range: [-1.5, 1.5],
isInequality: false,
};

// Act
const result = scoreNumberLine(userInput, rubric);
const score = scoreNumberLine(userInput, scoringData);

// Assert
expect(result).toHaveBeenAnsweredIncorrectly();
expect(score).toHaveBeenAnsweredIncorrectly();
});
});
41 changes: 19 additions & 22 deletions packages/perseus/src/widgets/number-line/score-number-line.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,41 @@
import {number as knumber} from "@khanacademy/kmath";

import validateNumberLine from "./validate-number-line";

import type {PerseusScore} from "../../types";
import type {
PerseusNumberLineRubric,
PerseusNumberLineScoringData,
PerseusNumberLineUserInput,
} from "../../validation.types";

function scoreNumberLine(
state: PerseusNumberLineUserInput,
rubric: PerseusNumberLineRubric,
userInput: PerseusNumberLineUserInput,
scoringData: PerseusNumberLineScoringData,
): PerseusScore {
const range = rubric.range;
const divisionRange = state.divisionRange;
const start = rubric.initialX != null ? rubric.initialX : range[0];
const startRel = rubric.isInequality ? "ge" : "eq";
const correctRel = rubric.correctRel || "eq";
const validationError = validateNumberLine(userInput);
if (validationError) {
return validationError;
}

const range = scoringData.range;
const start =
scoringData.initialX != null ? scoringData.initialX : range[0];
const startRel = scoringData.isInequality ? "ge" : "eq";
const correctRel = scoringData.correctRel || "eq";
const correctPos = knumber.equal(
state.numLinePosition,
rubric.correctX || 0,
userInput.numLinePosition,
scoringData.correctX || 0,
);
const outsideAllowedRange =
state.numDivisions > divisionRange[1] ||
state.numDivisions < divisionRange[0];

// TODO: I don't think isTickCrtl is a thing anymore
if (state.isTickCrtl && outsideAllowedRange) {
return {
type: "invalid",
message: "Number of divisions is outside the allowed range.",
};
}
if (correctPos && correctRel === state.rel) {
if (correctPos && correctRel === userInput.rel) {
return {
type: "points",
earned: 1,
total: 1,
message: null,
};
}
if (state.numLinePosition === start && state.rel === startRel) {
if (userInput.numLinePosition === start && userInput.rel === startRel) {
// We're where we started.
return {
type: "invalid",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import validateNumberLine from "./validate-number-line";

import type {PerseusNumberLineUserInput} from "../../validation.types";

describe("validateNumberLine", () => {
it("is invalid when outside allowed range", () => {
// Arrange
const userInput: PerseusNumberLineUserInput = {
isTickCrtl: true,
rel: "eq",
numDivisions: 10,
divisionRange: [-1, 1],
numLinePosition: 10,
};

// Act
const validationError = validateNumberLine(userInput);

// Assert
expect(validationError).toHaveInvalidInput(
"Number of divisions is outside the allowed range.",
);
});

it("returns null when validation passes", () => {
// Arrange
const userInput: PerseusNumberLineUserInput = {
isTickCrtl: true,
rel: "eq",
numDivisions: 10,
divisionRange: [-10, 10],
numLinePosition: -1.5,
};

// Act
const validationError = validateNumberLine(userInput);

// Assert
expect(validationError).toBeNull();
});
});
28 changes: 28 additions & 0 deletions packages/perseus/src/widgets/number-line/validate-number-line.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import type {PerseusScore} from "../../types";
import type {PerseusNumberLineUserInput} from "../../validation.types";

/**
* Checks user input is within the allowed range and not the same as the initial
* state.
* @param userInput
* @see 'scoreNumberLine' for the scoring logic.
*/
function validateNumberLine(
userInput: PerseusNumberLineUserInput,
): Extract<PerseusScore, {type: "invalid"}> | null {
const divisionRange = userInput.divisionRange;
const outsideAllowedRange =
userInput.numDivisions > divisionRange[1] ||
userInput.numDivisions < divisionRange[0];

// TODO: I don't think isTickCrtl is a thing anymore
if (userInput.isTickCrtl && outsideAllowedRange) {
return {
type: "invalid",
message: "Number of divisions is outside the allowed range.",
};
}
return null;
}

export default validateNumberLine;

0 comments on commit 051c50c

Please sign in to comment.