From 64cd38e6ee950b337f477a01ad22ddb7a6cce6ee Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 20 Oct 2026 13:27:08 +0300 Subject: [PATCH 01/10] Decode entities in tagless text and let wrapInlineRuns take a tag stripTags skipped entity decoding when the text had no tags, so hasRichTextContent counted a bare   as content. Inline rich text, such as a gap match answer, has no wrapper tag to avoid this. The XSD does not allow some inline content, such as qti-gap, directly inside a div, so wrapInlineRuns now takes the wrapper tag. It still defaults to div. --- .../serialization/__tests__/xml.spec.js | 21 ++++++++++++++++++- .../serialization/qti/QTISanitizer.js | 4 ++-- .../qti/__tests__/QTISanitizer.spec.js | 4 ++++ .../views/QTIEditor/serialization/xml.js | 6 ++++-- .../utils/__tests__/richText.spec.js | 1 + 5 files changed, 31 insertions(+), 5 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/xml.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/xml.spec.js index 4f97513d8f..8624bdb404 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/xml.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/xml.spec.js @@ -1,7 +1,7 @@ // Disabled because jest-dom matchers (toHaveAttribute/toHaveTextContent) are designed for // HTML and do not work reliably on strict XML elements generated by serialization. /* eslint-disable jest-dom/prefer-to-have-attribute, jest-dom/prefer-to-have-text-content */ -import { buildXmlNode, parseXML, getPromptHTML, serializeAsHtml } from '../xml'; +import { buildXmlNode, parseXML, getPromptHTML, serializeAsHtml, wrapInlineRuns } from '../xml'; import { VALID_CHOICE_ITEM_DOCUMENT } from '../../utils/testingFixtures'; const serializer = new XMLSerializer(); @@ -363,3 +363,22 @@ describe('serializeAsHtml', () => { expect(serializeAsHtml(children('

a

'))).toBe('

a

'); }); }); + +describe('wrapInlineRuns', () => { + const wrap = (innerHTML, ...args) => + wrapInlineRuns(buildXmlNode({ tag: 'div', innerHTML }), ...args).map(el => + serializer.serializeToString(el), + ); + + it('wraps each inline run in a div and keeps blocks as they are', () => { + expect(wrap('a b

c

d')).toEqual([ + '
a b
', + '

c

', + '
d
', + ]); + }); + + it('wraps each inline run in the given tag', () => { + expect(wrap('a

b

c', 'p')).toEqual(['

a

', '

b

', '

c

']); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/QTISanitizer.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/QTISanitizer.js index 97736127db..5f66ff7c9e 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/QTISanitizer.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/QTISanitizer.js @@ -91,8 +91,8 @@ export class QTISanitizer { */ static stripTags(value) { if (typeof value !== 'string') return String(value ?? ''); - // Fast path: if no `<` is present there is nothing to strip. - if (!value.includes('<')) return value; + // Fast path: with no `<` or `&` there is nothing to strip or decode. + if (!value.includes('<') && !value.includes('&')) return value; const doc = parseXML(value, 'text/html'); // Remove script and style elements entirely — we do NOT want their text content. doc.querySelectorAll('script, style').forEach(el => el.remove()); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/__tests__/QTISanitizer.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/__tests__/QTISanitizer.spec.js index 687c704e97..ccb797803a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/__tests__/QTISanitizer.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/qti/__tests__/QTISanitizer.spec.js @@ -114,6 +114,10 @@ describe('QTISanitizer.stripTags — XSS defence', () => { expect(QTISanitizer.stripTags('A & B')).toBe('A & B'); }); + it('decodes entities in a fragment without tags', () => { + expect(QTISanitizer.stripTags(' A & B')).toBe(' A & B'); + }); + it('handles non-string input by coercing to string', () => { expect(QTISanitizer.stripTags(42)).toBe('42'); expect(QTISanitizer.stripTags(null)).toBe(''); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js index 4434ba4edf..16a79a6b59 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js @@ -286,9 +286,11 @@ const BLOCK_TAGS = new Set([ * reopen, so this is stable across saves. Whitespace-only text is dropped. * * @param {Element} container Changed in place: inline runs move out into the new `
`s + * @param {string} [tag] The wrapper, for content the XSD allows only in some blocks: it + * allows `` in a `

` but not directly in a `

` * @returns {Element[]} The container's children, as blocks */ -export function wrapInlineRuns(container) { +export function wrapInlineRuns(container, tag = 'div') { const blocks = []; let run = null; for (const node of [...container.childNodes]) { @@ -297,7 +299,7 @@ export function wrapInlineRuns(container) { run = null; } else if (run || node.nodeType === Node.ELEMENT_NODE || node.nodeValue.trim()) { if (!run) { - run = buildXmlNode({ tag: 'div' }); + run = buildXmlNode({ tag }); blocks.push(run); } run.appendChild(node); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/richText.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/richText.spec.js index 982a2d359f..31ef06600e 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/richText.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/richText.spec.js @@ -11,6 +11,7 @@ describe('hasRichTextContent', () => { ['

', false], // The literal six characters a pattern strip leaves behind are not content. ['

 

', false], + ['  ', false], // A prompt, choice or hint can be entirely an image or a formula — from a converted // legacy question, or from the editor's own image and formula buttons. ['

', true], From 67b41e69de06494ed4b96509101d7982f937096f Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 20 Oct 2026 13:27:17 +0300 Subject: [PATCH 02/10] Add the gap match interaction descriptor Add the headless layer for gap match: parsing, XML building and validation. The descriptor is registered but stays headless, so the question type selector does not offer it until the editor lands. In the editor state, each gap holds its answer, so undo and paste keep the two together. The saved XML keeps every gap empty, with the answers written as gap texts. --- .../shared/views/QTIEditor/constants.js | 1 + .../QTIEditor/interactions/descriptors.js | 9 +- .../interactions/gapMatch/Descriptor.js | 38 ++ .../gapMatch/__tests__/Descriptor.spec.js | 83 ++++ .../gapMatch/__tests__/parse.spec.js | 380 ++++++++++++++++++ .../gapMatch/__tests__/validation.spec.js | 111 +++++ .../QTIEditor/interactions/gapMatch/parse.js | 175 ++++++++ .../interactions/gapMatch/validation.js | 44 ++ 8 files changed, 839 insertions(+), 2 deletions(-) create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js index 81c8f973b4..f4b1253328 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js @@ -102,6 +102,7 @@ export const QuestionType = Object.freeze({ ASSOCIATE: 'associate', MATCH: 'match', INLINE_CHOICE: 'inlineChoice', + GAP_MATCH: 'gapMatch', }); /** diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js index b7aa3360d9..d4c2847f78 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js @@ -5,6 +5,7 @@ import { orderingInteractionDescriptor } from './ordering/Descriptor'; import { associateInteractionDescriptor } from './associate/Descriptor'; import { matchInteractionDescriptor } from './match/Descriptor'; import { inlineChoiceInteractionDescriptor } from './inlineChoice/Descriptor'; +import { gapMatchInteractionDescriptor } from './gapMatch/Descriptor'; /** * Every interaction's descriptor: matching, parsing, building and validating XML. @@ -27,6 +28,7 @@ export const descriptors = [ associateInteractionDescriptor, matchInteractionDescriptor, inlineChoiceInteractionDescriptor, + gapMatchInteractionDescriptor, ]; /** @@ -36,9 +38,12 @@ export const registry = Object.fromEntries(descriptors.map(d => [d.type, d])); /** * Interactions with a descriptor but no editor in index.js yet. Items holding one are - * shown read-only. TODO: #6182 removes inline choice. + * shown read-only. TODO: #6182 removes inline choice, #6204 removes gap match. */ -export const HEADLESS_INTERACTIONS = Object.freeze([QtiInteraction.INLINE_CHOICE]); +export const HEADLESS_INTERACTIONS = Object.freeze([ + QtiInteraction.INLINE_CHOICE, + QtiInteraction.GAP_MATCH, +]); /** * Inline interactions whose editor holds one per item body. A body with several would diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js new file mode 100644 index 0000000000..77e190f43c --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js @@ -0,0 +1,38 @@ +import { QtiInteraction, QuestionType, BaseType, Cardinality } from '../../constants'; +import { InteractionDescriptor } from '../InteractionDescriptor'; +import { parseGapMatchInteraction, buildGapMatchInteractionXML } from './parse'; +import { validateGapMatchInteraction } from './validation'; + +class GapMatchInteractionDescriptor extends InteractionDescriptor { + constructor() { + super({ + type: QtiInteraction.GAP_MATCH, + questionTypes: [QuestionType.GAP_MATCH], + }); + } + + getQuestionType() { + return QuestionType.GAP_MATCH; + } + + getResponseDeclarationSchema() { + return { + baseType: BaseType.DIRECTED_PAIR, + cardinality: Cardinality.MULTIPLE, + }; + } + + parse(bodyXml, responseDeclarations) { + return parseGapMatchInteraction(bodyXml, responseDeclarations); + } + + buildXML(state, questionType) { + return buildGapMatchInteractionXML(state, questionType, this.getResponseDeclarationSchema()); + } + + validate(state) { + return validateGapMatchInteraction(state); + } +} + +export const gapMatchInteractionDescriptor = new GapMatchInteractionDescriptor(); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js new file mode 100644 index 0000000000..66da71c65b --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js @@ -0,0 +1,83 @@ +import { gapMatchInteractionDescriptor as descriptor } from '../Descriptor'; +import { getGaps } from '../parse'; +import { isSupportedItem, resolveDescriptor } from '../../resolveDescriptor'; +import { parseItem } from '../../../serialization/parseItem'; +import { parseXML } from '../../../serialization/xml'; +import { Placement, QtiInteraction, QuestionType } from '../../../constants'; +import { qtiEditorStrings } from '../../../qtiEditorStrings'; + +const ITEM = ` + + + + choice_w1n7erxx gap_a1b2c3d4 + choice_s9r1ngxx gap_e5f6g7h8 + + + + +

Identify the missing words.

 + winter + spring + summer +

Now is the of our discontent

+

Made glorious by this sun of York;

+ + +`; + +const toEl = xml => parseXML(xml).documentElement; + +describe('gapMatchInteractionDescriptor', () => { + it('is a block interaction', () => { + expect(descriptor.placement).toBe(Placement.BLOCK); + }); + + describe('matches', () => { + it('matches the gap match interaction element', () => { + expect(descriptor.matches(toEl(''))).toBe(true); + }); + + it('does not match a match interaction', () => { + expect(descriptor.matches(toEl(''))).toBe(false); + }); + }); + + describe('resolveDescriptor', () => { + it('resolves an item holding the interaction to gap match', () => { + const [{ bodyXml, responseDeclarations }] = parseItem(ITEM).interactions; + const result = resolveDescriptor(bodyXml, responseDeclarations); + expect(result.descriptor).toBe(descriptor); + expect(result.descriptor.type).toBe(QtiInteraction.GAP_MATCH); + expect(result.questionType).toBe(QuestionType.GAP_MATCH); + }); + + it('shows the item read-only until the interaction has an editor', () => { + const { interactions, itemBodyXml } = parseItem(ITEM); + expect(isSupportedItem(interactions, itemBodyXml)).toBe(false); + }); + }); + + it('parses a full item', () => { + const [{ bodyXml, responseDeclarations }] = parseItem(ITEM).interactions; + const state = descriptor.parse(bodyXml, responseDeclarations); + expect(state.prompt).toBe('

Identify the missing words.

'); + expect(getGaps(state.passage)).toEqual([ + { id: 'gap_a1b2c3d4', answerId: 'choice_w1n7erxx', answer: 'winter' }, + { id: 'gap_e5f6g7h8', answerId: 'choice_s9r1ngxx', answer: 'spring' }, + ]); + expect(state.distractors).toEqual([{ id: 'choice_s0mm3rxx', content: 'summer' }]); + }); + + it('buildXML() writes its own declaration schema', () => { + const [{ bodyXml, responseDeclarations }] = parseItem(ITEM).interactions; + const state = descriptor.parse(bodyXml, responseDeclarations); + const [declXml] = descriptor.buildXML(state, QuestionType.GAP_MATCH).responseDeclarations; + expect(declXml).toContain('base-type="directedPair"'); + expect(declXml).toContain('cardinality="multiple"'); + }); + + it('offers no question type option yet', () => { + expect(descriptor.getTypeOptions(qtiEditorStrings)).toEqual([]); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js new file mode 100644 index 0000000000..33ef3f1354 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js @@ -0,0 +1,380 @@ +/* eslint-disable jest-dom/prefer-to-have-attribute, jest-dom/prefer-to-have-text-content */ +// The jest-dom matchers reject XML nodes produced by DOMParser(..., 'text/xml'). + +import { + buildGapMatchInteractionXML as buildXML, + getGaps, + parseGapMatchInteraction as parse, +} from '../parse'; +import { BaseType, Cardinality, QuestionType } from '../../../constants'; +import { parseXML } from '../../../serialization/xml'; +import { QTIDeclaration } from '../../../serialization/qti/QTIDeclaration'; + +const SCHEMA = { baseType: BaseType.DIRECTED_PAIR, cardinality: Cardinality.MULTIPLE }; + +const gapText = (id, content, matchMax = 1) => + `${content}`; +const interaction = (...children) => + `${children.join('')}`; +const declaration = (...values) => + ` + ${values.map(v => `${v}`).join('')} + `; + +const PROMPT = '

Identify the missing words.

'; +const PASSAGE = + '

Now is the of our discontent

' + + '

Made glorious by this sun of York;

'; +const BODY = interaction( + PROMPT, + gapText('choice_winter', 'winter'), + gapText('choice_spring', 'spring'), + gapText('choice_summer', 'summer'), + gapText('choice_autumn', 'autumn'), + PASSAGE, +); +const DECL = declaration('choice_winter gap_a', 'choice_spring gap_b'); + +const gap = (id, answerId, answer) => + `${answer}`; +const blankGap = id => ``; + +function makeState(overrides = {}) { + return { + responseIdentifier: 'RESPONSE', + prompt: '

Identify the missing words.

', + passage: + `

Now is the ${gap('gap_a', 'choice_winter', 'winter')} of our discontent

` + + `

Made glorious ${gap('gap_b', 'choice_spring', 'spring')} by this sun of York;

`, + distractors: [{ id: 'choice_summer', content: 'summer' }], + ...overrides, + }; +} + +const build = state => { + const { bodyXml, responseDeclarations } = buildXML(state, QuestionType.GAP_MATCH, SCHEMA); + return { + root: parseXML(bodyXml).documentElement, + declaration: QTIDeclaration.fromXML(parseXML(responseDeclarations[0]).documentElement), + declarationXml: responseDeclarations[0], + }; +}; +const gapTextsOf = root => + [...root.querySelectorAll('qti-gap-text')].map(el => ({ + id: el.getAttribute('identifier'), + content: el.textContent, + matchMax: el.getAttribute('match-max'), + })); +const gapIdsOf = root => + [...root.querySelectorAll('qti-gap')].map(el => el.getAttribute('identifier')); +const roundTrip = state => { + const { bodyXml, responseDeclarations } = buildXML(state, QuestionType.GAP_MATCH, SCHEMA); + return parse(bodyXml, responseDeclarations); +}; + +describe('parse()', () => { + describe('fallbacks', () => { + it('returns an empty state when bodyXml is empty', () => { + expect(parse('', [DECL])).toEqual({ + responseIdentifier: 'RESPONSE', + prompt: '', + passage: '', + distractors: [], + }); + }); + + it('returns an empty state when bodyXml is invalid XML', () => { + expect(parse(' is absent', () => { + expect(parse(interaction(PASSAGE), []).prompt).toBe(''); + }); + }); + + it('reads the prompt HTML', () => { + expect(parse(BODY, [DECL]).prompt).toBe('

Identify the missing words.

'); + }); + + it('puts each gap answer inside its gap, in passage order', () => { + expect(getGaps(parse(BODY, [DECL]).passage)).toEqual([ + { id: 'gap_a', answerId: 'choice_winter', answer: 'winter' }, + { id: 'gap_b', answerId: 'choice_spring', answer: 'spring' }, + ]); + }); + + it('keeps the passage text around the gaps', () => { + expect(parse(BODY, [DECL]).passage).toBe( + `

Now is the ${gap('gap_a', 'choice_winter', 'winter')} of our discontent

` + + `

Made glorious ${gap('gap_b', 'choice_spring', 'spring')} by this sun of York;

`, + ); + }); + + it('reads a gap inside formatted content', () => { + const body = interaction( + gapText('choice_winter', 'winter'), + '
  • Now is the
', + ); + const { passage } = parse(body, [declaration('choice_winter gap_a')]); + expect(passage).toContain(`Now is the ${gap('gap_a', 'choice_winter', 'winter')}`); + }); + + it('keeps formatting and math in an answer', () => { + const body = interaction( + gapText('choice_x', 'x'), + '

', + ); + const [{ answer }] = getGaps(parse(body, [declaration('choice_x gap_a')]).passage); + expect(answer).toBe('x'); + }); + + it('reads the gap texts in no pair as distractors, in document order', () => { + expect(parse(BODY, [DECL]).distractors).toEqual([ + { id: 'choice_summer', content: 'summer' }, + { id: 'choice_autumn', content: 'autumn' }, + ]); + }); + + it('gives a shared gap text to every gap it is paired with', () => { + const body = interaction(gapText('choice_winter', 'winter', 2), PASSAGE); + const decl = declaration('choice_winter gap_a', 'choice_winter gap_b'); + expect(getGaps(parse(body, [decl]).passage)).toEqual([ + { id: 'gap_a', answerId: 'choice_winter', answer: 'winter' }, + { id: 'gap_b', answerId: 'choice_winter', answer: 'winter' }, + ]); + }); + + it('leaves a gap with no pair without an answer', () => { + const body = interaction(gapText('choice_winter', 'winter'), PASSAGE); + expect(getGaps(parse(body, [declaration('choice_winter gap_a')]).passage)[1]).toEqual({ + id: 'gap_b', + answerId: null, + answer: '', + }); + }); + + it('drops a value that names the gap before the gap text', () => { + const body = interaction(gapText('choice_winter', 'winter'), PASSAGE); + const state = parse(body, [declaration('gap_a choice_winter')]); + expect(getGaps(state.passage)[0].answerId).toBeNull(); + expect(state.distractors).toEqual([{ id: 'choice_winter', content: 'winter' }]); + }); + + it('drops a value that names a missing gap text', () => { + const state = parse(interaction(PASSAGE), [declaration('choice_missing gap_a')]); + expect(getGaps(state.passage)[0].answerId).toBeNull(); + }); + + describe('missing identifiers', () => { + it('gives a gap without an identifier a new one', () => { + const body = interaction('

Now is the

'); + expect(getGaps(parse(body, []).passage)[0].id).toMatch(/^gap_/); + }); + + it('gives a gap text without an identifier a new one', () => { + const body = interaction('summer', PASSAGE); + expect(parse(body, []).distractors[0].id).toMatch(/^choice_/); + }); + }); +}); + +describe('buildXML()', () => { + it('writes the prompt, the answers in gap order, the distractors, then the passage', () => { + const { root } = build( + makeState({ + passage: + `

${gap('gap_a', 'choice_spring', 'spring')}

` + + `

${gap('gap_b', 'choice_winter', 'winter')}

`, + }), + ); + expect([...root.children].map(el => el.localName)).toEqual([ + 'qti-prompt', + 'qti-gap-text', + 'qti-gap-text', + 'qti-gap-text', + 'p', + 'p', + ]); + expect(gapTextsOf(root).map(text => text.id)).toEqual([ + 'choice_spring', + 'choice_winter', + 'choice_summer', + ]); + }); + + it('writes the interaction attributes', () => { + const { root } = build(makeState()); + expect(root.getAttribute('response-identifier')).toBe('RESPONSE'); + expect(root.getAttribute('shuffle')).toBe('true'); + expect(root.getAttribute('max-associations')).toBe('2'); + }); + + it.each(['', '

'])('omits when the question is %p', prompt => { + const { root } = build(makeState({ prompt })); + expect(root.querySelector('qti-prompt')).toBeNull(); + }); + + it('writes every gap empty, with only its identifier', () => { + const { root } = build(makeState()); + for (const el of root.querySelectorAll('qti-gap')) { + expect(el.childNodes).toHaveLength(0); + expect(el.getAttributeNames()).toEqual(['identifier']); + } + }); + + it('writes every answer and distractor with match-max 1', () => { + expect(gapTextsOf(build(makeState()).root)).toEqual([ + { id: 'choice_winter', content: 'winter', matchMax: '1' }, + { id: 'choice_spring', content: 'spring', matchMax: '1' }, + { id: 'choice_summer', content: 'summer', matchMax: '1' }, + ]); + }); + + it('pairs each answer identifier with its gap identifier, in that order', () => { + const { declaration: decl, declarationXml } = build(makeState()); + expect(decl.correctResponse).toEqual([ + ['choice_winter', 'gap_a'], + ['choice_spring', 'gap_b'], + ]); + expect(declarationXml).toContain('base-type="directedPair"'); + expect(declarationXml).toContain('cardinality="multiple"'); + }); + + it('writes answers with the same content as one gap text paired with every gap', () => { + const { root, declaration: decl } = build( + makeState({ + passage: + `

${gap('gap_a', 'choice_winter', 'winter')}

` + + `

${gap('gap_b', 'choice_other', 'winter')}

`, + distractors: [], + }), + ); + expect(gapTextsOf(root)).toEqual([{ id: 'choice_winter', content: 'winter', matchMax: '2' }]); + expect(decl.correctResponse).toEqual([ + ['choice_winter', 'gap_a'], + ['choice_winter', 'gap_b'], + ]); + }); + + it('writes a blank gap with no gap text and no pair', () => { + const { root, declaration: decl } = build( + makeState({ + passage: `

${gap('gap_a', 'choice_winter', 'winter')} ${blankGap('gap_b')}

`, + distractors: [], + }), + ); + expect(gapIdsOf(root)).toEqual(['gap_a', 'gap_b']); + expect(gapTextsOf(root).map(text => text.id)).toEqual(['choice_winter']); + expect(decl.correctResponse).toEqual([['choice_winter', 'gap_a']]); + expect(root.getAttribute('max-associations')).toBe('2'); + }); + + it('writes no correct response when no gap has an answer', () => { + const { declarationXml } = build(makeState({ passage: `

${blankGap('gap_a')}

` })); + expect(declarationXml).not.toContain('qti-correct-response'); + }); + + it('keeps a gap inside formatted content in place', () => { + const { root } = build( + makeState({ passage: `

Now ${gap('gap_a', 'choice_w', 'winter')}

` }), + ); + expect(root.querySelector('p > strong > qti-gap').getAttribute('identifier')).toBe('gap_a'); + }); + + it('wraps passage text outside a block in a paragraph', () => { + const { root } = build(makeState({ passage: `Now ${gap('gap_a', 'choice_w', 'winter')}` })); + expect(root.querySelector('p > qti-gap')).not.toBeNull(); + }); + + describe('identifiers', () => { + it('keeps the other identifiers when a gap is removed', () => { + const { root } = build( + makeState({ + passage: `

Made glorious ${gap('gap_b', 'choice_spring', 'spring')}

`, + }), + ); + expect(gapIdsOf(root)).toEqual(['gap_b']); + expect(gapTextsOf(root).map(text => text.id)).toEqual(['choice_spring', 'choice_summer']); + }); + + it('keeps the identifiers when the gaps are reordered', () => { + const { root, declaration: decl } = build( + makeState({ + passage: + `

${gap('gap_b', 'choice_spring', 'spring')}

` + + `

${gap('gap_a', 'choice_winter', 'winter')}

`, + }), + ); + expect(gapIdsOf(root)).toEqual(['gap_b', 'gap_a']); + expect(decl.correctResponse).toEqual([ + ['choice_spring', 'gap_b'], + ['choice_winter', 'gap_a'], + ]); + }); + + it('gives a repeated gap identifier a new one', () => { + const { root } = build( + makeState({ + passage: `

${gap('gap_a', 'choice_w', 'winter')} ${gap('gap_a', 'choice_s', 'spring')}

`, + }), + ); + const [first, second] = gapIdsOf(root); + expect(first).toBe('gap_a'); + expect(second).toMatch(/^gap_/); + expect(second).not.toBe('gap_a'); + }); + + it('gives an answer whose identifier is taken by other content a new one', () => { + const { root } = build( + makeState({ + passage: `

${gap('gap_a', 'choice_w', 'winter')} ${gap('gap_b', 'choice_w', 'spring')}

`, + distractors: [], + }), + ); + const [first, second] = gapTextsOf(root).map(text => text.id); + expect(first).toBe('choice_w'); + expect(second).toMatch(/^choice_/); + expect(second).not.toBe('choice_w'); + }); + + it('gives an answer without an identifier a new one', () => { + const { root } = build( + makeState({ + passage: '

winter

', + distractors: [], + }), + ); + expect(gapTextsOf(root)[0].id).toMatch(/^choice_/); + }); + }); +}); + +describe('round trip', () => { + it('returns the same question, passage, answers, distractors and identifiers', () => { + const state = parse(BODY, [DECL]); + expect(roundTrip(state)).toEqual(state); + }); + + it('reopens an item with an empty question with an empty question', () => { + expect(roundTrip(makeState({ prompt: '' })).prompt).toBe(''); + }); + + it('reopens a shared answer as two gaps with that answer', () => { + const state = makeState({ + passage: + `

${gap('gap_a', 'choice_winter', 'winter')}

` + + `

${gap('gap_b', 'choice_winter', 'winter')}

`, + }); + expect(roundTrip(state)).toEqual(state); + }); + + it('reopens a blank gap with no answer', () => { + const state = makeState({ + passage: `

${gap('gap_a', 'choice_winter', 'winter')} ${blankGap('gap_b')}

`, + }); + expect(getGaps(roundTrip(state).passage)).toEqual([ + { id: 'gap_a', answerId: 'choice_winter', answer: 'winter' }, + { id: 'gap_b', answerId: null, answer: '' }, + ]); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js new file mode 100644 index 0000000000..30d8f824d3 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js @@ -0,0 +1,111 @@ +import { validateGapMatchInteraction as validate } from '../validation'; +import { ValidationError } from '../../../constants'; +import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; + +const gap = (id, answer) => + `${answer}`; +const distractor = (id, content) => ({ id, content }); + +function makeState(overrides = {}) { + return { + responseIdentifier: 'RESPONSE', + prompt: '

Identify the missing words.

', + passage: `

Now is the ${gap('gap_a', 'winter')} of our ${gap('gap_b', 'spring')}

`, + distractors: [distractor('choice_summer', 'summer')], + ...overrides, + }; +} + +const errorsWith = (state, code) => validate(state).filter(e => e.code === code); + +describe('validateGapMatchInteraction()', () => { + it('returns an empty array for a valid state', () => { + expect(validate(makeState())).toEqual([]); + }); + + it('accepts an empty question', () => { + expect(validate(makeState({ prompt: '' }))).toEqual([]); + }); + + it('accepts two gaps with the same answer', () => { + const passage = `

${gap('gap_a', 'winter')} ${gap('gap_b', 'winter')}

`; + expect(validate(makeState({ passage }))).toEqual([]); + }); + + describe('NO_CORRECT_ANSWER', () => { + it.each(['', '

Now is the winter of our discontent

'])( + 'flags a passage with no gaps: %p', + passage => { + expect(validate(makeState({ passage }))).toContainEqual({ + code: ValidationError.NO_CORRECT_ANSWER, + }); + }, + ); + }); + + describe('EMPTY_ANSWER_CONTENT', () => { + it('flags a gap with no answer by its gap identifier', () => { + const passage = `

${gap('gap_a', 'winter')}

`; + expect(errorsWith(makeState({ passage }), ValidationError.EMPTY_ANSWER_CONTENT)).toEqual([ + { code: ValidationError.EMPTY_ANSWER_CONTENT, id: 'gap_b' }, + ]); + }); + + it('flags a gap whose answer is only whitespace', () => { + const passage = `

${gap('gap_a', '  ')}

`; + expect(errorsWith(makeState({ passage }), ValidationError.EMPTY_ANSWER_CONTENT)).toEqual([ + { code: ValidationError.EMPTY_ANSWER_CONTENT, id: 'gap_a' }, + ]); + }); + + it('does not flag a math-only answer', () => { + const passage = `

${gap('gap_a', '')}

`; + expect(validate(makeState({ passage }))).toEqual([]); + }); + }); + + describe('EMPTY_CHOICE_CONTENT', () => { + it('flags a blank distractor by its identifier', () => { + const distractors = [distractor('choice_summer', 'summer'), distractor('choice_blank', '')]; + expect(errorsWith(makeState({ distractors }), ValidationError.EMPTY_CHOICE_CONTENT)).toEqual([ + { code: ValidationError.EMPTY_CHOICE_CONTENT, id: 'choice_blank' }, + ]); + }); + }); + + describe('DUPLICATE_DISTRACTOR_CONTENT', () => { + it('flags a distractor with the same content as an answer', () => { + const distractors = [distractor('choice_copy', 'winter')]; + expect( + errorsWith(makeState({ distractors }), ValidationError.DUPLICATE_DISTRACTOR_CONTENT), + ).toEqual([ + { + code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, + text: richTextComparisonKey('winter'), + }, + ]); + }); + + it('flags two distractors with the same content once', () => { + const distractors = [ + distractor('choice_summer', 'summer'), + distractor('choice_copy', 'summer'), + ]; + expect( + errorsWith(makeState({ distractors }), ValidationError.DUPLICATE_DISTRACTOR_CONTENT), + ).toEqual([ + { + code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, + text: richTextComparisonKey('summer'), + }, + ]); + }); + + it('does not compare blank distractors', () => { + const distractors = [distractor('choice_one', ''), distractor('choice_two', '')]; + expect( + errorsWith(makeState({ distractors }), ValidationError.DUPLICATE_DISTRACTOR_CONTENT), + ).toEqual([]); + }); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js new file mode 100644 index 0000000000..66752f6ad2 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -0,0 +1,175 @@ +import { QTIDeclaration } from '../../serialization/qti/QTIDeclaration'; +import CorrectResponse from '../../serialization/qti/declarations/correctResponse'; +import { + buildXmlNode, + getContentHTML, + getPromptHTML, + isContentNode, + parseXML, + serializeAsHtml, + wrapInlineRuns, +} from '../../serialization/xml'; +import { generateRandomSlug } from '../../utils/generateRandomSlug'; +import { hasRichTextContent } from '../../utils/richText'; +import { RESPONSE_IDENTIFIER } from '../../constants'; +import { _extractCorrectPairIds } from '../associate/parse'; +import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; + +const serializer = new XMLSerializer(); + +const GAP = 'qti-gap'; +const GAP_TEXT = 'qti-gap-text'; +const PROMPT = 'qti-prompt'; +const ANSWER_ATTR = 'data-studio-answer'; + +export function _defaultState() { + return { responseIdentifier: RESPONSE_IDENTIFIER, prompt: '', passage: '', distractors: [] }; +} + +export function getGaps(passage) { + const doc = parseXML(passage || '', 'text/html'); + return [...doc.querySelectorAll(GAP)].map(el => ({ + id: el.getAttribute('identifier'), + answerId: el.getAttribute(ANSWER_ATTR), + answer: el.innerHTML, + })); +} + +export function parseGapMatchInteraction(bodyXml, responseDeclarations) { + if (!bodyXml) return _defaultState(); + + let root; + try { + root = parseXML(bodyXml).documentElement; + } catch { + return _defaultState(); + } + + const contentNodes = [...root.childNodes].filter(isContentNode); + const gapTextEls = contentNodes.filter(node => node.localName === GAP_TEXT); + const passageNodes = contentNodes.filter(node => ![GAP_TEXT, PROMPT].includes(node.localName)); + + const gapTextById = new Map(); + for (const el of gapTextEls) { + if (!el.getAttribute('identifier')) { + el.setAttribute('identifier', generateRandomSlug('choice')); + } + const id = el.getAttribute('identifier'); + if (!gapTextById.has(id)) gapTextById.set(id, el); + } + + const answerIdByGapId = new Map(); + for (const [gapTextId, gapId] of _extractCorrectPairIds(responseDeclarations).filter(Boolean)) { + if (gapTextById.has(gapTextId) && !answerIdByGapId.has(gapId)) { + answerIdByGapId.set(gapId, gapTextId); + } + } + + const paired = new Set(); + for (const gapEl of root.querySelectorAll(GAP)) { + const originalId = gapEl.getAttribute('identifier'); + if (!originalId) gapEl.setAttribute('identifier', generateRandomSlug('gap')); + + const gapTextEl = gapTextById.get(answerIdByGapId.get(originalId)); + if (gapTextEl) { + gapEl.setAttribute(ANSWER_ATTR, gapTextEl.getAttribute('identifier')); + gapEl.append(...[...gapTextEl.childNodes].map(node => node.cloneNode(true))); + paired.add(gapTextEl); + } + } + + return { + responseIdentifier: root.getAttribute('response-identifier') || RESPONSE_IDENTIFIER, + prompt: getPromptHTML(root), + passage: serializeAsHtml(passageNodes, root.namespaceURI), + distractors: gapTextEls + .filter(el => !paired.has(el)) + .map(el => ({ id: el.getAttribute('identifier'), content: getContentHTML(el) })), + }; +} + +export function buildGapMatchInteractionXML(state, _questionType, declarationSchema) { + const { responseIdentifier = RESPONSE_IDENTIFIER, prompt, passage, distractors = [] } = state; + + const usedIds = new Set(); + function claim(id, prefix) { + const resolved = id && !usedIds.has(id) ? id : generateRandomSlug(prefix); + usedIds.add(resolved); + return resolved; + } + + const passageEl = buildXmlNode({ tag: 'div', innerHTML: passage || '' }); + const gapEls = [...passageEl.querySelectorAll(GAP)]; + + const idByAnswerKey = new Map(); + const answers = new Map(); + const correctPairs = []; + for (const gapEl of gapEls) { + const gapId = claim(gapEl.getAttribute('identifier'), 'gap'); + const content = getContentHTML(gapEl); + if (hasRichTextContent(content)) { + const key = richTextComparisonKey(content); + const answerId = idByAnswerKey.get(key) || claim(gapEl.getAttribute(ANSWER_ATTR), 'choice'); + idByAnswerKey.set(key, answerId); + + const answer = answers.get(answerId); + if (answer) { + answer.matchMax += 1; + } else { + answers.set(answerId, { id: answerId, content, matchMax: 1 }); + } + correctPairs.push([answerId, gapId]); + } + + gapEl.textContent = ''; + for (const name of gapEl.getAttributeNames()) gapEl.removeAttribute(name); + gapEl.setAttribute('identifier', gapId); + } + + const distractorChoices = distractors.map(({ id, content }) => ({ + id: claim(id, 'choice'), + content, + matchMax: 1, + })); + + const children = []; + if (hasRichTextContent(prompt)) { + children.push(buildXmlNode({ tag: PROMPT, innerHTML: prompt })); + } + for (const { id, content, matchMax } of [...answers.values(), ...distractorChoices]) { + children.push( + buildXmlNode({ + tag: GAP_TEXT, + attrs: { identifier: id, 'match-max': matchMax }, + innerHTML: content, + }), + ); + } + children.push(...wrapInlineRuns(passageEl, 'p')); + + const interactionEl = buildXmlNode({ + tag: 'qti-gap-match-interaction', + attrs: { + 'response-identifier': responseIdentifier, + shuffle: 'true', + 'max-associations': gapEls.length, + }, + children, + }); + + const { cardinality, baseType } = declarationSchema; + const declaration = new QTIDeclaration({ + identifier: responseIdentifier, + baseType, + cardinality, + tag: 'qti-response-declaration', + }); + if (correctPairs.length > 0) { + new CorrectResponse(correctPairs, declaration); + } + + return { + bodyXml: serializer.serializeToString(interactionEl), + responseDeclarations: [serializer.serializeToString(declaration.getXML())], + }; +} diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js new file mode 100644 index 0000000000..5617cfb764 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js @@ -0,0 +1,44 @@ +import { ValidationError } from '../../constants'; +import { hasRichTextContent } from '../../utils/richText'; +import { getGaps } from './parse'; +import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; + +export function validateGapMatchInteraction(state) { + const { passage, distractors = [] } = state; + const gaps = getGaps(passage); + if (gaps.length === 0) { + return [{ code: ValidationError.NO_CORRECT_ANSWER }]; + } + + const errors = []; + const filledKeys = []; + for (const { id, answer } of gaps) { + if (hasRichTextContent(answer)) { + filledKeys.push(richTextComparisonKey(answer)); + } else { + errors.push({ code: ValidationError.EMPTY_ANSWER_CONTENT, id }); + } + } + + const distractorKeys = []; + for (const { id, content } of distractors) { + if (hasRichTextContent(content)) { + distractorKeys.push(richTextComparisonKey(content)); + } else { + errors.push({ code: ValidationError.EMPTY_CHOICE_CONTENT, id }); + } + } + + const answerKeys = new Set(filledKeys); + const seen = new Set(); + const repeated = new Set(); + for (const key of distractorKeys) { + if (answerKeys.has(key) || seen.has(key)) repeated.add(key); + seen.add(key); + } + for (const text of repeated) { + errors.push({ code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, text }); + } + + return errors; +} From bbe981771ab2196fb5633a9dff1803aaff4d7b7f Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 20 Oct 2026 14:02:59 +0300 Subject: [PATCH 03/10] Share the repeated distractor check across interactions Associate, match and gap match each had their own copy of the rule that flags a distractor repeating an answer or another distractor. --- .../interactions/associate/validation.js | 17 ++------- .../interactions/gapMatch/validation.js | 36 ++++++++----------- .../interactions/match/validation.js | 14 ++++---- .../utils/__tests__/distractors.spec.js | 35 ++++++++++++++++++ .../views/QTIEditor/utils/distractors.js | 14 ++++++++ 5 files changed, 71 insertions(+), 45 deletions(-) create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/distractors.spec.js create mode 100644 contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js index d6628e9f94..de35e5a22c 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/associate/validation.js @@ -1,6 +1,7 @@ import flatten from 'lodash/flatten'; import { ValidationError } from '../../constants'; import { hasRichTextContent } from '../../utils/richText'; +import { duplicateDistractorErrors } from '../../utils/distractors'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** @@ -58,21 +59,7 @@ export function validateAssociateInteraction(state) { // A distractor is there to be the wrong answer, so repeating another // distractor or an item the author already paired makes it unanswerable. // Content reused across two pairs stays valid — only distractors are flagged. - const occurrences = new Map(); - for (const { filled, key } of allChoices) { - if (filled) { - occurrences.set(key, (occurrences.get(key) || 0) + 1); - } - } - - const repeated = new Set( - readDistractors - .filter(({ filled, key }) => filled && occurrences.get(key) > 1) - .map(({ key }) => key), - ); - for (const text of repeated) { - errors.push({ code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, text }); - } + errors.push(...duplicateDistractorErrors(flatten(readPairs), readDistractors)); return errors; } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js index 5617cfb764..05f9813ee2 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/validation.js @@ -1,5 +1,6 @@ import { ValidationError } from '../../constants'; import { hasRichTextContent } from '../../utils/richText'; +import { duplicateDistractorErrors } from '../../utils/distractors'; import { getGaps } from './parse'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; @@ -10,35 +11,26 @@ export function validateGapMatchInteraction(state) { return [{ code: ValidationError.NO_CORRECT_ANSWER }]; } + const read = (id, content) => ({ + id, + filled: hasRichTextContent(content), + key: richTextComparisonKey(content), + }); + const readAnswers = gaps.map(({ id, answer }) => read(id, answer)); + const readDistractors = distractors.map(({ id, content }) => read(id, content)); + const errors = []; - const filledKeys = []; - for (const { id, answer } of gaps) { - if (hasRichTextContent(answer)) { - filledKeys.push(richTextComparisonKey(answer)); - } else { + for (const { id, filled } of readAnswers) { + if (!filled) { errors.push({ code: ValidationError.EMPTY_ANSWER_CONTENT, id }); } } - - const distractorKeys = []; - for (const { id, content } of distractors) { - if (hasRichTextContent(content)) { - distractorKeys.push(richTextComparisonKey(content)); - } else { + for (const { id, filled } of readDistractors) { + if (!filled) { errors.push({ code: ValidationError.EMPTY_CHOICE_CONTENT, id }); } } - - const answerKeys = new Set(filledKeys); - const seen = new Set(); - const repeated = new Set(); - for (const key of distractorKeys) { - if (answerKeys.has(key) || seen.has(key)) repeated.add(key); - seen.add(key); - } - for (const text of repeated) { - errors.push({ code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, text }); - } + errors.push(...duplicateDistractorErrors(readAnswers, readDistractors)); return errors; } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js index 71195193e1..a7ac4d0037 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/match/validation.js @@ -1,5 +1,6 @@ import { ValidationError } from '../../constants'; import { hasRichTextContent } from '../../utils/richText'; +import { duplicateDistractorErrors } from '../../utils/distractors'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; /** @@ -71,15 +72,12 @@ export function validateMatchInteraction(state) { // An answer may serve several rows, so only distractors are flagged: one // repeating another distractor or any answer cannot be told apart from it. - const choiceKeyCounts = countFilledKeys(readChoices); - const repeated = new Set( - readDistractors - .filter(({ filled, key }) => filled && choiceKeyCounts.get(key) > 1) - .map(({ key }) => key), + errors.push( + ...duplicateDistractorErrors( + readRows.flatMap(row => row.matches), + readDistractors, + ), ); - for (const text of repeated) { - errors.push({ code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, text }); - } return errors; } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/distractors.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/distractors.spec.js new file mode 100644 index 0000000000..b937d1a00d --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/__tests__/distractors.spec.js @@ -0,0 +1,35 @@ +import { duplicateDistractorErrors } from '../distractors'; +import { ValidationError } from '../../constants'; + +const duplicate = text => ({ code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, text }); +const filled = (...keys) => keys.map(key => ({ filled: true, key })); +const blank = { filled: false, key: '[]' }; + +describe('duplicateDistractorErrors', () => { + it('returns no errors when every distractor is unique', () => { + expect(duplicateDistractorErrors(filled('a', 'b'), filled('c', 'd'))).toEqual([]); + }); + + it('flags a distractor that repeats an answer', () => { + expect(duplicateDistractorErrors(filled('a', 'b'), filled('b', 'c'))).toEqual([duplicate('b')]); + }); + + it('flags distractors that repeat each other once', () => { + expect(duplicateDistractorErrors(filled('a'), filled('c', 'c', 'c'))).toEqual([duplicate('c')]); + }); + + it('does not flag answers that repeat each other', () => { + expect(duplicateDistractorErrors(filled('a', 'a'), filled('b'))).toEqual([]); + }); + + it('does not compare blank choices', () => { + expect(duplicateDistractorErrors([blank], [blank, blank])).toEqual([]); + }); + + it('lists the repeated distractors in the order they appear', () => { + expect(duplicateDistractorErrors(filled('a'), filled('d', 'a', 'd'))).toEqual([ + duplicate('d'), + duplicate('a'), + ]); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js new file mode 100644 index 0000000000..c998617bf8 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js @@ -0,0 +1,14 @@ +import { ValidationError } from '../constants'; + +export function duplicateDistractorErrors(readAnswers, readDistractors) { + const filledKeys = readChoices => + readChoices.filter(({ filled }) => filled).map(({ key }) => key); + const distractorKeys = filledKeys(readDistractors); + + const counts = new Map(); + for (const key of [...filledKeys(readAnswers), ...distractorKeys]) { + counts.set(key, (counts.get(key) || 0) + 1); + } + const repeated = new Set(distractorKeys.filter(key => counts.get(key) > 1)); + return [...repeated].map(text => ({ code: ValidationError.DUPLICATE_DISTRACTOR_CONTENT, text })); +} From 007af1acc1141a43fb755e8ca243dbd8ee2c46d4 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 20 Oct 2026 14:03:04 +0300 Subject: [PATCH 04/10] Derive gap match identifiers instead of generating them The editor builds XML from its live state and never parses it again, so a random identifier for a missing or repeated id changed on every rebuild. A repeated id now takes the first free numbered suffix, and an answer without an id takes one from its gap. --- .../gapMatch/__tests__/parse.spec.js | 53 ++++++++++++++----- .../QTIEditor/interactions/gapMatch/parse.js | 43 +++++++++++---- 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js index 33ef3f1354..6935acae40 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js @@ -312,39 +312,68 @@ describe('buildXML()', () => { ]); }); - it('gives a repeated gap identifier a new one', () => { + it('gives a repeated gap identifier a numbered suffix', () => { const { root } = build( makeState({ passage: `

${gap('gap_a', 'choice_w', 'winter')} ${gap('gap_a', 'choice_s', 'spring')}

`, }), ); - const [first, second] = gapIdsOf(root); - expect(first).toBe('gap_a'); - expect(second).toMatch(/^gap_/); - expect(second).not.toBe('gap_a'); + expect(gapIdsOf(root)).toEqual(['gap_a', 'gap_a_2']); }); - it('gives an answer whose identifier is taken by other content a new one', () => { + it('gives an answer whose identifier is taken by other content a numbered suffix', () => { const { root } = build( makeState({ passage: `

${gap('gap_a', 'choice_w', 'winter')} ${gap('gap_b', 'choice_w', 'spring')}

`, distractors: [], }), ); - const [first, second] = gapTextsOf(root).map(text => text.id); - expect(first).toBe('choice_w'); - expect(second).toMatch(/^choice_/); - expect(second).not.toBe('choice_w'); + expect(gapTextsOf(root).map(text => text.id)).toEqual(['choice_w', 'choice_w_2']); }); - it('gives an answer without an identifier a new one', () => { + it('skips a numbered suffix that is already in use', () => { + const { root } = build( + makeState({ + passage: `

${gap('gap_a', 'choice_w', 'winter')} ${gap('gap_b', 'choice_w', 'spring')}

`, + distractors: [{ id: 'choice_w_2', content: 'summer' }], + }), + ); + expect(gapTextsOf(root).map(text => text.id)).toEqual([ + 'choice_w', + 'choice_w_3', + 'choice_w_2', + ]); + }); + + it('derives a missing answer identifier from its gap identifier', () => { const { root } = build( makeState({ passage: '

winter

', distractors: [], }), ); - expect(gapTextsOf(root)[0].id).toMatch(/^choice_/); + expect(gapTextsOf(root)[0].id).toBe('choice_a'); + }); + + it('keeps a distractor identifier that a derived answer identifier would take', () => { + const { root } = build( + makeState({ + passage: '

winter

', + distractors: [{ id: 'choice_a', content: 'summer' }], + }), + ); + expect(gapTextsOf(root).map(text => text.id)).toEqual(['choice_a_2', 'choice_a']); + }); + + it('writes the same XML each time it builds the same state', () => { + const state = makeState({ + passage: + `

winter ` + + `${gap('gap_b', 'choice_w', 'spring')} ${gap('gap_b', 'choice_w', 'autumn')}

`, + }); + expect(buildXML(state, QuestionType.GAP_MATCH, SCHEMA)).toEqual( + buildXML(state, QuestionType.GAP_MATCH, SCHEMA), + ); }); }); }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js index 66752f6ad2..159c3fa67e 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -91,25 +91,48 @@ export function parseGapMatchInteraction(bodyXml, responseDeclarations) { export function buildGapMatchInteractionXML(state, _questionType, declarationSchema) { const { responseIdentifier = RESPONSE_IDENTIFIER, prompt, passage, distractors = [] } = state; - const usedIds = new Set(); - function claim(id, prefix) { - const resolved = id && !usedIds.has(id) ? id : generateRandomSlug(prefix); - usedIds.add(resolved); - return resolved; - } - const passageEl = buildXmlNode({ tag: 'div', innerHTML: passage || '' }); const gapEls = [...passageEl.querySelectorAll(GAP)]; + const taken = new Set( + [ + ...gapEls.flatMap(el => [el.getAttribute('identifier'), el.getAttribute(ANSWER_ATTR)]), + ...distractors.map(({ id }) => id), + ].filter(Boolean), + ); + const claimed = new Set(); + function nextFree(base) { + let n = 2; + while (taken.has(`${base}_${n}`)) n++; + return `${base}_${n}`; + } + function claim(resolved) { + claimed.add(resolved); + taken.add(resolved); + return resolved; + } + function claimExisting(id, prefix) { + if (!id) return claim(generateRandomSlug(prefix)); + return claim(claimed.has(id) ? nextFree(id) : id); + } + function claimDerived(base) { + return claim(taken.has(base) ? nextFree(base) : base); + } + const idByAnswerKey = new Map(); const answers = new Map(); const correctPairs = []; for (const gapEl of gapEls) { - const gapId = claim(gapEl.getAttribute('identifier'), 'gap'); + const gapId = claimExisting(gapEl.getAttribute('identifier'), 'gap'); const content = getContentHTML(gapEl); if (hasRichTextContent(content)) { const key = richTextComparisonKey(content); - const answerId = idByAnswerKey.get(key) || claim(gapEl.getAttribute(ANSWER_ATTR), 'choice'); + const ownAnswerId = gapEl.getAttribute(ANSWER_ATTR); + const answerId = + idByAnswerKey.get(key) || + (ownAnswerId + ? claimExisting(ownAnswerId, 'choice') + : claimDerived(`choice_${gapId.replace(/^gap_/, '')}`)); idByAnswerKey.set(key, answerId); const answer = answers.get(answerId); @@ -127,7 +150,7 @@ export function buildGapMatchInteractionXML(state, _questionType, declarationSch } const distractorChoices = distractors.map(({ id, content }) => ({ - id: claim(id, 'choice'), + id: claimExisting(id, 'choice'), content, matchMax: 1, })); From 623b9bff679c5354e0fe01af1ca6f43df0bc1313 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 20 Oct 2026 14:04:35 +0300 Subject: [PATCH 05/10] Drop the gap match placement test and the defaultState export --- .../interactions/gapMatch/__tests__/Descriptor.spec.js | 6 +----- .../shared/views/QTIEditor/interactions/gapMatch/parse.js | 6 +++--- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js index 66da71c65b..85602a8af4 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js @@ -3,7 +3,7 @@ import { getGaps } from '../parse'; import { isSupportedItem, resolveDescriptor } from '../../resolveDescriptor'; import { parseItem } from '../../../serialization/parseItem'; import { parseXML } from '../../../serialization/xml'; -import { Placement, QtiInteraction, QuestionType } from '../../../constants'; +import { QtiInteraction, QuestionType } from '../../../constants'; import { qtiEditorStrings } from '../../../qtiEditorStrings'; const ITEM = ` @@ -29,10 +29,6 @@ const ITEM = ` const toEl = xml => parseXML(xml).documentElement; describe('gapMatchInteractionDescriptor', () => { - it('is a block interaction', () => { - expect(descriptor.placement).toBe(Placement.BLOCK); - }); - describe('matches', () => { it('matches the gap match interaction element', () => { expect(descriptor.matches(toEl(''))).toBe(true); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js index 159c3fa67e..0dae9e3b51 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -22,7 +22,7 @@ const GAP_TEXT = 'qti-gap-text'; const PROMPT = 'qti-prompt'; const ANSWER_ATTR = 'data-studio-answer'; -export function _defaultState() { +function defaultState() { return { responseIdentifier: RESPONSE_IDENTIFIER, prompt: '', passage: '', distractors: [] }; } @@ -36,13 +36,13 @@ export function getGaps(passage) { } export function parseGapMatchInteraction(bodyXml, responseDeclarations) { - if (!bodyXml) return _defaultState(); + if (!bodyXml) return defaultState(); let root; try { root = parseXML(bodyXml).documentElement; } catch { - return _defaultState(); + return defaultState(); } const contentNodes = [...root.childNodes].filter(isContentNode); From 0f2598283b33ebb9093df19dd91e909d28a2c579 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Tue, 20 Oct 2026 18:28:22 +0300 Subject: [PATCH 06/10] Test identifiers that the gap match build creates --- .../interactions/gapMatch/__tests__/parse.spec.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js index 6935acae40..ca85cf8304 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js @@ -365,7 +365,20 @@ describe('buildXML()', () => { expect(gapTextsOf(root).map(text => text.id)).toEqual(['choice_a_2', 'choice_a']); }); - it('writes the same XML each time it builds the same state', () => { + it('gives a gap and a distractor without an identifier new ones', () => { + const { root, declaration: decl } = build( + makeState({ + passage: '

winter

', + distractors: [{ content: 'summer' }], + }), + ); + const [gapId] = gapIdsOf(root); + expect(gapId).toMatch(/^gap_/); + expect(gapTextsOf(root)[1].id).toMatch(/^choice_/); + expect(decl.correctResponse).toEqual([['choice_w', gapId]]); + }); + + it('derives the same identifiers each time it builds the same state', () => { const state = makeState({ passage: `

winter ` + From e7cbccb2d4f31cd7af6510eaeec002f9001a0378 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Thu, 22 Oct 2026 12:06:49 +0300 Subject: [PATCH 07/10] Keep blank gap match items valid against the schema The schema needs at least one qti-gap-text and one passage block. A new question, or one whose gaps are all blank, had neither, so the server rejected the save. The build now writes a sentinel gap text, as inline choice does, and an empty paragraph for an empty passage. --- .../shared/views/QTIEditor/constants.js | 2 ++ .../gapMatch/__tests__/parse.spec.js | 32 ++++++++++++++++++- .../QTIEditor/interactions/gapMatch/parse.js | 25 ++++++++++++--- .../interactions/inlineChoice/parse.js | 2 +- 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js index f4b1253328..94ed070579 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js @@ -136,6 +136,8 @@ export const ValidationError = Object.freeze({ export const RESPONSE_IDENTIFIER = 'RESPONSE'; +export const SENTINEL_MARKER = 'data-studio-sentinel'; + /** * The QTI 3.0 standard response processing templates. Each one scores the response * variable named RESPONSE into SCORE, so it only fits an item whose one response diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js index ca85cf8304..c81f31b846 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js @@ -6,7 +6,7 @@ import { getGaps, parseGapMatchInteraction as parse, } from '../parse'; -import { BaseType, Cardinality, QuestionType } from '../../../constants'; +import { BaseType, Cardinality, QuestionType, SENTINEL_MARKER } from '../../../constants'; import { parseXML } from '../../../serialization/xml'; import { QTIDeclaration } from '../../../serialization/qti/QTIDeclaration'; @@ -410,6 +410,36 @@ describe('round trip', () => { expect(roundTrip(state)).toEqual(state); }); + describe('items that the schema needs filled', () => { + const sentinelsOf = root => root.querySelectorAll(`qti-gap-text[${SENTINEL_MARKER}]`); + const passageBlocksOf = root => [...root.children].filter(el => el.localName === 'p'); + + it('writes a sentinel gap text and an empty paragraph for a new question', () => { + const state = makeState({ prompt: '', passage: '', distractors: [] }); + const { root } = build(state); + expect(sentinelsOf(root)).toHaveLength(1); + expect(passageBlocksOf(root).map(p => p.childNodes.length)).toEqual([0]); + expect(roundTrip(state)).toEqual(state); + }); + + it('writes a sentinel gap text when the only gap is blank', () => { + const state = makeState({ + passage: `

Now is the ${blankGap('gap_a')} of our discontent

`, + distractors: [], + }); + expect(sentinelsOf(build(state).root)).toHaveLength(1); + expect(roundTrip(state)).toEqual(state); + }); + + it('writes an empty paragraph and no sentinel for an empty passage with a distractor', () => { + const state = makeState({ passage: '' }); + const { root } = build(state); + expect(sentinelsOf(root)).toHaveLength(0); + expect(passageBlocksOf(root).map(p => p.childNodes.length)).toEqual([0]); + expect(roundTrip(state)).toEqual(state); + }); + }); + it('reopens a blank gap with no answer', () => { const state = makeState({ passage: `

${gap('gap_a', 'choice_winter', 'winter')} ${blankGap('gap_b')}

`, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js index 0dae9e3b51..2f6fb3a575 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -11,7 +11,7 @@ import { } from '../../serialization/xml'; import { generateRandomSlug } from '../../utils/generateRandomSlug'; import { hasRichTextContent } from '../../utils/richText'; -import { RESPONSE_IDENTIFIER } from '../../constants'; +import { RESPONSE_IDENTIFIER, SENTINEL_MARKER } from '../../constants'; import { _extractCorrectPairIds } from '../associate/parse'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; @@ -21,6 +21,7 @@ const GAP = 'qti-gap'; const GAP_TEXT = 'qti-gap-text'; const PROMPT = 'qti-prompt'; const ANSWER_ATTR = 'data-studio-answer'; +const SENTINEL_ID = 'studio_sentinel'; function defaultState() { return { responseIdentifier: RESPONSE_IDENTIFIER, prompt: '', passage: '', distractors: [] }; @@ -46,8 +47,14 @@ export function parseGapMatchInteraction(bodyXml, responseDeclarations) { } const contentNodes = [...root.childNodes].filter(isContentNode); - const gapTextEls = contentNodes.filter(node => node.localName === GAP_TEXT); + const gapTextEls = contentNodes.filter( + node => node.localName === GAP_TEXT && !node.hasAttribute(SENTINEL_MARKER), + ); const passageNodes = contentNodes.filter(node => ![GAP_TEXT, PROMPT].includes(node.localName)); + const isEmptyParagraph = + passageNodes.length === 1 && + passageNodes[0].localName === 'p' && + !passageNodes[0].hasChildNodes(); const gapTextById = new Map(); for (const el of gapTextEls) { @@ -81,7 +88,7 @@ export function parseGapMatchInteraction(bodyXml, responseDeclarations) { return { responseIdentifier: root.getAttribute('response-identifier') || RESPONSE_IDENTIFIER, prompt: getPromptHTML(root), - passage: serializeAsHtml(passageNodes, root.namespaceURI), + passage: isEmptyParagraph ? '' : serializeAsHtml(passageNodes, root.namespaceURI), distractors: gapTextEls .filter(el => !paired.has(el)) .map(el => ({ id: el.getAttribute('identifier'), content: getContentHTML(el) })), @@ -168,7 +175,17 @@ export function buildGapMatchInteractionXML(state, _questionType, declarationSch }), ); } - children.push(...wrapInlineRuns(passageEl, 'p')); + if (!answers.size && !distractorChoices.length) { + children.push( + buildXmlNode({ + tag: GAP_TEXT, + attrs: { identifier: SENTINEL_ID, 'match-max': 1, [SENTINEL_MARKER]: '' }, + innerHTML: SENTINEL_ID, + }), + ); + } + const passageBlocks = wrapInlineRuns(passageEl, 'p'); + children.push(...(passageBlocks.length ? passageBlocks : [buildXmlNode({ tag: 'p' })])); const interactionEl = buildXmlNode({ tag: 'qti-gap-match-interaction', diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js index d5c84d10c0..ec9c782735 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/inlineChoice/parse.js @@ -2,13 +2,13 @@ import { QTIDeclaration } from '../../serialization/qti/QTIDeclaration'; import CorrectResponse from '../../serialization/qti/declarations/correctResponse'; import { buildXmlNode, parseXML, serializeAsHtml, wrapInlineRuns } from '../../serialization/xml'; import { generateRandomSlug } from '../../utils/generateRandomSlug'; +import { SENTINEL_MARKER } from '../../constants'; const serializer = new XMLSerializer(); export const DROPDOWN = 'qti-inline-choice-interaction'; export const OPTION = 'qti-inline-choice'; const PROMPT_MARKER = 'data-studio-prompt'; -const SENTINEL_MARKER = 'data-studio-sentinel'; export const CORRECT_ATTR = 'data-studio-correct'; const SENTINEL_ID = 'studio_sentinel'; const KEPT_DROPDOWN_ATTRS = ['response-identifier', CORRECT_ATTR]; From 81d11e73def5df4f10a1f77ea4ff0b294a3a45a7 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Thu, 22 Oct 2026 12:08:30 +0300 Subject: [PATCH 08/10] Open gap match items read-only when the build cannot keep them The build drops min-associations and match-group, and it moves a qti-gap-img into the passage, where it fails the schema and loses its pair. --- .../interactions/gapMatch/Descriptor.js | 11 ++++++ .../gapMatch/__tests__/Descriptor.spec.js | 34 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js index 77e190f43c..4bf6b19925 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/Descriptor.js @@ -1,4 +1,5 @@ import { QtiInteraction, QuestionType, BaseType, Cardinality } from '../../constants'; +import { contentChildrenOf } from '../../serialization/xml'; import { InteractionDescriptor } from '../InteractionDescriptor'; import { parseGapMatchInteraction, buildGapMatchInteractionXML } from './parse'; import { validateGapMatchInteraction } from './validation'; @@ -11,6 +12,16 @@ class GapMatchInteractionDescriptor extends InteractionDescriptor { }); } + /** @param {Element} bodyEl */ + isSupportedBody(bodyEl) { + if (!super.isSupportedBody(bodyEl)) return false; + const [interactionEl] = contentChildrenOf(bodyEl); + return ( + !interactionEl.hasAttribute('min-associations') && + !interactionEl.querySelector('qti-gap-img, [match-group]') + ); + } + getQuestionType() { return QuestionType.GAP_MATCH; } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js index 85602a8af4..c755c12e0d 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/Descriptor.spec.js @@ -54,6 +54,40 @@ describe('gapMatchInteractionDescriptor', () => { }); }); + describe('isSupportedBody', () => { + const isSupported = item => + descriptor.isSupportedBody(parseXML(parseItem(item).itemBodyXml).documentElement); + + it('supports an item that the build writes back unchanged', () => { + expect(isSupported(ITEM)).toBe(true); + }); + + it('does not support an item with an image gap text', () => { + const item = ITEM.replace( + 'A' + + ' { + const item = ITEM.replace( + '', + '', + ); + expect(isSupported(item)).toBe(false); + }); + + it('does not support an item with a minimum number of associations', () => { + const item = ITEM.replace( + 'max-associations="2"', + 'max-associations="2" min-associations="1"', + ); + expect(isSupported(item)).toBe(false); + }); + }); + it('parses a full item', () => { const [{ bodyXml, responseDeclarations }] = parseItem(ITEM).interactions; const state = descriptor.parse(bodyXml, responseDeclarations); From 98fecd6b419d92b93307db842905632361a3d6a6 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Thu, 22 Oct 2026 12:09:44 +0300 Subject: [PATCH 09/10] Read gap answers as the build does and rename the answer attribute getGaps read each answer with innerHTML, which keeps comments and scripts that the build drops, so validation and the build compared different strings. The gap attribute is now data-studio-correct, to match inline choice. --- .../interactions/gapMatch/__tests__/parse.spec.js | 11 +++++++++-- .../gapMatch/__tests__/validation.spec.js | 2 +- .../views/QTIEditor/interactions/gapMatch/parse.js | 12 ++++++------ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js index c81f31b846..c6dac9a1ef 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/parse.spec.js @@ -36,7 +36,7 @@ const BODY = interaction( const DECL = declaration('choice_winter gap_a', 'choice_spring gap_b'); const gap = (id, answerId, answer) => - `${answer}`; + `${answer}`; const blankGap = id => ``; function makeState(overrides = {}) { @@ -72,6 +72,13 @@ const roundTrip = state => { return parse(bodyXml, responseDeclarations); }; +describe('getGaps()', () => { + it('reads an answer as the build does, without comments or scripts', () => { + const passage = `

${gap('gap_a', 'choice_w', 'winter')}

`; + expect(getGaps(passage)[0].answer).toBe('winter'); + }); +}); + describe('parse()', () => { describe('fallbacks', () => { it('returns an empty state when bodyXml is empty', () => { @@ -368,7 +375,7 @@ describe('buildXML()', () => { it('gives a gap and a distractor without an identifier new ones', () => { const { root, declaration: decl } = build( makeState({ - passage: '

winter

', + passage: '

winter

', distractors: [{ content: 'summer' }], }), ); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js index 30d8f824d3..1f0d5b752d 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/__tests__/validation.spec.js @@ -3,7 +3,7 @@ import { ValidationError } from '../../../constants'; import { richTextComparisonKey } from 'shared/views/TipTapEditor/TipTapEditor/utils/comparisonKey'; const gap = (id, answer) => - `${answer}`; + `${answer}`; const distractor = (id, content) => ({ id, content }); function makeState(overrides = {}) { diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js index 2f6fb3a575..6e3f0e5cea 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -20,7 +20,7 @@ const serializer = new XMLSerializer(); const GAP = 'qti-gap'; const GAP_TEXT = 'qti-gap-text'; const PROMPT = 'qti-prompt'; -const ANSWER_ATTR = 'data-studio-answer'; +const CORRECT_ATTR = 'data-studio-correct'; const SENTINEL_ID = 'studio_sentinel'; function defaultState() { @@ -31,8 +31,8 @@ export function getGaps(passage) { const doc = parseXML(passage || '', 'text/html'); return [...doc.querySelectorAll(GAP)].map(el => ({ id: el.getAttribute('identifier'), - answerId: el.getAttribute(ANSWER_ATTR), - answer: el.innerHTML, + answerId: el.getAttribute(CORRECT_ATTR), + answer: getContentHTML(el), })); } @@ -79,7 +79,7 @@ export function parseGapMatchInteraction(bodyXml, responseDeclarations) { const gapTextEl = gapTextById.get(answerIdByGapId.get(originalId)); if (gapTextEl) { - gapEl.setAttribute(ANSWER_ATTR, gapTextEl.getAttribute('identifier')); + gapEl.setAttribute(CORRECT_ATTR, gapTextEl.getAttribute('identifier')); gapEl.append(...[...gapTextEl.childNodes].map(node => node.cloneNode(true))); paired.add(gapTextEl); } @@ -103,7 +103,7 @@ export function buildGapMatchInteractionXML(state, _questionType, declarationSch const taken = new Set( [ - ...gapEls.flatMap(el => [el.getAttribute('identifier'), el.getAttribute(ANSWER_ATTR)]), + ...gapEls.flatMap(el => [el.getAttribute('identifier'), el.getAttribute(CORRECT_ATTR)]), ...distractors.map(({ id }) => id), ].filter(Boolean), ); @@ -134,7 +134,7 @@ export function buildGapMatchInteractionXML(state, _questionType, declarationSch const content = getContentHTML(gapEl); if (hasRichTextContent(content)) { const key = richTextComparisonKey(content); - const ownAnswerId = gapEl.getAttribute(ANSWER_ATTR); + const ownAnswerId = gapEl.getAttribute(CORRECT_ATTR); const answerId = idByAnswerKey.get(key) || (ownAnswerId From 36c83562e64a1b0acd6a4ca58d3b2d96a42a1977 Mon Sep 17 00:00:00 2001 From: Samson Akol Date: Thu, 22 Oct 2026 12:11:15 +0300 Subject: [PATCH 10/10] Document the gap match state and the shared helpers --- .../QTIEditor/interactions/gapMatch/parse.js | 53 +++++++++++++++++++ .../views/QTIEditor/serialization/xml.js | 7 +-- .../views/QTIEditor/utils/distractors.js | 10 ++++ 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js index 6e3f0e5cea..ad5df9e584 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/gapMatch/parse.js @@ -23,10 +23,39 @@ const PROMPT = 'qti-prompt'; const CORRECT_ATTR = 'data-studio-correct'; const SENTINEL_ID = 'studio_sentinel'; +/** + * @typedef {object} GapMatchDistractor + * @property {string} id - QTI identifier of its + * @property {string} content - HTML content of its + */ + +/** + * @typedef {object} GapMatchState + * @property {string} responseIdentifier - Response identifier attribute + * @property {string} prompt - HTML content of ; '' when there is none + * @property {string} passage - HTML of the passage. Each holds its + * correct answer as content, and the answer's identifier in + * data-studio-correct. A blank gap has neither. + * @property {GapMatchDistractor[]} distractors - Gap texts that no gap takes + */ + +/** + * @typedef {object} GapMatchGap + * @property {string} id - QTI identifier of the + * @property {string|null} answerId - data-studio-correct; null when the gap has none + * @property {string} answer - HTML of the gap's correct answer; '' when blank + */ + function defaultState() { return { responseIdentifier: RESPONSE_IDENTIFIER, prompt: '', passage: '', distractors: [] }; } +/** + * Read every gap out of the passage, in document order. + * + * @param {string} passage - GapMatchState.passage + * @returns {GapMatchGap[]} + */ export function getGaps(passage) { const doc = parseXML(passage || '', 'text/html'); return [...doc.querySelectorAll(GAP)].map(el => ({ @@ -36,6 +65,17 @@ export function getGaps(passage) { })); } +/** + * Parse a into editor state. QTI keeps every empty + * and lists the answers as elements, paired to gaps in the correct + * response. The parse hydrates each paired gap: it copies the content of its gap text + * into the gap and sets data-studio-correct. A gap text that no gap takes becomes a + * distractor. The sentinel gap text that the build writes is dropped. + * + * @param {string} bodyXml + * @param {string[]} responseDeclarations + * @returns {GapMatchState} + */ export function parseGapMatchInteraction(bodyXml, responseDeclarations) { if (!bodyXml) return defaultState(); @@ -95,6 +135,19 @@ export function parseGapMatchInteraction(bodyXml, responseDeclarations) { }; } +/** + * Build the QTI for editor state. This reverses the parse: it takes each answer out of its + * hydrated gap, writes it as a , and leaves the gap empty, as QTI expects. + * Gaps with the same answer share one gap text. Missing or repeated identifiers are + * derived, so the same state always builds the same XML. If the schema needs a gap text + * or a passage block that the state does not give, the build writes a sentinel gap text + * or an empty paragraph. + * + * @param {GapMatchState} state + * @param {string} _questionType - unused; present to match the descriptor interface + * @param {{ baseType: string, cardinality: string }} declarationSchema + * @returns {{ bodyXml: string, responseDeclarations: string[] }} + */ export function buildGapMatchInteractionXML(state, _questionType, declarationSchema) { const { responseIdentifier = RESPONSE_IDENTIFIER, prompt, passage, distractors = [] } = state; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js index 16a79a6b59..61a64cca33 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js @@ -281,9 +281,10 @@ const BLOCK_TAGS = new Set([ /** * Wrap each run of top-level text and non-block elements (including TipTap's `` and - * ``, which the XSD only allows inside a block) in one `
`, so `` - * holds only blocks and every child can carry a marker attribute. TipTap unwraps a `
` on - * reopen, so this is stable across saves. Whitespace-only text is dropped. + * ``, which the XSD only allows inside a block) in one wrapper element, so + * `` holds only blocks and every child can carry a marker attribute. TipTap + * unwraps a `
` on reopen and keeps a `

` as a paragraph, so either wrapper is stable + * across saves. Whitespace-only text is dropped. * * @param {Element} container Changed in place: inline runs move out into the new `

`s * @param {string} [tag] The wrapper, for content the XSD allows only in some blocks: it diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js index c998617bf8..8ee26c5069 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/distractors.js @@ -1,5 +1,15 @@ import { ValidationError } from '../constants'; +/** + * A distractor must differ from every answer and from every other distractor. Answers can + * repeat each other, because one answer can be correct in more than one place. + * + * @param {Array<{ filled: boolean, key: string }>} readAnswers - The answers, each read as + * whether it has content and its comparison key + * @param {Array<{ filled: boolean, key: string }>} readDistractors - The distractors, read + * the same way + * @returns {Array<{ code: string, text: string }>} One error for each repeated distractor key + */ export function duplicateDistractorErrors(readAnswers, readDistractors) { const filledKeys = readChoices => readChoices.filter(({ filled }) => filled).map(({ key }) => key);