Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-27114

Can't remove course prerequisites

    Details

    • 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, MOODLE_26_STABLE, MOODLE_27_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      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.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            agordon Andrea Gregory (Gordon) added a comment -

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

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

            Hi Andrea,

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

            Cheers,
            Aaron

            Show
            sry_not4sale Aaron Barnes added a comment - Hi Andrea, I have just submitted a fix for 2.0, 2.1, 2.2 Cheers, Aaron
            Hide
            samhemelryk 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
            samhemelryk 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
            sry_not4sale Aaron Barnes added a comment -

            Hi Sam,

            Took your comments on board - how about this?

            Thanks,
            Aaron

            Show
            sry_not4sale Aaron Barnes added a comment - Hi Sam, Took your comments on board - how about this? Thanks, Aaron
            Hide
            samhemelryk 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
            samhemelryk 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
            sry_not4sale 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
            sry_not4sale 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
            samhemelryk 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
            samhemelryk 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
            amirelion Amir Elion added a comment -

            Are there any news regarding this issue?

            Show
            amirelion Amir Elion added a comment - Are there any news regarding this issue?
            Hide
            salvetore 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
            salvetore 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
            sry_not4sale Aaron Barnes added a comment -

            Hi Michael,

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

            Cheers,
            Aaron

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

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

            Show
            musolinom Mia Musolino added a comment - Can anyone say when this might be fixed? It is a big problem for us.
            Hide
            chiefvas 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
            chiefvas 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
            sry_not4sale Aaron Barnes added a comment -

            Hi,

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

            Cheers,
            Aaron

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

            Hi Aaron

            Is there any news about the patch?

            thanks,
            Ilan.

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

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

            Show
            tsala Helen Foster added a comment - Adding 2.5 as affects version to make sure this issue doesn't slip under the radar.
            Hide
            peverist 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
            peverist 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
            Hide
            marycooch Mary Cooch added a comment - - edited

            Adding later versions as it still appears to be a problem. See https://moodle.org/mod/forum/discuss.php?d=262304#p1136482

            Show
            marycooch Mary Cooch added a comment - - edited Adding later versions as it still appears to be a problem. See https://moodle.org/mod/forum/discuss.php?d=262304#p1136482
            Hide
            abarbary Adam Barbary added a comment -

            Can verify, still an issue in 2.6.4 Good to see development in progress though.

            Show
            abarbary Adam Barbary added a comment - Can verify, still an issue in 2.6.4 Good to see development in progress though.
            Hide
            amanda.doughty Amanda Doughty added a comment -

            Is development in progress? It looks like it stalled 19 months ago. Can we have your patch Aaron?

            Show
            amanda.doughty Amanda Doughty added a comment - Is development in progress? It looks like it stalled 19 months ago. Can we have your patch Aaron?
            Hide
            sry_not4sale Aaron Barnes added a comment -

            I've been looking into this!

            MDL-30940 sort of fixes this, as you can Ctrl-click items to deselect them, and Moodle saves this correctly. That fix is in 2.3 onwards.

            We still need to improve the UI however... so I'll get my patch ready for 2.8 tomorrow.

            Show
            sry_not4sale Aaron Barnes added a comment - I've been looking into this! MDL-30940 sort of fixes this, as you can Ctrl-click items to deselect them, and Moodle saves this correctly. That fix is in 2.3 onwards. We still need to improve the UI however... so I'll get my patch ready for 2.8 tomorrow.
            Hide
            abarbary Adam Barbary added a comment - - edited

            Ha! Well that wasn't immediately obvious, but yes, it does work. 2.6.4

            Show
            abarbary Adam Barbary added a comment - - edited Ha! Well that wasn't immediately obvious, but yes, it does work. 2.6.4
            Hide
            amanda.doughty Amanda Doughty added a comment -

            Thanks Aaron

            Show
            amanda.doughty Amanda Doughty added a comment - Thanks Aaron

              People

              • Votes:
                22 Vote for this issue
                Watchers:
                24 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Fix Release Date:
                  1/Jul/11