MDL-73602 questions: Prevent restore of qtype_random questions
This completely removes the step of creating a question with qtype 'random' during the restore process. Previously, we would create the question bank entry, question version and question record, then rely on the activity using it to convert it to a question set reference and delete it. If the activity was never restored, the question remained in the database. Now, we store the random question data in a temporary record. The activity still converts this to a set reference, and now directly to the new filter condition format. The temporary record will automatically be cleaned up after the restore. If the restore process attempts to create a question record with qtype 'random', this will now throw a coding exception as this must never happen.
This commit is contained in:
@@ -5226,6 +5226,18 @@ class restore_create_categories_and_questions extends restore_structure_step {
|
||||
// we have loaded qcatids there for all parsed questions.
|
||||
$data->category = $this->get_mappingid('question_category', $questionmapping->parentitemid);
|
||||
$this->process_question_legacy_data($data);
|
||||
if ($data->qtype === 'random') {
|
||||
// Random questions do not exist anymore. Store the data in a temporary record so it can be converted to a set
|
||||
// reference by the activity that uses it.
|
||||
restore_dbops::set_backup_ids_record(
|
||||
$this->get_restoreid(),
|
||||
'qtype_random_data',
|
||||
$data->id,
|
||||
$questionmapping->id,
|
||||
info: $data,
|
||||
);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// In the past, there were some very sloppy values of penalty. Fix them.
|
||||
@@ -5290,6 +5302,11 @@ class restore_create_categories_and_questions extends restore_structure_step {
|
||||
}
|
||||
|
||||
// Now store the question.
|
||||
if ($data->qtype === 'random') {
|
||||
throw new \core\exception\coding_exception(
|
||||
'You cannot restore a question with qtype "random". It must be converted to a set reference instead.',
|
||||
);
|
||||
}
|
||||
$newitemid = $DB->insert_record('question', $data);
|
||||
$this->set_mapping('question', $oldid, $newitemid);
|
||||
// Also annotate them as question_created, we need
|
||||
@@ -5609,8 +5626,8 @@ class restore_move_module_questions_categories extends restore_execution_step {
|
||||
$references = $DB->get_records('question_set_references', ['usingcontextid' => $newcontext->newitemid]);
|
||||
foreach ($references as $reference) {
|
||||
$filtercondition = json_decode($reference->filtercondition);
|
||||
if (!empty($filtercondition->questioncategoryid) &&
|
||||
in_array($filtercondition->questioncategoryid, $categoryids)) {
|
||||
$categoryid = reset($filtercondition->filter->category->values);
|
||||
if (!empty($categoryid) && in_array($categoryid, $categoryids)) {
|
||||
// This is one of ours, update the questionscontextid.
|
||||
$DB->set_field('question_set_references',
|
||||
'questionscontextid', $newcontext->newitemid,
|
||||
|
||||
@@ -45,11 +45,6 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st
|
||||
/** @var stdClass */
|
||||
protected $oldquizlayout;
|
||||
|
||||
/**
|
||||
* @var array Track old question ids that need to be removed at the end of the restore.
|
||||
*/
|
||||
protected $oldquestionids = [];
|
||||
|
||||
protected function define_structure() {
|
||||
|
||||
$paths = [];
|
||||
@@ -365,7 +360,9 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st
|
||||
$question = $DB->get_record_sql($sql, [$questionid]);
|
||||
$module = $DB->get_record('quiz', ['id' => $data->quizid]);
|
||||
|
||||
if ($question->qtype === 'random') {
|
||||
if (!$question) {
|
||||
$randomquestion = restore_dbops::get_backup_ids_record($this->get_restoreid(), 'qtype_random_data', $data->questionid);
|
||||
$question = $randomquestion->info;
|
||||
// Set reference data.
|
||||
$questionsetreference = new stdClass();
|
||||
$questionsetreference->usingcontextid = $this->task->get_contextid();
|
||||
@@ -374,16 +371,20 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st
|
||||
$questionsetreference->itemid = $data->id;
|
||||
// If, in the orginal quiz that was backed up, this random question was pointing to a
|
||||
// category in the quiz question bank, then (for reasons explained in {@see restore_move_module_questions_categories})
|
||||
// right now, $question->questioncontextid will incorrectly point to the course contextid.
|
||||
// right now, $questionsetreference->questionscontextid will incorrectly point to the course contextid.
|
||||
// This will get fixed up later in restore_move_module_questions_categories
|
||||
// as part of moving the question categories to the right place.
|
||||
$questionsetreference->questionscontextid = $question->questioncontextid;
|
||||
$filtercondition = new stdClass();
|
||||
$filtercondition->questioncategoryid = $question->category;
|
||||
$filtercondition->includingsubcategories = $data->includingsubcategories ?? false;
|
||||
$questionsetreference->questionscontextid = $DB->get_field(
|
||||
'question_categories',
|
||||
'contextid',
|
||||
['id' => $question->category],
|
||||
);
|
||||
$filtercondition = core_question\question_reference_manager::convert_legacy_set_reference_filter_condition([
|
||||
'questioncategoryid' => $question->category,
|
||||
'includingsubcategories' => $data->includingsubcategories ?? false,
|
||||
]);
|
||||
$questionsetreference->filtercondition = json_encode($filtercondition);
|
||||
$DB->insert_record('question_set_references', $questionsetreference);
|
||||
$this->oldquestionids[$question->questionid] = 1;
|
||||
} else {
|
||||
// Reference data.
|
||||
$questionreference = new \stdClass();
|
||||
@@ -651,12 +652,4 @@ class restore_quiz_activity_structure_step extends restore_questions_activity_st
|
||||
'shufflequestions' => $this->legacyshufflequestionsoption]);
|
||||
}
|
||||
}
|
||||
|
||||
protected function after_restore() {
|
||||
parent::after_restore();
|
||||
// Delete old random questions that have been converted to set references.
|
||||
foreach (array_keys($this->oldquestionids) as $oldquestionid) {
|
||||
question_delete_question($oldquestionid);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,8 +76,10 @@ final class restore_39_test extends advanced_testcase {
|
||||
foreach ($references as $reference) {
|
||||
$filtercondition = json_decode($reference->filtercondition);
|
||||
// Confirm the questionscontextid is set correctly, which is from filter question category id.
|
||||
$this->assertEquals($reference->questionscontextid,
|
||||
$qcats[$filtercondition->questioncategoryid]->contextid);
|
||||
$this->assertEquals(
|
||||
$reference->questionscontextid,
|
||||
$qcats[$filtercondition->filter->category->values[0]]->contextid,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user