L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-85721 qtype_multianswer: Don't delete qtype data for subquestions

Browse Source 
Historically, multianswer would re-use question IDs from subquestions,
so it was necessary to delete qtype-specific data if the qtype changed.
Since versioning was introduced, editing creates new versions of the
qtype and subquestions. Deleting the data means the original versions of
the subquestions cannot function correctly, so we shouldn't do that
anymore.
This commit is contained in:
Mark Johnson
2025-07-01 11:54:49 +01:00
parent c36274a15a
commit 7c0600ae16
2 changed files with 64 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/type/multianswer/questiontype.php
+5 -23
View File
@@ -162,9 +162,9 @@ class qtype_multianswer extends question_type {
// This function needs to be able to handle the case where the existing set of wrapped
// questions does not match the new set of wrapped questions so that some need to be
// created, some modified and some deleted.
// Unfortunately the code currently simply overwrites existing ones in sequence. This
// will make re-marking after a re-ordering of wrapped questions impossible and
// will also create difficulties if questiontype specific tables reference the id.
// Thanks to versioning, we no longer overwrite existing questions in the sequence
// by re-using IDs, but instead create a new version of each wrapped question for
// the new version of the parent.
// First we get all the existing wrapped questions.
$oldwrappedquestions = [];
@@ -186,30 +186,12 @@ class qtype_multianswer extends question_type {
$sequence = array();
foreach ($question->options->questions as $wrapped) {
if (!empty($wrapped)) {
// If we still have some old wrapped question ids, reuse the next of them.
// If we still have some old wrapped question ids, reuse the next of them to save
// the new version against its question bank entry.
$wrapped->id = 0;
if (is_array($oldwrappedquestions) &&
$oldwrappedquestion = array_shift($oldwrappedquestions)) {
$wrapped->oldid = $oldwrappedquestion->id;
if ($oldwrappedquestion->qtype != $wrapped->qtype) {
switch ($oldwrappedquestion->qtype) {
case 'multichoice':
$DB->delete_records('qtype_multichoice_options',
array('questionid' => $oldwrappedquestion->id));
break;
case 'shortanswer':
$DB->delete_records('qtype_shortanswer_options',
array('questionid' => $oldwrappedquestion->id));
break;
case 'numerical':
$DB->delete_records('question_numerical',
array('question' => $oldwrappedquestion->id));
break;
default:
throw new moodle_exception('qtypenotrecognized',
'qtype_multianswer', '', $oldwrappedquestion->qtype);
}
}
}
}
$wrapped->name = $question->name;
question/type/multianswer/tests/question_type_test.php
+59
View File
@@ -20,7 +20,9 @@ use qtype_multianswer;
use qtype_multianswer_edit_form;
use qtype_multichoice_base;
use question_bank;
use stdClass;
use test_question_maker;
use core\context;
defined('MOODLE_INTERNAL') || die();
@@ -79,11 +81,13 @@ final class question_type_test extends \advanced_testcase {
$q->timemodified = time();
$q->createdby = $USER->id;
$q->modifiedby = $USER->id;
$q->options = new stdClass();
$sadata = new \stdClass();
$sadata->id = 1;
$sadata->qtype = 'shortanswer';
$sadata->defaultmark = 1;
$sadata->options = new stdClass();
$sadata->options->usecase = true;
$sadata->options->answers[1] = (object) array('answer' => 'Bow-wow', 'fraction' => 0);
$sadata->options->answers[2] = (object) array('answer' => 'Wiggly worm', 'fraction' => 0);
@@ -93,6 +97,7 @@ final class question_type_test extends \advanced_testcase {
$mcdata->id = 1;
$mcdata->qtype = 'multichoice';
$mcdata->defaultmark = 1;
$mcdata->options = new stdClass();
$mcdata->options->single = true;
$mcdata->options->answers[1] = (object) array('answer' => 'Dog', 'fraction' => 0);
$mcdata->options->answers[2] = (object) array('answer' => 'Owl', 'fraction' => 1);
@@ -434,4 +439,58 @@ final class question_type_test extends \advanced_testcase {
$this->assertCount(2, $questiondata->options->questions);
$this->assertEquals('subquestion_replacement', $questiondata->options->questions[$questiontodeletekey]->qtype);
}
/**
* Saving a new version of the question should retain the original subquestion versions, with their own qtype data.
*/
public function test_save_question_options(): void {
global $DB;
$this->resetAfterTest(true);
$this->setAdminUser();
$generator = $this->getDataGenerator()->get_plugin_generator('core_question');
$cat = $generator->create_question_category([]);
$question = $generator->create_question('multianswer', 'twosubq', ['category' => $cat->id]);
get_question_options($question);
$originalsubq1 = reset($question->options->questions);
$originalsubq2 = next($question->options->questions);
// Assert that the original subquestions are the expected types, and they have options records.
$this->assertEquals('shortanswer', $originalsubq1->qtype);
$this->assertTrue($DB->record_exists('qtype_shortanswer_options', ['questionid' => $originalsubq1->id]));
$this->assertEquals('multichoice', $originalsubq2->qtype);
$this->assertTrue($DB->record_exists('qtype_multichoice_options', ['questionid' => $originalsubq2->id]));
// Edit the question, replacing the subquestions with two new questions of different types.
$editedquestion = test_question_maker::get_question_data('multianswer', 'twosubq');
$editedquestion->id = $question->id;
$editedquestion->category = $cat->id;
$editedquestion->context = context::instance_by_id($cat->contextid);
$editedsubq1 = test_question_maker::get_question_form_data('multichoice', 'one_of_four');
$editedsubq1->id = $originalsubq1->id;
$editedsubq1->qtype = 'multichoice';
$editedsubq2 = test_question_maker::get_question_form_data('shortanswer', 'frogtoad');
$editedsubq2->id = $originalsubq2->id;
$editedsubq2->qtype = 'shortanswer';
$editedquestion->options->questions = [$editedsubq1, $editedsubq2];
$this->qtype->save_question_options($editedquestion);
$newquestion = $DB->get_record('question', ['id' => $question->id]);
get_question_options($newquestion);
$newsubq1 = reset($newquestion->options->questions);
$newsubq2 = next($newquestion->options->questions);
// The new subquestions are different types, and did not re-use IDs from the original subquestions.
$this->assertEquals('multichoice', $newsubq1->qtype);
$this->assertFalse(in_array($newsubq1->id, [$originalsubq1->id, $originalsubq2->id]));
$this->assertEquals('shortanswer', $newsubq2->qtype);
$this->assertFalse(in_array($newsubq2->id, [$originalsubq1->id, $originalsubq2->id]));
// The original questions and option records still exist.
$this->assertTrue($DB->record_exists('question', ['id' => $originalsubq1->id]));
$this->assertTrue($DB->record_exists('qtype_shortanswer_options', ['questionid' => $originalsubq1->id]));
$this->assertTrue($DB->record_exists('question', ['id' => $originalsubq2->id]));
$this->assertTrue($DB->record_exists('qtype_multichoice_options', ['questionid' => $originalsubq2->id]));
}
}