MDL-78025 question generator: make the behaviour less surprising

* The object returned by update_question is alwasy a new clone
  and the $question passed in will not be modified.

* The returned object has the fields like questionbankentryid and
  the ones related to versionning, so it is more like the data
  returned by question_bank::load_question_data.
This commit is contained in:
Tim Hunt 2023-04-25 17:58:58 +01:00
parent 37e8115d68
commit 9775be13e9
3 changed files with 35 additions and 13 deletions

View File

@ -118,11 +118,12 @@ class core_question_generator extends component_generator_base {
* @param string $which as for the corresponding argument of
* {@link question_test_helper::get_question_form_data}. null for the default one.
* @param array|stdClass $overrides any fields that should be different from the base example.
* @return stdClass the question data.
* @return stdClass the question data, including version info and questionbankentryid
*/
public function update_question($question, $which = null, $overrides = null) {
global $CFG, $DB;
require_once($CFG->dirroot . '/question/engine/tests/helpers.php');
$question = clone($question);
$qtype = $question->qtype;
@ -144,6 +145,11 @@ class core_question_generator extends component_generator_base {
}
$DB->update_record('question', $question);
}
$questionversion = $DB->get_record('question_versions', ['questionid' => $question->id], '*', MUST_EXIST);
$question->versionid = $questionversion->id;
$question->questionbankentryid = $questionversion->questionbankentryid;
$question->version = $questionversion->version;
$question->status = $questionversion->status;
return $question;
}

View File

@ -16,6 +16,7 @@
namespace core_question;
use core_question\local\bank\question_version_status;
use question_bank;
/**
@ -89,7 +90,7 @@ class version_test extends \advanced_testcase {
$this->assertEquals($questionversion->questionbankentryid, $questiondefinition->questionbankentryid);
// If a question is updated, a new version should be created.
$this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
$question = $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
$newquestiondefinition = question_bank::load_question($question->id);
// The version should be 2.
$this->assertEquals('2', $newquestiondefinition->version);
@ -112,7 +113,7 @@ class version_test extends \advanced_testcase {
$questionfirstversionid = $question->id;
// Create a new version and try to remove it.
$this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
$question = $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
// The new version and bank entry record should exist.
$sql = "SELECT q.id, qv.id AS versionid, qv.questionbankentryid
@ -158,12 +159,14 @@ class version_test extends \advanced_testcase {
* @covers ::question_delete_question
*/
public function test_delete_question_in_use() {
global $DB;
$qcategory = $this->qgenerator->create_question_category(['contextid' => $this->context->id]);
$question = $this->qgenerator->create_question('shortanswer', null, ['category' => $qcategory->id]);
$questionfirstversionid = $question->id;
// Create a new version and try to remove it after adding it to a quiz.
$this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
$question = $this->qgenerator->update_question($question, null, ['name' => 'This is a new version']);
// Add it to the quiz.
quiz_add_quiz_question($question->id, $this->quiz);
@ -173,11 +176,13 @@ class version_test extends \advanced_testcase {
// Try to delete old version.
question_delete_question($questionfirstversionid);
// The questions should exist even after trying to remove it.
$questionversion1 = question_bank::load_question($question->id);
$questionversion2 = question_bank::load_question($questionfirstversionid);
$this->assertEquals($questionversion1->id, $question->id);
$this->assertEquals($questionversion2->id, $questionfirstversionid);
// The used question version should exist even after trying to remove it, but now hidden.
$questionversion2 = question_bank::load_question($question->id);
$this->assertEquals($question->id, $questionversion2->id);
$this->assertEquals(question_version_status::QUESTION_STATUS_HIDDEN, $questionversion2->status);
// The unused version should be completely gone.
$this->assertFalse($DB->record_exists('question', ['id' => $questionfirstversionid]));
}
/**
@ -233,10 +238,10 @@ class version_test extends \advanced_testcase {
$questionid1 = $question->id;
// Create a new version and try to remove it after adding it to a quiz.
$this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
$question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
$questionid2 = $question->id;
// Change the id number and get the question object.
$this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
$question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
$questionid3 = $question->id;
// The new version and bank entry record should exist.
@ -270,10 +275,10 @@ class version_test extends \advanced_testcase {
$questionid1 = $question->id;
// Create a new version.
$this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
$question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id2']);
$questionid2 = $question->id;
// Change the id number and get the question object.
$this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
$question = $this->qgenerator->update_question($question, null, ['idnumber' => 'id3']);
$questionid3 = $question->id;
$questiondefinition = question_bank::get_all_versions_of_question($question->id);

View File

@ -1,5 +1,16 @@
This files describes API changes for code that uses the question API.
=== 4.3 ===
1) The core_question_generator::update_question has been changed so that it no longer modifies the $question
object that was passed in. Instead, the update question is returned (which was already the case).
If you were relying on the old behavioru in your tests, you will need a change like
$questiongenerator->update_question($question, ...);
to
$question = $questiongenerator->update_question($question, ...);
Also, the $question object returned now has fields questionbankentryid, versionid, version and status.
=== 4.2 ===
1) The question/qengine.js has been deprecated. We create core_question/question_engine