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 fa3af04e77..4f3c409f17 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/__tests__/validateItem.spec.js @@ -5,10 +5,13 @@ import { CHOICE_ITEM_DOCUMENT_NO_PROMPT, CHOICE_ITEM_DOCUMENT_NO_CORRECT_ANSWER, NO_INTERACTION_ITEM_DOCUMENT, + INLINE_CHOICE_ITEM_DOCUMENT, VALID_MATCH_ITEM_DOCUMENT, MATCH_THREE_SETS_XML, MATCH_XML, MULTI_TEXT_ENTRY_ITEM_DOCUMENT, + MULTI_INTERACTION_ITEM_DOCUMENT, + UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT, } from '../utils/testingFixtures'; const codesOf = errors => errors.map(error => error.code); @@ -30,11 +33,9 @@ describe('validateQtiItem', () => { ); }); - it('validates an item with several inline interactions as one question', () => { - // Every inline interaction is handed the whole body, so validating one per interaction - // would report the same problem once for each of them. + it('does not apply editor rules to an item with several text entries', () => { const xml = MULTI_TEXT_ENTRY_ITEM_DOCUMENT.replace(/>Sun<').replace(/>Moon<'); - expect(codesOf(validateQtiItem(xml))).toEqual([ValidationError.EMPTY_ANSWER_CONTENT]); + expect(validateQtiItem(xml)).toEqual([]); }); it('reports an item whose body holds no interaction', () => { @@ -43,6 +44,33 @@ describe('validateQtiItem', () => { ]); }); + it('reports nothing for an item with an interaction with no descriptor, which is shown read-only', () => { + expect(validateQtiItem(UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT)).toEqual([]); + expect( + validateQtiItem(UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT, { allowFreeResponse: false }), + ).toEqual([]); + }); + + it('reports a three-set match interaction inside a multi-interaction item as unparseable', () => { + const xml = MULTI_INTERACTION_ITEM_DOCUMENT.replace( + //, + MATCH_THREE_SETS_XML, + ); + expect(codesOf(validateQtiItem(xml))).toContain(ValidationError.PARSE_ERROR); + }); + + it('does not apply the editor rules to a multi-interaction item', () => { + const xml = MULTI_INTERACTION_ITEM_DOCUMENT.replace( + /First question<\/qti-prompt>/, + '', + ); + expect(validateQtiItem(xml, { allowFreeResponse: false })).toEqual([]); + }); + + it('does not apply the editor rules to an interaction with no editor', () => { + expect(validateQtiItem(INLINE_CHOICE_ITEM_DOCUMENT)).toEqual([]); + }); + it('reports an item with no raw data at all', () => { expect(validateQtiItem('')).toEqual([{ code: ValidationError.NO_INTERACTION }]); expect(validateQtiItem(undefined)).toEqual([{ code: ValidationError.NO_INTERACTION }]); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js index 360d681956..71643822a7 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js @@ -27,6 +27,9 @@ const renderSection = (props = {}) => routes: new VueRouter(), }); +// jest_config/setup.js adds a hidden csrf input to the document. +const isAuthorInput = el => el.name !== 'csrfmiddlewaretoken'; + describe('InteractionSection', () => { describe('choice interaction', () => { it('renders the prompt from the XML via ChoiceInteractionEditor', () => { @@ -142,11 +145,11 @@ describe('InteractionSection', () => { }); describe('unknown interaction type', () => { - it('falls back silently when the interaction tag is unrecognized', () => { - // Should not throw — just renders the fallback component - expect(() => - renderSection({ interaction: interactionBlock(UNKNOWN_INTERACTION_XML) }), - ).not.toThrow(); + it('renders nothing editable', () => { + renderSection({ interaction: interactionBlock(UNKNOWN_INTERACTION_XML), mode: 'edit' }); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + expect(screen.queryAllByRole('textbox').filter(isAuthorInput)).toHaveLength(0); + expect(screen.queryByRole('radio')).not.toBeInTheDocument(); }); }); }); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue index 080bc5bc7a..0762cf6780 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/index.vue @@ -7,7 +7,7 @@ > {{ parseError }}

-
+
{ delete Element.prototype.scrollIntoView; }); +// jest_config/setup.js adds a hidden csrf input to the document. +const isAuthorInput = el => el.name !== 'csrfmiddlewaretoken'; + describe('QTIItemEditor', () => { beforeEach(() => scrollIntoView.mockClear()); afterEach(() => jest.restoreAllMocks()); @@ -230,6 +237,57 @@ describe('QTIItemEditor', () => { }); expect(screen.getByText(unsupportedItemMessage$())).toBeInTheDocument(); }); + + describe('items without exactly one editable interaction', () => { + const documents = { + 'two interactions': MULTI_INTERACTION_ITEM_DOCUMENT, + 'an interaction with no descriptor': UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT, + 'an interaction with no editor': INLINE_CHOICE_ITEM_DOCUMENT, + 'no interaction': NO_INTERACTION_ITEM_DOCUMENT, + 'several blanks in one text entry': MULTI_TEXT_ENTRY_ITEM_DOCUMENT, + }; + const renderDocument = (raw_data, props = {}) => + renderComponent({ + item: { assessment_id: 'item-id', type: AssessmentItemTypes.QTI, raw_data }, + ...props, + }); + + describe.each(Object.entries(documents))('with %s', (_, raw_data) => { + test('shows the unsupported message', () => { + renderDocument(raw_data, { mode: 'view' }); + expect(screen.getByText(unsupportedItemMessage$())).toBeInTheDocument(); + }); + + test('offers no editable controls or hints in edit mode', () => { + renderDocument(raw_data, { mode: 'edit' }); + expect(screen.queryByText(hintsLabel$())).not.toBeInTheDocument(); + const buttons = screen.getAllByRole('button'); + expect(buttons).toHaveLength(1); + expect(buttons[0]).toHaveAccessibleName(closeBtnLabel$()); + expect(screen.queryByRole('radio')).not.toBeInTheDocument(); + expect(screen.queryByRole('checkbox')).not.toBeInTheDocument(); + expect(screen.queryByRole('combobox')).not.toBeInTheDocument(); + expect(screen.queryAllByRole('textbox').filter(isAuthorInput)).toHaveLength(0); + }); + }); + + test('leaves raw_data untouched after the card is opened and closed', async () => { + const { emitted, updateProps } = renderDocument(MULTI_INTERACTION_ITEM_DOCUMENT, { + mode: 'edit', + }); + + await fireEvent.click(screen.getByRole('button', { name: closeBtnLabel$() })); + await updateProps({ mode: 'view' }); + await nextTick(); + + expect(emitted()['update:rawData']).toBeUndefined(); + }); + + test('shows the hints of an unsupported item when answers are shown', () => { + renderDocument(MULTI_INTERACTION_ITEM_DOCUMENT, { mode: 'view', showAnswers: true }); + expect(screen.getByRole('button', { name: hintsLabel$() })).toBeInTheDocument(); + }); + }); }); describe('incomplete indicator', () => { @@ -257,11 +315,26 @@ describe('QTIItemEditor', () => { expect(screen.getByText(incompleteItemIndicatorLabel$())).toBeInTheDocument(); }); - test('is shown for an item with no interaction at all', async () => { + test('is shown for an unsupported item with no interaction, which blocks publishing', async () => { await renderAndValidate(NO_INTERACTION_ITEM_DOCUMENT); expect(screen.getByText(incompleteItemIndicatorLabel$())).toBeInTheDocument(); }); + test('is shown for an item whose XML cannot be parsed', async () => { + await renderAndValidate(' { + await renderAndValidate(VALID_MATCH_ITEM_DOCUMENT.replace(MATCH_XML, MATCH_THREE_SETS_XML)); + expect(screen.getByText(incompleteItemIndicatorLabel$())).toBeInTheDocument(); + }); + + test('is not shown for a valid item with unsupported interactions', async () => { + await renderAndValidate(MULTI_INTERACTION_ITEM_DOCUMENT); + expect(screen.queryByText(incompleteItemIndicatorLabel$())).not.toBeInTheDocument(); + }); + test('is shown for a free-response question where those are not accepted', async () => { renderComponent({ allowFreeResponse: false, @@ -403,23 +476,6 @@ describe('QTIItemEditor', () => { expect(screen.getByText(hintsLabel$())).toBeInTheDocument(); }); - test('keeps the body of a question that has no interaction when a hint changes', async () => { - // Nothing mounts an interaction editor here, so the editor holds no body of its own. - // Assembling from that empty state would replace the question's text with an empty - // — a hint edit silently deleting the question. - const { emitted } = renderComponent({ - item: { ...defaultProps.item, raw_data: NO_INTERACTION_ITEM_WITH_HINTS }, - mode: 'edit', - }); - await fireEvent.click(screen.getByRole('button', { name: hintsLabel$() })); - await fireEvent.click(screen.getAllByRole('button', { name: 'Delete hint' })[0]); - await nextTick(); - - const [xml] = emitted()['update:rawData'].at(-1); - expect(xml).toContain('What is the capital of France?'); - expect(xml).not.toContain(''); - }); - test('reports the item XML when a hint changes', async () => { const { emitted } = renderComponent({ item: { ...defaultProps.item, raw_data: CHOICE_ITEM_DOCUMENT_WITH_HINTS }, 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 6e9eef8858..edb1017d5a 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/index.vue +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/index.vue @@ -80,7 +80,7 @@ props.item.type === AssessmentItemTypes.QTI); /** - * Items authored outside this editor (e.g. Perseus questions) and items whose XML - * cannot be read are shown as read-only cards. + * 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. */ - const isUnsupported = computed( - () => props.item.type !== AssessmentItemTypes.QTI || Boolean(parseError.value), + const isBlank = !props.item.raw_data; + const isEditableQti = computed( + () => !parseError.value && (isBlank || isSupportedItem(interactions.value)), ); + /** + * Items authored outside this editor (e.g. Perseus questions) and QTI it cannot edit + * faithfully are shown as read-only cards. + */ + const isUnsupported = computed(() => !isQti.value || !isEditableQti.value); + /* * Seed the editor refs from the parsed item's first block, which for an inline * passage holds every declaration. - * - * The body is seeded even when there is no interaction to edit. Such an item still has - * content — its own text, and any interaction this editor has no descriptor for — and - * anything else the author can change, a hint, reassembles the whole item. Leaving the - * body unseeded would write an empty over that text. */ - currentBodyXml.value = interactions.value[0]?.bodyXml ?? itemBodyXml.value; if (interactions.value.length > 0) { + currentBodyXml.value = interactions.value[0].bodyXml; currentResponseDeclarations.value = interactions.value[0].responseDeclarations; } @@ -262,6 +264,10 @@ */ const hasHints = hints.value.length > 0; + const showHints = computed( + () => hasHints && (props.mode === 'edit' ? !isUnsupported.value : props.showAnswers), + ); + function onUpdateHints(newHints) { editedHere = props.mode === 'edit'; hints.value = newHints; @@ -315,7 +321,13 @@ */ const isIncomplete = computed(() => { if (isUnsupported.value) { - return false; + // Unreadable or empty QTI can't be fixed here and blocks publishing; other + // unsupported items are publishable. + return ( + isQti.value && + validateQtiItem(props.item.raw_data, { allowFreeResponse: props.allowFreeResponse }) + .length > 0 + ); } const itemErrors = validateItemShape({ interactions: interactions.value, @@ -343,7 +355,7 @@ onUpdateInteraction, onUpdateErrors, hints, - hasHints, + showHints, onUpdateHints, }; }, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useInteractionDescriptor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useInteractionDescriptor.spec.js index 0549f256fd..41ef7c54c8 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useInteractionDescriptor.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useInteractionDescriptor.spec.js @@ -62,38 +62,20 @@ describe('useInteractionDescriptor', () => { }); describe('with an unrecognized interaction type', () => { - it('falls back to the default descriptor without a parse error', async () => { + it('resolves no descriptor and no parse error', async () => { const { result } = renderDescriptor(UNKNOWN_INTERACTION_XML); await nextTick(); + expect(result.descriptor.value).toBeNull(); expect(result.parseError.value).toBeNull(); }); - - it('still returns a defined fallback descriptor', async () => { - const { result } = renderDescriptor(UNKNOWN_INTERACTION_XML); - await nextTick(); - expect(result.descriptor.value).toBeDefined(); - expect(typeof result.descriptor.value.matches).toBe('function'); - }); }); describe('with a null or empty bodyXmlRef', () => { - it('returns the default descriptor when bodyXmlRef is null', async () => { - const { result } = renderDescriptor(null); - await nextTick(); - expect(result.descriptor.value).toBeDefined(); - expect(result.parseError.value).toBeNull(); - }); - - it('returns null questionType when bodyXmlRef is null', async () => { - const { result } = renderDescriptor(null); + it.each([null, ''])('resolves no descriptor for %p', async xml => { + const { result } = renderDescriptor(xml); await nextTick(); + expect(result.descriptor.value).toBeNull(); expect(result.questionType.value).toBeNull(); - }); - - it('returns the default descriptor when bodyXmlRef is an empty string', async () => { - const { result } = renderDescriptor(''); - await nextTick(); - expect(result.descriptor.value).toBeDefined(); expect(result.parseError.value).toBeNull(); }); }); @@ -106,10 +88,10 @@ describe('useInteractionDescriptor', () => { expect(result.parseError.value).toBe('This question could not be loaded'); }); - it('still returns a defined fallback descriptor on parse error', async () => { + it('resolves no descriptor on parse error', async () => { const { result } = renderDescriptor(' - descriptors.find(d => d.questionTypes.includes(questionType.value)) ?? - registry[DEFAULT_INTERACTION], + () => descriptors.find(d => d.questionTypes.includes(questionType.value)) ?? null, ); return { descriptor, questionType, parseError }; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js index deb8008d21..63847bec11 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useQtiItem.js @@ -17,7 +17,6 @@ import { assembleItemXml } from '../serialization/assembleItem'; * identifier: import('vue').Ref, * title: import('vue').Ref, * language: import('vue').Ref, - * itemBodyXml: import('vue').Ref, * interactions: import('vue').Ref>, * hints: import('vue').Ref>, * parseError: import('vue').Ref, @@ -29,8 +28,6 @@ export default function useQtiItem(rawXml, { bodyXml, responseDeclarations } = { const title = ref(''); const language = ref(''); const interactions = ref([]); - /** The item's `` as parsed, whether or not it holds an interaction. */ - const itemBodyXml = ref(''); /** * Hints belong to the item, not to any one interaction, so they live here beside * identifier and title — mutable, and read back by the rawData computed below. @@ -45,7 +42,6 @@ export default function useQtiItem(rawXml, { bodyXml, responseDeclarations } = { title.value = model.title; language.value = model.language; interactions.value = model.interactions; - itemBodyXml.value = model.itemBodyXml; hints.value = model.hints; } catch (e) { parseError.value = e.message; @@ -72,7 +68,6 @@ export default function useQtiItem(rawXml, { bodyXml, responseDeclarations } = { identifier, title, language, - itemBodyXml, interactions, hints, parseError, diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js index a0645220d4..81c8f973b4 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/constants.js @@ -50,13 +50,28 @@ export const Orientation = Object.freeze({ * so they serve as both type keys and CSS selectors for querySelectorAll. */ export const QtiInteraction = Object.freeze({ - CHOICE: 'qti-choice-interaction', - ORDER: 'qti-order-interaction', - MATCH: 'qti-match-interaction', ASSOCIATE: 'qti-associate-interaction', - TEXT_ENTRY: 'qti-text-entry-interaction', + CHOICE: 'qti-choice-interaction', + CUSTOM: 'qti-custom-interaction', + DRAWING: 'qti-drawing-interaction', + END_ATTEMPT: 'qti-end-attempt-interaction', EXTENDED_TEXT: 'qti-extended-text-interaction', + GAP_MATCH: 'qti-gap-match-interaction', + GRAPHIC_ASSOCIATE: 'qti-graphic-associate-interaction', + GRAPHIC_GAP_MATCH: 'qti-graphic-gap-match-interaction', + GRAPHIC_ORDER: 'qti-graphic-order-interaction', + HOTSPOT: 'qti-hotspot-interaction', + HOTTEXT: 'qti-hottext-interaction', INLINE_CHOICE: 'qti-inline-choice-interaction', + MATCH: 'qti-match-interaction', + MEDIA: 'qti-media-interaction', + ORDER: 'qti-order-interaction', + PORTABLE_CUSTOM: 'qti-portable-custom-interaction', + POSITION_OBJECT: 'qti-position-object-interaction', + SELECT_POINT: 'qti-select-point-interaction', + SLIDER: 'qti-slider-interaction', + TEXT_ENTRY: 'qti-text-entry-interaction', + UPLOAD: 'qti-upload-interaction', }); export const QTI_INTERACTION_TAGS = Object.freeze(Object.values(QtiInteraction)); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/registry.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/registry.spec.js index 23760422c0..f3499824b7 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/registry.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/registry.spec.js @@ -1,5 +1,5 @@ -import { descriptors, editors, registry, DEFAULT_INTERACTION } from '../index'; -import { isInlineInteraction } from '../descriptors'; +import { descriptors, editors, registry } from '../index'; +import { HEADLESS_INTERACTIONS, isInlineInteraction } from '../descriptors'; import { Placement, QtiInteraction } from '../../constants'; /** @@ -10,9 +10,7 @@ import { Placement, QtiInteraction } from '../../constants'; * opens the question. */ describe('interaction registry', () => { - // Descriptors whose editor is not built yet. TODO: #6182 removes this exemption. - const HEADLESS_DESCRIPTORS = [QtiInteraction.INLINE_CHOICE]; - const editable = descriptors.filter(d => !HEADLESS_DESCRIPTORS.includes(d.type)); + const editable = descriptors.filter(d => !HEADLESS_INTERACTIONS.includes(d.type)); it('registers an editor for every descriptor', () => { const missing = editable.filter(d => !editors[d.type]).map(d => d.type); @@ -32,10 +30,6 @@ describe('interaction registry', () => { expect(Object.keys(registry).sort()).toEqual(descriptors.map(d => d.type).sort()); }); - it('has a descriptor for the fallback interaction', () => { - expect(registry[DEFAULT_INTERACTION]).toBeDefined(); - }); - describe('isInlineInteraction', () => { it('reports the placement each descriptor declares', () => { for (const descriptor of descriptors) { diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/resolveDescriptor.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/resolveDescriptor.spec.js new file mode 100644 index 0000000000..0b67080ee0 --- /dev/null +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/resolveDescriptor.spec.js @@ -0,0 +1,76 @@ +import { parseItem } from '../../serialization/parseItem'; +import { QtiInteraction, ValidationError } from '../../constants'; +import { isSupportedItem, resolveDescriptor } from '../resolveDescriptor'; +import { + CHOICE_SINGLE_SELECT_XML, + INLINE_CHOICE_ITEM_DOCUMENT, + MATCH_THREE_SETS_XML, + MULTI_INTERACTION_ITEM_DOCUMENT, + MULTI_TEXT_ENTRY_ITEM_DOCUMENT, + TWO_INTERACTIONS_DOCUMENT, + UNDESCRIBED_INTERACTION, + UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT, + VALID_CHOICE_ITEM_DOCUMENT, + VALID_MATCH_ITEM_DOCUMENT, +} from '../../utils/testingFixtures'; + +const UNDESCRIBED_XML = `<${UNDESCRIBED_INTERACTION} response-identifier="RESPONSE"/>`; + +const interactionsOf = document => parseItem(document).interactions; + +describe('resolveDescriptor', () => { + it('resolves no descriptor for an interaction nothing is registered for', () => { + expect(resolveDescriptor(UNDESCRIBED_XML, [])).toEqual({ + descriptor: null, + questionType: null, + error: null, + }); + }); + + it('resolves no descriptor for an empty body', () => { + expect(resolveDescriptor('', []).descriptor).toBeNull(); + }); + + it('resolves the descriptor of a recognized interaction', () => { + const { descriptor, error } = resolveDescriptor(CHOICE_SINGLE_SELECT_XML, []); + expect(descriptor.type).toBe(QtiInteraction.CHOICE); + expect(error).toBeNull(); + }); + + it('keeps the descriptor when its question type cannot be read', () => { + const { descriptor, error } = resolveDescriptor(MATCH_THREE_SETS_XML, []); + expect(descriptor.type).toBe(QtiInteraction.MATCH); + expect(error).toBe(ValidationError.PARSE_ERROR); + }); + + it('resolves no descriptor for malformed XML', () => { + expect(resolveDescriptor(' { + it.each([ + ['no interactions', []], + ['two choice interactions', interactionsOf(MULTI_INTERACTION_ITEM_DOCUMENT)], + ['a choice and a text entry', interactionsOf(TWO_INTERACTIONS_DOCUMENT)], + ['an interaction with no descriptor', interactionsOf(UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT)], + ['an interaction with no editor', interactionsOf(INLINE_CHOICE_ITEM_DOCUMENT)], + ['several text entries in one body', interactionsOf(MULTI_TEXT_ENTRY_ITEM_DOCUMENT)], + [ + 'an interaction whose question type cannot be read', + [{ bodyXml: MATCH_THREE_SETS_XML, responseDeclarations: [] }], + ], + ])('rejects %s', (_, interactions) => { + expect(isSupportedItem(interactions)).toBe(false); + }); + + it.each([ + ['one choice interaction', VALID_CHOICE_ITEM_DOCUMENT], + ['a match interaction', VALID_MATCH_ITEM_DOCUMENT], + ])('accepts %s', (_, document) => { + expect(isSupportedItem(interactionsOf(document))).toBe(true); + }); +}); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js index bdc145d08a..395196005d 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/descriptors.js @@ -15,12 +15,6 @@ import { inlineChoiceInteractionDescriptor } from './inlineChoice/Descriptor'; * Registering a new interaction means adding its descriptor here and its editor in index.js */ -/** - * The default interaction type used as fallback when no descriptor matches - * the interaction element found in the XML body. - */ -export const DEFAULT_INTERACTION = QtiInteraction.CHOICE; - /** * Ordered list of all registered interaction descriptors. * Searched in order; the first whose `matches(el)` returns true wins. @@ -39,6 +33,18 @@ export const descriptors = [ */ 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. + */ +export const HEADLESS_INTERACTIONS = Object.freeze([QtiInteraction.INLINE_CHOICE]); + +/** + * Inline interactions whose editor holds one per item body. A body with several would + * lose all but one on edit, so it is shown read-only. + */ +export const SINGLE_INSTANCE_INTERACTIONS = Object.freeze([QtiInteraction.TEXT_ENTRY]); + /** * Whether an interaction is authored inline, and so needs the whole item body to parse * rather than its own element. Read off the descriptor's placement, so declaring it there diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/index.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/index.js index a20d0958f5..312242e5ea 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/index.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/index.js @@ -21,4 +21,4 @@ export const editors = Object.freeze({ [QtiInteraction.MATCH]: MatchEditor, }); -export { DEFAULT_INTERACTION, descriptors, registry } from './descriptors'; +export { descriptors, registry } from './descriptors'; diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js index c02f778346..de5a8aa17b 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/resolveDescriptor.js @@ -1,6 +1,6 @@ import { parseXML } from '../serialization/xml'; import { ValidationError } from '../constants'; -import { descriptors, registry, DEFAULT_INTERACTION } from './descriptors'; +import { HEADLESS_INTERACTIONS, SINGLE_INSTANCE_INTERACTIONS, descriptors } from './descriptors'; /** * Resolve the interaction descriptor and question type for a single interaction block. @@ -12,31 +12,52 @@ import { descriptors, registry, DEFAULT_INTERACTION } from './descriptors'; * interactions) * @param {string[]} [responseDeclarations] * @returns {{ - * descriptor: object, + * descriptor: object|null, * questionType: string|null, * error: string|null, - * }} `error` is a ValidationError code; callers own how it is presented. + * }} `descriptor` is null when no descriptor matches the interaction. `error` is a + * ValidationError code; callers own how it is presented. */ export function resolveDescriptor(bodyXml, responseDeclarations) { if (!bodyXml) { - return { descriptor: registry[DEFAULT_INTERACTION], questionType: null, error: null }; + return { descriptor: null, questionType: null, error: null }; } + let descriptor = null; try { const interactionEl = parseXML(bodyXml).documentElement; - const descriptor = - descriptors.find(d => d.matches(interactionEl)) ?? registry[DEFAULT_INTERACTION]; + descriptor = descriptors.find(d => d.matches(interactionEl)) ?? null; return { descriptor, - questionType: descriptor.getQuestionType(interactionEl, responseDeclarations) ?? null, + questionType: descriptor?.getQuestionType(interactionEl, responseDeclarations) ?? null, error: null, }; } catch (e) { // eslint-disable-next-line no-console console.error('[QTI] Failed to parse interaction XML:', e.message); - return { - descriptor: registry[DEFAULT_INTERACTION], - questionType: null, - error: ValidationError.PARSE_ERROR, - }; + return { descriptor, questionType: null, error: ValidationError.PARSE_ERROR }; + } +} + +/** + * Whether the editor can edit an item faithfully: it holds exactly one interaction block + * (one interaction, for descriptors in SINGLE_INSTANCE_INTERACTIONS), and a descriptor + * with an editor exists for it that could read the interaction. + * + * @param {Array<{ bodyXml: string, responseDeclarations: string[] }>} interactions - As + * returned by parseItem + * @returns {boolean} + */ +export function isSupportedItem(interactions) { + if (interactions.length !== 1) { + return false; + } + const [{ bodyXml, responseDeclarations }] = interactions; + const { descriptor, error } = resolveDescriptor(bodyXml, responseDeclarations); + if (descriptor === null || error || HEADLESS_INTERACTIONS.includes(descriptor.type)) { + return false; } + return ( + !SINGLE_INSTANCE_INTERACTIONS.includes(descriptor.type) || + parseXML(bodyXml).getElementsByTagName(descriptor.type).length <= 1 + ); } diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/parseItem.spec.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/parseItem.spec.js index 28555b966b..adea6f6e72 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/parseItem.spec.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/serialization/__tests__/parseItem.spec.js @@ -4,6 +4,8 @@ import { parseXML } from '../xml'; import { VALID_CHOICE_ITEM_DOCUMENT, TWO_INTERACTIONS_DOCUMENT, + UNDESCRIBED_INTERACTION, + UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT, MULTI_TEXT_ENTRY_ITEM_DOCUMENT, } from '../../utils/testingFixtures'; @@ -59,6 +61,12 @@ describe('parseItem — interaction blocks', () => { expect(model.interactions).toHaveLength(2); }); + it('finds an interaction the editor has no descriptor for', () => { + const model = parseItem(UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT); + expect(model.interactions).toHaveLength(1); + expect(model.interactions[0].bodyXml).toContain(`<${UNDESCRIBED_INTERACTION}`); + }); + it('merges inline interactions of one type into one block, in body order', () => { const model = parseItem(MULTI_TEXT_ENTRY_ITEM_DOCUMENT); expect(model.interactions).toHaveLength(1); diff --git a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js index e5f7dbe10f..ccf4bfc825 100644 --- a/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js +++ b/contentcuration/contentcuration/frontend/shared/views/QTIEditor/utils/testingFixtures.js @@ -1,5 +1,8 @@ // Centralized QTI Mock XML Fixtures for Unit Tests +import { QTI_INTERACTION_TAGS } from '../constants'; +import { registry } from '../interactions/descriptors'; + export const CHOICE_SINGLE_SELECT_XML = ` Which planet is closest to the Sun? Mercury @@ -299,35 +302,6 @@ export const CHOICE_ITEM_DOCUMENT_WITH_HINTS = ` `; -/** - * A converted question with nothing to answer, which still carries its text in the body and - * hints in its catalog. No interaction means no interaction editor, so the only thing an - * author can edit here is a hint — and that must not cost the body. - */ -export const NO_INTERACTION_ITEM_WITH_HINTS = ` - - -

What is the capital of France?

- - - - -

It is on the Seine

 - - -

Starts with a P

 - - - -`; - export const VALID_ASSOCIATE_ITEM_DOCUMENT = ` `; +/** Two choice interactions and a hint catalog: parses, but cannot be edited faithfully. */ +export const MULTI_INTERACTION_ITEM_DOCUMENT = ` + + + A + + + C + + + + + First question + Yes + No + + + Second question + Yes + No + + + + + +

Think about it

 + + + +`; + +/** + * An interaction QTI defines but no registered descriptor handles. Picked from the + * registry so a fixture stays unrecognized when a descriptor is added for another type. + */ +export const UNDESCRIBED_INTERACTION = QTI_INTERACTION_TAGS.find(tag => !registry[tag]); + +export const UNRECOGNIZED_INTERACTION_ITEM_DOCUMENT = ` + + + + + <${UNDESCRIBED_INTERACTION} response-identifier="RESPONSE"> + Describe the capital + + +`; + +/** + * Inline choice dropdowns, which have a descriptor but no editor yet. The second dropdown + * has no correct answer, which the editor's rules would report. + */ +export const INLINE_CHOICE_ITEM_DOCUMENT = ` + + + + blue + + + + + +

The sky is bluered and grass is greenpurple.

+ +`; + /** * Two text entries whose declarations are listed out of body order. - * Text entry, the only inline interaction parseItem finds until #6180, stands in for - * inline choice. */ export const MULTI_TEXT_ENTRY_ITEM_DOCUMENT = ` } Empty when the item is valid + * @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. */ export function validateQtiItem(rawData, { allowFreeResponse = true } = {}) { if (!rawData) { @@ -59,6 +60,11 @@ export function validateQtiItem(rawData, { allowFreeResponse = true } = {}) { ...resolveDescriptor(interaction.bodyXml, interaction.responseDeclarations), })); + if (item.interactions.length && !isSupportedItem(item.interactions)) { + // Shown read-only: the editor's rules don't apply, only unreadable interactions count. + return resolved.filter(({ error }) => error).map(({ error }) => ({ code: error })); + } + const errors = validateItemShape({ interactions: item.interactions, questionTypes: resolved.map(({ questionType }) => questionType),