|
question.php is quite intricate so I cannot say I master it.
however save as a new question is saved as a new question in save_question() of questiontype.php the question->id has been set to 0 by question.php if (!empty($fromform->makecopy)) { $question->id = 0; // causes a new question to be created. $question->hidden = 0; // Copies should not be hidden } then in save_question() of questiontype.php // 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); } I have to check how the form set $form->categorymoveto more later I duplicated Joseph's problem on demo.moodle.org. I discovered that in order to edit a question and save it back into the same System category, you need 'move' in addition to 'add' and 'view'. Without 'move' you get the Save button, but it gives the error message.
Oddly, you do not need 'edit' to edit questions, although Tim Hunt said that you do. See: in edit_question_form.php
if (Unable to render embedded object: File (isset($this->question->id)){ //adding question $mform->addElement('questioncategory', 'category', get_string('category', 'quiz'), array('contexts' => $this->contexts->having_cap('moodle/question:add'))); } elseif () not found.($this->question->formoptions->canmove || $this->question->formoptions->cansaveasnew)){ //editing question with no permission to move from category. $mform->addElement('questioncategory', 'category', get_string('category', 'quiz'), array('contexts' => array($this->categorycontext))); } elseif ($this->question->formoptions->movecontext){ //moving question to another context. $mform->addElement('questioncategory', 'categorymoveto', get_string('category', 'quiz'), array('contexts' => $this->contexts->having_cap('moodle/question:add'))); } else { So as you don't have formoptions->canmove the element categorymoveto is not created P.S.Joseph, I am using these comments also to complete my knowledge ...( and perhaps yours...) In edit_question_form.php
if (!empty($this->question->id)){ //editing / moving question if ($this->question->formoptions->movecontext){ $buttonarray[] = &$mform->createElement('submit', 'submitbutton', get_string('moveq', 'question')); } elseif ($this->question->formoptions->canedit || $this->question->formoptions->canmove ||$this->question->formoptions->movecontext){ $buttonarray[] = &$mform->createElement('submit', 'submitbutton', get_string('savechanges')); } if ($this->question->formoptions->cansaveasnew){ $buttonarray[] = &$mform->createElement('submit', 'makecopy', get_string('makecopy', 'quiz')); } $buttonarray[] = &$mform->createElement('cancel'); } else { // adding new question $buttonarray[] = &$mform->createElement('submit', 'submitbutton', get_string('savechanges')); $buttonarray[] = &$mform->createElement('cancel'); } So effectively canedit is not the condition to have the savechanges button. Perhaps a solution will be to have only one place that controls what you can do with a question. Clearly this is not in the edit_question_form() because it can be modified by question types question.php should be the place to do it. This could necessitate the creation of specific formoptions for all actions possible in editing a question Or perhaps more precise formoptions related to the buttons themselves i.e savasnew_buttond
An additional argument is that the control of where to save in moodle should not be controlled by the question types although they can control when all the data is ready to be saved using the next_wizard_form (i.e.calculated question ).
I committed changes to 19 stable and HEAD to fix this. The question form submission and processing logic is quite complex because of the number of different cases. Tested moving, saving as new and moving context for users with and without move permission. Also tested with random questions. All seems to work fine.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Bug
Resolved
Major
In question bank, problem with moodle/question:edit* when do not have moodle/question:move*
 |
|
All content on this web site is made available under the GNU General Public License, unless otherwise stated.
// Validate the question category.
if (!$category = get_record('question_categories', 'id', $question->category)) {
print_error('categorydoesnotexist', 'question', $returnurl);
}
//permissions
$question->formoptions = new object();
$categorycontext = get_context_instance_by_id($category->contextid);
$addpermission = has_capability('moodle/question:add', $categorycontext);
if ($id) {
$canview = question_has_capability_on($question, 'view');
if ($movecontext){ $question->formoptions->canedit = false; $question->formoptions->canmove = (question_has_capability_on($question, 'move') && $contexts->have_cap('moodle/question:add')); $question->formoptions->cansaveasnew = false; $question->formoptions->repeatelements = false; $question->formoptions->movecontext = true; $formeditable = true; question_require_capability_on($question, 'view'); } else {
$question->formoptions->canedit = question_has_capability_on($question, 'edit');
$question->formoptions->canmove = (question_has_capability_on($question, 'move') && $addpermission);
$question->formoptions->cansaveasnew = (($canview ||question_has_capability_on($question, 'edit')) && $addpermission);
$question->formoptions->repeatelements = ($question->formoptions->canedit || $question->formoptions->cansaveasnew);
$formeditable = $question->formoptions->canedit || $question->formoptions->cansaveasnew || $question->formoptions->canmove;
$question->formoptions->movecontext = false;
if (!$formeditable){ question_require_capability_on($question, 'view'); }
}
I notice that you don'it have permission to move although $question->formoptions->canmove is necessary in all cases...