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.

question/tests/generator/lib.php

@@ -118,11 +118,12 @@ public function create_question_tag(array $data): void {
      * @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 @@ public function update_question($question, $which = null, $overrides = null) {
             }
             $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;
     }

question/tests/version_test.php

@@ -16,6 +16,7 @@
 
 namespace core_question;
 
+use core_question\local\bank\question_version_status;
 use question_bank;
 
 /**
@@ -89,7 +90,7 @@ public function test_make_question_create_version_and_bank_entry() {
         $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 @@ public function test_delete_question_delete_versions() {
         $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 @@ public function test_delete_question_delete_versions() {
      * @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 @@ public function test_delete_question_in_use() {
         // 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 @@ public function test_id_number_in_bank_entry() {
         $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.

question/upgrade.txt

@@ -1,5 +1,16 @@
 This files describes API changes for code that uses the question API.
 
+=== 4.0.9 ===
+
+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.0.5 ===
 
 1) Question bank plugins can now define more than one bulk action. Therefore, plugin_features_base::get_bulk_actions has been