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

Tolerance should not be required for the answer * in numerical/calculated questions

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.2, 2.2
    • Fix Version/s: 2.1.3
    • Component/s: Questions
    • Labels:
      None
    • Testing Instructions:
      Hide

      For each of Numerical, Calculated and Calculated simple qtypes:

      1. Create a new question of this type with two answers, the second answer being *. First try to submit the form when both answers have a tolerance that is blank. You should get a validation for answer 1, but not answer 2.

      2. Then put in a tolerance for answer 1, but leave the tolerance for answer 2 blank. You should now be able to successfully sumbit the form.

      Show
      For each of Numerical, Calculated and Calculated simple qtypes: 1. Create a new question of this type with two answers, the second answer being *. First try to submit the form when both answers have a tolerance that is blank. You should get a validation for answer 1, but not answer 2. 2. Then put in a tolerance for answer 1, but leave the tolerance for answer 2 blank. You should now be able to successfully sumbit the form.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      When the teacher uses the answer '*' to match 'any other response' we should not insist that they enter a tolerance.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            timhunt Tim Hunt added a comment -

            Pierre, does this seem like a sensible change to you?

            Show
            timhunt Tim Hunt added a comment - Pierre, does this seem like a sensible change to you?
            Hide
            ppichet Pierre Pichet added a comment -

            This is effectively a logical change as * means any value.

            Should we extend this to any answers being or not numerical?

            So we could ask why should a teacher put a valid grade to anything written as response?
            This does not appears logical in any way ...

            Logic has his own limit and we should submit this to Aristote.

            And then we have the problem of !is_numeric($data['tolerance'][$key]) when using french ,

            So being logical your solution is OK

            but giving a grade to * does not appears logical as also any feedback to an unknown response.
            But this is another story.

            Show
            ppichet Pierre Pichet added a comment - This is effectively a logical change as * means any value. Should we extend this to any answers being or not numerical? So we could ask why should a teacher put a valid grade to anything written as response? This does not appears logical in any way ... Logic has his own limit and we should submit this to Aristote. And then we have the problem of !is_numeric($data ['tolerance'] [$key] ) when using french , So being logical your solution is OK but giving a grade to * does not appears logical as also any feedback to an unknown response. But this is another story.
            Hide
            timhunt Tim Hunt added a comment -

            Well, the is_numerical bit has not changed from what was there before, and I think we already have another open bug about that, so I will consider that a separate issue, and not attempt to address it here.

            It is certainly useful to have feedback for answer '*', particularly in interactive/adaptive mode. The fact that the student did not even make a common mistake suggest that they are really struggling, and are particularly in need of advice about how to do better.

            Adding validation to force the grade for '*' to be None is an interesting idea. It probably is logical, but again I think that is a separate issue, so I don't plan to address it here. Feel free to create a new issue if you think it is important.

            I will submit this change for integration now.

            Thanks for your review.

            Show
            timhunt Tim Hunt added a comment - Well, the is_numerical bit has not changed from what was there before, and I think we already have another open bug about that, so I will consider that a separate issue, and not attempt to address it here. It is certainly useful to have feedback for answer '*', particularly in interactive/adaptive mode. The fact that the student did not even make a common mistake suggest that they are really struggling, and are particularly in need of advice about how to do better. Adding validation to force the grade for '*' to be None is an interesting idea. It probably is logical, but again I think that is a separate issue, so I don't plan to address it here. Feel free to create a new issue if you think it is important. I will submit this change for integration now. Thanks for your review.
            Hide
            ppichet Pierre Pichet added a comment -

            This is not important to generate a specific bug.
            So your solution remains logical given the code and it is correct to apply it.

            What we don't have already but there discussions about it, is the detection of a non numerical response from the student to which we could apply a specific analysis to detect (a kind of regexp) a badly formatted number.

            Then * could have a specific signification of any or not valid number.

            Show
            ppichet Pierre Pichet added a comment - This is not important to generate a specific bug. So your solution remains logical given the code and it is correct to apply it. What we don't have already but there discussions about it, is the detection of a non numerical response from the student to which we could apply a specific analysis to detect (a kind of regexp) a badly formatted number. Then * could have a specific signification of any or not valid number.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            PS: Note this is the last message of this type that you will receive along the whole November month, because we are running continuous integration this weeks while QA tests for release of Moodle 2.2 (Dec 1st) are being performed.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao PS: Note this is the last message of this type that you will receive along the whole November month, because we are running continuous integration this weeks while QA tests for release of Moodle 2.2 (Dec 1st) are being performed.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Looks perfect, let's be tolerant with wildcards, integrated!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Looks perfect, let's be tolerant with wildcards, integrated!
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            simple calculated and calculated mufti-choice questions are broken in integration master and 21 atm.(Might be due to some other code push)
            Stopping test until those are fixed.

            Show
            ankit_frenz Ankit Agarwal added a comment - simple calculated and calculated mufti-choice questions are broken in integration master and 21 atm.(Might be due to some other code push) Stopping test until those are fixed.
            Hide
            timhunt Tim Hunt added a comment -

            Ankit, please don't say totally vague things like '... are broken' in but reports. At the very least copy and paste the exact error message. Hopefully also explain exactly what you were doing when the error occurred (that is, give precise steps to reproduce.)

            Show
            timhunt Tim Hunt added a comment - Ankit, please don't say totally vague things like '... are broken' in but reports. At the very least copy and paste the exact error message. Hopefully also explain exactly what you were doing when the error occurred (that is, give precise steps to reproduce.)
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Aah sorry,
            Here is steps to reproduce

            • Try to create a "calculated multi-choice question" with debug set to developer this generates following errors

              Warning: Missing argument 1 for HTML_QuickForm_advcheckbox::getPrivateName(), called in H:\wamp\www\repo2\master\moodle\lib\formslib.php on line 1974 and defined in H:\wamp\www\repo2\master\moodle\lib\pear\HTML\QuickForm\advcheckbox.php on line 99
              Notice: Undefined variable: elementName in H:\wamp\www\repo2\master\moodle\lib\pear\HTML\QuickForm\advcheckbox.php on line 101

            • Try creating a "calculated simple question"
            • Click on "find wildcards present in the correct answer formulas"
            • This generates following noise

              Invalid array parameter detected in required_param(): answer
               
                  line 583 of \lib\moodlelib.php: call to debugging()
                  line 151 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param()
                  line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct()
                  line 197 of \question\question.php: call to question_type->create_editing_form()
               
              Invalid array parameter detected in required_param(): fraction
               
                  line 583 of \lib\moodlelib.php: call to debugging()
                  line 153 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param()
                  line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct()
                  line 197 of \question\question.php: call to question_type->create_editing_form()
               
              Invalid array parameter detected in required_param(): tolerance
               
                  line 583 of \lib\moodlelib.php: call to debugging()
                  line 154 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param()
                  line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct()
                  line 197 of \question\question.php: call to question_type->create_editing_form()
               
              Invalid array parameter detected in required_param(): tolerancetype
               
                  line 583 of \lib\moodlelib.php: call to debugging()
                  line 155 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param()
                  line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct()
                  line 197 of \question\question.php: call to question_type->create_editing_form()
               
              Invalid array parameter detected in required_param(): correctanswerlength
               
                  line 583 of \lib\moodlelib.php: call to debugging()
                  line 156 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param()
                  line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct()
                  line 197 of \question\question.php: call to question_type->create_editing_form()
               
              Invalid array parameter detected in required_param(): correctanswerformat
               
                  line 583 of \lib\moodlelib.php: call to debugging()
                  line 157 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param()
                  line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct()
                  line 197 of \question\question.php: call to question_type->create_editing_form()

            • Moreover I was not able to save simple calculated questions as well
            Show
            ankit_frenz Ankit Agarwal added a comment - Aah sorry, Here is steps to reproduce Try to create a "calculated multi-choice question" with debug set to developer this generates following errors Warning: Missing argument 1 for HTML_QuickForm_advcheckbox::getPrivateName(), called in H:\wamp\www\repo2\master\moodle\lib\formslib.php on line 1974 and defined in H:\wamp\www\repo2\master\moodle\lib\pear\HTML\QuickForm\advcheckbox.php on line 99 Notice: Undefined variable: elementName in H:\wamp\www\repo2\master\moodle\lib\pear\HTML\QuickForm\advcheckbox.php on line 101 Try creating a "calculated simple question" Click on "find wildcards present in the correct answer formulas" This generates following noise Invalid array parameter detected in required_param(): answer   line 583 of \lib\moodlelib.php: call to debugging() line 151 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param() line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct() line 197 of \question\question.php: call to question_type->create_editing_form()   Invalid array parameter detected in required_param(): fraction   line 583 of \lib\moodlelib.php: call to debugging() line 153 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param() line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct() line 197 of \question\question.php: call to question_type->create_editing_form()   Invalid array parameter detected in required_param(): tolerance   line 583 of \lib\moodlelib.php: call to debugging() line 154 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param() line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct() line 197 of \question\question.php: call to question_type->create_editing_form()   Invalid array parameter detected in required_param(): tolerancetype   line 583 of \lib\moodlelib.php: call to debugging() line 155 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param() line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct() line 197 of \question\question.php: call to question_type->create_editing_form()   Invalid array parameter detected in required_param(): correctanswerlength   line 583 of \lib\moodlelib.php: call to debugging() line 156 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param() line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct() line 197 of \question\question.php: call to question_type->create_editing_form()   Invalid array parameter detected in required_param(): correctanswerformat   line 583 of \lib\moodlelib.php: call to debugging() line 157 of \question\type\calculatedsimple\edit_calculatedsimple_form.php: call to optional_param() line 221 of \question\type\questiontypebase.php: call to qtype_calculatedsimple_edit_form->__construct() line 197 of \question\question.php: call to question_type->create_editing_form() Moreover I was not able to save simple calculated questions as well
            Hide
            timhunt Tim Hunt added a comment -

            Ah right. The first warning is MDL-27045. The necessary changes in MDL-30186 mean that 27045 is now affecting the calculated qtypes

            The other one is caused by the non-backwards-compatible change MDL-26796 that Petr introduced some time ago without fixing all effected code. I have created MDL-30322 so we don't forget to fix that.

            If you can ignore those errors, then you can probably finish testing this issue.

            Show
            timhunt Tim Hunt added a comment - Ah right. The first warning is MDL-27045 . The necessary changes in MDL-30186 mean that 27045 is now affecting the calculated qtypes The other one is caused by the non-backwards-compatible change MDL-26796 that Petr introduced some time ago without fixing all effected code. I have created MDL-30322 so we don't forget to fix that. If you can ignore those errors, then you can probably finish testing this issue.
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Eloy can you reset the testing phase please?

            Show
            ankit_frenz Ankit Agarwal added a comment - Eloy can you reset the testing phase please?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            (done)

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - (done)
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            Tested for calculated and numerical.
            Works great
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - Tested for calculated and numerical. Works great Thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            U P S T R E A M I Z E D !

            Thanks! Closing...

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Thanks! Closing...

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  28/Nov/11