refactor: replace incorrect choice-interaction fallback with explicit parse error handling in QTI editor

Signed-off-by: Abhishek-Punhani <punhani.manavabhi@gmail.com>

Author: Abhishek-Punhani

contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/InteractionSection/__tests__/InteractionSection.spec.js

@@ -17,10 +17,6 @@ const renderSection = (props = {}) =>
     routes: new VueRouter(),
   });
 
-// ---------------------------------------------------------------------------
-// Tests
-// ---------------------------------------------------------------------------
-
 describe('InteractionSection', () => {
   describe('choice interaction', () => {
     it('renders the prompt from the XML via ChoiceInteractionEditor', () => {
@@ -43,11 +39,9 @@ describe('InteractionSection', () => {
   });
 
   describe('parse error handling', () => {
-    it('gracefully falls back to default interaction state when XML is malformed', () => {
+    it('shows a parse error when XML is malformed', () => {
       renderSection({ interaction: interactionBlock('not-xml<{{') });
-      // It should render exactly 1 choice fallback element
-      const inputs = screen.queryAllByRole('radio').concat(screen.queryAllByRole('checkbox'));
-      expect(inputs).toHaveLength(1);
+      expect(screen.getByText('This question could not be loaded')).toBeInTheDocument();
     });
   });
 

contentcuration/contentcuration/frontend/shared/views/QTIEditor/components/QTIItemEditor/index.vue

@@ -62,6 +62,7 @@
 
   import { computed, ref, watch } from 'vue';
   import { qtiEditorStrings } from '../../qtiEditorStrings';
+  import { QuestionType } from '../../constants';
   import useQtiItem from '../../composables/useQtiItem';
   import InteractionSection from '../InteractionSection/index.vue';
 
@@ -119,9 +120,14 @@
       const interactionTypeLabel = computed(() => {
         const type = currentQuestionType.value;
         if (!type) return unknownTypeLabel$();
-        // Dynamic access to the localized string method (e.g. 'singleSelectLabel$()')
-        const methodKey = `${type}Label$`;
-        return qtiEditorStrings[methodKey]?.() ?? unknownTypeLabel$();
+        const QUESTION_TYPE_LABELS = {
+          [QuestionType.SINGLE_SELECT]: qtiEditorStrings.singleSelectLabel$,
+          [QuestionType.MULTI_SELECT]: qtiEditorStrings.multiSelectLabel$,
+          [QuestionType.NUMERIC]: qtiEditorStrings.numericLabel$,
+          [QuestionType.TEXT_ENTRY]: qtiEditorStrings.textEntryLabel$,
+          [QuestionType.FREE_RESPONSE]: qtiEditorStrings.freeResponseLabel$,
+        };
+        return (QUESTION_TYPE_LABELS[type] ?? unknownTypeLabel$)();
       });
 
       const questionNumberAndTypeLabel = computed(() =>

contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useChoiceInteraction.spec.js

@@ -30,15 +30,11 @@ function makeBlock(choices, questionType = QuestionType.SINGLE_SELECT) {
 }
 
 function setup(choices, questionType = QuestionType.SINGLE_SELECT) {
-  const qt = ref(questionType);
+  const questionTypeRef = ref(questionType);
   const block = makeBlock(choices, questionType);
-  return { qt, ...useChoiceInteraction(block, qt) };
+  return { questionTypeRef, ...useChoiceInteraction(block, questionTypeRef) };
 }
 
-// ---------------------------------------------------------------------------
-// Tests
-// ---------------------------------------------------------------------------
-
 describe('useChoiceInteraction', () => {
   describe('addChoice()', () => {
     it('appends a new choice to the list', () => {
@@ -118,33 +114,33 @@ describe('useChoiceInteraction', () => {
 
   describe('toggleCorrectChoice()', () => {
     it('singleSelect: sets only the target as correct and clears others', () => {
-      const { state, toggleCorrectChoice, qt } = setup([
+      const { state, toggleCorrectChoice, questionTypeRef } = setup([
         makeAnswer({ id: 'a', correct: true }),
         makeAnswer({ id: 'b', correct: false }),
       ]);
-      qt.value = QuestionType.SINGLE_SELECT;
+      questionTypeRef.value = QuestionType.SINGLE_SELECT;
       toggleCorrectChoice('b');
       expect(state.value.choices.find(a => a.id === 'b').correct).toBe(true);
       expect(state.value.choices.find(a => a.id === 'a').correct).toBe(false);
     });
 
     it('multiSelect: toggles only the target, leaves others unchanged', () => {
-      const { state, toggleCorrectChoice, qt } = setup(
+      const { state, toggleCorrectChoice, questionTypeRef } = setup(
         [makeAnswer({ id: 'a', correct: true }), makeAnswer({ id: 'b', correct: false })],
         QuestionType.MULTI_SELECT,
       );
-      qt.value = QuestionType.MULTI_SELECT;
+      questionTypeRef.value = QuestionType.MULTI_SELECT;
       toggleCorrectChoice('b');
       expect(state.value.choices.find(a => a.id === 'b').correct).toBe(true);
       expect(state.value.choices.find(a => a.id === 'a').correct).toBe(true);
     });
 
     it('multiSelect: toggles correct off when already correct', () => {
-      const { state, toggleCorrectChoice, qt } = setup(
+      const { state, toggleCorrectChoice, questionTypeRef } = setup(
         [makeAnswer({ id: 'a', correct: true }), makeAnswer({ id: 'b', correct: true })],
         QuestionType.MULTI_SELECT,
       );
-      qt.value = QuestionType.MULTI_SELECT;
+      questionTypeRef.value = QuestionType.MULTI_SELECT;
       toggleCorrectChoice('a');
       expect(state.value.choices.find(a => a.id === 'a').correct).toBe(false);
       expect(state.value.choices.find(a => a.id === 'b').correct).toBe(true);

contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useInteraction.spec.js

@@ -9,10 +9,6 @@ jest.mock('lodash/debounce', () => {
   });
 });
 
-// ---------------------------------------------------------------------------
-// Minimal descriptor stub
-// ---------------------------------------------------------------------------
-
 function makeDescriptor({ parseReturn = {}, buildReturn = null, validateReturn = [] } = {}) {
   return {
     parse: jest.fn(() => parseReturn),
@@ -76,7 +72,6 @@ describe('useInteraction', () => {
       questionType,
     );
 
-    // Because of immediate: true and mocked debounce, validate runs immediately
     expect(errors.value).toEqual(validateReturn);
   });
 
@@ -92,7 +87,6 @@ describe('useInteraction', () => {
 
     expect(errors.value).toEqual([]);
 
-    // Change the mock to return something else to simulate state change
     descriptor.validate.mockReturnValueOnce([{ code: 'NEW_ERROR' }]);
     runValidation();
 
@@ -163,15 +157,12 @@ describe('useInteraction', () => {
       questionType,
     );
 
-    // With mocked debounce, validate fires synchronously on immediate watcher.
-    // errors are already populated from the initial watcher run.
     expect(errors.value).toEqual(validateReturn);
 
-    // Reset mock and update state — validate should be called again.
     descriptor.validate.mockReset();
     descriptor.validate.mockReturnValue([{ code: 'UPDATED_ERROR' }]);
     state.value = { prompt: 'updated' };
-    await nextTick(); // flush Vue watcher queue
+    await nextTick();
 
     expect(descriptor.validate).toHaveBeenCalledWith({ prompt: 'updated' }, 'singleSelect');
     expect(errors.value).toEqual([{ code: 'UPDATED_ERROR' }]);

contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useInteractionDescriptor.spec.js

@@ -16,12 +16,6 @@ import {
 
 jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor');
 
-// ---------------------------------------------------------------------------
-// Helper: renders a wrapper component that runs the composable inside setup().
-// Because questionType is now set on mount (not as a computed), we must wait
-// for the component to mount before checking reactive values.
-// ---------------------------------------------------------------------------
-
 function renderDescriptor(initialXml = null, declarations = []) {
   const interactionRef = ref(
     initialXml ? { bodyXml: initialXml, responseDeclarations: declarations } : null,
@@ -40,10 +34,6 @@ function renderDescriptor(initialXml = null, declarations = []) {
   return { result, interactionRef };
 }
 
-// ---------------------------------------------------------------------------
-// Tests
-// ---------------------------------------------------------------------------
-
 describe('useInteractionDescriptor', () => {
   describe('with a valid choice interaction', () => {
     it('resolves the Choice descriptor by its type', async () => {
@@ -110,11 +100,10 @@ describe('useInteractionDescriptor', () => {
 
   describe('with malformed XML', () => {
     it('returns a parse error for malformed XML', async () => {
-      // inferFromXml uses text/xml which throws a parser error for malformed fragments.
       const { result } = renderDescriptor('<unclosed');
       await nextTick();
       expect(typeof result.parseError.value).toBe('string');
-      expect(result.parseError.value).toMatch(/parse error/);
+      expect(result.parseError.value).toBe('This question could not be loaded');
     });
 
     it('still returns a defined fallback descriptor on parse error', async () => {
@@ -132,22 +121,14 @@ describe('useInteractionDescriptor', () => {
       expect(result.questionType.value).toBe(QuestionType.SINGLE_SELECT);
       expect(result.descriptor.value.type).toBe(QtiInteraction.CHOICE);
 
-      // Simulate user switching question type via the selector
       result.questionType.value = QuestionType.MULTI_SELECT;
       await nextTick();
 
-      // Still the same descriptor (choice handles both), but questionType changed
       expect(result.descriptor.value.type).toBe(QtiInteraction.CHOICE);
       expect(result.questionType.value).toBe(QuestionType.MULTI_SELECT);
     });
   });
 
-  // ---------------------------------------------------------------------------
-  // Regression: inline placement — bodyXml is a full <qti-item-body>
-  // Before the fix, documentElement was <qti-item-body> and no descriptor
-  // matched it, so the code fell back to the choice descriptor for every
-  // text-entry item, showing them as "Multiple Choice / Single Choice".
-  // ---------------------------------------------------------------------------
   describe('with an inline text-entry interaction (bodyXml is qti-item-body)', () => {
     it('resolves the TextEntry descriptor for a numeric declaration', async () => {
       const { result } = renderDescriptor(TEXT_ENTRY_BODY_XML, [TEXT_ENTRY_NUMERIC_DECL_XML]);
@@ -169,13 +150,5 @@ describe('useInteractionDescriptor', () => {
       expect(result.descriptor.value.type).toBe(QtiInteraction.TEXT_ENTRY);
       expect(result.questionType.value).toBe(QuestionType.FREE_RESPONSE);
     });
-
-    it('does NOT resolve as choice when bodyXml is a qti-item-body (regression)', async () => {
-      // This is the exact bug: without the fix every text-entry item was
-      // treated as a choice interaction because documentElement was <qti-item-body>.
-      const { result } = renderDescriptor(TEXT_ENTRY_BODY_XML, [TEXT_ENTRY_FREE_DECL_XML]);
-      await nextTick();
-      expect(result.descriptor.value.type).not.toBe(QtiInteraction.CHOICE);
-    });
   });
 });

contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/__tests__/useTextEntryInteraction.spec.js

@@ -2,8 +2,6 @@ import { ref } from 'vue';
 import { useTextEntryInteraction } from '../useTextEntryInteraction';
 import { QuestionType, ValidationError } from '../../constants';
 
-// ─── Helpers ───────────────────────────────────────────────────────────────
-
 function makeNumericBlock(answerValues = ['12']) {
   const values = answerValues.map(v => `<qti-value>${v}</qti-value>`).join('');
   const cardinality = answerValues.length > 1 ? 'multiple' : 'single';
@@ -21,17 +19,15 @@ function makeFreeBlock() {
 }
 
 function setupNumeric(answerValues = ['12']) {
-  const qt = ref(QuestionType.NUMERIC);
-  return { qt, ...useTextEntryInteraction(makeNumericBlock(answerValues), qt) };
+  const questionType = ref(QuestionType.NUMERIC);
+  return { questionType, ...useTextEntryInteraction(makeNumericBlock(answerValues), questionType) };
 }
 
 function setupFree() {
-  const qt = ref(QuestionType.FREE_RESPONSE);
-  return { qt, ...useTextEntryInteraction(makeFreeBlock(), qt) };
+  const questionType = ref(QuestionType.FREE_RESPONSE);
+  return { questionType, ...useTextEntryInteraction(makeFreeBlock(), questionType) };
 }
 
-// ─── Tests ─────────────────────────────────────────────────────────────────
-
 describe('useTextEntryInteraction', () => {
   describe('initial state', () => {
     it('parses existing answers from the block', () => {

contentcuration/contentcuration/frontend/shared/views/QTIEditor/composables/useInteractionDescriptor.js

@@ -1,23 +1,21 @@
-import { computed, ref, onMounted } from 'vue';
+import { computed, ref } from 'vue';
 import { parseXML } from '../serialization/parseItem';
 import { descriptors, registry, DEFAULT_INTERACTION } from '../interactions/index';
+import { qtiEditorStrings } from '../qtiEditorStrings';
+
+const { errorParsingQuestion$ } = qtiEditorStrings;
 
 /**
- * Composable that manages the question type and descriptor for a single
- * interaction block.
+ * Composable that resolves the interaction descriptor and question type for a
+ * single interaction block.
  *
  * @param {import('vue').Ref<object>} interactionRef
  *   Ref to the interaction block { bodyXml, responseDeclarations }.
  */
 export default function useInteractionDescriptor(interactionRef) {
-  /** Writable question type — set from XML on mount, then driven by UI selections. */
-  const questionType = ref(null);
-  /** Any parse error message from the initial XML parse; null when clean. */
-  const parseError = ref(null);
-
   /**
-   * Parses bodyXml and returns { descriptor, questionType } without touching
-   * any reactive state — pure helper used only on mount.
+   * Parses bodyXml and returns the matching descriptor, resolved
+   * question type, and any parse error without touching reactive state.
    */
   function inferFromXml(xml, declarations) {
     if (!xml) {
@@ -33,29 +31,35 @@ export default function useInteractionDescriptor(interactionRef) {
         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: e.message,
+        error: errorParsingQuestion$(),
       };
     }
   }
 
-  /** Parse XML once when the host component mounts to set the initial state. */
-  onMounted(() => {
-    const inferred = inferFromXml(
-      interactionRef.value?.bodyXml,
-      interactionRef.value?.responseDeclarations,
-    );
-    questionType.value = inferred.questionType;
-    parseError.value = inferred.error;
-  });
+  /**
+   * Parse the initial XML synchronously during component setup.
+   *
+   * This ensures `questionType` is immediately available for downstream components
+   * on first render, avoiding prop validation warnings that would occur if
+   * initialization was deferred to a lifecycle hook.
+   */
+  const initial = inferFromXml(
+    interactionRef.value?.bodyXml,
+    interactionRef.value?.responseDeclarations,
+  );
+
+  /** Writable ref driven by UI selections after initial parse. */
+  const questionType = ref(initial.questionType);
+  const parseError = ref(initial.error);
 
   /**
-   * Descriptor is derived from the current questionType ref.
-   * Uses each descriptor's `questionTypes` array so the lookup stays accurate
-   * when the user changes the question type via the selector.
-   * Falls back to the default descriptor when no match is found.
+   * Derived from questionType so the descriptor updates when the user switches
+   * question types via the selector. Falls back to the default when no match.
    */
   const descriptor = computed(
     () =>

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/__tests__/defineInteraction.spec.js

@@ -31,8 +31,6 @@ describe('defineInteraction', () => {
     expect(descriptor.editorComponent).toBe(component);
   });
 
-  // Keys that must be present on the descriptor itself (editorComponent is injected
-  // by defineInteraction from the second argument, so it is excluded here).
   const REQUIRED_DESCRIPTOR_KEYS = [
     'type',
     'placement',
@@ -56,7 +54,6 @@ describe('defineInteraction', () => {
 
   it('throws when editorComponent is not passed as the second argument', () => {
     const descriptor = makeValidDescriptor();
-    // Calling with no second arg means editorComponent is undefined — still flagged.
     expect(() => defineInteraction(descriptor)).toThrow(/missing required key "editorComponent"/i);
   });
 

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

@@ -144,7 +144,6 @@ describe('parse()', () => {
 });
 
 describe('buildXML()', () => {
-  // Helper: parse an XML string and return the document root element.
   function parseXmlString(xml) {
     const parser = new DOMParser();
     const doc = parser.parseFromString(xml, 'text/xml');

contentcuration/contentcuration/frontend/shared/views/QTIEditor/interactions/index.js

@@ -9,9 +9,8 @@ import textEntryDescriptor from './textEntry/index';
 export const DEFAULT_INTERACTION = QtiInteraction.CHOICE;
 
 /**
- * Ordered array of all registered interaction descriptors.
- * InteractionSection iterates this to find the first descriptor whose
- * matches(el) returns true.
+ * Ordered list of all registered interaction descriptors.
+ * Searched in order; the first whose `matches(el)` returns true wins.
  */
 export const descriptors = [choiceDescriptor, textEntryDescriptor];
 
@@ -32,13 +31,3 @@ export const registry = Object.fromEntries(descriptors.map(d => [d.type, d]));
 export function getDescriptorForQuestionType(questionType) {
   return descriptors.find(d => d.questionTypes.includes(questionType));
 }
-
-/**
- * Find the interaction descriptor for a given QTI interaction tag name.
- *
- * @param {string} tagName
- * @returns {import('./defineInteraction').InteractionDescriptor|undefined}
- */
-export function getDescriptorForInteractionType(tagName) {
-  return registry[tagName];
-}