Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
import { render, screen } from '@testing-library/vue';
import { render, screen, fireEvent, within } from '@testing-library/vue';
import { nextTick } from 'vue';
import VueRouter from 'vue-router';
import InteractionSection from '../index.vue';
import { qtiEditorStrings as tr } from '../../../qtiEditorStrings';

import {
CHOICE_SINGLE_SELECT_XML,
CHOICE_MULTI_SELECT_XML,
UNKNOWN_INTERACTION_XML,
mockInteractionBlock as interactionBlock,
} from '../../../utils/testingFixtures';

jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor');
jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => {
const { ref } = require('vue');
return {
__esModule: true,
default: () => ({ windowIsSmall: ref(false) }),
};
});

const renderSection = (props = {}) =>
render(InteractionSection, {

@rtibblesbot rtibblesbot Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved — addressed in the current code.

nitpick: InteractionSection generates settingsTargetId internally and declares no targetId prop — this looks like a leftover from an earlier iteration, and lands as a stray DOM attribute.

Expand All @@ -36,12 +45,72 @@ describe('InteractionSection', () => {
expect(screen.getByText('Mercury')).toBeInTheDocument();
expect(screen.getByText('Venus')).toBeInTheDocument();
});

it('teleports answer settings into the question type selector header', async () => {
renderSection({ interaction: interactionBlock(CHOICE_MULTI_SELECT_XML) });
await nextTick();
const targetDiv = document.querySelector('.answer-settings-group');
expect(targetDiv).toBeInTheDocument();
expect(
within(targetDiv).getByRole('checkbox', { name: tr.$tr('shuffleAnswersLabel') }),
).toBeInTheDocument();
});
});

describe('parse error handling', () => {
it('shows a parse error when XML is malformed', () => {
renderSection({ interaction: interactionBlock('not-xml<{{') });
expect(screen.getByText('This question could not be loaded')).toBeInTheDocument();
expect(screen.getByText(tr.$tr('errorParsingQuestion'))).toBeInTheDocument();
});
});

describe('type switching', () => {
it('preserves the prompt but resets choices when switching from choice to text-entry', async () => {
Comment thread
AlexVelezLl marked this conversation as resolved.
const Wrapper = {
components: { InteractionSection },
template: `
<InteractionSection
mode="edit"
:interaction="interactionBlock"
@update:interaction="onUpdate"
/>
`,
data() {
return {
interactionBlock: {
bodyXml: CHOICE_SINGLE_SELECT_XML,
responseDeclarations: [],
},
};
},
methods: {
onUpdate(val) {
this.interactionBlock = val;
this.$emit('wrapper-update', val);
},
},
};

const { emitted } = render(Wrapper, {
routes: new VueRouter(),
});

await nextTick();

const selectedOption = screen.getAllByText(tr.$tr('singleSelectLabel'))[0];
await fireEvent.click(selectedOption);

const textEntryOption = screen.getByText(tr.$tr('textEntryLabel'));
await fireEvent.click(textEntryOption);

await nextTick();

const emits = emitted()['wrapper-update'];
const switchXml = emits.at(-1)[0].bodyXml;

expect(switchXml).toContain('Which planet is closest to the Sun?');
expect(switchXml).toContain('<qti-text-entry-interaction');
expect(switchXml).not.toContain('Mercury');
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,25 @@
>
{{ parseError }}
</p>
<component
:is="descriptor.editorComponent"
v-else
:key="descriptor.type"
:questionType="questionType"
:interaction="interaction"
:mode="mode"
:showAnswers="showAnswers"
@update:interaction="interaction => $emit('update:interaction', interaction)"
/>
<div v-else>
<QuestionTypeSelector
Comment thread
AlexVelezLl marked this conversation as resolved.
v-if="mode === 'edit'"
:questionType="questionType"
:settingsTargetId="settingsTargetId"
@update:questionType="onUpdateQuestionType"
/>

<component
:is="descriptor.editorComponent"
:key="descriptor.type"
:questionType="questionType"
:interaction="interaction"
:mode="mode"
:showAnswers="showAnswers"
:teleportTargetId="settingsTargetId"
Comment thread
AlexVelezLl marked this conversation as resolved.
Comment thread
AlexVelezLl marked this conversation as resolved.
@update:interaction="onUpdateInteraction"
/>
</div>
</div>

</template>
Expand All @@ -26,10 +35,17 @@

import { computed, watch } from 'vue';
import useInteractionDescriptor from '../../composables/useInteractionDescriptor';
import QuestionTypeSelector from '../QuestionTypeSelector/index.vue';
import { generateRandomSlug } from '../../utils/generateRandomSlug';
import { descriptors } from '../../interactions';

export default {
name: 'InteractionSection',

components: {
QuestionTypeSelector,
},

setup(props, { emit }) {
const interactionRef = computed(() => props.interaction);
const { descriptor, questionType, parseError } = useInteractionDescriptor(interactionRef);
Expand All @@ -42,7 +58,37 @@
{ immediate: true },
);

return { descriptor, questionType, parseError };
const onUpdateQuestionType = newType => {

@rtibblesbot rtibblesbot Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved — addressed in the current code.

suggestion: untested, and the only path that can lose authored content — freshState keeps prompt only, dropping every choice.

const newDescriptor = descriptors.find(d => d.questionTypes.includes(newType));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: descriptors.find(d => d.questionTypes.includes(newType)) is exactly getDescriptorForQuestionType, already exported from interactions/index.js:31. Importing the helper instead of descriptors keeps the registry lookup in one place.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: this is exactly getDescriptorForQuestionType (interactions/index.js:31), which is exported and currently unused. useInteractionDescriptor.js:66 open-codes the same rule, so importing it here keeps the lookup in one place rather than adding a third copy.

if (newDescriptor && newDescriptor !== descriptor.value) {

@rtibblesbot rtibblesbot Aug 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved — addressed in the current code.

blocking: SINGLE_SELECT / MULTI_SELECT share choiceDescriptor, so state.choices survives. buildChoiceInteractionXML writes every correct id (parse.js:187) under cardinality: 'single'; update:rawData persists it and validateChoiceInteraction raises TOO_MANY_CORRECT_ANSWERS. Fix: watch questionType in useChoiceInteraction and collapse choices to one correct, as toggleCorrectChoice does.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it should be upto user which option to mark as only correct option or we just take option randomly ? @AlexVelezLl

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not random — deterministic, and the UI already makes the pick. ChoiceInteractionEditor.vue:339 computes correctChoiceId as state.choices.find(a => a.correct)?.id, and the radios bind to it, so right after a multi→single switch the author already sees exactly one option selected: the first correct in document order. The extra correct flags survive only in state and in the XML buildChoiceInteractionXML writes. Collapsing to that same first-correct choice imposes nothing new on the author — it just makes the persisted data match what's already on screen.

The banner does fire (tooManyCorrectError), so it isn't silent, but it points at choices the author can't see as selected, and it goes out to update:rawData before they touch anything.

Clearing every correct flag instead is the other defensible option — explicit re-pick — but it swaps one error for NO_CORRECT_ANSWER (validation.js:42) and throws away the answer key. My preference is keep-first; either way it's @AlexVelezLl's call on the UX.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll handle this in a follow-up!

const oldState = descriptor.value.parse(
props.interaction.bodyXml,
props.interaction.responseDeclarations,
);
const freshState = newDescriptor.parse('', []);
const withPrompt = { ...freshState, prompt: oldState.prompt ?? '' };
const newInteraction = newDescriptor.buildXML(withPrompt, newType);
emit('update:interaction', newInteraction);
}

questionType.value = newType;
emit('update:questionType', newType);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: line 74 already triggers the watch at line 53, which emits update:questionType. This emits it a second time — harmless, but one of the two should go.

};

const onUpdateInteraction = updatedInteraction => {
emit('update:interaction', updatedInteraction);
};

const settingsTargetId = generateRandomSlug('answer-settings');

return {
descriptor,
questionType,
parseError,
onUpdateQuestionType,
onUpdateInteraction,
settingsTargetId,
};
},

props: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import { qtiEditorStrings } from '../../../qtiEditorStrings';
import { AssessmentItemTypes } from '../../../constants';

jest.mock('shared/views/TipTapEditor/TipTapEditor/TipTapEditor');
jest.mock('kolibri-design-system/lib/composables/useKResponsiveWindow', () => {
const { ref } = require('vue');
return {
__esModule: true,
default: () => ({ windowIsSmall: ref(false) }),
};
});

const { closeBtnLabel$, questionContentPlaceholder$ } = qtiEditorStrings;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<div class="question-card-body">
<InteractionSection
v-if="interactions.length > 0"
:interaction="interactions[0]"
:interaction="currentInteraction"
:mode="mode"
:showAnswers="showAnswers"
@update:questionType="type => (currentQuestionType = type)"
Expand Down Expand Up @@ -103,6 +103,11 @@
currentResponseDeclarations.value = interactions.value[0].responseDeclarations;
}

const currentInteraction = computed(() => ({
bodyXml: currentBodyXml.value,
responseDeclarations: currentResponseDeclarations.value,
}));

const questionNumberLabel = computed(() =>
questionNumberLabel$({
number: props.index + 1,
Expand Down Expand Up @@ -155,6 +160,7 @@
return {
currentQuestionType,
interactions,
currentInteraction,
questionNumberLabel,
questionNumberAndTypeLabel,
closeBtnLabel$,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { render, screen, fireEvent, within } from '@testing-library/vue';
import VueRouter from 'vue-router';
import QuestionTypeSelector from '../index.vue';
import { QuestionType } from '../../../constants';
import { qtiEditorStrings as tr } from '../../../qtiEditorStrings';

const defaultProps = {

@rtibblesbot rtibblesbot Aug 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved — addressed in the current code.

suggestion: defaultProps doesn't match the component's props. QuestionTypeSelector declares exactly two, questionType and settingsTargetId (both required); this passes questionTypeOptions and mode, neither of which exists, and omits settingsTargetId — so every render logs a missing-required-prop warning and the target div renders without an id. questionTypeOptions here is inert; the real options come from the jest.mock of ../../../interactions. If the blocking finding is taken, questionTypeOptions becomes a real prop and the mock goes away.

Also, it('disables selector when only one option available') asserts is-disabled on document.querySelector('.ui-select') — a KDS-internal class name that will break silently on a KDS upgrade. toBeDisabled() on the combobox role would survive it.

questionType: QuestionType.SINGLE_SELECT,
settingsTargetId: 'test-settings-target',
};

const renderHeader = (props = {}) =>
render(QuestionTypeSelector, {
props: { ...defaultProps, ...props },
routes: new VueRouter(),
});

describe('QuestionTypeSelector', () => {
it('renders the type meta-label in edit mode', () => {
renderHeader();
expect(screen.getByText(tr.$tr('typeLabel'))).toBeInTheDocument();
});

it('renders a KSelect with the selected option label (not raw enum)', () => {
renderHeader();
expect(screen.getByText(tr.$tr('singleSelectLabel'))).toBeInTheDocument();
expect(screen.queryByText(QuestionType.SINGLE_SELECT)).not.toBeInTheDocument();
});

it('renders the selected type label inside the type group', () => {
renderHeader();
const group = screen.getByRole('group', { name: tr.$tr('typeLabel') });
expect(within(group).getByText(tr.$tr('singleSelectLabel'))).toBeInTheDocument();
});

it('opens type info modal when info button clicked', async () => {
renderHeader();

const helpButton = screen.getByRole('button', { name: tr.$tr('responseTypeInfoTitle') });
await fireEvent.click(helpButton);

expect(screen.getByRole('dialog')).toBeInTheDocument();
expect(screen.getByText(tr.$tr('singleChoiceDescription'))).toBeInTheDocument();
expect(screen.getByText(tr.$tr('multipleSelectionDescription'))).toBeInTheDocument();
});

it('closes type info modal when Close button clicked', async () => {
renderHeader();

await fireEvent.click(screen.getByRole('button', { name: tr.$tr('responseTypeInfoTitle') }));
expect(screen.getByRole('dialog')).toBeInTheDocument();

await fireEvent.click(screen.getByRole('button', { name: tr.$tr('closeBtnLabel') }));
expect(screen.queryByRole('dialog')).not.toBeInTheDocument();
});

it('emits update:questionType when a new type is selected', async () => {
const { emitted } = renderHeader();

// Click the currently selected option to open the dropdown
await fireEvent.click(screen.getByText(tr.$tr('singleSelectLabel')));

// Click the new option from the dropdown menu
await fireEvent.click(screen.getByText(tr.$tr('multiSelectLabel')));

expect(emitted()['update:questionType']).toBeTruthy();
expect(emitted()['update:questionType'][0]).toEqual([QuestionType.MULTI_SELECT]);
});
});
Loading
Loading