Skip to content

Commit a54f46b

Browse files
rtibblesbotclaude
andcommitted
refactor: read text-entry answers through QTIDeclaration.fromXML
_extractAnswers parsed <qti-correct-response> by hand while a separate helper built the case-sensitivity lookup via QTIDeclaration. Fold both into one parse: the declaration class now supplies the correct responses as well as the mapping. A declaration too malformed for QTIDeclaration to model (e.g. a missing identifier) now yields no answers rather than falling back to raw DOM reads, matching how an unsupported base-type is already handled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 78e7163 commit a54f46b

2 files changed

Lines changed: 31 additions & 58 deletions

File tree

  • contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/__tests__/parse.spec.js

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ describe('_defaultState', () => {
9595
});
9696

9797
describe('_extractAnswers', () => {
98+
afterEach(() => {
99+
jest.restoreAllMocks();
100+
});
101+
98102
it('returns [] when no declaration is provided', () => {
99103
expect(_extractAnswers([])).toEqual([]);
100104
});
@@ -128,11 +132,13 @@ describe('_extractAnswers', () => {
128132
expect(result[0].id).not.toBe(result[1].id);
129133
});
130134

131-
describe('mapping-derived case sensitivity', () => {
132-
afterEach(() => {
133-
jest.restoreAllMocks();
134-
});
135+
it('returns [] when the declaration is too malformed to model', () => {
136+
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
137+
expect(_extractAnswers([DECLARATION_WITHOUT_IDENTIFIER])).toEqual([]);
138+
expect(errorSpy).toHaveBeenCalled();
139+
});
135140

141+
describe('mapping-derived case sensitivity', () => {
136142
it('reads caseSensitive by map-key, silently ignoring unmatched entries', () => {
137143
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
138144
const result = _extractAnswers([TEXT_ENTRY_DECLARATION_WITH_MAPPING]);
@@ -154,17 +160,12 @@ describe('_extractAnswers', () => {
154160
});
155161

156162
it('matches a map entry for a blank answer value', () => {
157-
// Empty map-key coerces to null (QTI NULL) while the value stays ''; the entry is
158-
// case-sensitive="true" so a missed lookup can't slip through the false fallback.
163+
// Both the empty map-key and the empty <qti-value> coerce to null (QTI NULL) and
164+
// format back to ''; the entry is case-sensitive="true" so a missed lookup can't
165+
// slip through the false fallback.
159166
const result = _extractAnswers([BLANK_VALUE_DECLARATION]);
160167
expect(result).toEqual([expect.objectContaining({ value: '', caseSensitive: true })]);
161168
});
162-
163-
it('keeps the answers when the declaration is too malformed to model', () => {
164-
jest.spyOn(console, 'warn').mockImplementation(() => {});
165-
const result = _extractAnswers([DECLARATION_WITHOUT_IDENTIFIER]);
166-
expect(result).toEqual([expect.objectContaining({ value: 'Paris', caseSensitive: false })]);
167-
});
168169
});
169170
});
170171

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js

Lines changed: 18 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -70,34 +70,6 @@ function extractPromptHTML(bodyEl) {
7070
return clone.innerHTML.trim();
7171
}
7272

73-
/**
74-
* Build a value → caseSensitive lookup from a declaration's <qti-mapping>.
75-
*
76-
* @param {Element} declEl - The <qti-response-declaration> element
77-
* @returns {Map<string, boolean>}
78-
*/
79-
function caseSensitivityByValue(declEl) {
80-
let declaration;
81-
try {
82-
declaration = QTIDeclaration.fromXML(declEl);
83-
} catch (err) {
84-
// QTIDeclaration validates more strictly than answer extraction needs (a missing
85-
// identifier makes it unmodellable), so degrade rather than drop the answers.
86-
// eslint-disable-next-line no-console
87-
console.warn('[QTI Editor] Could not read <qti-mapping> case sensitivity:', err);
88-
return new Map();
89-
}
90-
91-
// Key on the XML string form: map-key is coerced on parse (empty → null under QTI
92-
// NULL semantics) while answer values stay raw <qti-value> text.
93-
return new Map(
94-
(declaration.mapping?.entries ?? []).map(entry => [
95-
declaration.formatValue(entry.mapKey),
96-
entry.caseSensitive,
97-
]),
98-
);
99-
}
100-
10173
/**
10274
* Extract correct answer values from the response declaration string.
10375
* Returns an array of `{ id, value, caseSensitive }` objects, or [] when no
@@ -115,40 +87,40 @@ export function _extractAnswers(responseDeclarations) {
11587
if (!declXml) return [];
11688

11789
try {
118-
const declEl = parseXML(declXml).documentElement;
119-
const isFloat = declEl.getAttribute('base-type') === BaseType.FLOAT;
120-
const isString = declEl.getAttribute('base-type') === BaseType.STRING;
90+
const declaration = QTIDeclaration.fromXML(parseXML(declXml).documentElement);
91+
const { baseType, correctResponse } = declaration;
12192

122-
if (!isFloat && !isString) {
93+
if (baseType !== BaseType.FLOAT && baseType !== BaseType.STRING) {
12394
// eslint-disable-next-line no-console
124-
console.error(
125-
`[QTI Editor] Unsupported text-entry base-type: ${declEl.getAttribute('base-type')}`,
126-
);
95+
console.error(`[QTI Editor] Unsupported text-entry base-type: ${baseType}`);
12796
return [];
12897
}
12998

130-
const correctResponseEl = declEl.querySelector('qti-correct-response');
131-
if (!correctResponseEl) {
132-
if (isFloat) {
99+
if (correctResponse === null) {
100+
if (baseType === BaseType.FLOAT) {
133101
// eslint-disable-next-line no-console
134102
console.error('[QTI Editor] Missing <qti-correct-response> for numeric interaction');
135103
}
136104
return [];
137105
}
138106

139-
const valueEls = [...correctResponseEl.querySelectorAll('qti-value')];
140-
if (valueEls.length === 0) return [];
141-
142-
const caseSensitivity = isString ? caseSensitivityByValue(declEl) : null;
107+
// Case sensitivity is a string-only concept, so numeric answers never read the mapping.
108+
const mapEntries = baseType === BaseType.STRING ? (declaration.mapping?.entries ?? []) : [];
109+
// Key on the XML string form: both map-key and correct-response values are coerced
110+
// on parse (empty → null under QTI NULL semantics), so formatting both back matches
111+
// them on equal terms.
112+
const caseSensitivity = new Map(
113+
mapEntries.map(entry => [declaration.formatValue(entry.mapKey), entry.caseSensitive]),
114+
);
143115

144-
return valueEls.map(el => {
145-
const value = el.textContent.trim();
116+
return correctResponse.map(value => {
117+
const formatted = declaration.formatValue(value);
146118
return {
147119
id: generateRandomSlug('answer'),
148-
value,
120+
value: formatted,
149121
// An answer with no matching qti-map-entry — including every answer in an
150122
// item authored before mappings were written — takes the XSD default, false.
151-
caseSensitive: caseSensitivity?.get(value) ?? false,
123+
caseSensitive: caseSensitivity.get(formatted) ?? false,
152124
};
153125
});
154126
} catch (err) {

0 commit comments

Comments
 (0)