Repository navigation
Exercise: Import and export matching questions in QTI2, and export fill-in-the-blanks questions - #9281
Merged
ywarnier merged 9 commits intoOct 20, 2026
Conversation
…o#1377 QTI 2.1 packages containing a matchInteraction were imported without their matching questions: the legacy importer (qti2.php) only accepted a fixed list of types and the API importer (ExerciseQti2ImportService) detected type 4 but did not support it, so both skipped the question. Import: - Legacy importer: accept MATCHING, build option rows (correct = 0) followed by item rows whose correct value is the option position, with the score from the mapping (or the default), so Answer::save() links them to the option ids. Questions without associations are skipped with a warning. - ExerciseQti2ImportService: add Matching to the supported types, create the option answers first, flush to get their ids and then create the item answers pointing to them; question score is the sum of the item scores. Questions without associations are reported as skipped. - Both standard (multiple/directedPair) and older Chamilo (single/identifier) response declarations are read. QTI cannot tell Matching and Matching draggable apart, so both import as Matching. Export (needed for the round trip, matching was not exported): - ExerciseQti2ExportService: export Matching and Matching draggable as a matchInteraction with a directedPair response declaration and per-pair scores. - Legacy ImsAnswerMatching/qti2_export: list options and items from the stored rows, emit the prompt and a multiple/directedPair declaration, and accept matching questions as exportable. Tests: PHPUnit for the service (incl. export/import round trip) and the legacy parser, plus a Playwright scenario importing a QTI2 zip with a matching question.
Matching options and items are displayed without any HTML filtering in the exercise views (unique and multiple answers go through Security::remove_XSS() when displayed), so importing them as-is from an uploaded QTI2 package let the package plant scripts, event handlers or javascript: URLs that run for students and teachers. Both QTI2 importers now clean the matching texts with HTMLPurifier (QtiImportHtmlSanitizer) before storing them; formatting is kept.
Fill in blanks questions were not exported to QTI2 by either exporter. Export (ExerciseQti2ExportService and legacy ImsAnswerFillInBlanks, both through the new Qti2FillBlanksWriter so they write the same markup): - typed blanks ([word], [word||alternative]) as textEntryInteraction with a string response, menus ([correct|wrong1|wrong2]) as inlineChoiceInteraction, each blank in its own fill_N declaration with its score in the mapping (caseSensitive="false" for case-insensitive questions), processed with map_response; - the text around the blanks is kept in spans, as escaped HTML in CDATA like the other texts of Chamilo QTI2 exports; any blank separator is read. Import (both QTI2 importers), so that the export round-trips: - each blank is rebuilt from its own interaction, so a question can mix menus and typed blanks (one kind used to be turned into "[]" or into the choice identifier); - each blank takes its score from its own response mapping (menu scores were lost and identical words shared one score); - spans without attributes are read as plain text; - caseSensitive="false" sets the question as case insensitive. Input sizes, the blank separator and "allow answers order switches" have no QTI equivalent and are imported with the defaults.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #9281 +/- ##
============================================
+ Coverage 17.20% 17.56% +0.35%
- Complexity 62176 62303 +127
============================================
Files 2522 2527 +5
Lines 233319 233745 +426
============================================
+ Hits 40150 41053 +903
+ Misses 193169 192692 -477 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1377
Problem
The issue asked for more question types in QTI exports, naming fill-in-the-blanks and matching.
never writes them. On current master (local portal, Oct 20), importing a zip with one matching
question and one unique-answer question through the Vue "Import exercises Qti2" page gives
Imported questions: 1 / Skipped questions: 1. The legacyqti2.phpimport keeps only theunique-answer question.
questions:
ExerciseQti2ExportServiceskips them and the legacyexport_question_qti()rejects themin
isValid(). The legacyImsAnswerFillInBlanksclass exists but can't work: it treats thewhole stored answer row (text plus the
::scores:sizes:separatorpart) as one blank word, so no blankis ever replaced, it declares the response as
identifierand writesmapKeywithout escaping.This PR adds matching import and export, and fill-in-the-blanks export, in both code paths, so a
Chamilo-to-Chamilo round trip works for both types. Since packages are untrusted input and the legacy
export entry points now export two more question types, it also cleans the imported HTML of both types
and checks that an exported question or exercise belongs to the current course.
Cause
public/main/exercise/export/exercise_import.inc.php): it accepts a fixed listof question types. MATCHING is not on the list, so the parsed question is skipped.
ExerciseQti2ImportService: it mapsmatchInteractionto type 4, but type 4 is missing fromSUPPORTED_TYPES, so the question is counted as skipped.ExerciseQti2ExportServiceexports neither matching nor fill-in-the-blanks questions.export_question_qti()(qti2_export.php) rejects both inisValid().ImsAnswerMatchingloops overleftListandrightList, which nothing fills, writes the rawquestion text inside the interaction and declares the response as
single/identifier.Fix
The two importers already duplicate their parsing logic (for example
buildFibAnswerString), so thematching import logic is duplicated the same way. The fill-in-the-blanks writer is shared
(
Qti2FillBlanksWriter), because both exporters must write exactly the same XML.Matching import
simpleMatchSetblocks. Set 1 holds the items (left) and set 2 theoptions (right). Associations come from the
correctResponsepairs<item> <option>. A reversedpair is swapped. Each item keeps its first pair. Scores come from
mapping/mapEntry, falling back tothe default value.
multiple/directedPair, and thesingle/identifierthat the old Chamilo exporter wrote.(
correct = 0), then the item rows, whosecorrectpoints to their option and whoseponderationis the pair's score. The question score is the sum of the item scores.
QtiImportHtmlSanitizer) before they arestored. Matching texts are rendered as HTML in the exercise, so without this a package could store
script in an answer (see "Security review").
ExerciseQuestionImportProvidernow list matching questions.Matching export
ExerciseQti2ExportServiceexports Matching and Matching draggable as amatchInteractionwith aprompt, set 1 with the items
left_<iid>(matchMax="1"), set 2 with the optionsright_<iid>(
matchMax="0") and amultiple/directedPairdeclaration with onemapEntryper pair.ImsAnswerMatchingbuilds its lists from the stored rows and writes the same structure.Fill-in-the-blanks export
Chamilo stores a FIB question as one answer row,
<html text with [blanks]>::<scores>:<input sizes>:<separator>@<switchable>. The newQti2FillBlanksWriterparses it with the same rules asFillBlanks::getAnswerInfo()and writes oneQTI 2.1 interaction per blank, with identifiers
fill_1,fill_2, …:[PHP]textEntryInteraction;responseDeclarationsingle/string, correct valuePHP,mapEntry mapKey="PHP" mappedValue="<score>"[1.11||1.11.x]mapEntryper alternative with the blank's score; the correct value is1.11||1.11.x(read back by Chamilo's importers)[Linux|Windows|macOS](first option is correct)inlineChoiceInteraction shuffle="false"with oneinlineChoiceper option;single/identifierdeclaration, correct value andmapEntryon the first optioncase:false)caseSensitive="false"on every typedmapEntrymap_responsetemplate, with anoutcomeDeclaration SCORE.<span><![CDATA[…]]></span>, HTML-escaped inside the CDATA,like the other Chamilo QTI texts. Options are written the same way,
mapKeyand identifiers asXML-escaped attributes. A
]]>in the content can't close the CDATA because it is escaped first.[] {} () ** ## %% $$is read; scores are written as finite numbers only.ExerciseQti2ExportServiceand the legacyImsAnswerFillInBlanks/qti2_export.php(
isValid(),map_responsefor FIB,extracopied for the case option) use the writer.Fill-in-the-blanks import fixes needed for the round trip
Both importers already accepted
textEntryInteractionandinlineChoiceInteraction, but a round tripthrough them lost data. The fixes are small and shared by both importers:
question was handled as one kind of blank, and menus became
[fill_2_1], see the Playwright run).mapEntry. Before, a menu got 1 point and two blanks withthe same word shared one score.
<span>wrapper around text is unwrapped, so the text doesn't gain extra tags on each roundtrip.
caseSensitive="false"on a blank sets the question's "Ignore case" option.QtiImportHtmlSanitizer(HTMLPurifier) as thematching texts: scripts, event handlers and
javascript:URLs are removed, formatting is kept. Theblanks are swapped for plain tokens before purifying and put back afterwards, so
[word],[a||b],[a|b|c]and the other separators come out unchanged. Inside a blank,<and>areescaped (and quotes, when the blank sits inside an attribute), so a blank can't bring markup back.
A blank that the purifier removes with its element (inside a
<script>, for example) is droppedwith its score, so the scores still match the blanks.
Legacy export entry points
admin.php?action=exportqti2&questionId=exported any question id, from any course, to a teacher ofany course;
exercise.php?action=exportqti2&exerciseId=did the same with exercises. Both nowanswer with the usual "not allowed" page unless the question/exercise has a live resource link in the
current course (and session, or the base course in a session). The exercise uses the existing
CQuizRepository::findInCourseContext(); the question uses a newCQuizQuestionRepository::findInCourseContext()written the same way. The API export(
/api/exercise/{id}/qti2-export) already did this check.Call to undefined method Question::getPictureFilename()inImsAssessmentItem::export()). Themethod was removed in 3.x and the value is only read by the hotspot exporter, which isn't reachable,
so an empty string is passed instead. Without this, the legacy exporter changes above couldn't be
used at all.
What QTI 2.1 can't carry (by design)
[]. A question using another separatorwhose text contains literal
[]may be read differently after import.@1) is not exported.question case-insensitive.
mapEntryvalues; thea||bcorrect value isChamilo-specific and other tools see it as one literal answer.
Files:
public/main/exercise/admin.php,public/main/exercise/exercise.php(course check)public/main/exercise/export/exercise_import.inc.phppublic/main/exercise/export/qti2/ImsAnswerFillInBlanks.phppublic/main/exercise/export/qti2/ImsAnswerMatching.phppublic/main/exercise/export/qti2/qti2_export.phpsrc/CoreBundle/Service/Exercise/ExerciseQti2ExportService.phpsrc/CoreBundle/Service/Exercise/ExerciseQti2ImportService.phpsrc/CoreBundle/Service/Exercise/Qti2FillBlanksWriter.php(new)src/CoreBundle/Service/Exercise/QtiImportHtmlSanitizer.php(new)src/CoreBundle/State/Exercise/ExerciseQuestionImportProvider.php(import notes only)src/CourseBundle/Repository/CQuizQuestionRepository.php(findInCourseContext())Tests:
tests/CoreBundle/Service/Exercise/ExerciseQti2ImportServiceTest.phptests/CoreBundle/Service/Exercise/LegacyQti2ImportMatchingTest.phptests/CoreBundle/Service/Exercise/ExerciseQti2FillBlanksExportTest.php(new)tests/playwright/features/toolExerciseQti2Import.feature, its generated spec,tests/playwright/fixtures/qti2_matching.zipandtests/playwright/fixtures/qti2_fill_blanks.zipcommon.steps.ts:the row for {string} should contain {string},I save the file linked by the {string} icon in the row for {string} as {string},the fill in blanks preview should show {string}, {string} and {string}andthe QTI2 export of the question {string} should work in course {string} but be refused in course {string}How it was tested
The branch is rebased on master f84ab3f. All runs below are from Oct 20.
php bin/phpunit tests/CoreBundle/Service/Exercise/ExerciseQti2ImportServiceTest.php: OK, 7 tests,59 assertions. Matching import with both declaration styles, rows, scores, positions, an export →
import round trip, the skipped-question report and the HTML sanitizing of matching texts; the HTML
sanitizing of a fill-in-the-blanks text (script,
onclick/onerror,javascript:removed; bold,link, image, menu and alternatives kept; the blank inside the
<script>dropped with its score),other separators, and a blank inside an attribute.
php bin/phpunit tests/CoreBundle/Service/Exercise/LegacyQti2ImportMatchingTest.php: OK, 5 tests,16 assertions. The same through the legacy
qti_parse_file()+buildMatchingAnswerRows()/buildFibAnswerString().php bin/phpunit tests/CoreBundle/Service/Exercise/ExerciseQti2FillBlanksExportTest.php: OK,16 tests, 157 assertions. For typed, alternatives, menu and mixed blanks, the
{}separator, HTML,entities and
]]>in the text, and two equal words with different scores, it exports with theservice and with the legacy exporter (both must write the same item), imports the XML again through
both importers and compares the stored answer string and the question score. It also checks the
XML structure with XPath, the escaping, the "Ignore case" option and a question without blanks.
master's importers, 13 failures. With the production files reverted to master, the matching tests
fail as well (3 errors + 1 failure, and 3 errors).
tests/CoreBundle/Service/Exercisedirectory: 64 tests. The 44 tests that need noDB pass. The 20 DB-backed tests error with
SQLSTATE[HY000] [1698] Access denied for user 'root'@'localhost', from the local test-DB setup, the same without this change.toolExerciseQti2Import.feature, admin in the seeded courseTEMP):Imported questions: 2, no skipped questions, total score 7, thedistractor
D: Lisbonand the item/option rows in the preview.qti2_fill_blanks.zip(one question with a typed blank, a 3-option menuand a typed blank with alternatives, 1 + 2 + 3 points), checks the total score 6 and the three blanks
in the question preview; then downloads the exercise with the list's "Export QTI2" link, imports
that file and checks the same things again. Cleanup scenarios delete the exercises.
admin.php?action=exportqti2andexercise.php?action=exportqti2URLs return a zip in the courseTEMPand the "not allowed" page (no zip) in the courseAIACT. With the course check reverted,admin.phpinAIACTreturns the zip of theTEMPquestion (the step fails).blank is shown as
[fill_2_1]; the export → import gives no "File imported"; the cleanup can't findthe second FIB exercise).
toolExerciseAdmin.feature+toolExerciseQti2Import.feature27/27 passed (12:43 CEST).
npx playwright test --config=tests/playwright/playwright.config.ts <spec> --retries=0 --workers=1vendor/bin/phpstan analyseon the changedsrc/files: no errors.vendor/bin/ecs checkon the changedsrc/and test files: clean. The legacy files have the samefindings as on master, none on the changed lines.
Security review
An OWASP-style review of the branch was done (Oct 20):
LIBXML_NOENT, networkdisabled) and libxml rejects entity amplification ("billion laughs").
were stored as raw HTML and rendered as HTML. This PR made matching importable and FIB round-trippable,
so both are now sanitized with HTMLPurifier (
QtiImportHtmlSanitizer), with regression tests in bothimporter test files.
identifiers are generated and scores are written as finite numbers.
exportqti2actions now check that the question/exercise belongs tothe current course (see above); the API export already did. No new route, endpoint or form.
AnswerAPIs only.Found on master and not changed here (to be reported separately): the text of questions of other
types imported by the legacy importer and the description of imported questions are stored without
HTMLPurifier (the service uses a regex-based cleanup),
qti2.phphas no CSRF token on its uploadform, zip entries are read without a size limit, and a score such as
1e309becomesINF.Not covered / notes for reviewers
teachers (on 1.10.x). This was not checked on 3.x.
stored rows are the same as those the question editors create.
association are dropped on import (Chamilo can't store them); distractor options are kept.
3.x/en/on the docs repository'sallbranch is attached tothis PR (
qti2-import-matching.md), to be merged into the docs manually.