From: tjhunt Date: Fri, 13 Feb 2009 06:11:42 +0000 (+0000) Subject: question bank: MDL-12719, MDL-15774, MDL-17105, and MDL-18174 X-Git-Url: http://git.mjollnir.org/gw?a=commitdiff_plain;h=24e8b9b684f1069083086f89bcbae45504da5796;p=moodle.git question bank: MDL-12719, MDL-15774, MDL-17105, and MDL-18174 These are all to do with the combination of the $form->category, and $form->categorymoveto fields on the question editing forms since Moodle 1.9. These are now resolved to a single ->category value in question.php, which simplifies the logic for the individual question types. MDL-15774 and MDL-12719 errors editing Cloze question MDL-17105 problems with qustion saving logic. MDL-18174 'Select from subcategories' option not preserved when editing a random question. --- diff --git a/question/question.php b/question/question.php index a93ffaf954..aea20da477 100644 --- a/question/question.php +++ b/question/question.php @@ -17,15 +17,13 @@ require_once($CFG->libdir . '/formslib.php'); $id = optional_param('id', 0, PARAM_INT); // question id $qtype = optional_param('qtype', '', PARAM_FILE); $categoryid = optional_param('category', 0, PARAM_INT); - $cmid = optional_param('cmid', 0, PARAM_INT); $courseid = optional_param('courseid', 0, PARAM_INT); -$wizardnow = optional_param('wizardnow', '', PARAM_ALPHA); -$movecontext = optional_param('movecontext', 0, PARAM_BOOL);//switch to make question - //uneditable - form is displayed to edit category only +$wizardnow = optional_param('wizardnow', '', PARAM_ALPHA); +$movecontext = optional_param('movecontext', 0, PARAM_BOOL); // Switch to make + // question uneditable - form is displayed to edit category only $returnurl = optional_param('returnurl', 0, PARAM_LOCALURL); $appendqnumstring = optional_param('appendqnumstring', '', PARAM_ALPHA); - $inpopup = optional_param('inpopup', 0, PARAM_BOOL); if ($movecontext && !$id){ @@ -146,20 +144,39 @@ if ($mform->is_cancelled()){ } else { redirect($returnurl); } -} elseif ($fromform = $mform->get_data()){ - $returnurl = new moodle_url($returnurl); - //select category that question has been saved in / moved to when we return to question bank - if (!empty($fromform->categorymoveto)){ - $returnurl->param('category', $fromform->categorymoveto); - } else if (!empty($fromform->category)){ - $returnurl->param('category', $fromform->category); - } - $returnurl = $returnurl->out(); +} elseif ($fromform = $mform->get_data()) { + /// If we are saving as a copy, break the connection to the old question. if (!empty($fromform->makecopy)) { - $question->id = 0; // causes a new question to be created. + $question->id = 0; $question->hidden = 0; // Copies should not be hidden } - if ($movecontext){ + + /// Process the combination of usecurrentcat, categorymoveto and category form + /// fields, so the save_question method only has to consider $fromform->category + if (!empty($fromform->usecurrentcat)) { + // $fromform->category is the right category to save in. + } else { + if (!empty($fromform->categorymoveto)) { + $fromform->category = $fromform->categorymoveto; + } else { + // $fromform->category is the right category to save in. + } + } + + /// If we are moving a question, check we have permission to move it from + /// whence it came. (Where we are moving to is validated by the form.) + list($newcatid) = explode(',', $fromform->category); + if (!empty($question->id) && $newcatid != $question->category) { + question_require_capability_on($question, 'move'); + } + + /// Ensure we redirect back to the category the question is being saved into. + $returnurl = new moodle_url($returnurl); + $returnurl->param('category', $fromform->category); + $returnurl = $returnurl->out(); + + /// Call the appropriate method. + if ($movecontext) { list($tocatid, $tocontextid) = explode(',', $fromform->categorymoveto); $tocontext = get_context_instance_by_id($tocontextid); require_capability('moodle/question:add', $tocontext); diff --git a/question/type/multianswer/questiontype.php b/question/type/multianswer/questiontype.php index ce5c89b6e2..8d2404af5e 100644 --- a/question/type/multianswer/questiontype.php +++ b/question/type/multianswer/questiontype.php @@ -173,7 +173,6 @@ class embedded_cloze_qtype extends default_questiontype { $question->id = $authorizedquestion->id; } - $question->category = $authorizedquestion->category; $form->course = $course; // To pass the course object to // save_question_options, where it is diff --git a/question/type/questiontype.php b/question/type/questiontype.php index 3b10d63708..4d6d465d7c 100644 --- a/question/type/questiontype.php +++ b/question/type/questiontype.php @@ -266,22 +266,28 @@ class default_questiontype { } /** - * Saves or updates a question after editing by a teacher + * Saves (creates or updates) a question. * * Given some question info and some data about the answers * this function parses, organises and saves the question * It is used by {@link question.php} when saving new data from * a form, and also by {@link import.php} when importing questions * This function in turn calls {@link save_question_options} - * to save question-type specific options - * @param object $question the question object which should be updated - * @param object $form the form submitted by the teacher - * @param object $course the course we are in + * to save question-type specific data. + * + * Whether we are saving a new question or updating an existing one can be + * determined by testing !empty($question->id). If it is not empty, we are updating. + * + * The question will be saved in category $form->category. + * + * @param object $question the question object which should be updated. For a new question will be mostly empty. + * @param object $form the object containing the information to save, as if from the question editing form. + * @param object $course not really used any more. * @return object On success, return the new question object. On failure, * return an object as follows. If the error object has an errors field, * display that as an error message. Otherwise, the editing form will be * redisplayed with validation errors, from validation_errors field, which - * is itself an object, shown next to the form fields. + * is itself an object, shown next to the form fields. (I don't think this is accurate any more.) */ function save_question($question, $form, $course) { global $USER, $DB; @@ -289,15 +295,15 @@ class default_questiontype { // question types. // First, save the basic question itself - $question->name = trim($form->name); - $question->questiontext = trim($form->questiontext); + $question->name = trim($form->name); + $question->questiontext = trim($form->questiontext); $question->questiontextformat = $form->questiontextformat; - $question->parent = isset($form->parent)? $form->parent : 0; + $question->parent = isset($form->parent) ? $form->parent : 0; $question->length = $this->actual_number_of_questions($question); $question->penalty = isset($form->penalty) ? $form->penalty : 0; if (empty($form->image)) { - $question->image = ""; + $question->image = ''; } else { $question->image = $form->image; } @@ -323,33 +329,18 @@ class default_questiontype { $question->defaultgrade = $form->defaultgrade; } - if (!empty($question->id) && !empty($form->categorymoveto)) { // Question already exists - list($movetocategory, $movetocontextid) = explode(',', $form->categorymoveto); - if ($movetocategory != $question->category){ - question_require_capability_on($question, 'move'); - $question->category = $movetocategory; - //don't need to test add permission of category we are moving question to. - //Only categories that we have permission to add - //a question to will get through the form cleaning code for the select box. - } - // keep existing unique stamp code - $question->stamp = $DB->get_field('question', 'stamp', array('id' => $question->id)); + list($question->category) = explode(',', $form->category); + + if (!empty($question->id)) { + /// Question already exists, update. $question->modifiedby = $USER->id; $question->timemodified = time(); if (!$DB->update_record('question', $question)) { print_error('cannotupdatequestion', 'question'); } - } else { // Question is a new one - if (isset($form->categorymoveto)){ - // Doing save as new question, and we have move rights. - list($question->category, $notused) = explode(',', $form->categorymoveto); - //don't need to test add permission of category we are moving question to. - //Only categories that we have permission to add - //a question to will get through the form cleaning code for the select box. - } else { - // Really a new question. - list($question->category, $notused) = explode(',', $form->category); - } + + } else { + /// New question. // Set the unique code $question->stamp = make_unique_id_code(); $question->createdby = $USER->id; @@ -362,7 +353,6 @@ class default_questiontype { } // Now to save all the answers and type-specific options - $form->id = $question->id; $form->qtype = $question->qtype; $form->category = $question->category; diff --git a/question/type/random/edit_random_form.php b/question/type/random/edit_random_form.php index 9d0682d5d8..87e8f8611d 100644 --- a/question/type/random/edit_random_form.php +++ b/question/type/random/edit_random_form.php @@ -36,6 +36,14 @@ class question_edit_random_form extends question_edit_form { $mform->addElement('advcheckbox', 'questiontext', get_string("recurse", "quiz"), 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); diff --git a/question/type/random/questiontype.php b/question/type/random/questiontype.php index 31e5f8663e..f37b566bb2 100644 --- a/question/type/random/questiontype.php +++ b/question/type/random/questiontype.php @@ -108,31 +108,22 @@ class random_qtype extends default_questiontype { return get_string('random', 'quiz') .' ('. $category->name .')'; } - function save_question($question, $form, $course) { + function save_question_options($question) { global $DB; - // If the category is changing, set things up as default_questiontype::save_question expects. - list($formcategory, $unused) = explode(',', $form->category); - if (isset($question->id) && $formcategory != $question->category) { - $form->categorymoveto = $form->category; - } - $form->name = ''; - $question = parent::save_question($question, $form, $course); + + // No options, as such, but we set the parent field to the question's + // own id. Setting the parent field has the effect of hiding this + // question in various places. + $updateobject = new stdClass; + $updateobject->id = $question->id; + $updateobject->parent = $question->id; + + // We also force the question name to be 'Random (categoryname)'. if (!$category = $DB->get_record('question_categories', array('id' => $question->category))) { print_error('cannotretrieveqcat', 'question'); } - $question->name = $this->question_name($category); - if (!$DB->set_field('question', 'name', $question->name, array('id' => $question->id))) { - print_error('cannotupdaterandomqname', 'question'); - } - return $question; - } - - function save_question_options($question) { - global $DB; - // No options, but we set the parent field to the question's own id. - // Setting the parent field has the effect of hiding this question in - // various places. - return ($DB->set_field('question', 'parent', $question->id, array('id' => $question->id)) ? true : false); + $updateobject->name = $this->question_name($category); + return $DB->update_record('question', $updateobject); } /**