Moodle

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

Details

  • Type: Bug Bug
  • Status: Closed 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

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

duplicates

Bug - A problem which impairs or prevents Moodle from functioning correctly. CONTRIB-485 It is not possible for students to edit questions that they have created. They can only save these questions 'as new'.

  • Critical - Crashes server, loss of data, severe memory leak
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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

Vote (1)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: