MDL-30854 quiz/question editing: fix create calc question & add on page.

This was one of those innocent seeming issues where, once you start digging, you find a mess. In this case, the code that is now in question_wizard_form::add_hidden_fields used to exist in four different places, in four inconsistent versions. This is now all nicely re-factored, and that solves the problem. Along the way, I found and fixed some wrong string references in qtype_random, and stripped out some unnecessary &s in function declarations.
moodle · Jan 30, 2012 · 7255316 · 7255316
1 parent 216f6d8
commit 7255316
Show file tree

Hide file tree

Showing 9 changed files with 62 additions and 123 deletions.
diff --git a/question/type/calculated/datasetdefinitions_form.php b/question/type/calculated/datasetdefinitions_form.php
@@ -26,14 +26,16 @@
 
 defined('MOODLE_INTERNAL') || die();
 
+require_once($CFG->dirroot . '/question/type/edit_question_form.php');
+
 
 /**
  * Calculated question data set definitions editing form definition.
  *
  * @copyright  2007 Jamie Pratt me@jamiep.org
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class question_dataset_dependent_definitions_form extends moodleform {
+class question_dataset_dependent_definitions_form extends question_wizard_form {
     /**
      * Question object with options and answers already loaded by get_question_options
      * Be careful how you use this it is needed sometimes to set up the structure of the
@@ -149,26 +151,11 @@ protected function definition() {
 
         $this->add_action_buttons(false, get_string('nextpage', 'qtype_calculated'));
 
-        // Hidden elements
-        $mform->addElement('hidden', 'returnurl');
-        $mform->setType('returnurl', PARAM_LOCALURL);
-        $mform->setDefault('returnurl', 0);
-        $mform->addElement('hidden', 'id');
-        $mform->setType('id', PARAM_INT);
+        $this->add_hidden_fields();
 
         $mform->addElement('hidden', 'category');
-        $mform->setType('category', PARAM_RAW);
-        $mform->setDefault('category', array('contexts' => array($this->categorycontext)));
-
-        $mform->addElement('hidden', 'courseid');
-        $mform->setType('courseid', PARAM_INT);
-        $mform->setDefault('courseid', 0);
-
-        $mform->addElement('hidden', 'cmid');
-        $mform->setType('cmid', PARAM_INT);
-        $mform->setDefault('cmid', 0);
+        $mform->setType('category', PARAM_SEQUENCE);
 
-        $mform->setType('id', PARAM_INT);
         $mform->addElement('hidden', 'wizard', 'datasetitems');
         $mform->setType('wizard', PARAM_ALPHA);
     }

diff --git a/question/type/calculated/datasetitems_form.php b/question/type/calculated/datasetitems_form.php
@@ -26,14 +26,16 @@
 
 defined('MOODLE_INTERNAL') || die();
 
+require_once($CFG->dirroot . '/question/type/edit_question_form.php');
+
 
 /**
  * Calculated question data set items editing form definition.
  *
  * @copyright  2007 Jamie Pratt me@jamiep.org
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-class question_dataset_dependent_items_form extends moodleform {
+class question_dataset_dependent_items_form extends question_wizard_form {
     /**
      * Question object with options and answers already loaded by get_question_options
      * Be careful how you use this it is needed sometimes to set up the structure of the
@@ -328,28 +330,14 @@ protected function definition() {
             $mform->addElement('submit', 'savechanges', get_string('savechanges'));
             $mform->closeHeaderBefore('savechanges');
         }
-        //hidden elements
-        $mform->addElement('hidden', 'id');
-        $mform->setType('id', PARAM_INT);
 
-        $mform->addElement('hidden', 'courseid');
-        $mform->setType('courseid', PARAM_INT);
-        $mform->setDefault('courseid', 0);
+        $this->add_hidden_fields();
 
         $mform->addElement('hidden', 'category');
-        $mform->setType('category', PARAM_RAW);
-        $mform->setDefault('category', array('contexts' => array($this->categorycontext)));
-
-        $mform->addElement('hidden', 'cmid');
-        $mform->setType('cmid', PARAM_INT);
-        $mform->setDefault('cmid', 0);
+        $mform->setType('category', PARAM_SEQUENCE);
 
         $mform->addElement('hidden', 'wizard', 'datasetitems');
         $mform->setType('wizard', PARAM_ALPHA);
-
-        $mform->addElement('hidden', 'returnurl');
-        $mform->setType('returnurl', PARAM_LOCALURL);
-        $mform->setDefault('returnurl', 0);
     }
 
     public function set_data($question) {

diff --git a/question/type/calculated/questiontype.php b/question/type/calculated/questiontype.php
@@ -384,15 +384,15 @@ public function validate_form($form) {
         }
         return true;
     }
-    public function finished_edit_wizard(&$form) {
+    public function finished_edit_wizard($form) {
         return isset($form->savechanges);
     }
     public function wizardpagesnumber() {
         return 3;
     }
     // This gets called by editquestion.php after the standard question is saved
-    public function print_next_wizard_page(&$question, &$form, $course) {
-        global $CFG, $USER, $SESSION, $COURSE;
+    public function print_next_wizard_page($question, $form, $course) {
+        global $CFG, $SESSION, $COURSE;
 
         // Catch invalid navigation & reloads
         if (empty($question->id) && empty($SESSION->calculated)) {
@@ -456,19 +456,20 @@ public function &next_wizard_form($submiturl, $question, $wizardnow) {
      * @param object $question
      * @param string $wizardnow is '' for first page.
      */
-    public function display_question_editing_page(&$mform, $question, $wizardnow) {
+    public function display_question_editing_page($mform, $question, $wizardnow) {
         global $OUTPUT;
         switch ($wizardnow) {
             case '':
-                //on first page default display is fine
+                // On the first page, the default display is fine.
                 parent::display_question_editing_page($mform, $question, $wizardnow);
                 return;
-                break;
+
             case 'datasetdefinitions':
                 echo $OUTPUT->heading_with_help(
                         get_string('choosedatasetproperties', 'qtype_calculated'),
                         'questiondatasets', 'qtype_calculated');
                 break;
+
             case 'datasetitems':
                 echo $OUTPUT->heading_with_help(get_string('editdatasets', 'qtype_calculated'),
                         'questiondatasets', 'qtype_calculated');

diff --git a/question/type/calculatedsimple/edit_calculatedsimple_form.php b/question/type/calculatedsimple/edit_calculatedsimple_form.php
@@ -316,13 +316,7 @@ protected function definition_inner($mform) {
                 get_string('findwildcards', 'qtype_calculatedsimple'));
         $mform->registerNoSubmitButton('analyzequestion');
         $mform->closeHeaderBefore('analyzequestion');
-        if (optional_param('analyzequestion', false, PARAM_BOOL)) {
-
-            $this->wizarddisplay = true;
-
-        } else {
-            $this->wizwarddisplay = false;
-        }
+        $this->wizarddisplay = optional_param('analyzequestion', false, PARAM_BOOL);
         if ($this->maxnumber != -1) {
             $this->noofitems = $this->maxnumber;
         } else {

diff --git a/question/type/calculatedsimple/questiontype.php b/question/type/calculatedsimple/questiontype.php
@@ -232,7 +232,7 @@ public function save_question_options($question) {
         return true;
     }
 
-    public function finished_edit_wizard(&$form) {
+    public function finished_edit_wizard($form) {
         return true;
     }
 

diff --git a/question/type/edit_question_form.php b/question/type/edit_question_form.php
@@ -27,6 +27,36 @@
 defined('MOODLE_INTERNAL') || die();
 
 
+abstract class question_wizard_form extends moodleform {
+    /**
+     * Add all the hidden form fields used by question/question.php.
+     */
+    protected function add_hidden_fields() {
+        $mform = $this->_form;
+
+        $mform->addElement('hidden', 'id');
+        $mform->setType('id', PARAM_INT);
+
+        $mform->addElement('hidden', 'inpopup');
+        $mform->setType('inpopup', PARAM_INT);
+
+        $mform->addElement('hidden', 'cmid');
+        $mform->setType('cmid', PARAM_INT);
+
+        $mform->addElement('hidden', 'courseid');
+        $mform->setType('courseid', PARAM_INT);
+
+        $mform->addElement('hidden', 'returnurl');
+        $mform->setType('returnurl', PARAM_LOCALURL);
+
+        $mform->addElement('hidden', 'scrollpos');
+        $mform->setType('scrollpos', PARAM_INT);
+
+        $mform->addElement('hidden', 'appendqnumstring');
+        $mform->setType('appendqnumstring', PARAM_ALPHA);
+    }
+}
+
 /**
  * Form definition base class. This defines the common fields that
  * all question types need. Question types should define their own
@@ -36,7 +66,7 @@
  * @copyright  2006 The Open University
  * @license    http://www.gnu.org/copyleft/gpl.html GNU Public License
  */
-abstract class question_edit_form extends moodleform {
+abstract class question_edit_form extends question_wizard_form {
     const DEFAULT_NUM_HINTS = 2;
 
     /**
@@ -193,45 +223,17 @@ protected function definition() {
             }
         }
 
-        // Standard fields at the end of the form.
-        $mform->addElement('hidden', 'id');
-        $mform->setType('id', PARAM_INT);
-
-        $mform->addElement('hidden', 'qtype');
-        $mform->setType('qtype', PARAM_ALPHA);
-
-        $mform->addElement('hidden', 'inpopup');
-        $mform->setType('inpopup', PARAM_INT);
-
-        $mform->addElement('hidden', 'versioning');
-        $mform->setType('versioning', PARAM_BOOL);
+        $this->add_hidden_fields();
 
         $mform->addElement('hidden', 'movecontext');
         $mform->setType('movecontext', PARAM_BOOL);
 
-        $mform->addElement('hidden', 'cmid');
-        $mform->setType('cmid', PARAM_INT);
-        $mform->setDefault('cmid', 0);
-
-        $mform->addElement('hidden', 'courseid');
-        $mform->setType('courseid', PARAM_INT);
-        $mform->setDefault('courseid', 0);
-
-        $mform->addElement('hidden', 'returnurl');
-        $mform->setType('returnurl', PARAM_LOCALURL);
-        $mform->setDefault('returnurl', 0);
-
-        $mform->addElement('hidden', 'scrollpos');
-        $mform->setType('scrollpos', PARAM_INT);
-        $mform->setDefault('scrollpos', 0);
-
-        $mform->addElement('hidden', 'appendqnumstring');
-        $mform->setType('appendqnumstring', PARAM_ALPHA);
-        $mform->setDefault('appendqnumstring', 0);
+        $mform->addElement('hidden', 'qtype');
+        $mform->setType('qtype', PARAM_ALPHA);
 
         $buttonarray = array();
         if (!empty($this->question->id)) {
-            //editing / moving question
+            // Editing / moving question
             if ($this->question->formoptions->movecontext) {
                 $buttonarray[] = $mform->createElement('submit', 'submitbutton',
                         get_string('moveq', 'question'));
@@ -247,7 +249,7 @@ protected function definition() {
             }
             $buttonarray[] = $mform->createElement('cancel');
         } else {
-            // adding new question
+            // Adding new question
             $buttonarray[] = $mform->createElement('submit', 'submitbutton',
                     get_string('savechanges'));
             $buttonarray[] = $mform->createElement('cancel');

diff --git a/question/type/missingtype/questiontype.php b/question/type/missingtype/questiontype.php
@@ -87,7 +87,7 @@ public function get_random_guess_score($questiondata) {
         return null;
     }
 
-    public function display_question_editing_page(&$mform, $question, $wizardnow) {
+    public function display_question_editing_page($mform, $question, $wizardnow) {
         global $OUTPUT;
         echo $OUTPUT->heading(get_string('warningmissingtype', 'qtype_missingtype'));
 

diff --git a/question/type/random/edit_random_form.php b/question/type/random/edit_random_form.php
@@ -42,11 +42,6 @@ class qtype_random_edit_form extends question_edit_form {
      * override this method and remove the ones you don't want with $mform->removeElement().
      */
     protected function definition() {
-        global $COURSE, $CFG;
-
-        $qtype = $this->qtype();
-        $langfile = "qtype_$qtype";
-
         $mform = $this->_form;
 
         // Standard fields at the start of the form.
@@ -58,41 +53,10 @@ protected function definition() {
         $mform->addElement('advcheckbox', 'questiontext[text]',
                 get_string('includingsubcategories', 'qtype_random'), null, null, array(0, 1));
 
-        $mform->addElement('hidden', 'name');
-        $mform->setType('name', PARAM_ALPHA);
-        $mform->setDefault('name', '');
-
-        $mform->addElement('hidden', 'tags[]');
-        $mform->setType('tags[]', PARAM_ALPHA);
-        $mform->setDefault('tags[]', '');
-
-        // Standard fields at the end of the form.
-        $mform->addElement('hidden', 'questiontextformat', 0);
-        $mform->setType('questiontextformat', PARAM_INT);
-
-        $mform->addElement('hidden', 'id');
-        $mform->setType('id', PARAM_INT);
-
         $mform->addElement('hidden', 'qtype');
         $mform->setType('qtype', PARAM_ALPHA);
 
-        $mform->addElement('hidden', 'inpopup');
-        $mform->setType('inpopup', PARAM_INT);
-
-        $mform->addElement('hidden', 'versioning');
-        $mform->setType('versioning', PARAM_BOOL);
-
-        $mform->addElement('hidden', 'cmid');
-        $mform->setType('cmid', PARAM_INT);
-        $mform->setDefault('cmid', 0);
-
-        $mform->addElement('hidden', 'courseid');
-        $mform->setType('courseid', PARAM_INT);
-        $mform->setDefault('courseid', 0);
-
-        $mform->addElement('hidden', 'returnurl');
-        $mform->setType('returnurl', PARAM_LOCALURL);
-        $mform->setDefault('returnurl', 0);
+        $this->add_hidden_fields();
 
         $buttonarray = array();
         $buttonarray[] = $mform->createElement('submit', 'submitbutton', get_string('savechanges'));

diff --git a/question/type/random/questiontype.php b/question/type/random/questiontype.php
@@ -115,10 +115,10 @@ protected function init_qtype_lists() {
         $this->manualqtypes = implode(',', $manualqtypes);
     }
 
-    public function display_question_editing_page(&$mform, $question, $wizardnow) {
+    public function display_question_editing_page($mform, $question, $wizardnow) {
         global $OUTPUT;
         $heading = $this->get_heading(empty($question->id));
-        echo $OUTPUT->heading_with_help($heading, $this->name(), $this->plugin_name());
+        echo $OUTPUT->heading_with_help($heading, 'pluginname', $this->plugin_name());
         $mform->display();
     }
 
@@ -151,6 +151,9 @@ protected function set_selected_question_name($question, $randomname) {
 
     public function save_question($question, $form) {
         $form->name = '';
+        $form->questiontextformat = FORMAT_MOODLE;
+        $form->tags = array();
+
         // Name is not a required field for random questions, but
         // parent::save_question Assumes that it is.
         return parent::save_question($question, $form);