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:

      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:

        Gliffy Diagrams

          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: