Moodle

"disabledif" function doesn't work in "repeat_elements" function if the condition is a repeat element

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.3, 2.0
  • Fix Version/s: 1.9.5
  • Component/s: Forms Library
  • Labels:
    None
  • Database:
    MySQL
  • Affected Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

All is in the title.

To correct this problem, i modify the "repeat_elements" function.
When the function clone the differents elements, i copy all of them in an array.
When the function applies the differents options for the cloned elements, in the case of a 'disabledif' option, i compare the name of the conditon of the element with the array. If it's in it, i change the name of the condition with the nice bracket [no].

It seems to work fine.

Attachments

  1. Text File
    formslib.php.patch
    12/Jul/08 6:24 AM
    2 kB
    Matthieu Nué
  1. screenshot-1.jpg
    109 kB
    25/Jul/08 4:57 PM
  2. screenshot-2.jpg
    110 kB
    25/Jul/08 4:59 PM
  3. screenshot-3.jpg
    130 kB
    25/Jul/08 5:01 PM
  4. screenshot-4.jpg
    124 kB
    25/Jul/08 5:02 PM

Issue Links

This issue is duplicated by:
MDL-15602 problem with "disabledif" function in "repeat_elements" function (with a solution) Minor Closed

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Matthieu Nué added a comment -

don't read : When the function clone the differents elements, i copy all of them in an array.

but : When the function clone the differents elements, i copy all the name of them in an array.

Show
Matthieu Nué added a comment - don't read : When the function clone the differents elements, i copy all of them in an array. but : When the function clone the differents elements, i copy all the name of them in an array.
Hide
Permalink
Anthony Borrow added a comment -

Matthieu - Similar to CONTRIB-575, a couple of screen shots will help folks better understand what you are doing and why. Thanks - Anthony

Show
Anthony Borrow added a comment - Matthieu - Similar to CONTRIB-575, a couple of screen shots will help folks better understand what you are doing and why. Thanks - Anthony
Hide
Permalink
Matthieu Nué added a comment - - edited

Screenshot_1
When there is only one condition (in the circle) for all the proposition, there is no problem. When Désactiver (desactivate) is on, we can't change the "limite".

Show
Matthieu Nué added a comment - - edited Screenshot_1 When there is only one condition (in the circle) for all the proposition, there is no problem. When Désactiver (desactivate) is on, we can't change the "limite".
Hide
Permalink
Matthieu Nué added a comment - - edited

Screenshot_2
When "activer" is on, we can change the "limite". No problem.

Show
Matthieu Nué added a comment - - edited Screenshot_2 When "activer" is on, we can change the "limite". No problem.
Hide
Permalink
Matthieu Nué added a comment - - edited

Screenshot_3
But, if there are a condition for each "limite" option, ther is a problem. In the screenshot, "désactiver" is on but we can change the limit.

Show
Matthieu Nué added a comment - - edited Screenshot_3 But, if there are a condition for each "limite" option, ther is a problem. In the screenshot, "désactiver" is on but we can change the limit.
Hide
Permalink
Matthieu Nué added a comment - - edited

Screenshot_4
With the patch, there is no problem. We can add a condition for each proposition.

Show
Matthieu Nué added a comment - - edited Screenshot_4 With the patch, there is no problem. We can add a condition for each proposition.
Hide
Permalink
Matthieu Nué added a comment -

For the example, i use the choice activity module which used the function "repeat_elements".

Show
Matthieu Nué added a comment - For the example, i use the choice activity module which used the function "repeat_elements".
Hide
Permalink
Anthony Borrow added a comment -

Matthieu - Thanks for explaining this further. Now it makes sense to me and I think others will more easily understand what you are getting at. One more question, does this affect Moodle 1.9 too? I will go ahead and move this into the main Moodle project since it affects CORE code rather than CONTRIB code. For simplicity, I will make it part of the choice component even though you patch could be useful in other places. Peace - Anthony

Show
Anthony Borrow added a comment - Matthieu - Thanks for explaining this further. Now it makes sense to me and I think others will more easily understand what you are getting at. One more question, does this affect Moodle 1.9 too? I will go ahead and move this into the main Moodle project since it affects CORE code rather than CONTRIB code. For simplicity, I will make it part of the choice component even though you patch could be useful in other places. Peace - Anthony
Hide
Permalink
Matthieu Nué added a comment -

Yes, it do affect too in 1.9

Show
Matthieu Nué added a comment - Yes, it do affect too in 1.9
Hide
Permalink
Anthony Borrow added a comment -

Petr - Since this is primarily reported as an issue with /lib/formslib.php I am assigning this to you as I think you have the best overview of how such situations should be handled. Between the screen shots and the patch I figured you could make a quick assessment and comment/reassign accordingly. That being said, I have not been able to reproduce this in HEAD so that it gives me the option to disactivate individual choice options. Peace - Anthony

Show
Anthony Borrow added a comment - Petr - Since this is primarily reported as an issue with /lib/formslib.php I am assigning this to you as I think you have the best overview of how such situations should be handled. Between the screen shots and the patch I figured you could make a quick assessment and comment/reassign accordingly. That being said, I have not been able to reproduce this in HEAD so that it gives me the option to disactivate individual choice options. Peace - Anthony
Hide
Permalink
Anthony Borrow added a comment -

Matthieu - Thanks for the quick response, I am unable to reproduce this error using choice. Could you detail the steps that you use to get the options to individually activate/deactivate whether to enforce limits. Thanks - Anthony

Show
Anthony Borrow added a comment - Matthieu - Thanks for the quick response, I am unable to reproduce this error using choice. Could you detail the steps that you use to get the options to individually activate/deactivate whether to enforce limits. Thanks - Anthony
Hide
Permalink
Anthony Borrow added a comment -

Mathieu indicated that this also impacts Moodle 1.9 so I am adding it to the affected versions.

Show
Anthony Borrow added a comment - Mathieu indicated that this also impacts Moodle 1.9 so I am adding it to the affected versions.
Hide
Permalink
Matthieu Nué added a comment -

I make change in the file mod_form.php for the choice mod :

this code is the original code of the choice mod_form.php file.

  1. line26
    $repeatarray=array();
    $repeatarray[] = &MoodleQuickForm::createElement('header', '', get_string('choice','choice').' {no}');
    $repeatarray[] = &MoodleQuickForm::createElement('text', 'option', get_string('choice','choice'));
    $repeatarray[] = &MoodleQuickForm::createElement('text', 'limit', get_string('limit','choice'));
    $repeatarray[] = &MoodleQuickForm::createElement('hidden', 'optionid', 0);

    $menuoptions=array();
    $menuoptions[0] = get_string('disable');
    $menuoptions[1] = get_string('enable');
    $mform->addElement('header', 'timerestricthdr', get_string('limit', 'choice'));
    $mform->addElement('select', 'limitanswers', get_string('limitanswers', 'choice'), $menuoptions);
    $mform->setHelpButton('limitanswers', array('limit', get_string('limit', 'choice'), 'choice'));
    #line37

    this is the code with the change :
    #
    $menuoptions=array();
    $menuoptions[0] = get_string('disable');
    $menuoptions[1] = get_string('enable');

    $repeatarray=array();
    $repeatarray[] = &MoodleQuickForm::createElement('header', '', get_string('choice','choice').' {no}');
    $repeatarray[] = &MoodleQuickForm::createElement('text', 'option', get_string('choice','choice'));
    $repeatarray[] = &MoodleQuickForm::createElement('text', 'limit', get_string('limit','choice'));
    $repeatarray[] = &MoodleQuickForm::createElement('hidden', 'optionid', 0);
    $repeatarray[] = &MoodleQuickForm::createElement('select', 'limitanswers', get_string('limitanswers', 'choice'), $menuoptions);

//$mform->addElement('header', 'timerestricthdr', get_string('limit', 'choice'));
//$mform->setHelpButton('limitanswers', array('limit', get_string('limit', 'choice'), 'choice'));
#

To be sure, i don't want to change the activity module choice. I use "choice" for show the problem with the function repeat_element.

Show
Matthieu Nué added a comment - I make change in the file mod_form.php for the choice mod : this code is the original code of the choice mod_form.php file.
  1. line26 $repeatarray=array(); $repeatarray[] = &MoodleQuickForm::createElement('header', '', get_string('choice','choice').' {no}'); $repeatarray[] = &MoodleQuickForm::createElement('text', 'option', get_string('choice','choice')); $repeatarray[] = &MoodleQuickForm::createElement('text', 'limit', get_string('limit','choice')); $repeatarray[] = &MoodleQuickForm::createElement('hidden', 'optionid', 0); $menuoptions=array(); $menuoptions[0] = get_string('disable'); $menuoptions[1] = get_string('enable'); $mform->addElement('header', 'timerestricthdr', get_string('limit', 'choice')); $mform->addElement('select', 'limitanswers', get_string('limitanswers', 'choice'), $menuoptions); $mform->setHelpButton('limitanswers', array('limit', get_string('limit', 'choice'), 'choice')); #line37 this is the code with the change : # $menuoptions=array(); $menuoptions[0] = get_string('disable'); $menuoptions[1] = get_string('enable'); $repeatarray=array(); $repeatarray[] = &MoodleQuickForm::createElement('header', '', get_string('choice','choice').' {no}'); $repeatarray[] = &MoodleQuickForm::createElement('text', 'option', get_string('choice','choice')); $repeatarray[] = &MoodleQuickForm::createElement('text', 'limit', get_string('limit','choice')); $repeatarray[] = &MoodleQuickForm::createElement('hidden', 'optionid', 0); $repeatarray[] = &MoodleQuickForm::createElement('select', 'limitanswers', get_string('limitanswers', 'choice'), $menuoptions);
//$mform->addElement('header', 'timerestricthdr', get_string('limit', 'choice')); //$mform->setHelpButton('limitanswers', array('limit', get_string('limit', 'choice'), 'choice')); # To be sure, i don't want to change the activity module choice. I use "choice" for show the problem with the function repeat_element.
Hide
Permalink
Anthony Borrow added a comment -

Mathieu - Yes, I understand now what you did. It became clearer to me when I was looking at MDL-15829. Between Petr and Jamie I think they will be able assess the patch and whether or not it seems consistent with the direction that things are going with the form lib. Peace - Anthony

Show
Anthony Borrow added a comment - Mathieu - Yes, I understand now what you did. It became clearer to me when I was looking at MDL-15829. Between Petr and Jamie I think they will be able assess the patch and whether or not it seems consistent with the direction that things are going with the form lib. Peace - Anthony
Hide
Permalink
Dan Marsden added a comment -

just removing the Choice module from the components that this bug involves - The reporter has used a customised hack of the Choice module to demonstrate this issue - it doesn't currently affect the standard choice module in Moodle.
Thanks!

Dan

Show
Dan Marsden added a comment - just removing the Choice module from the components that this bug involves - The reporter has used a customised hack of the Choice module to demonstrate this issue - it doesn't currently affect the standard choice module in Moodle. Thanks! Dan
Hide
Permalink
Tim Hunt added a comment -

Note, I have just done some other fixes to disabledif that probable break this patch. I only saw this bug after doing my changes.

Show
Tim Hunt added a comment - Note, I have just done some other fixes to disabledif that probable break this patch. I only saw this bug after doing my changes.
Hide
Permalink
Tim Hunt added a comment -

Actually, the patch did still apply with very little work, and it looks good to me. I have committed it. Thanks Mattheiu. That saved me some work.

Show
Tim Hunt added a comment - Actually, the patch did still apply with very little work, and it looks good to me. I have committed it. Thanks Mattheiu. That saved me some work.
Hide
Permalink
Sam Hemelryk added a comment -

Tested and confirmed working. Thanks all

Show
Sam Hemelryk added a comment - Tested and confirmed working. Thanks all

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.