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

Lesson detailed statistics not displayed for whole class

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.4, 2.1.1, 2.2
    • Fix Version/s: 2.0.5, 2.1.2
    • Component/s: Lesson
    • Labels:
    • Testing Instructions:
      Hide

      login as teacher/admin
      #create lesson with question

      login as student
      #attempt the lesson

      #login as teacher/admin

      1. select the lesson
      2. select "report"
      3. select "Detailed statistics"

      make sure it displays the class statistics.

      note: if there's no attempt on the lesson, the page will display 'No attempts have been made on this lesson.' message.

      Show
      login as teacher/admin #create lesson with question login as student #attempt the lesson #login as teacher/admin select the lesson select "report" select "Detailed statistics" make sure it displays the class statistics. note: if there's no attempt on the lesson, the page will display 'No attempts have been made on this lesson.' message.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      Lesson detailed statistics not displayed for whole class.
      See this discussion: http://moodle.org/mod/forum/discuss.php?d=183600

      1.- Suggested fix (for moodle 2.0 and 2.1)
      in mod/lesson/report.php, line 563
      change

      if (!empty($page->answerdata) && isset($page->answerdata->response)) {

      to:

      if (!empty($page->answerdata) && (empty($userid) || (!empty($userid) && isset($page->answerdata->response)))) {

      2.- Plus: when student did not answer a question, in the Detailed (individual) statistics it would be nicer to display "Did not answer this question." rather than "0".
      in mod/lesson/report.php, line 577 change

      $table->data[] = array(0, " ");

      to:

      $table->data[] = array(get_string('didnotanswerquestion', 'lesson'), " ");

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Thanks for reporting that and providing a solution.

            Show
            salvetore Michael de Raadt added a comment - Thanks for reporting that and providing a solution.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Joseph,

            Thanks for providing the patch.

            I simplified the logic for the page answer data and response to:
            line 563:

            if (!empty($page->answerdata)) {

            Then do the following to the answer data response (line 572):

            if (isset($page->answerdata->response) && $page->answerdata->response != NULL) {

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Joseph, Thanks for providing the patch. I simplified the logic for the page answer data and response to: line 563: if (!empty($page->answerdata)) { Then do the following to the answer data response (line 572): if (isset($page->answerdata->response) && $page->answerdata->response != NULL) {
            Hide
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks for fixing this Rossie
            Just one thing:
            You probably want to remove redundant check

            mod/lesson/report.php - line 572

            if (isset($page->answerdata->response) && $page->answerdata->response != NULL) {

            isset determines if a variable is set and is not NULL, so checking it again is redundant

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Rossie Just one thing: You probably want to remove redundant check mod/lesson/report.php - line 572 if (isset($page->answerdata->response) && $page->answerdata->response != NULL) { isset determines if a variable is set and is not NULL, so checking it again is redundant
            Hide
            rwijaya Rossiani Wijaya added a comment -

            updating patch and change the check for response to !empty.

            Show
            rwijaya Rossiani Wijaya added a comment - updating patch and change the check for response to !empty.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Thanks for reviewing Raj.

            Submitting for integration.

            Show
            rwijaya Rossiani Wijaya added a comment - Thanks for reviewing Raj. Submitting for integration.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Rosie,

            Sending this back around this week as there is a bug with the patch.
            It should not be using empty() as the user may have provided a valid response of 0 (considered empty).

            On another note while testing this I found that it is impossible to create a short answer question with an answer (either correct or otherwise) of 0.
            Likely the same sort of issue popping up again. I'll create a new bug for this issue now.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Rosie, Sending this back around this week as there is a bug with the patch. It should not be using empty() as the user may have provided a valid response of 0 (considered empty). On another note while testing this I found that it is impossible to create a short answer question with an answer (either correct or otherwise) of 0. Likely the same sort of issue popping up again. I'll create a new bug for this issue now. Cheers Sam
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Linking to MDL-29230 to fix the 0 answer bug I spotted.

            Show
            samhemelryk Sam Hemelryk added a comment - Linking to MDL-29230 to fix the 0 answer bug I spotted.
            Hide
            rwijaya Rossiani Wijaya added a comment -

            Hi Sam,

            I updated the patch by removing the empty() check for response. As Raj mentioned above that isset() is also checking for NULL value, it is not necessary to add != null check.

            Re-submitting for integration.

            Show
            rwijaya Rossiani Wijaya added a comment - Hi Sam, I updated the patch by removing the empty() check for response. As Raj mentioned above that isset() is also checking for NULL value, it is not necessary to add != null check. Re-submitting for integration.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for fixing that up Rosie, this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for fixing that up Rosie, this has been integrated now
            Hide
            ankit_frenz Ankit Agarwal added a comment -

            test passed
            Thanks

            Show
            ankit_frenz Ankit Agarwal added a comment - test passed Thanks
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            git & cvs repositories updated with your gorgeous code. Many thanks!

            Closing and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - git & cvs repositories updated with your gorgeous code. Many thanks! Closing and ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  10/Oct/11