Skip to content

Commit 1bc1464

Browse files
feat(LEMS-3070): Add IDs to radio choices (#2762)
## Summary: Adds IDs as a required field for radio choices - Legacy content's IDs are formatted `radio-choice-{index}` - New content's IDs are UUIDs Issue: LEMS-3070 ## Test plan: ~ ZND Incoming ~ ## Screenshots https://github.com/user-attachments/assets/9c745f0a-9efc-4e7d-9180-6e33e2065d5a Author: anakaren-rojas Reviewers: anakaren-rojas, handeyeco, SonicScrewdriver, benchristel, ivyolamit, mark-fitzgerald, nishasy Required Reviewers: Approved By: handeyeco, SonicScrewdriver, benchristel Checks: ✅ 10 checks were successful Pull Request URL: #2762
1 parent f48e917 commit 1bc1464

Some content is hidden

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

44 files changed

+1316
-350
lines changed

.changeset/ten-adults-count.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"@khanacademy/perseus": minor
3+
"@khanacademy/perseus-core": minor
4+
"@khanacademy/perseus-editor": minor
5+
"@khanacademy/perseus-score": minor
6+
---
7+
8+
adds choice id as a required field to Radio Widget

config/test/crypto-polyfill.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
/**
2+
* Polyfill crypto.randomUUID for Jest test environment
3+
* This runs before all test files to ensure crypto is available during module imports
4+
*/
5+
Object.defineProperty(global, "crypto", {
6+
value: {
7+
randomUUID: () => "0-0-0-0-0",
8+
},
9+
writable: true,
10+
configurable: true,
11+
});

config/test/test.config.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ swcrc.jsc.experimental.plugins.push(["swc_mut_cjs_exports", {}]);
4949
/** @type {import('jest').Config} */
5050
module.exports = {
5151
rootDir: path.join(__dirname, "../../"),
52+
setupFiles: [],
5253
transform: {
5354
"^.+\\.(j|t)sx?$": ["@swc/jest", swcrc],
5455
// Compile .svg files using a custom transformer that returns the
@@ -73,6 +74,7 @@ module.exports = {
7374
setupFilesAfterEnv: [
7475
"<rootDir>/config/test/test-setup.ts",
7576
"<rootDir>/config/test/custom-matchers.ts",
77+
"<rootDir>/config/test/crypto-polyfill.js",
7678
],
7779
moduleNameMapper: {
7880
...pkgMap,

packages/perseus-core/src/accessibility.test.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,16 @@ describe("isItemAccessible", () => {
8686
type: "radio",
8787
options: {
8888
choices: [
89-
{content: "Option 1", correct: true},
90-
{content: "Option 2", correct: false},
89+
{
90+
id: "0-0-0-0-0",
91+
content: "Option 1",
92+
correct: true,
93+
},
94+
{
95+
id: "1-1-1-1-1",
96+
content: "Option 2",
97+
correct: false,
98+
},
9199
],
92100
},
93101
},

packages/perseus-core/src/data-schema.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,11 @@ export type PerseusRadioWidgetOptions = {
13781378
export type PerseusRadioChoice = {
13791379
// Translatable Markdown; The label for this choice
13801380
content: string;
1381+
/**
1382+
* An opaque string that uniquely identifies this choice within
1383+
* the radio widget. The format of this ID is subject to change.
1384+
*/
1385+
id: string;
13811386
// Translatable Markdown; Rationale to give the user when they get it wrong
13821387
rationale?: string;
13831388
// Whether this option is a correct answer or not

packages/perseus-core/src/parse-perseus-json/general-purpose-parsers/array.test.ts

Lines changed: 163 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import {assertFailure, success} from "../result";
22

3-
import {array} from "./array";
3+
import {array, arrayWithIndex} from "./array";
4+
import {defaulted} from "./defaulted";
5+
import {object} from "./object";
46
import {string} from "./string";
57
import {anyFailure, ctx, parseFailureWith} from "./test-helpers";
68

@@ -11,11 +13,11 @@ describe("array", () => {
1113
expect(arrayOfStrings([], ctx())).toEqual(success([]));
1214
});
1315

14-
it("accepts a array of one element", () => {
16+
it("accepts an array of one element", () => {
1517
expect(arrayOfStrings(["hi"], ctx())).toEqual(success(["hi"]));
1618
});
1719

18-
it("accepts a array of multiple elements", () => {
20+
it("accepts an array of multiple elements", () => {
1921
expect(arrayOfStrings(["a", "b", "c"], ctx())).toEqual(
2022
success(["a", "b", "c"]),
2123
);
@@ -25,11 +27,11 @@ describe("array", () => {
2527
expect(arrayOfStrings("blah", ctx())).toEqual(anyFailure);
2628
});
2729

28-
it("rejects a array with one element of the wrong type", () => {
30+
it("rejects an array with one element of the wrong type", () => {
2931
expect(arrayOfStrings([99], ctx())).toEqual(anyFailure);
3032
});
3133

32-
it("rejects a array with multiple elements if any has the wrong type", () => {
34+
it("rejects an array with multiple elements if any has the wrong type", () => {
3335
expect(arrayOfStrings(["ok", 99, "ok"], ctx())).toEqual(anyFailure);
3436
});
3537

@@ -107,3 +109,159 @@ describe("array", () => {
107109
);
108110
});
109111
});
112+
113+
describe("arrayWithIndex ", () => {
114+
// Test using object with defaulted field that uses index
115+
const arrayOfItemsWithIndexedIds = arrayWithIndex((index) =>
116+
object({
117+
name: string,
118+
id: defaulted(string, () => `item-${index}`),
119+
}),
120+
);
121+
122+
it("accepts an empty array", () => {
123+
expect(arrayOfItemsWithIndexedIds([], ctx())).toEqual(success([]));
124+
});
125+
126+
it("accepts an array of one element", () => {
127+
const input = [{name: "first"}];
128+
expect(arrayOfItemsWithIndexedIds(input, ctx())).toEqual(
129+
success([{name: "first", id: "item-0"}]),
130+
);
131+
});
132+
133+
it("accepts an array of multiple elements", () => {
134+
const input = [
135+
{name: "first"},
136+
{name: "second", id: "custom"},
137+
{name: "third"},
138+
];
139+
expect(arrayOfItemsWithIndexedIds(input, ctx())).toEqual(
140+
success([
141+
{name: "first", id: "item-0"},
142+
{name: "second", id: "custom"},
143+
{name: "third", id: "item-2"},
144+
]),
145+
);
146+
});
147+
148+
it("rejects a string", () => {
149+
expect(arrayOfItemsWithIndexedIds("blah", ctx())).toEqual(anyFailure);
150+
});
151+
152+
it("rejects an array with one element of the wrong type", () => {
153+
expect(arrayOfItemsWithIndexedIds([99], ctx())).toEqual(anyFailure);
154+
});
155+
156+
it("rejects an array with multiple elements if any has the wrong type", () => {
157+
expect(
158+
arrayOfItemsWithIndexedIds([{name: "ok"}, 99, {name: "ok"}], ctx()),
159+
).toEqual(anyFailure);
160+
});
161+
162+
it("indicates the bad element in the failure message", () => {
163+
const theArray = [
164+
{name: "ok"},
165+
{name: "ok"},
166+
99, // index 2
167+
];
168+
169+
const result = arrayOfItemsWithIndexedIds(theArray, ctx());
170+
171+
expect(result).toEqual(
172+
parseFailureWith({
173+
expected: ["object"],
174+
badValue: 99,
175+
path: [2],
176+
}),
177+
);
178+
});
179+
180+
it("lists all mismatches", () => {
181+
const theArray = [{name: "ok"}, 4, 99];
182+
183+
const result = arrayOfItemsWithIndexedIds(theArray, ctx());
184+
185+
assertFailure(result);
186+
187+
expect(result.detail).toEqual([
188+
{
189+
expected: ["object"],
190+
badValue: 4,
191+
path: [1],
192+
},
193+
{
194+
expected: ["object"],
195+
badValue: 99,
196+
path: [2],
197+
},
198+
]);
199+
});
200+
201+
it("uses the index to generate default IDs", () => {
202+
const input = [
203+
{name: "first"}, // Should get id: "item-0"
204+
{name: "second"}, // Should get id: "item-1"
205+
{name: "third"}, // Should get id: "item-2"
206+
];
207+
208+
const result = arrayOfItemsWithIndexedIds(input, ctx());
209+
210+
expect(result).toEqual(
211+
success([
212+
{name: "first", id: "item-0"},
213+
{name: "second", id: "item-1"},
214+
{name: "third", id: "item-2"},
215+
]),
216+
);
217+
});
218+
219+
it("pinpoints mismatches in nested arrays", () => {
220+
const arrayOfArrayOfItems = arrayWithIndex(() =>
221+
array(object({name: string})),
222+
);
223+
const theArray = [
224+
[{name: ""}],
225+
[{name: ""}],
226+
[],
227+
[{name: ""}, 99, {name: ""}],
228+
];
229+
230+
const result = arrayOfArrayOfItems(theArray, ctx());
231+
232+
expect(result).toEqual(parseFailureWith({path: [3, 1]}));
233+
});
234+
235+
it("lists multiple mismatches in nested arrays", () => {
236+
const arrayOfArrayOfItems = arrayWithIndex(() =>
237+
array(object({name: string})),
238+
);
239+
const theArray = [
240+
[{name: ""}, {name: ""}, 4],
241+
[9, {name: ""}],
242+
[],
243+
[{name: ""}, 99, {name: ""}],
244+
];
245+
246+
const result = arrayOfArrayOfItems(theArray, ctx());
247+
248+
assertFailure(result);
249+
250+
expect(result.detail.map((d) => d.path)).toEqual([
251+
[0, 2],
252+
[1, 0],
253+
[3, 1],
254+
]);
255+
});
256+
257+
it("describes the problem if given a non-array", () => {
258+
const result = arrayOfItemsWithIndexedIds(99, ctx());
259+
260+
expect(result).toEqual(
261+
parseFailureWith({
262+
expected: ["array"],
263+
badValue: 99,
264+
}),
265+
);
266+
});
267+
});

packages/perseus-core/src/parse-perseus-json/general-purpose-parsers/array.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,27 @@ export function array<T>(elementParser: Parser<T>): Parser<T[]> {
1414
};
1515
}
1616

17+
/**
18+
* Like the standard `array` parser, but passes the array index to a parser factory function.
19+
* This allows element parsers to generate index-based default values.
20+
*/
21+
export function arrayWithIndex<T>(
22+
elementParserFactory: (index: number) => Parser<T>,
23+
): Parser<T[]> {
24+
return (rawValue: unknown, ctx: ParseContext) => {
25+
if (!Array.isArray(rawValue)) {
26+
return ctx.failure("array", rawValue);
27+
}
28+
29+
const elementResults = rawValue.map((elem, index) => {
30+
const elementParser = elementParserFactory(index);
31+
return elementParser(elem, ctx.forSubtree(index));
32+
});
33+
34+
return Result.all(elementResults, concat);
35+
};
36+
}
37+
1738
function concat<T>(a: T[], b: T[]): T[] {
1839
return [...a, ...b];
1940
}

packages/perseus-core/src/parse-perseus-json/perseus-parsers/radio-widget.mockData.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,7 @@ export const v2Widget = {
119119
clue: "I am some rationale for a choice",
120120
isNoneOfTheAbove: false,
121121
},
122-
{
123-
content: "I am a choice with only content",
124-
},
122+
{content: "I am a choice with only content"},
125123
],
126124
numCorrect: 1,
127125
hasNoneOfTheAbove: false,
@@ -145,29 +143,32 @@ export const v3Widget = {
145143
options: {
146144
choices: [
147145
{
146+
id: "radio-choice-0",
148147
content: "I am a correct choice",
149148
correct: true,
150149
rationale: "I am some rationale for a choice",
151150
isNoneOfTheAbove: true,
152151
},
153152
{
153+
id: "radio-choice-1",
154154
content: "I am an incorrect choice",
155155
correct: false,
156156
rationale: "I am some rationale for a choice",
157+
widgets: undefined,
157158
},
158159
{
160+
id: "radio-choice-2",
159161
content: "I am an incorrect choice",
160162
correct: false,
161163
isNoneOfTheAbove: false,
162164
},
163165
{
166+
id: "radio-choice-3",
164167
content: "I am content for a choice",
165168
rationale: "I am some rationale for a choice",
166169
isNoneOfTheAbove: false,
167170
},
168-
{
169-
content: "I am a choice with only content",
170-
},
171+
{id: "radio-choice-4", content: "I am a choice with only content"},
171172
],
172173
numCorrect: 1,
173174
hasNoneOfTheAbove: false,

0 commit comments

Comments
 (0)