Moodle
  1. Moodle
  2. MDL-27114

Can't remove course prerequisites

    Details

    • Type: Bug Bug
    • Status: Development in progress
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 2.1.4, 2.2.1, 2.5
    • Fix Version/s: 2.1
    • Environment:
      PHP Version 5.3.3
      Apache/2.2.16
      MySQL 5.1.49
    • Testing Instructions:
      Hide

      Create 3 courses with completion enabled, enable "self completion" via completion settings for each.

      Go to one of the courses, select both other courses in the "course prerequisites" select box in completion settings, save.

      Revisit the form, deselect both courses and save.

      Before fix:
      The change would not have been saved.

      After fix:
      Changes would be saved, no prerequisites would be selected.
      "Select none" checkbox also exists for same purpose.

      Show
      Create 3 courses with completion enabled, enable "self completion" via completion settings for each. Go to one of the courses, select both other courses in the "course prerequisites" select box in completion settings, save. Revisit the form, deselect both courses and save. Before fix: The change would not have been saved. After fix: Changes would be saved, no prerequisites would be selected. "Select none" checkbox also exists for same purpose.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      16757

      Description

      When editing course completion settings, one can chose one or several courses as "course prerequisites". If one makes a mistake, one can modify the chosen elements (courses) in this "select multiple" HTML element. However, if one wants to remove all selection (i.e. not chose any course as prerequisite anymore), the choice is not saved. The application acts as if not receiving any input from this part of the form.

      It is therefore not possible to remove all course prerequisites after having chosen one (or several) in the first place.

        Issue Links

          Activity

          Hide
          Andrea Gregory (Gordon) added a comment -

          Does anyone know when this will be resolved? It is still present in 2.1.1.

          Show
          Andrea Gregory (Gordon) added a comment - Does anyone know when this will be resolved? It is still present in 2.1.1.
          Hide
          Aaron Barnes added a comment -

          Hi Andrea,

          I have just submitted a fix for 2.0, 2.1, 2.2

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hi Andrea, I have just submitted a fix for 2.0, 2.1, 2.2 Cheers, Aaron
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          I've just been looking at this now. First up thanks for working on it!
          Presently I think the solution needs a little more work so I am sending it back.
          What you've got at the moment works, however it's not a great solution because the data_submitted that is being used is in fact avoiding all other validation and data manipulation that the form may be doing, or preceeding code for that matter. Given $data is there I really think we need to use this in which the solution has to exist somewhere in form defintion or processing.
          I have a couple of ideas in regards to finding a solution for this:

          1. See whether removing the set_default call and instead calling $form->set_data results in those defaults not being present in the select box.
          2. Perhaps a better solution, certainly for usability, would be to include a `none` option in the select box and use that as the default if nothing has been previously set. I think this is certainly going to aid usability (non-technical users likely won't know how to unselect the last element from a select box) and it makes it easy to know when nothing is wanted

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, I've just been looking at this now. First up thanks for working on it! Presently I think the solution needs a little more work so I am sending it back. What you've got at the moment works, however it's not a great solution because the data_submitted that is being used is in fact avoiding all other validation and data manipulation that the form may be doing, or preceeding code for that matter. Given $data is there I really think we need to use this in which the solution has to exist somewhere in form defintion or processing. I have a couple of ideas in regards to finding a solution for this: See whether removing the set_default call and instead calling $form->set_data results in those defaults not being present in the select box. Perhaps a better solution, certainly for usability, would be to include a `none` option in the select box and use that as the default if nothing has been previously set. I think this is certainly going to aid usability (non-technical users likely won't know how to unselect the last element from a select box) and it makes it easy to know when nothing is wanted Cheers Sam
          Hide
          Aaron Barnes added a comment -

          Hi Sam,

          Took your comments on board - how about this?

          Thanks,
          Aaron

          Show
          Aaron Barnes added a comment - Hi Sam, Took your comments on board - how about this? Thanks, Aaron
          Hide
          Sam Hemelryk added a comment -

          Hi Aaron,

          Thanks for working with those suggestions certainly I think this is a good improvement.
          The only thing further that I can see would be to re-arrange things a little in the form (very trivial stuff).
          I've had a play myself real quick and came up with the following solution:

          diff --git a/course/completion_form.php b/course/completion_form.php
          index 96595db..d942f73 100644
          --- a/course/completion_form.php
          +++ b/course/completion_form.php
          @@ -106,13 +106,13 @@ class course_completion_form extends moodleform {
                       $mform->addElement('select', 'criteria_course', get_string('coursesavailable', 'completion'), $selectbox, array('multiple' => 'multiple', 'size' => 6));
                       $mform->disabledIf('criteria_course', 'criteria_course_none', 'eq', 1);
           
          -            // Explain list
          -            $mform->addElement('static', 'criteria_courses_explaination', '', get_string('coursesavailableexplaination', 'completion'));
          -
                       // Show select none checkbox
          -            $mform->addElement('checkbox', 'criteria_course_none', get_string('selectnone', 'completion'));
          +            $mform->addElement('checkbox', 'criteria_course_none', null, get_string('selectnone', 'completion'));
                       $mform->setType('checkbox', PARAM_BOOL);
           
          +            // Explain list
          +            $mform->addElement('static', 'criteria_courses_explaination', '', get_string('coursesavailableexplaination', 'completion'));
          +
                   } else {
                       $mform->addElement('static', 'nocourses', '', get_string('err_nocourses', 'completion'));
                   }
          

          This basically puts the select none text on the other side of the checkbox and moved the explanation below it.
          I'll attach a screenshot shortly.

          Does that look OK to you?

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Aaron, Thanks for working with those suggestions certainly I think this is a good improvement. The only thing further that I can see would be to re-arrange things a little in the form (very trivial stuff). I've had a play myself real quick and came up with the following solution: diff --git a/course/completion_form.php b/course/completion_form.php index 96595db..d942f73 100644 --- a/course/completion_form.php +++ b/course/completion_form.php @@ -106,13 +106,13 @@ class course_completion_form extends moodleform { $mform->addElement('select', 'criteria_course', get_string('coursesavailable', 'completion'), $selectbox, array('multiple' => 'multiple', 'size' => 6)); $mform->disabledIf('criteria_course', 'criteria_course_none', 'eq', 1); - // Explain list - $mform->addElement(' static ', 'criteria_courses_explaination', '', get_string('coursesavailableexplaination', 'completion')); - // Show select none checkbox - $mform->addElement('checkbox', 'criteria_course_none', get_string('selectnone', 'completion')); + $mform->addElement('checkbox', 'criteria_course_none', null , get_string('selectnone', 'completion')); $mform->setType('checkbox', PARAM_BOOL); + // Explain list + $mform->addElement(' static ', 'criteria_courses_explaination', '', get_string('coursesavailableexplaination', 'completion')); + } else { $mform->addElement(' static ', 'nocourses', '', get_string('err_nocourses', 'completion')); } This basically puts the select none text on the other side of the checkbox and moved the explanation below it. I'll attach a screenshot shortly. Does that look OK to you? Cheers Sam
          Hide
          Aaron Barnes added a comment -

          Hi Sam,

          Talked to our UI guy here - have a couple more ideas for improvements

          Will get back to you with some more improvements.

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hi Sam, Talked to our UI guy here - have a couple more ideas for improvements Will get back to you with some more improvements. Cheers, Aaron
          Hide
          Sam Hemelryk added a comment -

          Ok great thanks Aaron, reopening this now.
          I'm looking forward to see the next iteration which will be spot on for sure!

          Show
          Sam Hemelryk added a comment - Ok great thanks Aaron, reopening this now. I'm looking forward to see the next iteration which will be spot on for sure!
          Hide
          Amir Elion added a comment -

          Are there any news regarding this issue?

          Show
          Amir Elion added a comment - Are there any news regarding this issue?
          Hide
          Michael de Raadt added a comment -

          It would be good to resolve this issue. It has been sitting in its current state for a few months now and a few people seem to be waiting for it to be resolved.

          Aaron, could you please return to this issue?

          Show
          Michael de Raadt added a comment - It would be good to resolve this issue. It has been sitting in its current state for a few months now and a few people seem to be waiting for it to be resolved. Aaron, could you please return to this issue?
          Hide
          Aaron Barnes added a comment -

          Hi Michael,

          Sure, I'll have a look at this again.

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hi Michael, Sure, I'll have a look at this again. Cheers, Aaron
          Hide
          Mia Musolino added a comment -

          Can anyone say when this might be fixed? It is a big problem for us.

          Show
          Mia Musolino added a comment - Can anyone say when this might be fixed? It is a big problem for us.
          Hide
          Paul Vasilauskis added a comment -

          I'm running 2.4 and this bug is still there! What would have made this intuitive is the GUI style that has the two columns where you Add and Remove items between the two with the selected items in the Left column and the available items in the Right column. Then to remove all selections you simply remove all the items from the Left column.

          Show
          Paul Vasilauskis added a comment - I'm running 2.4 and this bug is still there! What would have made this intuitive is the GUI style that has the two columns where you Add and Remove items between the two with the selected items in the Left column and the available items in the Right column. Then to remove all selections you simply remove all the items from the Left column.
          Hide
          Aaron Barnes added a comment -

          Hi,

          Have a patch for this - will tidy up and post for review

          Cheers,
          Aaron

          Show
          Aaron Barnes added a comment - Hi, Have a patch for this - will tidy up and post for review Cheers, Aaron
          Hide
          ilan cohen added a comment -

          Hi Aaron

          Is there any news about the patch?

          thanks,
          Ilan.

          Show
          ilan cohen added a comment - Hi Aaron Is there any news about the patch? thanks, Ilan.
          Hide
          Helen Foster added a comment -

          Adding 2.5 as affects version to make sure this issue doesn't slip under the radar.

          Show
          Helen Foster added a comment - Adding 2.5 as affects version to make sure this issue doesn't slip under the radar.
          Hide
          Phil Everist added a comment -

          I have hit this problem with a 2.3 moodle course - is there a fix?
          Is it an issue in 2.6 (planned upgrade)?

          Thanks
          Phil

          Show
          Phil Everist added a comment - I have hit this problem with a 2.3 moodle course - is there a fix? Is it an issue in 2.6 (planned upgrade)? Thanks Phil

            People

            • Votes:
              17 Vote for this issue
              Watchers:
              21 Start watching this issue

              Dates

              • Created:
                Updated: