Moodle
  1. Moodle
  2. MDL-33548

Quiz comment.php fails to validate the grade that was input, leading to a weird error.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.6, 2.2.3, 2.3
    • Fix Version/s: 2.1.7, 2.2.4
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      1. As teacher: Created a quiz with a single essay question
      2. As student: Attempted quiz
      3. As teacher: reviewed attempt, clicked 'Make comment or override mark' to get to: http://oracle/moodle/mod/quiz/comment.php
      4. Attempted to mark the answer by adding a comment and writing adding a mark greater than the maximum mark
      5. Form would warn about mark added greater than maximum mark.
      6. Now correct the mark to be in range, and try saving again. It should work.

      Show
      1. As teacher: Created a quiz with a single essay question 2. As student: Attempted quiz 3. As teacher: reviewed attempt, clicked 'Make comment or override mark' to get to: http://oracle/moodle/mod/quiz/comment.php 4. Attempted to mark the answer by adding a comment and writing adding a mark greater than the maximum mark 5. Form would warn about mark added greater than maximum mark. 6. Now correct the mark to be in range, and try saving again. It should work.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      41476

      Description

      While running MDLQA-2517 I did the following:

      1. As teacher: Created a quiz with a single essay question
      2. As student: Attempted quiz
      3. As teacher: reviewed attempt, clicked 'Make comment or override mark' to get to: http://oracle/moodle/mod/quiz/comment.php
      4. Attempted to mark the answer by adding a comment and writing adding a mark greater than the maximum mark

      Expected result:

      • Form would warn about mark added greater than maximum mark.

      Actual result:

        Issue Links

          Activity

          Hide
          Dan Poltawski added a comment -

          Doing the same on postgres:
          test essay question

          Fatal error: Uncaught exception 'coding_exception' with message 'Coding error detected, it must be fixed by a programmer: PHP catchable fatal error' in /Users/danp/git/integration/lib/setuplib.php on line 397 coding_exception: Coding error detected, it must be fixed by a programmer: PHP catchable fatal error in /Users/danp/git/integration/lib/setuplib.php on line 397 Call Stack: 0.3034 48565448 1. default_exception_handler() /Users/danp/git/integration/lib/setuplib.php:0 0.3042 48561232 2. get_exception_info() /Users/danp/git/integration/lib/setuplib.php:351 0.3042 48563904 3. default_error_handler() /Users/danp/git/integration/lib/setuplib.php:493

          Show
          Dan Poltawski added a comment - Doing the same on postgres: test essay question Fatal error: Uncaught exception 'coding_exception' with message 'Coding error detected, it must be fixed by a programmer: PHP catchable fatal error' in /Users/danp/git/integration/lib/setuplib.php on line 397 coding_exception: Coding error detected, it must be fixed by a programmer: PHP catchable fatal error in /Users/danp/git/integration/lib/setuplib.php on line 397 Call Stack: 0.3034 48565448 1. default_exception_handler() /Users/danp/git/integration/lib/setuplib.php:0 0.3042 48561232 2. get_exception_info() /Users/danp/git/integration/lib/setuplib.php:351 0.3042 48563904 3. default_error_handler() /Users/danp/git/integration/lib/setuplib.php:493
          Hide
          Dan Poltawski added a comment -

          (I guess I didn't have error logging in oracle)

          Show
          Dan Poltawski added a comment - (I guess I didn't have error logging in oracle)
          Hide
          Tim Hunt added a comment -

          What does that error mean? "PHP catchable fatal error" that is a PHP error of some sort. How does that become a coding_exception? And why do we not get any useful part of the error message of the original PHP error. I will try to reproduce it and work out what is going on.

          Show
          Tim Hunt added a comment - What does that error mean? "PHP catchable fatal error" that is a PHP error of some sort. How does that become a coding_exception? And why do we not get any useful part of the error message of the original PHP error. I will try to reproduce it and work out what is going on.
          Hide
          Tim Hunt added a comment -

          OK, so the cryptic was caused by me passing an object to an exception as $debuginfo.

          The real bug was missing validation code in comment.php in the quiz.

          Fix coming soon.

          Show
          Tim Hunt added a comment - OK, so the cryptic was caused by me passing an object to an exception as $debuginfo. The real bug was missing validation code in comment.php in the quiz. Fix coming soon.
          Hide
          Dan Poltawski added a comment -

          Linking to the exception issue MDL-33565

          Show
          Dan Poltawski added a comment - Linking to the exception issue MDL-33565
          Hide
          Aparup Banerjee added a comment -

          Hi Tim,
          a quick question even though this has tested fine for me and hardly affects the integration of this patch.
          within is_manual_grade_in_range(), did you want to

          return is_null($mark) || ($mark >= $minfraction * $maxmark && $mark <= $maxmark);
          or rather
          return is_null($mark) or ($mark >= $minfraction * $maxmark && $mark <= $maxmark); //note precedence
          
          Show
          Aparup Banerjee added a comment - Hi Tim, a quick question even though this has tested fine for me and hardly affects the integration of this patch. within is_manual_grade_in_range(), did you want to return is_null($mark) || ($mark >= $minfraction * $maxmark && $mark <= $maxmark); or rather return is_null($mark) or ($mark >= $minfraction * $maxmark && $mark <= $maxmark); //note precedence
          Hide
          Tim Hunt added a comment -

          The two alternatives are exactly the same. Look at the brackets that make the precedence clear, even if you have not memorised the entire PHP operator precedence table.

          Also, it would strike me as horrible coding style to mix or and && in the same expression.

          So, I meant exactly what I wrote.

          Show
          Tim Hunt added a comment - The two alternatives are exactly the same. Look at the brackets that make the precedence clear, even if you have not memorised the entire PHP operator precedence table. Also, it would strike me as horrible coding style to mix or and && in the same expression. So, I meant exactly what I wrote.
          Hide
          Aparup Banerjee added a comment -

          all good then, thats been integrated into 21, 22 and master.

          Show
          Aparup Banerjee added a comment - all good then, thats been integrated into 21, 22 and master.
          Hide
          Aparup Banerjee added a comment -

          this tested fine for me; passing here.

          Show
          Aparup Banerjee added a comment - this tested fine for me; passing here.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

          Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

          Many thanks for your collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: