diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/README.md index 95a7373557..fed169cce9 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`; `QTIItemEditor` emits `update:rawData`. `validateItem.js` `isEditableItem` shows read-only any item holding content outside the body that this rebuild doesn't write or regenerate. `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..382b9f78e5 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js @@ -1,4 +1,4 @@ -import { validateItemShape, validateQtiItem } from '../validateItem'; +import { isEditableItem, validateItemShape, validateQtiItem } from '../validateItem'; import { QuestionType, ValidationError } from '../constants'; import { assembleItemXml } from '../serialization/assembleItem'; import { parseItem } from '../serialization/parseItem'; @@ -9,6 +9,9 @@ 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_SCHEMA_LOCATION, + STYLESHEET_ITEM_DOCUMENT_NO_PROMPT, NO_INTERACTION_ITEM_DOCUMENT, INLINE_CHOICE_ITEM_DOCUMENT, VALID_MATCH_ITEM_DOCUMENT, @@ -53,6 +56,25 @@ describe('validateQtiItem', () => { expect(validateQtiItem(noPrompt, { allowFreeResponse: false })).toEqual([]); }); + it('applies editor rules to an item with content an edit would drop', () => { + expect(codesOf(validateQtiItem(STYLESHEET_ITEM_DOCUMENT_NO_PROMPT))).toContain( + ValidationError.PROMPT_REQUIRED, + ); + const templatedAnswer = CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + /[\s\S]*<\/qti-correct-response>/, + '', + ).replace( + ' ', + ` + + choice-a + + + `, + ); + expect(codesOf(validateQtiItem(templatedAnswer))).toContain(ValidationError.NO_CORRECT_ANSWER); + }); + it('reports an item whose body holds no interaction', () => { expect(validateQtiItem(NO_INTERACTION_ITEM_DOCUMENT)).toEqual([ { code: ValidationError.NO_INTERACTION }, @@ -247,6 +269,185 @@ describe('validateQtiItem', () => { }); }); +describe('isEditableItem', () => { + const isEditable = xml => isEditableItem(parseItem(xml), xml); + const withRoot = attrs => + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('xml:lang="en"', `xml:lang="en" ${attrs}`); + const withScoring = (outcomeDeclarations, responseProcessing = '') => + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '\n\n ', + `\n ${outcomeDeclarations}\n `, + ).replace('\n', `\n ${responseProcessing}\n`); + const FEEDBACK_OUTCOME = + ''; + const withHintCard = card => + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '', + `\n ${card}`, + ); + + it.each([ + ['hints', CHOICE_ITEM_DOCUMENT_WITH_HINTS], + ['xsi:schemaLocation', CHOICE_ITEM_DOCUMENT_WITH_SCHEMA_LOCATION], + ['converter metadata', withRoot('label="L" tool-name="other" tool-version="2"')], + ['adaptive="0"', CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('adaptive="false"', 'adaptive="0"')], + [ + 'time-dependent="0"', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('time-dependent="false"', 'time-dependent="0"'), + ], + [ + 'explicit match_correct rules', + withScoring( + '', + ` + + + + 1 + + + `, + ), + ], + [ + 'a non-zero default SCORE', + withScoring(` + 1 + `), + ], + [ + 'a score maximum and a MAXSCORE outcome', + withScoring( + ` + `, + ), + ], + [ + 'a response declaration no interaction uses', + withScoring( + '', + ), + ], + [ + 'a pretty-printed bare-text hint', + withHintCard(` + + Try halving it first + + `), + ], + [ + 'an empty hint card', + withHintCard(''), + ], + ])('is true for an item with %s', (_, xml) => { + expect(isEditable(xml)).toBe(true); + }); + + it.each([ + [ + 'hint cards in a catalog by another id', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '', + '', + ), + ], + [ + 'an attribute on the catalog info', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '', + '', + ), + ], + [ + '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 language on a hint card', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replaceAll( + '', + '', + ), + ], + [ + 'a language on hint content', + withHintCard( + '

Hola

', + ), + ], + ['an unknown root attribute', withRoot('foo="bar"')], + ['a root attribute in another namespace', withRoot('xmlns:x="urn:x" x:label="L"')], + [ + 'adaptive="true"', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('adaptive="false"', 'adaptive="true"'), + ], + [ + 'time-dependent="true"', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('time-dependent="false"', 'time-dependent="true"'), + ], + [ + 'a known child in another namespace', + withScoring(''), + ], + ['text outside the body', withScoring('Stray text')], + [ + 'a root language xml:lang does not accept', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace('xml:lang="en"', 'language="en_US"'), + ], + [ + 'feedback on an outcome an edit drops', + withScoring(FEEDBACK_OUTCOME).replace( + '>A', + '>AYes', + ), + ], + [ + 'a printed outcome an edit drops', + withScoring(FEEDBACK_OUTCOME).replace( + 'Pick one.', + 'Pick one. ', + ), + ], + [ + 'a printed SCORE', + withScoring( + '', + ).replace('Pick one.', 'Pick one. '), + ], + [ + 'feedback on SCORE', + CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '>A', + '>AYes', + ), + ], + [ + 'a hint card in another namespace', + withHintCard( + '

hidden

', + ), + ], + ])('is false for an item with %s', (_, xml) => { + expect(isEditable(xml)).toBe(false); + }); +}); + // What the editor asks about an item it is already showing, which is everything an // interaction cannot answer for itself. describe('validateItemShape', () => { 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..691a74bfb3 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,11 @@ 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, + STYLESHEET_ITEM_DOCUMENT_NO_PROMPT, VALID_ASSOCIATE_ITEM_DOCUMENT, VALID_MATCH_ITEM_DOCUMENT, MATCH_THREE_SETS_XML, @@ -25,6 +30,7 @@ import { UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT, INLINE_CHOICE_ITEM_DOCUMENT, } from '../../../utils/testingFixtures'; +import { QTI_SCHEMA_LOCATION, XSI_NS } from '../../../serialization/xml'; jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor'); jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => { @@ -47,6 +53,7 @@ const { questionNumberAndTypeLabel$, unknownTypeLabel$, responsePoolLabel$, + deleteHintBtn$, } = qtiEditorStrings; const defaultProps = { @@ -252,10 +259,14 @@ 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, + 'modal feedback': CHOICE_ITEM_DOCUMENT_WITH_MODAL_FEEDBACK, + 'a catalog beside the hints': CHOICE_ITEM_DOCUMENT_WITH_HINTS_AND_GLOSSARY, }; const documents = { ...publishableDocuments, 'no interaction': NO_INTERACTION_ITEM_DOCUMENT, + 'a stylesheet and no prompt': STYLESHEET_ITEM_DOCUMENT_NO_PROMPT, }; test.each(Object.entries(publishableDocuments))( @@ -308,6 +319,7 @@ describe('QTIItemEditor', () => { 'a match interaction that cannot be read', VALID_MATCH_ITEM_DOCUMENT.replace(MATCH_XML, MATCH_THREE_SETS_XML), ], + ['a stylesheet and no prompt', STYLESHEET_ITEM_DOCUMENT_NO_PROMPT], ])( 'asks the author to delete a question with %s instead of only saying it cannot be edited', (_, raw_data) => { @@ -532,6 +544,37 @@ 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')).toBe(QTI_SCHEMA_LOCATION); + }); + + test('keeps the label through an edit', async () => { + const { emitted } = renderComponent({ + item: { + ...defaultProps.item, + raw_data: CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + 'xml:lang="en"', + 'xml:lang="en" label="L"', + ), + }, + 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); + expect(xml).toContain(' label="L"'); + }); + 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..be848c42f9 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js @@ -27,6 +27,7 @@ import { assembleItemXml } from '../serialization/assembleItem'; export default function useQtiItem(rawXml, { bodyXml, responseDeclarations } = {}) { const identifier = ref(''); const title = ref(''); + const label = ref(''); const language = ref(''); const itemBodyXml = ref(''); const interactions = ref([]); @@ -42,6 +43,7 @@ export default function useQtiItem(rawXml, { bodyXml, responseDeclarations } = { const model = parseItem(rawXml); identifier.value = model.identifier; title.value = model.title; + label.value = model.label; language.value = model.language; itemBodyXml.value = model.itemBodyXml; interactions.value = model.interactions; @@ -60,6 +62,7 @@ export default function useQtiItem(rawXml, { bodyXml, responseDeclarations } = { assembleItemXml({ identifier: identifier.value, title: title.value, + label: label.value, language: language.value, bodyXml: bodyXml?.value ?? '', responseDeclarations: responseDeclarations?.value ?? [], diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/InteractionDescriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/InteractionDescriptor.js index 3cce104b50..2baee54b36 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/InteractionDescriptor.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/InteractionDescriptor.js @@ -11,7 +11,7 @@ */ import { Placement } from '../constants'; -import { isContentNode } from '../serialization/xml'; +import { contentChildrenOf } from '../serialization/xml'; /** * Methods a subclass has to implement. `matches` and `getTypeOptions` are not listed @@ -78,7 +78,7 @@ export class InteractionDescriptor { * @returns {boolean} */ isSupportedBody(bodyEl) { - const content = [...bodyEl.childNodes].filter(isContentNode); + const content = contentChildrenOf(bodyEl); return content.length === 1 && this.matches(content[0]); } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js index 48e025faee..ccdc053163 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/textEntry/parse.js @@ -1,6 +1,7 @@ import { QTIDeclaration } from '../../serialization/qti/QTIDeclaration'; import { buildXmlNode, + contentChildrenOf, hasNonNamespaceAttributes, isContentNode, parseXML, @@ -51,10 +52,6 @@ export function _defaultState() { }; } -function contentOf(el) { - return [...el.childNodes].filter(isContentNode); -} - function hasContentAfter(node) { for (let sibling = node.nextSibling; sibling; sibling = sibling.nextSibling) { if (isContentNode(sibling)) { @@ -78,7 +75,7 @@ export function isSupportedTextEntryBody(bodyEl) { if ( paragraph.localName !== 'p' || hasNonNamespaceAttributes(paragraph) || - contentOf(paragraph).length > 1 || + contentChildrenOf(paragraph).length > 1 || hasContentAfter(paragraph) ) { return false; @@ -89,7 +86,7 @@ export function isSupportedTextEntryBody(bodyEl) { (container.localName === 'div' && !hasNonNamespaceAttributes(container) && container.parentElement === bodyEl && - contentOf(bodyEl).length === 1) + contentChildrenOf(bodyEl).length === 1) ); } 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..22867eea52 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 @@ -360,4 +360,23 @@ describe('assembleItemXml', () => { expect(parseXML(xml).querySelectorAll('qti-response-processing')).toHaveLength(1); }); }); + + describe('root metadata', () => { + const rootOf = params => + parseXML( + assembleItemXml({ + identifier: 'item-1', + title: 'T', + language: 'en', + bodyXml: '', + responseDeclarations: [], + ...params, + }), + ).documentElement; + + it('writes a label only when given one', () => { + expect(rootOf({ label: 'L' }).getAttribute('label')).toBe('L'); + expect(rootOf().hasAttribute('label')).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..593cc431c2 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 @@ -13,31 +13,53 @@ import fs from 'fs'; import path from 'path'; import { parseItem } from '../parseItem'; import { assembleItemXml } from '../assembleItem'; -import { isSupportedItem } from '../../interactions/resolveDescriptor'; +import { isEditableItem } from '../../validateItem'; 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, + label: item.label, language: item.language, bodyXml: item.interactions[0].bodyXml, responseDeclarations: item.interactions[0].responseDeclarations, hints: item.hints, }); +}; + +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)); - expect(isSupportedItem(interactions, itemBodyXml)).toBe(true); + it.each(FIXTURE_NAMES)('%s', name => { + const xml = read(name); + expect(isEditableItem(parseItem(xml), xml)).toBe(true); + }); + + // Before c51c5e035 the converter wrote the root language as `language`. + it('single_selection, as converted before c51c5e035', () => { + const stored = read('single_selection').replace(' xml:lang="', ' language="'); + expect(isEditableItem(parseItem(stored), stored)).toBe(true); + }); +}); + +// Keyed by namespace, since jsdom writes `xsi:` back under another prefix. +const rootAttributes = xml => + [...new DOMParser().parseFromString(xml, 'text/xml').documentElement.attributes] + .filter(attr => attr.prefix !== 'xmlns' && attr.name !== 'xmlns') + .map(attr => `{${attr.namespaceURI ?? ''}}${attr.localName}=${attr.value}`); + +describe('converted items, once edited', () => { + it.each(FIXTURE_NAMES)('%s keeps every root attribute', name => { + const xml = read(name); + expect(rootAttributes(rebuild(xml))).toEqual(expect.arrayContaining(rootAttributes(xml))); }); }); @@ -49,13 +71,19 @@ 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('writes a root language as converted before c51c5e035 back as xml:lang', () => { + const xml = rebuild(original.replace(' xml:lang="', ' language="')); 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'); @@ -65,7 +93,7 @@ describe('a converted single-selection item', () => { }); 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..80e1872d17 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/assembleItem.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/assembleItem.js @@ -5,7 +5,7 @@ import { HINT_CATALOG_ID, HINT_SUPPORT, hintHasContent } from './hints'; import { QTIDeclaration } from './qti/QTIDeclaration'; -import { buildFloatNode, buildXmlNode, parseXML } from './xml'; +import { QTI_SCHEMA_LOCATION, XSI_NS, buildFloatNode, buildXmlNode, parseXML } from './xml'; const serializer = new XMLSerializer(); @@ -164,6 +164,7 @@ function buildScoringNodes(declNodes) { * @param {object} params * @param {string} params.identifier - Item identifier attribute * @param {string} params.title - Item title attribute + * @param {string} [params.label] - Item label attribute, or '' to omit it * @param {string} params.language - Language tag, or '' to omit it * @param {string} params.bodyXml - Serialized interaction element XML string * @param {string[]} params.responseDeclarations - Array of serialized declaration XML strings @@ -173,6 +174,7 @@ function buildScoringNodes(declNodes) { export function assembleItemXml({ identifier, title, + label, language, bodyXml, responseDeclarations, @@ -206,11 +208,15 @@ export function assembleItemXml({ // only cover items assembled from XML that never carried them. identifier: identifier || 'item', title: title || '', + label: label || null, adaptive: 'false', 'time-dependent': 'false', // Omitted rather than guessed when the item has no language: the schema allows an // item without one. 'xml:lang': language || null, + // As the converter writes them (utils/assessment/qti/assessment_item.py). + 'tool-name': 'kolibri', + 'tool-version': '0.1', }, // The schema fixes this order: declarations, the body, the catalog, the processing. children: [ @@ -221,6 +227,8 @@ export function assembleItemXml({ ...(responseProcessing ? [responseProcessing] : []), ], }); + // As the converter writes it; setAttribute would leave it outside the xsi namespace. + assessmentItemNode.setAttributeNS(XSI_NS, 'xsi:schemaLocation', QTI_SCHEMA_LOCATION); return `\n${serializer.serializeToString(assessmentItemNode)}`; } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js index ddfd43aff6..b9f43d529e 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/hints.js @@ -16,7 +16,13 @@ import { generateRandomSlug } from '../utils/generateRandomSlug'; import { hasRichTextContent } from '../utils/richText'; -import { getContentHTML } from './xml'; +import { + attributesOf, + contentChildrenOf, + getContentHTML, + hasNonNamespaceAttributes, + isChildElement, +} from './xml'; /** The catalog this editor writes hints into. */ export const HINT_CATALOG_ID = 'kolibri-hints'; @@ -48,6 +54,49 @@ export function parseHints(doc) { }); } +/** + * Whether this is a hint card in the shape the editor writes, which parseHints reads in full. + * + * @param {Node} card + * @returns {boolean} + */ +function isHintCard(card) { + if (!isChildElement(card, 'qti-card')) { + return false; + } + 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 || + (isChildElement(content, 'qti-html-content') && !hasNonNamespaceAttributes(content))) + ); +} + +/** + * Whether the item's `` holds only the hint catalog, with only cards + * parseHints reads in full, so an edit that rewrites it drops nothing. Change it with + * parseHints. + * + * @param {Element} catalogInfo + * @returns {boolean} + */ +export function holdsOnlyHints(catalogInfo) { + return ( + !hasNonNamespaceAttributes(catalogInfo) && + contentChildrenOf(catalogInfo).every( + catalog => + isChildElement(catalog, 'qti-catalog') && + attributesOf(catalog).length === 1 && + catalog.getAttribute('id') === HINT_CATALOG_ID && + contentChildrenOf(catalog).every(isHintCard), + ) + ); +} + /** * Whether a hint holds anything worth writing. * diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/parseItem.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/parseItem.js index f059897f4e..f75c60e541 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/parseItem.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/parseItem.js @@ -25,6 +25,7 @@ const serializer = new XMLSerializer(); * @returns {{ * identifier: string, * title: string, + * label: string, * language: string, * itemBodyXml: string, * interactions: Array<{ bodyXml: string, responseDeclarations: string[] }>, @@ -37,7 +38,9 @@ export function parseItem(rawData) { const root = doc.querySelector('qti-assessment-item'); const identifier = root?.getAttribute('identifier') ?? ''; const title = root?.getAttribute('title') ?? ''; - const language = root?.getAttribute('xml:lang') ?? ''; + const label = root?.getAttribute('label') ?? ''; + // Items converted before c51c5e035 carry the root language as `language`. + const language = root?.getAttribute('xml:lang') ?? root?.getAttribute('language') ?? ''; const body = doc.querySelector('qti-item-body'); @@ -79,6 +82,7 @@ export function parseItem(rawData) { return { identifier, title, + label, language, itemBodyXml: body ? serializer.serializeToString(body) : '', interactions, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js index b5223546b4..4434ba4edf 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/xml.js @@ -133,6 +133,22 @@ 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'; + +/** The `xsi:schemaLocation` the converter writes on an item. */ +export const QTI_SCHEMA_LOCATION = + 'http://www.imsglobal.org/xsd/imsqtiasi_v3p0 https://purl.imsglobal.org/spec/qti/v3p0/schema/xsd/imsqti_asiv3p0p1_v1p0.xsd'; + +export const XML_NS = 'http://www.w3.org/XML/1998/namespace'; + +/** + * @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 +156,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; } /** @@ -158,6 +174,16 @@ export function isContentNode(node) { ); } +/** @returns {boolean} Whether the node is this element, in its parent's namespace */ +export function isChildElement(node, localName) { + return node.localName === localName && node.namespaceURI === node.parentNode.namespaceURI; +} + +/** @returns {Node[]} The node's children that isContentNode counts */ +export function contentChildrenOf(node) { + return [...node.childNodes].filter(isContentNode); +} + /** * Serialize XML nodes as an HTML string, for state that a rich text editor parses as HTML. * diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js index 4096e3684e..1f698802bc 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 { QTI_SCHEMA_LOCATION, XSI_NS } from '../serialization/xml'; export const CHOICE_SINGLE_SELECT_XML = ` Which planet is closest to the Sun? @@ -333,6 +334,54 @@ 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="${QTI_SCHEMA_LOCATION}" +>`, +); + +export const CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET = CHOICE_ITEM_DOCUMENT_WITH_HINTS.replace( + '\n\n ', + ` + + `, +); + +export const STYLESHEET_ITEM_DOCUMENT_NO_PROMPT = CHOICE_ITEM_DOCUMENT_WITH_STYLESHEET.replace( + 'Pick one.', + '', +); + +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 = ` + EDITOR_CHILDREN.some(name => isChildElement(el, name)) && + (el.localName !== 'qti-catalog-info' || holdsOnlyHints(el)), + ) && + !readsOutcome(root) + ); +} + +/** + * Whether the editor can edit this parsed, non-blank item faithfully; it shows any other + * item read-only. + * + * @param {{ interactions: Array, itemBodyXml: string }} item - The parsed item + * @param {string} rawData - The item's XML + * @returns {boolean} + */ +export function isEditableItem({ interactions, itemBodyXml }, rawData) { + return isSupportedItem(interactions, itemBodyXml) && keepsItemContent(rawData); +} + /** * Validate a QTI assessment item from its raw XML, without rendering it. * @@ -40,8 +133,8 @@ export function validateItemShape({ interactions, questionTypes = [], allowFreeR * @param {object} [options] * @param {boolean} [options.allowFreeResponse] - Whether a free-response question counts * as valid. Consumers that only accept scorable questions pass false. - * @returns {Array<{ code: string, id?: string }>} Empty when the item is valid. Items the - * editor shows read-only report only unreadable XML or a missing interaction. + * @returns {Array<{ code: string, id?: string }>} Empty when the item is valid. Items whose + * body the editor can't read in full report only unreadable XML or a missing interaction. */ export function validateQtiItem(rawData, { allowFreeResponse = true } = {}) { if (!rawData) { @@ -61,7 +154,7 @@ export function validateQtiItem(rawData, { allowFreeResponse = true } = {}) { })); if (item.interactions.length && !isSupportedItem(item.interactions, item.itemBodyXml)) { - // Shown read-only: the editor's rules don't apply, only unreadable interactions count. + // The editor can't read the body in full: only unreadable interactions count. return resolved.filter(({ error }) => error).map(({ error }) => ({ code: error })); } diff --git a/contentcuration/contentcuration/tests/utils/qti/fixtures/single_selection_with_hints.xml b/contentcuration/contentcuration/tests/utils/qti/fixtures/single_selection_with_hints.xml new file mode 100644 index 0000000000..9689c1b731 --- /dev/null +++ b/contentcuration/contentcuration/tests/utils/qti/fixtures/single_selection_with_hints.xml @@ -0,0 +1,29 @@ + + + + + choice_0 + + + + + + +

What is 2+2?

+
+

4

+

3

+
+
+ + + +

Count on your fingers

+
+ +

See diagram

+
+
+
+ +
diff --git a/contentcuration/contentcuration/tests/utils/qti/test_convert.py b/contentcuration/contentcuration/tests/utils/qti/test_convert.py index e7cdbb1c01..83bc884f70 100644 --- a/contentcuration/contentcuration/tests/utils/qti/test_convert.py +++ b/contentcuration/contentcuration/tests/utils/qti/test_convert.py @@ -680,6 +680,20 @@ def test_no_hints_produces_no_catalog_info(self): self.assertNotIn("