From 0f4e0d15fd4b792f4accab76afac65c1a39a9787 Mon Sep 17 00:00:00 2001 From: rtibblesbot Date: Wed, 7 Oct 2026 22:15:18 -0700 Subject: [PATCH] fix(qti): show items with content an edit would drop as read-only An item opens read-only when assembling it unedited would not write back every root attribute and child outside the body unchanged. Edits keep xsi:schemaLocation and the converter's root metadata. Like other read-only items, these skip the editor's validation rules. Co-Authored-By: Claude Opus 5.5 --- .../frontend/shared/views/QTIEditor/README.md | 2 +- .../QTIEditor/__tests__/validateItem.spec.js | 10 + .../__tests__/QTIItemEditor.spec.js | 26 +++ .../components/QTIItemEditor/index.vue | 14 +- .../views/QTIEditor/composables/useQtiItem.js | 2 + .../__tests__/assembleItem.spec.js | 210 ++++++++++++++++- .../__tests__/convertedItem.spec.js | 50 +++-- .../QTIEditor/serialization/assembleItem.js | 212 +++++++++++++++++- .../views/QTIEditor/serialization/xml.js | 12 +- .../views/QTIEditor/utils/testingFixtures.js | 44 ++++ .../shared/views/QTIEditor/validateItem.js | 15 +- 11 files changed, 573 insertions(+), 24 deletions(-) diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md index 95a7373557..009b76f6bc 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md @@ -7,7 +7,7 @@ Edits an exercise's assessment items stored as [QTI 3](https://www.imsglobal.org 2. `composables/useQtiItem.js` parses `raw_data` with `serialization/parseItem.js` into interaction blocks (`bodyXml` + `responseDeclarations`), hints and item metadata. 3. `components/InteractionSection` resolves each block's descriptor and question type with `composables/useInteractionDescriptor.js`. 4. The interaction's `Editor.vue` (`interactions/index.js`) edits a plain state object through `composables/useInteraction.js`: `descriptor.parse` → state → `descriptor.buildXML` → `descriptor.validate`. -5. `useQtiItem` rebuilds the full item with `serialization/assembleItem.js`; `QTIItemEditor` emits `update:rawData`. +5. `useQtiItem` rebuilds the full item with `serialization/assembleItem.js`, keeping `xsi:schemaLocation` and the converter's metadata from `raw_data`; `QTIItemEditor` emits `update:rawData`. `validateItem.js` `validateQtiItem` runs the same parse and validation headless, without the Vue editors. diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js index 58d645f9be..c167cb540e 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js @@ -9,6 +9,8 @@ import { CHOICE_ITEM_DOCUMENT_NO_PROMPT, CHOICE_ITEM_DOCUMENT_NO_CORRECT_ANSWER, CHOICE_ITEM_DOCUMENT_NO_CORRECT_ANSWER_WITH_STIMULUS, + CHOICE_ITEM_DOCUMENT_WITH_HINTS, + CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET, NO_INTERACTION_ITEM_DOCUMENT, INLINE_CHOICE_ITEM_DOCUMENT, VALID_MATCH_ITEM_DOCUMENT, @@ -53,6 +55,14 @@ describe('validateQtiItem', () => { expect(validateQtiItem(noPrompt, { allowFreeResponse: false })).toEqual([]); }); + it('does not apply editor rules to an item with content an edit would drop', () => { + const noPrompt = xml => xml.replace('Pick one.', ''); + expect(codesOf(validateQtiItem(noPrompt(CHOICE_ITEM_DOCUMENT_WITH_HINTS)))).toContain( + ValidationError.PROMPT_REQUIRED, + ); + expect(validateQtiItem(noPrompt(CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET))).toEqual([]); + }); + it('reports an item whose body holds no interaction', () => { expect(validateQtiItem(NO_INTERACTION_ITEM_DOCUMENT)).toEqual([ { code: ValidationError.NO_INTERACTION }, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/__tests__/QTIItemEditor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/__tests__/QTIItemEditor.spec.js index dbc33766d4..60328085c1 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/__tests__/QTIItemEditor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/__tests__/QTIItemEditor.spec.js @@ -14,6 +14,10 @@ import { FREE_RESPONSE_ITEM_DOCUMENT, NO_INTERACTION_ITEM_DOCUMENT, CHOICE_ITEM_DOCUMENT_WITH_HINTS, + CHOICE_ITEM_DOCUMENT_WITH_HINTS_AND_GLOSSARY, + CHOICE_ITEM_DOCUMENT_WITH_MODAL_FEEDBACK, + CHOICE_ITEM_DOCUMENT_WITH_SCHEMA_LOCATION, + CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET, VALID_ASSOCIATE_ITEM_DOCUMENT, VALID_MATCH_ITEM_DOCUMENT, MATCH_THREE_SETS_XML, @@ -25,6 +29,7 @@ import { UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT, INLINE_CHOICE_ITEM_DOCUMENT, } from '../../../utils/testingFixtures'; +import { XSI_NS } from '../../../serialization/xml'; jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor'); jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => { @@ -47,6 +52,7 @@ const { questionNumberAndTypeLabel$, unknownTypeLabel$, responsePoolLabel$, + deleteHintBtn$, } = qtiEditorStrings; const defaultProps = { @@ -252,6 +258,13 @@ describe('QTIItemEditor', () => { CHOICE_ITEM_DOCUMENT_WITH_HINTS_AND_STIMULUS, 'text sharing the text entry paragraph': TEXT_ENTRY_ITEM_DOCUMENT_SHARED_PARAGRAPH, 'content after the text entry paragraph': TEXT_ENTRY_ITEM_DOCUMENT_TRAILING_CONTENT, + 'a stylesheet': CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET, + 'a stylesheet and no prompt': CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET.replace( + 'Pick one.', + '', + ), + 'modal feedback': CHOICE_ITEM_DOCUMENT_WITH_MODAL_FEEDBACK, + 'a catalog beside the hints': CHOICE_ITEM_DOCUMENT_WITH_HINTS_AND_GLOSSARY, }; const documents = { ...publishableDocuments, @@ -532,6 +545,19 @@ describe('QTIItemEditor', () => { }); }); + test('keeps xsi:schemaLocation through an edit', async () => { + const { emitted } = renderComponent({ + item: { ...defaultProps.item, raw_data: CHOICE_ITEM_DOCUMENT_WITH_SCHEMA_LOCATION }, + mode: 'edit', + }); + await fireEvent.click(screen.getByRole('button', { name: hintsLabel$() })); + await fireEvent.click(screen.getAllByRole('button', { name: deleteHintBtn$() })[0]); + await nextTick(); + const [xml] = emitted()['update:rawData'].at(-1); + const root = new DOMParser().parseFromString(xml, 'text/xml').documentElement; + expect(root.getAttributeNS(XSI_NS, 'schemaLocation')).toContain('imsqti_asiv3p0p1_v1p0.xsd'); + }); + describe('associate interaction', () => { const renderAssociateItem = () => renderComponent({ diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/index.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/index.vue index db003f725d..c0498bdf76 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/index.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/index.vue @@ -115,8 +115,7 @@ import { qtiEditorStrings } from '../../qtiEditorStrings'; import { AssessmentItemTypes, QuestionType } from '../../constants'; import useQtiItem from '../../composables/useQtiItem'; - import { validateItemShape, validateQtiItem } from '../../validateItem'; - import { isSupportedItem } from '../../interactions/resolveDescriptor'; + import { isEditableItem, validateItemShape, validateQtiItem } from '../../validateItem'; import InteractionSection from '../InteractionSection/index.vue'; import HintsSection from '../HintsSection/index.vue'; @@ -159,12 +158,17 @@ /** * Whether this editor can edit the item's XML faithfully: it is readable, and it is * either blank or holds exactly one interaction this editor knows, in the body shape its - * builder writes. + * builder writes, and nothing outside the body that an edit would drop. */ - const isBlank = !props.item.raw_data; + const sourceXml = props.item.raw_data; const isEditableQti = computed( () => - !parseError.value && (isBlank || isSupportedItem(interactions.value, itemBodyXml.value)), + !parseError.value && + (!sourceXml || + isEditableItem( + { interactions: interactions.value, itemBodyXml: itemBodyXml.value }, + sourceXml, + )), ); /** diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js index 5ac183c70a..ac29d0de3d 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js @@ -64,6 +64,8 @@ export default function useQtiItem(rawXml, { bodyXml, responseDeclarations } = { bodyXml: bodyXml?.value ?? '', responseDeclarations: responseDeclarations?.value ?? [], hints: hints.value, + // parseXML throws on XML that failed to parse, which would break this computed. + sourceXml: parseError.value ? null : rawXml, }), ); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/assembleItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/assembleItem.spec.js index 8278c26045..d63a2028bf 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/assembleItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/assembleItem.spec.js @@ -1,12 +1,16 @@ // 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 { assembleItemXml } from '../assembleItem.js'; +import { assembleItemXml, keepsItemContent } from '../assembleItem.js'; import { parseItem } from '../parseItem.js'; import { buildXmlNode, parseXML } from '../xml.js'; import { normalizeXML } from '../qti/__tests__/testUtils.js'; import { ResponseProcessingTemplate } from '../../constants'; -import { MULTI_TEXT_ENTRY_ITEM_DOCUMENT } from '../../utils/testingFixtures'; +import { + CHOICE_ITEM_DOCUMENT_WITH_HINTS, + CHOICE_ITEM_DOCUMENT_WITH_SCHEMA_LOCATION, + MULTI_TEXT_ENTRY_ITEM_DOCUMENT, +} from '../../utils/testingFixtures'; const serializer = new XMLSerializer(); @@ -360,4 +364,206 @@ describe('assembleItemXml', () => { expect(parseXML(xml).querySelectorAll('qti-response-processing')).toHaveLength(1); }); }); + + describe('with the source item', () => { + const sourceXml = CHOICE_ITEM_DOCUMENT_WITH_SCHEMA_LOCATION; + const reassemble = (params = {}) => { + const item = parseItem(sourceXml); + return assembleItemXml({ + identifier: item.identifier, + title: item.title, + language: item.language, + bodyXml: item.interactions[0].bodyXml, + responseDeclarations: item.interactions[0].responseDeclarations, + hints: item.hints, + sourceXml, + ...params, + }); + }; + + it('keeps no source prefix declaration that its own elements would take', () => { + const xml = reassemble({ + sourceXml: sourceXml.replace( + 'xmlns:xsi=', + 'xmlns:qti="http://www.imsglobal.org/xsd/imsqtiasi_v3p0"\n xmlns:xsi=', + ), + }); + expect(xml).not.toContain('qti:'); + expect(xml).toContain('xsi:schemaLocation'); + expect(parseItem(xml).interactions).toHaveLength(1); + }); + + it('writes its own root attributes over the source ones', () => { + const root = parseXML(reassemble({ identifier: 'renamed', language: '' })).documentElement; + expect(root.getAttribute('identifier')).toBe('renamed'); + expect(root.hasAttribute('xml:lang')).toBe(false); + }); + }); +}); + +describe('keepsItemContent', () => { + const withRoot = attrs => + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('xml:lang="en"', `xml:lang="en" ${attrs}`); + const SCORE_DECLARATION = + ''; + const withScoring = (outcomeDeclaration, responseProcessing = '') => + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '\n\n ', + `\n ${outcomeDeclaration}\n `, + ).replace('\n', `\n ${responseProcessing}\n`); + const withDefaultScore = (value, identifier = 'SCORE') => + withScoring( + ` + ${value} + `, + ); + const withTemplate = attrs => + withScoring(SCORE_DECLARATION, ``); + const withHintCard = card => + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '', + `\n ${card}`, + ); + const EMPTY_HINT_CARD = ''; + + it.each([ + ['hints', CHOICE_ITEM_DOCUMENT_WITH_HINTS], + ['xsi:schemaLocation', CHOICE_ITEM_DOCUMENT_WITH_SCHEMA_LOCATION], + ['converter metadata', withRoot('label="L" tool-name="kolibri" tool-version="0.1"')], + [ + 'scoring the editor regenerates', + withTemplate(`template="${ResponseProcessingTemplate.MATCH_CORRECT}"`), + ], + [ + 'a standard template with no .xml', + withTemplate('template="https://purl.imsglobal.org/spec/qti/v3p0/rptemplates/match_correct"'), + ], + [ + 'a standard template in its imsglobal.org form', + withTemplate( + 'template="http://www.imsglobal.org/question/qti_v3p0/rptemplates/match_correct"', + ), + ], + [ + 'a standard template by template-location', + withTemplate(`template-location="${ResponseProcessingTemplate.MATCH_CORRECT}"`), + ], + [ + 'a standard template other than the one the editor writes', + withTemplate(`template="${ResponseProcessingTemplate.MAP_RESPONSE}"`), + ], + ['a zero default SCORE', withDefaultScore(0)], + ['a zero default RAW_SCORE that one response leaves unread', withDefaultScore(0, 'RAW_SCORE')], + [ + 'math in a hint, in the namespace the converter leaves it', + withHintCard( + '

x

', + ), + ], + [ + 'a pretty-printed bare-text hint', + withHintCard(` + + Try halving it first + + `), + ], + ...[ + ['a table', '
a
'], + ['CDATA', '

'], + ['a data attribute', '

a

'], + ['a div in a paragraph', '

a

'], + ].map(([name, html]) => [ + `${name} in a hint, which the hint editor rewrites`, + withHintCard( + `${html}`, + ), + ]), + ['an empty hint card', withHintCard(EMPTY_HINT_CARD)], + [ + 'only empty hint cards', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + /[^]*<\/qti-catalog>/, + `${EMPTY_HINT_CARD}`, + ), + ], + ])('is true for an item with %s', (_, xml) => { + expect(keepsItemContent(xml)).toBe(true); + }); + + it.each([ + [ + 'hint cards in a catalog the body references by another id', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '', + '', + ).replace('', 'Term'), + ], + [ + 'a non-hint card in the hint catalog', + withHintCard( + '

Term

', + ), + ], + [ + 'a hint card in two languages', + withHintCard(` +

Hi

 +

Hola

 + `), + ], + [ + 'a hint card pointing at a file', + withHintCard( + 'hint.html', + ), + ], + [ + 'a response declaration no interaction uses', + withScoring( + '', + ), + ], + ['an unknown root attribute', withRoot('foo="bar"')], + [ + 'adaptive="true"', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('adaptive="false"', 'adaptive="true"'), + ], + [ + 'a language on a hint card', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replaceAll( + '', + '', + ), + ], + [ + 'a language on hint content', + withHintCard( + '

Hola

', + ), + ], + [ + 'a score maximum', + withScoring( + '', + ), + ], + ['a non-zero default SCORE', withDefaultScore(1)], + ['a non-zero default RAW_SCORE', withDefaultScore(1, 'RAW_SCORE')], + [ + 'response processing the editor does not write', + withScoring( + '', + ` + 2 + `, + ), + ], + [ + 'a template that is not a standard one', + withTemplate('template="https://example.com/rptemplates/match_correct.xml"'), + ], + ])('is false for an item with %s', (_, xml) => { + expect(keepsItemContent(xml)).toBe(false); + }); }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js index 9881ac94e1..7d6793c221 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/convertedItem.spec.js @@ -12,32 +12,50 @@ import fs from 'fs'; import path from 'path'; import { parseItem } from '../parseItem'; -import { assembleItemXml } from '../assembleItem'; +import { assembleItemXml, keepsItemContent } from '../assembleItem'; import { isSupportedItem } from '../../interactions/resolveDescriptor'; +import { XSI_NS } from '../xml'; const FIXTURES = path.join(__dirname, '../../../../../../tests/utils/qti/fixtures'); const read = name => fs.readFileSync(path.join(FIXTURES, `${name}.xml`), 'utf8'); -const rebuild = item => - assembleItemXml({ +const rebuild = original => { + const item = parseItem(original); + return assembleItemXml({ identifier: item.identifier, title: item.title, language: item.language, bodyXml: item.interactions[0].bodyXml, responseDeclarations: item.interactions[0].responseDeclarations, hints: item.hints, + sourceXml: original, }); +}; + +const FIXTURE_NAMES = fs + .readdirSync(FIXTURES) + .filter(file => file.endsWith('.xml')) + .map(file => path.basename(file, '.xml')); describe('converted items the editor opens', () => { - it.each( - fs - .readdirSync(FIXTURES) - .filter(file => file.endsWith('.xml')) - .map(file => path.basename(file, '.xml')), - )('%s', name => { - const { interactions, itemBodyXml } = parseItem(read(name)); + it.each(FIXTURE_NAMES)('%s', name => { + const xml = read(name); + const { interactions, itemBodyXml } = parseItem(xml); expect(isSupportedItem(interactions, itemBodyXml)).toBe(true); + expect(keepsItemContent(xml)).toBe(true); + }); + + // Before 90b0a08b1 the converter wrote every template as match_correct with no .xml. + it.each(FIXTURE_NAMES)('%s, as converted before 90b0a08b1', name => { + const stored = read(name).replace(/rptemplates\/\w+\.xml/, 'rptemplates/match_correct'); + expect(keepsItemContent(stored)).toBe(true); + }); + + // Before c51c5e035 the converter wrote the root language as `language`. + it.each(FIXTURE_NAMES)('%s, as converted before c51c5e035', name => { + const stored = read(name).replace(' xml:lang="', ' language="'); + expect(keepsItemContent(stored)).toBe(true); }); }); @@ -49,13 +67,13 @@ describe('a converted single-selection item', () => { }); it('writes the language back as xml:lang, the attribute QTI declares', () => { - const xml = rebuild(parseItem(original)); + const xml = rebuild(original); expect(xml).toContain('xml:lang="en-US"'); expect(xml).not.toContain(' language="'); }); it('keeps its scoring outcome and response processing', () => { - const xml = rebuild(parseItem(original)); + const xml = rebuild(original); const doc = new DOMParser().parseFromString(xml, 'text/xml'); expect(doc.querySelector('parsererror')).toBeNull(); expect(doc.querySelector('qti-outcome-declaration').getAttribute('identifier')).toBe('SCORE'); @@ -64,8 +82,14 @@ describe('a converted single-selection item', () => { ); }); + it('keeps the root attributes the editor does not write', () => { + const root = new DOMParser().parseFromString(rebuild(original), 'text/xml').documentElement; + expect(root.getAttributeNS(XSI_NS, 'schemaLocation')).toContain('imsqti_asiv3p0p1_v1p0.xsd'); + expect(root.getAttribute('tool-name')).toBe('kolibri'); + }); + it('puts the children in the order the schema fixes', () => { - const xml = rebuild(parseItem(original)); + const xml = rebuild(original); const order = [...xml.matchAll(/<(qti-[a-z-]+)/g)] .map(m => m[1]) .filter(tag => diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/assembleItem.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/assembleItem.js index cee17841e5..b129e536bb 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/assembleItem.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/assembleItem.js @@ -3,9 +3,19 @@ * body, hints, and the outcome declarations and response processing that score it. */ +import { ResponseProcessingTemplate } from '../constants'; import { HINT_CATALOG_ID, HINT_SUPPORT, hintHasContent } from './hints'; +import { parseItem } from './parseItem'; import { QTIDeclaration } from './qti/QTIDeclaration'; -import { buildFloatNode, buildXmlNode, parseXML } from './xml'; +import { + XSI_NS, + attributesOf, + buildFloatNode, + buildXmlNode, + hasNonNamespaceAttributes, + isContentNode, + parseXML, +} from './xml'; const serializer = new XMLSerializer(); @@ -153,6 +163,192 @@ function buildScoringNodes(declNodes) { }; } +/** Root metadata the converter writes and nothing in the item points at. */ +const METADATA_ROOT_ATTRIBUTES = ['label', 'tool-name', 'tool-version', 'language']; + +/** + * Whether an edit copies this source root attribute, which the editor doesn't write. + * + * @param {Attr} attr + * @returns {boolean} + */ +function isCarriedRootAttribute(attr) { + return ( + (attr.namespaceURI === XSI_NS && attr.localName === 'schemaLocation') || + (!attr.namespaceURI && METADATA_ROOT_ATTRIBUTES.includes(attr.localName)) + ); +} + +/** @returns {Node[]} The node's children, less whitespace-only text */ +function contentChildrenOf(node) { + return [...node.childNodes].filter(isContentNode); +} + +/** + * Whether this is a hint card in the shape the editor writes, whose content the hint editor + * owns. + * + * @param {Element} card + * @returns {boolean} + */ +function isHintCard(card) { + const attrs = attributesOf(card); + const [content, ...rest] = contentChildrenOf(card); + return ( + attrs.length === 1 && + attrs[0].name === 'support' && + attrs[0].value === HINT_SUPPORT && + !rest.length && + (!content || (content.localName === 'qti-html-content' && !hasNonNamespaceAttributes(content))) + ); +} + +/** + * Whether an edit drops nothing the editor doesn't own by not writing this node as it is: + * - a hint card; + * - the hint catalog, holding only hint cards; + * - a zero default SCORE or RAW_SCORE, which every processing the editor writes sets first; + * - a RAW_SCORE declared as the editor declares it; the editor redeclares it wherever it reads it. + * + * @param {Node} node + * @returns {boolean} + */ +function holdsNothing(node) { + switch (node.localName) { + case 'qti-card': + return isHintCard(node); + case 'qti-catalog': + return ( + attributesOf(node).length === 1 && + node.getAttribute('id') === HINT_CATALOG_ID && + contentChildrenOf(node).every(holdsNothing) + ); + case 'qti-catalog-info': + return contentChildrenOf(node).every(holdsNothing); + case 'qti-outcome-declaration': + return ( + attributesOf(node).length === 3 && + node.getAttribute('identifier') === RAW_SCORE && + node.getAttribute('cardinality') === 'single' && + node.getAttribute('base-type') === 'float' && + contentChildrenOf(node).every(holdsNothing) + ); + case 'qti-default-value': + return ( + node.parentNode.localName === 'qti-outcome-declaration' && + [SCORE, RAW_SCORE].includes(node.parentNode.getAttribute('identifier')) && + node.children.length === 1 && + parseFloat(node.textContent) === 0 + ); + default: + return false; + } +} + +/** + * @param {Node} a + * @param {Node} b + * @returns {boolean} Whether two nodes hold the same content + */ +function isSameContent(a, b) { + if (a.nodeType !== Node.ELEMENT_NODE || b.nodeType !== Node.ELEMENT_NODE) { + return a.nodeType === b.nodeType && a.nodeValue === b.nodeValue; + } + const attrs = attributesOf(a); + const children = contentChildrenOf(a).filter(child => !holdsNothing(child)); + const otherChildren = contentChildrenOf(b).filter(child => !holdsNothing(child)); + return ( + a.namespaceURI === b.namespaceURI && + a.localName === b.localName && + attrs.length === attributesOf(b).length && + attrs.every(attr => b.getAttributeNS(attr.namespaceURI, attr.localName) === attr.value) && + children.length === otherChildren.length && + children.every((child, i) => isSameContent(child, otherChildren[i])) + ); +} + +/** Every spelling of a standard template that Kolibri resolves. */ +const STANDARD_TEMPLATE_URIS = new Set( + Object.values(ResponseProcessingTemplate).flatMap(uri => { + const withoutSuffix = uri.replace(/\.xml$/, ''); + const name = withoutSuffix.split('/').pop(); + return [uri, withoutSuffix, `http://www.imsglobal.org/question/qti_v3p0/rptemplates/${name}`]; + }), +); + +/** + * @param {Element} el + * @returns {boolean} Whether this is response processing by a standard template alone + */ +function isStandardTemplate(el) { + const attrs = attributesOf(el); + return ( + el.localName === 'qti-response-processing' && + attrs.length === 1 && + ['template', 'template-location'].includes(attrs[0].localName) && + STANDARD_TEMPLATE_URIS.has(attrs[0].value) && + !el.children.length + ); +} + +/** + * @param {string} sourceXml + * @returns {boolean} Whether assembling the item unedited writes back each source root + * attribute and child as it was + */ +function writesBackItemContent(sourceXml) { + const item = parseItem(sourceXml); + const [interaction] = item.interactions; + const source = parseXML(sourceXml).documentElement; + const written = parseXML( + assembleItemXml({ + identifier: item.identifier, + title: item.title, + language: item.language, + bodyXml: interaction.bodyXml, + responseDeclarations: interaction.responseDeclarations, + hints: item.hints, + sourceXml, + }), + ).documentElement; + return ( + attributesOf(source).every( + attr => written.getAttributeNS(attr.namespaceURI, attr.localName) === attr.value, + ) && + [...source.children].every( + el => + el.localName === 'qti-item-body' || + holdsNothing(el) || + // Response processing is regenerated on every save, and a standard template is + // all of what it was. + isStandardTemplate(el) || + [...written.children].some(writtenEl => isSameContent(el, writtenEl)), + ) + ); +} + +/** Verdicts by source XML; each card and the validation getter ask about the same item. */ +const keepsItemContentCache = new Map(); +const KEEPS_ITEM_CONTENT_CACHE_SIZE = 500; + +/** + * Whether an edit keeps everything in this supported item outside its body. The editor shows + * any other item read-only: what an edit drops or rewrites could leave the rest pointing at + * nothing. + * + * @param {string} sourceXml + * @returns {boolean} + */ +export function keepsItemContent(sourceXml) { + if (!keepsItemContentCache.has(sourceXml)) { + if (keepsItemContentCache.size >= KEEPS_ITEM_CONTENT_CACHE_SIZE) { + keepsItemContentCache.delete(keepsItemContentCache.keys().next().value); + } + keepsItemContentCache.set(sourceXml, writesBackItemContent(sourceXml)); + } + return keepsItemContentCache.get(sourceXml); +} + /** * Assembles a full QTI assessment-item XML string from its constituent parts. * @@ -168,6 +364,8 @@ function buildScoringNodes(declNodes) { * @param {string} params.bodyXml - Serialized interaction element XML string * @param {string[]} params.responseDeclarations - Array of serialized declaration XML strings * @param {Array<{ content: string }>} [params.hints] - Item hints, in order + * @param {string} [params.sourceXml] - The item's XML before this edit, whose + * `xsi:schemaLocation` and converter metadata attributes are kept * @returns {string} Full QTI XML string */ export function assembleItemXml({ @@ -177,6 +375,7 @@ export function assembleItemXml({ bodyXml, responseDeclarations, hints = [], + sourceXml, }) { // Parse each serialized declaration string back into a DOM node so it can be // adopted into the assessment item tree via buildXmlNode's importNode logic. @@ -221,6 +420,17 @@ export function assembleItemXml({ ...(responseProcessing ? [responseProcessing] : []), ], }); + const sourceAttrs = sourceXml ? [...parseXML(sourceXml).documentElement.attributes] : []; + const carriedAttrs = sourceAttrs.filter(isCarriedRootAttribute); + // A root prefix declaration would make the serializer prefix the editor's own elements + // (qti:qti-item-body), so only those a carried attribute uses are copied. + const usedPrefixes = new Set(carriedAttrs.map(attr => attr.prefix)); + const usedPrefixDeclarations = sourceAttrs.filter( + attr => attr.prefix === 'xmlns' && usedPrefixes.has(attr.localName), + ); + for (const attr of [...usedPrefixDeclarations, ...carriedAttrs]) { + assessmentItemNode.setAttributeNS(attr.namespaceURI, attr.name, attr.value); + } return `\n${serializer.serializeToString(assessmentItemNode)}`; } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js index b5223546b4..08e8721b12 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js @@ -133,6 +133,16 @@ function adoptNode(node, doc = xmlDoc, plainNamespace = XHTML_NS) { const XMLNS_NS = 'http://www.w3.org/2000/xmlns/'; +export const XSI_NS = 'http://www.w3.org/2001/XMLSchema-instance'; + +/** + * @param {Element} el + * @returns {Attr[]} The element's attributes, less namespace declarations + */ +export function attributesOf(el) { + return [...el.attributes].filter(attr => attr.namespaceURI !== XMLNS_NS); +} + /** * Whether an element has attributes beyond namespace declarations. * @@ -140,7 +150,7 @@ const XMLNS_NS = 'http://www.w3.org/2000/xmlns/'; * @returns {boolean} */ export function hasNonNamespaceAttributes(el) { - return [...el.attributes].some(attr => attr.namespaceURI !== XMLNS_NS); + return attributesOf(el).length > 0; } /** diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js index 4096e3684e..fa332e0f8a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js @@ -2,6 +2,7 @@ import { QTI_INTERACTION_TAGS } from '../constants'; import { registry } from '../interactions/descriptors'; +import { XSI_NS } from '../serialization/xml'; export const CHOICE_SINGLE_SELECT_XML = ` Which planet is closest to the Sun? @@ -333,6 +334,49 @@ export const CHOICE_ITEM_DOCUMENT_WITH_HINTS_AND_STIMULUS = CHOICE_ITEM_DOCUMENT '

Read the passage.

', ); +/** A glossary catalog beside the hints, which the prompt points at. */ +export const CHOICE_ITEM_DOCUMENT_WITH_HINTS_AND_GLOSSARY = CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + 'Pick one.', + 'Pick one.', +).replace( + ' ', + ` + +

Exactly one.

 + + + `, +); + +export const CHOICE_ITEM_DOCUMENT_WITH_SCHEMA_LOCATION = CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + 'xml:lang="en"\n>', + `xml:lang="en" + xmlns:xsi="${XSI_NS}" + xsi:schemaLocation="http://www.imsglobal.org/xsd/imsqtiasi_v3p0 https://purl.imsglobal.org/spec/qti/v3p0/schema/xsd/imsqti_asiv3p0p1_v1p0.xsd" +>`, +); + +export const CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET = CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '\n\n ', + ` + + `, +); + +export const CHOICE_ITEM_DOCUMENT_WITH_MODAL_FEEDBACK = CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '\n\n ', + ` + + `, +).replace( + '\n', + ` + +

Well done.

 + +`, +); + export const VALID_ASSOCIATE_ITEM_DOCUMENT = ` error).map(({ error }) => ({ code: error })); }