Skip to content

Commit 67f7a2b

Browse files
committed
MDL-26511 qtype_multianswer: Improve support for embedded files
1 parent 32ba862 commit 67f7a2b

10 files changed

Lines changed: 601 additions & 38 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
issueNumber: MDL-26511
2+
notes:
3+
qtype_multianswer:
4+
- message: >-
5+
Improved embedded file support for qtype_multianswer (Cloze) questions.
6+
This change is backwards compatible with existing and imported
7+
questions, with a new adhoc task for migrating legacy files to the
8+
correct file area.
9+
type: improved

public/question/type/multianswer/backup/moodle2/restore_qtype_multianswer_plugin.class.php

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,26 @@ public function after_execute_question() {
136136
$rs->close();
137137
}
138138

139+
/**
140+
* Migrate legacy answer files once all restore file processing has completed.
141+
*/
142+
public function after_restore_question() {
143+
global $DB;
144+
145+
$restoredquestionids = $DB->get_fieldset_sql(
146+
"SELECT qma.question
147+
FROM {question_multianswer} qma
148+
JOIN {backup_ids_temp} bi ON bi.newitemid = qma.question
149+
WHERE bi.backupid = ?
150+
AND bi.itemname = 'question_created'",
151+
[$this->get_restoreid()]
152+
);
153+
154+
if (!empty($restoredquestionids)) {
155+
qtype_multianswer\task\copy_legacy_answer_files::copy_legacy_answer_files_to_questiontext($restoredquestionids);
156+
}
157+
}
158+
139159
public function recode_response($questionid, $sequencenumber, array $response) {
140160
global $DB;
141161

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
<?php
2+
// This file is part of Moodle - https://moodle.org/
3+
//
4+
// Moodle is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// Moodle is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with Moodle. If not, see <https://www.gnu.org/licenses/>.
16+
17+
namespace qtype_multianswer\task;
18+
19+
/**
20+
* Migrate legacy Cloze files from child answer areas to the parent questiontext file area.
21+
*
22+
* Historically, files embedded in multichoice subquestions were stored within the child
23+
* question itself. Modern versions consolidate these files into the parent question.
24+
*
25+
* Filenames must be unique within the new parent questiontext area. If a single
26+
* question contains identically named but differing files across its child areas,
27+
* only the first unique filename is preserved. As a result, the affected question
28+
* may inadvertently render a duplicated file in place of the intended one.
29+
*
30+
* @package qtype_multianswer
31+
* @copyright 2026 Catalyst IT Australia Pty Ltd
32+
* @license https://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
33+
*/
34+
class copy_legacy_answer_files extends \core\task\adhoc_task {
35+
#[\Override]
36+
public function execute() {
37+
$customdata = (array) $this->get_custom_data();
38+
$questionids = $customdata['questionids'] ?? [];
39+
40+
$copied = self::copy_legacy_answer_files_to_questiontext($questionids);
41+
mtrace("Copied {$copied} legacy Cloze answer files to parent questiontext areas.");
42+
}
43+
44+
/**
45+
* Copy legacy files from embedded subquestion answer areas to the parent Cloze questiontext area.
46+
* This process is immutable and idempotent as questions and files are not modified.
47+
*
48+
* @param array $questionids Optionally limit the query to specific questions for performance.
49+
* @return int The number of files copied.
50+
*/
51+
public static function copy_legacy_answer_files_to_questiontext(array $questionids = []): int {
52+
global $DB;
53+
54+
if (!empty($questionids)) {
55+
[$insql, $params] = $DB->get_in_or_equal($questionids, SQL_PARAMS_NAMED);
56+
$questionjoin = "AND parent.id {$insql}";
57+
} else {
58+
$params = [];
59+
$questionjoin = '';
60+
}
61+
62+
// NOT EXISTS excludes already migrated and colliding filenames.
63+
// GROUP BY deduplicates cases where answer and answerfeedback both carry the same filename.
64+
$sql = "SELECT MIN(f.id) AS fileid,
65+
f.filepath,
66+
f.filename,
67+
parent.id AS parentid,
68+
parentcontext.contextid AS parentcontextid
69+
FROM {files} f
70+
JOIN {question_answers} qa ON qa.id = f.itemid
71+
JOIN {question} child ON child.id = qa.question
72+
JOIN {question} parent ON parent.id = child.parent
73+
JOIN {question_versions} parentversion ON parentversion.questionid = parent.id
74+
JOIN {question_bank_entries} parententry ON parententry.id = parentversion.questionbankentryid
75+
JOIN {question_categories} parentcontext ON parentcontext.id = parententry.questioncategoryid
76+
WHERE f.component = 'question'
77+
AND f.filearea IN ('answer', 'answerfeedback')
78+
AND f.filename <> '.'
79+
AND parent.qtype = 'multianswer'
80+
{$questionjoin}
81+
AND NOT EXISTS (
82+
SELECT 1
83+
FROM {files} target
84+
WHERE target.contextid = parentcontext.contextid
85+
AND target.component = 'question'
86+
AND target.filearea = 'questiontext'
87+
AND target.itemid = parent.id
88+
AND target.filepath = f.filepath
89+
AND target.filename = f.filename
90+
)
91+
GROUP BY f.filepath, f.filename, parent.id, parentcontext.contextid
92+
ORDER BY f.filepath, f.filename, parent.id";
93+
94+
$fs = get_file_storage();
95+
$copied = 0;
96+
$files = $DB->get_recordset_sql($sql, $params);
97+
try {
98+
foreach ($files as $filedata) {
99+
$file = $fs->get_file_by_id($filedata->fileid);
100+
if (!$file) {
101+
continue;
102+
}
103+
$fs->create_file_from_storedfile([
104+
'contextid' => $filedata->parentcontextid,
105+
'component' => 'question',
106+
'filearea' => 'questiontext',
107+
'itemid' => $filedata->parentid,
108+
'filepath' => $filedata->filepath,
109+
'filename' => $filedata->filename,
110+
], $file);
111+
$copied++;
112+
}
113+
} finally {
114+
$files->close();
115+
}
116+
117+
return $copied;
118+
}
119+
}

public/question/type/multianswer/db/upgrade.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,13 @@ function xmldb_qtype_multianswer_upgrade($oldversion) {
5252
// Automatically generated Moodle v5.2.0 release upgrade line.
5353
// Put any upgrade step following this.
5454

55+
if ($oldversion < 2026042001) {
56+
mtrace('Queueing legacy Cloze answer file copy task.');
57+
$task = new \qtype_multianswer\task\copy_legacy_answer_files();
58+
\core\task\manager::queue_adhoc_task($task, true);
59+
60+
upgrade_plugin_savepoint(true, 2026042001, 'qtype', 'multianswer');
61+
}
62+
5563
return true;
5664
}

public/question/type/multianswer/questiontype.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,10 @@ function qtype_multianswer_initialise_multichoice_subquestion($wrapped) {
407407

408408
function qtype_multianswer_extract_question($text) {
409409
// Variable $text is an array [text][format][itemid].
410+
411+
$rewritefileurls = fn($content): string =>
412+
empty($text['itemid']) ? $content : file_rewrite_urls_to_pluginfile($content, $text['itemid']);
413+
410414
$question = new stdClass();
411415
$question->qtype = 'multianswer';
412416
$question->questiontext = $text;
@@ -498,7 +502,7 @@ function qtype_multianswer_extract_question($text) {
498502
$wrapped->answer = array();
499503
$wrapped->fraction = array();
500504
$wrapped->feedback = array();
501-
$wrapped->questiontext['text'] = $answerregs[0];
505+
$wrapped->questiontext['text'] = $rewritefileurls($answerregs[0]);
502506
$wrapped->questiontext['format'] = FORMAT_HTML;
503507
$wrapped->questiontext['itemid'] = '';
504508
$answerindex = 0;
@@ -519,7 +523,7 @@ function qtype_multianswer_extract_question($text) {
519523
$feedback = html_entity_decode(
520524
$altregs[ANSWER_ALTERNATIVE_REGEX_FEEDBACK], ENT_QUOTES, 'UTF-8');
521525
$feedback = str_replace('\}', '}', $feedback);
522-
$wrapped->feedback["{$answerindex}"]['text'] = str_replace('\#', '#', $feedback);
526+
$wrapped->feedback["{$answerindex}"]['text'] = $rewritefileurls(str_replace('\#', '#', $feedback));
523527
$wrapped->feedback["{$answerindex}"]['format'] = FORMAT_HTML;
524528
$wrapped->feedback["{$answerindex}"]['itemid'] = '';
525529
} else {
@@ -543,7 +547,7 @@ function qtype_multianswer_extract_question($text) {
543547
$answer = html_entity_decode(
544548
$altregs[ANSWER_ALTERNATIVE_REGEX_ANSWER], ENT_QUOTES, 'UTF-8');
545549
$answer = str_replace('\}', '}', $answer);
546-
$wrapped->answer["{$answerindex}"] = str_replace('\#', '#', $answer);
550+
$wrapped->answer["{$answerindex}"] = $rewritefileurls(str_replace('\#', '#', $answer));
547551
if ($wrapped->qtype == 'multichoice') {
548552
$wrapped->answer["{$answerindex}"] = array(
549553
'text' => $wrapped->answer["{$answerindex}"],

public/question/type/multianswer/renderer.php

Lines changed: 64 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,19 @@ abstract public function subquestion(question_attempt $qa,
151151
question_display_options $options, $index,
152152
question_graded_automatically $subq);
153153

154+
/**
155+
* Format question text and process embedded files using the parent question context.
156+
*
157+
* @param question_attempt $qa the current attempt.
158+
* @param string $text the text to format.
159+
* @param int $format the text format.
160+
* @return string formatted text.
161+
*/
162+
protected function format_embedded_text(question_attempt $qa, string $text, int $format): string {
163+
$question = $qa->get_question();
164+
return $question->format_text($text, $format, $qa, 'question', 'questiontext', $question->id);
165+
}
166+
154167
/**
155168
* Render the feedback pop-up contents.
156169
*
@@ -327,15 +340,21 @@ public function subquestion(question_attempt $qa, question_display_options $opti
327340
$correctanswer = $subq->get_correct_answer();
328341
}
329342

330-
$feedbackpopup = $this->feedback_popup($subq, $matchinganswer->fraction,
331-
$subq->format_text($matchinganswer->feedback, $matchinganswer->feedbackformat,
332-
$qa, 'question', 'answerfeedback', $matchinganswer->id),
333-
s($correctanswer->answer), $options);
343+
$feedbackpopup = $this->feedback_popup(
344+
$subq,
345+
$matchinganswer->fraction,
346+
$this->format_embedded_text($qa, $matchinganswer->feedback, $matchinganswer->feedbackformat),
347+
s($correctanswer->answer),
348+
$options
349+
);
334350

335351
$output = html_writer::start_tag('span', ['class' => 'subquestion']);
336352

337-
$output .= html_writer::tag('label', $this->get_answer_label(),
338-
array('class' => 'subq accesshide', 'for' => $inputattributes['id']));
353+
$output .= html_writer::tag(
354+
'label',
355+
$this->get_answer_label(),
356+
['class' => 'subq accesshide', 'for' => $inputattributes['id']]
357+
);
339358
$output .= html_writer::empty_tag('input', $inputattributes);
340359
$output .= $this->get_feedback_image($feedbackimg, $feedbackpopup);
341360
$output .= html_writer::end_tag('span');
@@ -368,8 +387,7 @@ public function subquestion(question_attempt $qa, question_display_options $opti
368387
$rightanswer = null;
369388
foreach ($subq->get_order($qa) as $value => $ansid) {
370389
$ans = $subq->answers[$ansid];
371-
$choices[$value] = $subq->format_text($ans->answer, $ans->answerformat,
372-
$qa, 'question', 'answer', $ansid);
390+
$choices[$value] = $this->format_embedded_text($qa, $ans->answer, $ans->answerformat);
373391
if ($subq->is_choice_selected($response, $value)) {
374392
$matchinganswer = $ans;
375393
}
@@ -397,15 +415,20 @@ public function subquestion(question_attempt $qa, question_display_options $opti
397415
if (!$matchinganswer) {
398416
$matchinganswer = new question_answer(0, '', null, '', FORMAT_HTML);
399417
}
400-
$feedbackpopup = $this->feedback_popup($subq, $matchinganswer->fraction,
401-
$subq->format_text($matchinganswer->feedback, $matchinganswer->feedbackformat,
402-
$qa, 'question', 'answerfeedback', $matchinganswer->id),
403-
$subq->format_text($rightanswer->answer, $rightanswer->answerformat,
404-
$qa, 'question', 'answer', $rightanswer->id), $options);
418+
$feedbackpopup = $this->feedback_popup(
419+
$subq,
420+
$matchinganswer->fraction,
421+
$this->format_embedded_text($qa, $matchinganswer->feedback, $matchinganswer->feedbackformat),
422+
$this->format_embedded_text($qa, $rightanswer->answer, $rightanswer->answerformat),
423+
$options
424+
);
405425

406426
$output = html_writer::start_tag('span', array('class' => 'subquestion'));
407-
$output .= html_writer::tag('label', $this->get_answer_label(),
408-
array('class' => 'subq accesshide', 'for' => $inputattributes['id']));
427+
$output .= html_writer::tag(
428+
'label',
429+
$this->get_answer_label(),
430+
['class' => 'subq accesshide', 'for' => $inputattributes['id']]
431+
);
409432
$output .= $select;
410433
$output .= $this->get_feedback_image($feedbackimg, $feedbackpopup);
411434
$output .= html_writer::end_tag('span');
@@ -468,16 +491,19 @@ public function subquestion(question_attempt $qa, question_display_options $opti
468491

469492
$result .= $this->choice_wrapper_start($class);
470493
$result .= html_writer::empty_tag('input', $inputattributes);
471-
$result .= html_writer::tag('label', $subq->format_text($ans->answer,
472-
$ans->answerformat, $qa, 'question', 'answer', $ansid),
473-
array('for' => $inputattributes['id'], 'class' => 'form-check-label text-body'));
494+
$result .= html_writer::tag(
495+
'label',
496+
$this->format_embedded_text($qa, $ans->answer, $ans->answerformat),
497+
['for' => $inputattributes['id'], 'class' => 'form-check-label text-body']
498+
);
474499
$result .= $feedbackimg;
475500

476501
if ($options->feedback && $isselected && trim($ans->feedback)) {
477-
$result .= html_writer::tag('div',
478-
$subq->format_text($ans->feedback, $ans->feedbackformat,
479-
$qa, 'question', 'answerfeedback', $ansid),
480-
array('class' => 'specificfeedback'));
502+
$result .= html_writer::tag(
503+
'div',
504+
$this->format_embedded_text($qa, $ans->feedback, $ans->feedbackformat),
505+
['class' => 'specificfeedback']
506+
);
481507
}
482508

483509
$result .= $this->choice_wrapper_end();
@@ -499,9 +525,11 @@ public function subquestion(question_attempt $qa, question_display_options $opti
499525
foreach ($subq->answers as $ans) {
500526
if (question_state::graded_state_for_fraction($ans->fraction) ==
501527
question_state::$gradedright) {
502-
$feedback[] = get_string('correctansweris', 'qtype_multichoice',
503-
$subq->format_text($ans->answer, $ans->answerformat,
504-
$qa, 'question', 'answer', $ansid));
528+
$feedback[] = get_string(
529+
'correctansweris',
530+
'qtype_multichoice',
531+
$this->format_embedded_text($qa, $ans->answer, $ans->answerformat)
532+
);
505533
break;
506534
}
507535
}
@@ -661,16 +689,19 @@ public function subquestion(question_attempt $qa, question_display_options $opti
661689

662690
$result .= $this->choice_wrapper_start($class);
663691
$result .= html_writer::empty_tag('input', $inputattributes);
664-
$result .= html_writer::tag('label', $subq->format_text($ans->answer,
665-
$ans->answerformat, $qa, 'question', 'answer', $ansid),
666-
['for' => $inputattributes['id'], 'class' => 'form-check-label text-body']);
692+
$result .= html_writer::tag(
693+
'label',
694+
$this->format_embedded_text($qa, $ans->answer, $ans->answerformat),
695+
['for' => $inputattributes['id'], 'class' => 'form-check-label text-body']
696+
);
667697
$result .= $feedbackimg;
668698

669699
if ($options->feedback && $isselected && trim($ans->feedback)) {
670-
$result .= html_writer::tag('div',
671-
$subq->format_text($ans->feedback, $ans->feedbackformat,
672-
$qa, 'question', 'answerfeedback', $ansid),
673-
array('class' => 'specificfeedback'));
700+
$result .= html_writer::tag(
701+
'div',
702+
$this->format_embedded_text($qa, $ans->feedback, $ans->feedbackformat),
703+
['class' => 'specificfeedback']
704+
);
674705
}
675706

676707
$result .= $this->choice_wrapper_end();
@@ -692,7 +723,7 @@ public function subquestion(question_attempt $qa, question_display_options $opti
692723
$correct = [];
693724
foreach ($subq->answers as $ans) {
694725
if (question_state::graded_state_for_fraction($ans->fraction) != question_state::$gradedwrong) {
695-
$correct[] = $subq->format_text($ans->answer, $ans->answerformat, $qa, 'question', 'answer', $ans->id);
726+
$correct[] = $this->format_embedded_text($qa, $ans->answer, $ans->answerformat);
696727
}
697728
}
698729
$correct = '<ul><li>'.implode('</li><li>', $correct).'</li></ul>';

0 commit comments

Comments
 (0)