Moodle
  1. Moodle
  2. MDL-14676

In question bank, problem with moodle/question:edit* when do not have moodle/question:move*

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9
    • Fix Version/s: 1.9.3
    • Component/s: Questions, Roles / Access
    • Labels:
      None
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      30945

      Description

      Using moodle 1.9.

      1- As Admin,
      a. create a new role called CanCreateSystemQuestions with Allowed permissions: moodle/question:add; moodle/question:editmine.
      b. create a new role called CanUseSystemQuestions with Allowed permission: moodle/question:useall
      Assign these 2 roles to all teachers on your moodle site.

      2- Log in as teacher A
      a. create question "Question A" in System default and save it.
      b. try to edit "Question A" : you can click the Edit icon, you see the Edit question screen which says:
      You have permission to :

      • Edit this question
      • Save this as a new question
        Edit your question and click on the Save changes to save it in the same (System) category. You see an error message:
        Sorry, but you do not currently have permissions to do that (move).

      There seems to be a contradiction of "permissions" somewhere...

      Joseph

      PS.-- Clicking on the "More information about this error link" takes me to the http://docs.moodle.org/en/error/moodle/nopermissions page which informs me that "If you believe that your roles for that user DO allow that action you were attempting, then you may have found a bug. Congratulations!" Hence this bug report...

        Issue Links

          Activity

          Hide
          Pierre Pichet added a comment -

          Can you look at this part of code in question.php to see if it conflicts with what you are doing
          // 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...

          Show
          Pierre Pichet added a comment - Can you look at this part of code in question.php to see if it conflicts with what you are doing // 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...
          Hide
          Pierre Pichet added a comment -

          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

          Show
          Pierre Pichet added a comment - 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
          Hide
          John Isner added a comment -

          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:

          http://moodle.org/mod/forum/discuss.php?d=94773#p419350

          Show
          John Isner added a comment - 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: http://moodle.org/mod/forum/discuss.php?d=94773#p419350
          Hide
          Pierre Pichet added a comment -

          in edit_question_form.php
          if (!isset($this->question->id))

          { //adding question $mform->addElement('questioncategory', 'category', get_string('category', 'quiz'), array('contexts' => $this->contexts->having_cap('moodle/question:add'))); }

          elseif (!($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...)

          Show
          Pierre Pichet added a comment - in edit_question_form.php if (!isset($this->question->id)) { //adding question $mform->addElement('questioncategory', 'category', get_string('category', 'quiz'), array('contexts' => $this->contexts->having_cap('moodle/question:add'))); } elseif (!($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...)
          Hide
          Pierre Pichet added a comment -

          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
          and either in save_question() function where all this should be set before.

          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
          i.e saveas, edit, create, move in new context, move only in one context , save as new etc.

          Show
          Pierre Pichet added a comment - 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 and either in save_question() function where all this should be set before. 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 i.e saveas, edit, create, move in new context, move only in one context , save as new etc.
          Hide
          Pierre Pichet added a comment -

          Or perhaps more precise formoptions related to the buttons themselves i.e savasnew_buttond

          Show
          Pierre Pichet added a comment - Or perhaps more precise formoptions related to the buttons themselves i.e savasnew_buttond
          Hide
          Pierre Pichet added a comment -

          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 ).

          Show
          Pierre Pichet added a comment - 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 ).
          Hide
          Tim Hunt added a comment -

          Jamie, do you have any thoughts on this?

          Show
          Tim Hunt added a comment - Jamie, do you have any thoughts on this?
          Hide
          Jamie Pratt added a comment -

          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.

          Show
          Jamie Pratt added a comment - 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.

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: