MDL-66259 qtypes: get_question_options() always call parent::

To get the question->options initialised, children must
call parent::get_question_options() always. Also, it is
just general good practice. Subclasses are meant to be
adaptations of the base class, not something completely
different.

Note, there are some changes in the data structure
produced (see changes in the tests) but these changes
are not wrong.

question/type/calculated/questiontype.php

@@ -61,6 +61,7 @@ class qtype_calculated extends question_type {
         // First get the datasets and default options.
         // The code is used for calculated, calculatedsimple and calculatedmulti qtypes.
         global $CFG, $DB, $OUTPUT;
+        parent::get_question_options($question);
         if (!$question->options = $DB->get_record('question_calculated_options',
                 array('question' => $question->id))) {
             $question->options = new stdClass();

question/type/calculated/tests/questiontype_test.php

@@ -112,9 +112,8 @@ class qtype_calculated_test extends advanced_testcase {
         // Options.
         $this->assertEquals($questiondata->id, $questiondata->options->question);
         $this->assertEquals([], $questiondata->options->units);
-        // TODO: Are this four correct? The first one doesn't match, hence commented out.
-        // $this->assertEquals($fromform->unitgradingtypes, $questiondata->options->unitgradingtype);
-        $this->assertEquals($fromform->unitrole, $questiondata->options->showunits);
+        $this->assertEquals(qtype_numerical::UNITNONE, $questiondata->options->showunits);
+        $this->assertEquals(0, $questiondata->options->unitgradingtype); // Unit role is none, so this is 0.
         $this->assertEquals($fromform->unitpenalty, $questiondata->options->unitpenalty);
         $this->assertEquals($fromform->unitsleft, $questiondata->options->unitsleft);
 

question/type/multianswer/questiontype.php

@@ -45,6 +45,7 @@ class qtype_multianswer extends question_type {
     public function get_question_options($question) {
         global $DB, $OUTPUT;
 
+        parent::get_question_options($question);
         // Get relevant data indexed by positionkey from the multianswers table.
         $sequence = $DB->get_field('question_multianswer', 'sequence',
                 array('question' => $question->id), MUST_EXIST);

question/type/multianswer/tests/questiontype_test.php

@@ -131,7 +131,8 @@ class qtype_multianswer_test extends advanced_testcase {
         $question->qtype = 'multianswer';
         $question->createdby = 0;
 
-        // TODO: Just check why is not consistent with others, $question should came back modified.
+        // Note, $question gets modified during save because of the way subquestions
+        // are extracted.
         $question = $this->qtype->save_question($question, $fromform);
 
         $questiondata = question_bank::load_question_data($question->id);
@@ -181,9 +182,12 @@ class qtype_multianswer_test extends advanced_testcase {
         $this->assertEquals($expectedhints, array_values($gothints));
 
         // Options.
-        $this->assertEquals(['questions'], array_keys(get_object_vars($questiondata->options)));
+        $this->assertEquals(['answers', 'questions'], array_keys(get_object_vars($questiondata->options)));
         $this->assertEquals(count($fromform->options->questions), count($questiondata->options->questions));
 
+        // Option answers.
+        $this->assertEquals([], $questiondata->options->answers);
+
         // Build the expected questions. We aren't going deeper to subquestion answers, options... that's another qtype job.
         $expectedquestions = [];
         foreach ($fromform->options->questions as $key => $value) {

question/type/random/questiontype.php

@@ -116,6 +116,7 @@ class qtype_random extends question_type {
     }
 
     public function get_question_options($question) {
+        parent::get_question_options($question);
         return true;
     }
 

question/type/random/tests/questiontype_test.php

@@ -82,25 +82,26 @@ class qtype_random_test extends advanced_testcase {
         $this->assertEquals(['id', 'category', 'parent', 'name', 'questiontext', 'questiontextformat',
                 'generalfeedback', 'generalfeedbackformat', 'defaultmark', 'penalty', 'qtype',
                 'length', 'stamp', 'version', 'hidden', 'timecreated', 'timemodified',
-                'createdby', 'modifiedby', 'idnumber', 'contextid', 'categoryobject'],
+                'createdby', 'modifiedby', 'idnumber', 'contextid', 'options', 'hints', 'categoryobject'],
                 array_keys(get_object_vars($questiondata)));
         $this->assertEquals($category->id, $questiondata->category);
-        // TODO: All this is nosense (in my brain, requires review.
-        $this->assertEquals($questiondata->id, $questiondata->parent); // parent points to self?
-        // $this->assertEquals($fromform->name, $questiondata->name); // fromform has empty name.
-        $this->assertEquals($fromform->questiontext, $questiondata->questiontext); // questiontext has 0 ?
-        $this->assertEquals($fromform->questiontextformat, $questiondata->questiontextformat);
-        $this->assertEquals('', $questiondata->generalfeedback);
-        $this->assertEquals(0, $questiondata->generalfeedbackformat);
-        $this->assertEquals(1, $questiondata->defaultmark); // Nothing @ fromform?
-        $this->assertEquals(0, $questiondata->penalty);
+
+        // Random questions are not real questions. This is signaled by parent
+        // being non-zero - and in fact equal to question id.
+        $this->assertEquals($questiondata->id, $questiondata->parent);
+        $this->assertEquals('Random (' . $category->name . ')', $questiondata->name);
+        $this->assertEquals(0, $questiondata->questiontext); // Used to store 'Select from subcategories'.
         $this->assertEquals('random', $questiondata->qtype);
         $this->assertEquals(1, $questiondata->length);
         $this->assertEquals(0, $questiondata->hidden);
-        $this->assertEquals($question->createdby, $questiondata->createdby);
-        $this->assertEquals($question->createdby, $questiondata->modifiedby);
-        $this->assertEquals('', $questiondata->idnumber);
-        $this->assertEquals($syscontext->id, $questiondata->contextid);
+        $this->assertEquals($category->contextid, $questiondata->contextid);
+
+        // Options - not used.
+        $this->assertEquals(['answers'], array_keys(get_object_vars($questiondata->options)));
+        $this->assertEquals([], $questiondata->options->answers);
+
+        // Hints - not used.
+        $this->assertEquals([], $questiondata->hints);
     }
 
     public function test_get_possible_responses() {

question/type/truefalse/questiontype.php

@@ -113,6 +113,7 @@ class qtype_truefalse extends question_type {
      */
     public function get_question_options($question) {
         global $DB, $OUTPUT;
+        parent::get_question_options($question);
         // Get additional information from database
         // and attach it to the question object.
         if (!$question->options = $DB->get_record('question_truefalse',

question/type/truefalse/tests/questiontype_test.php

@@ -83,7 +83,7 @@ class qtype_truefalse_test extends advanced_testcase {
         $this->assertEquals(['id', 'category', 'parent', 'name', 'questiontext', 'questiontextformat',
                 'generalfeedback', 'generalfeedbackformat', 'defaultmark', 'penalty', 'qtype',
                 'length', 'stamp', 'version', 'hidden', 'timecreated', 'timemodified',
-                'createdby', 'modifiedby', 'idnumber', 'contextid', 'options', 'categoryobject'],
+                'createdby', 'modifiedby', 'idnumber', 'contextid', 'options', 'hints', 'categoryobject'],
                 array_keys(get_object_vars($questiondata)));
         $this->assertEquals($category->id, $questiondata->category);
         $this->assertEquals(0, $questiondata->parent);
@@ -101,6 +101,8 @@ class qtype_truefalse_test extends advanced_testcase {
         $this->assertEquals($question->createdby, $questiondata->modifiedby);
         $this->assertEquals('', $questiondata->idnumber);
         $this->assertEquals($syscontext->id, $questiondata->contextid);
+
+        // Options.
         $this->assertEquals($questiondata->id, $questiondata->options->question);
         $this->assertEquals('True', $questiondata->options->answers[$questiondata->options->trueanswer]->answer);
         $this->assertEquals('False', $questiondata->options->answers[$questiondata->options->falseanswer]->answer);
@@ -112,6 +114,9 @@ class qtype_truefalse_test extends advanced_testcase {
                 $questiondata->options->answers[$questiondata->options->falseanswer]->feedback);
         $this->assertEquals(FORMAT_HTML, $questiondata->options->answers[$questiondata->options->trueanswer]->feedbackformat);
         $this->assertEquals(FORMAT_HTML, $questiondata->options->answers[$questiondata->options->falseanswer]->feedbackformat);
+
+        // Hints.
+        $this->assertEquals([], $questiondata->hints);
     }
 
     public function test_get_possible_responses() {