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

Quiz statistics report does not show most of the overall data

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3.9, 2.4.6, 2.5.2
    • Fix Version/s: 2.4.7, 2.5.3, 2.6
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      Please test this on PHP 5.4 or later.

      1. Create a quiz, and attempt it a few times as different students.

      2. Go to Results -> Statistics, and verify that Quiz information table contains overall statistics like "Average grade of all attempts".

      Show
      Please test this on PHP 5.4 or later. 1. Create a quiz, and attempt it a few times as different students. 2. Go to Results -> Statistics, and verify that Quiz information table contains overall statistics like "Average grade of all attempts".
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      As far as I can see there has been a silly bug in the code since Moodle 2.1 which stops most of the quiz statistics from displaying!

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              timhunt Tim Hunt added a comment -

              Ready for peer review.

              Show
              timhunt Tim Hunt added a comment - Ready for peer review.
              Hide
              timhunt Tim Hunt added a comment -

              This was a regression caused by MDL-20636.

              Adding Jamie as a watcher, since he is doing work on the statistics report now.

              Show
              timhunt Tim Hunt added a comment - This was a regression caused by MDL-20636 . Adding Jamie as a watcher, since he is doing work on the statistics report now.
              Show
              timhunt Tim Hunt added a comment - Regression introduced here: https://github.com/moodle/moodle/commit/04853f273aa788586799a534e1028c3645dcaa77#L7R446
              Hide
              jmvedrine Jean-Michel Vedrine added a comment -

              That's weird ! How wasn't this noticed by all of us looking at quiz statistics ?

              Show
              jmvedrine Jean-Michel Vedrine added a comment - That's weird ! How wasn't this noticed by all of us looking at quiz statistics ?
              Hide
              jamiesensei Jamie Pratt added a comment -

              It looks like I inadvertently fixed this bug in MDL-41888 which just went in the integration for the master branch.

              I removed the second test altogether, you think we should test to see if someone has edited the $todisplay array and included a false or empty value to indicate non display?

              Show
              jamiesensei Jamie Pratt added a comment - It looks like I inadvertently fixed this bug in MDL-41888 which just went in the integration for the master branch. I removed the second test altogether, you think we should test to see if someone has edited the $todisplay array and included a false or empty value to indicate non display?
              Hide
              timhunt Tim Hunt added a comment -

              I think the idea is that it would be easy for people to alter the display by changing the value in the array to an empty string.

              Shall we keep this issue for fixing things on stable branches, and do you want to amend your commit to match mine?

              Show
              timhunt Tim Hunt added a comment - I think the idea is that it would be easy for people to alter the display by changing the value in the array to an empty string. Shall we keep this issue for fixing things on stable branches, and do you want to amend your commit to match mine?
              Hide
              jamiesensei Jamie Pratt added a comment -

              I have amended MDL-41888 for consistency. I added a commit to the branch and resubmitted for integration review. So hopefully the extra commit will get pulled into the integration.

              Show
              jamiesensei Jamie Pratt added a comment - I have amended MDL-41888 for consistency. I added a commit to the branch and resubmitted for integration review. So hopefully the extra commit will get pulled into the integration.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Looks obvious, so sending to integration, thanks!

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Looks obvious, so sending to integration, thanks!
              Hide
              timhunt Tim Hunt added a comment -

              I did some more experimentation, and found out why no-one has noticed this before.

              The issue is the behaviour of empty($format[$property]), when $format and $property are both strings.

              $format[$property] is interpreted as $format[0], or the first character of $format.

              What about empty($format[$property])? In PHP 5.3 that is false, and so this bug does not acutally cause a problem. In PHP 5.4 it is true. That is why this bug has only been noticed recently.

              Show
              timhunt Tim Hunt added a comment - I did some more experimentation, and found out why no-one has noticed this before. The issue is the behaviour of empty($format [$property] ), when $format and $property are both strings. $format [$property] is interpreted as $format [0] , or the first character of $format. What about empty($format [$property] )? In PHP 5.3 that is false, and so this bug does not acutally cause a problem. In PHP 5.4 it is true. That is why this bug has only been noticed recently.
              Hide
              marina Marina Glancy added a comment -

              Thanks Tim, it has been integrated in 2.4, 2.5 and master

              Show
              marina Marina Glancy added a comment - Thanks Tim, it has been integrated in 2.4, 2.5 and master
              Hide
              dmonllao David Monllaó added a comment -

              It passes, tested in 24, 25 and master

              Show
              dmonllao David Monllaó added a comment - It passes, tested in 24, 25 and master
              Hide
              marina Marina Glancy added a comment -

              And THANK YOU again for making Moodle better every day!

              Another weekly release has been released.

              Show
              marina Marina Glancy added a comment - And THANK YOU again for making Moodle better every day! Another weekly release has been released.

                People

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

                  Dates

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