Moodle
  1. Moodle
  2. MDL-30199

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor 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:
    • Rank:
      26123

      Description

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

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

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

          Show
          Tim Hunt added a comment - Pierre, does this seem like a sensible change to you?
          Hide
          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
          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
          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
          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
          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
          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
          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
          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
          Eloy Lafuente (stronk7) added a comment -

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

          Show
          Eloy Lafuente (stronk7) added a comment - Looks perfect, let's be tolerant with wildcards, integrated!
          Hide
          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 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
          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
          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 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 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
          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
          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 Agarwal added a comment -

          Eloy can you reset the testing phase please?

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

          (done)

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

          Tested for calculated and numerical.
          Works great
          Thanks

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

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

          Thanks! Closing...

          Show
          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: