Moodle
  1. Moodle
  2. MDL-36680

Overview report gives wrong course total by including hidden item grades

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.3.4, 2.4.1
    • Component/s: Gradebook
    • Labels:
    • Testing Instructions:
      Hide

      For this test you will find it easier to have two browsers ie Firefox and Chrome.

      Create a course containing two assignments. Have a student submit something for both assignments.

      As a teacher grade the student submissions. Give the a different grade. For example 50 and 70 so that you can see if they are being used in averages.

      Hide one assignment on the course page.

      As teacher go to the course gradebook and go to the categories and items page. Click the settings icon for the course itself. Its the cog icon in the actions column. Set aggregation to "mean of grades" if it isn't already.

      As a teacher or admin go the course gradebook. Go to Settings > Course in the gradebook navigation drop down.
      Look for the Overview report setting "Hide totals if they contain hidden items" and make sure it is set to "hide".

      As the student go to your overview report and check that the course total is hidden. It should have been replaced with "-".

      As teacher, set "Hide totals if they contain hidden items" to "show totals excluding hidden items".
      As student, refresh the overview report. Check that the course total is equal to the grade for the visible activity.

      As teacher, set "Hide totals if they contain hidden items" to "show totals including hidden items".
      As student, refresh the overview report. Check that the course total is equal to the average of the two activity grades.

      Show
      For this test you will find it easier to have two browsers ie Firefox and Chrome. Create a course containing two assignments. Have a student submit something for both assignments. As a teacher grade the student submissions. Give the a different grade. For example 50 and 70 so that you can see if they are being used in averages. Hide one assignment on the course page. As teacher go to the course gradebook and go to the categories and items page. Click the settings icon for the course itself. Its the cog icon in the actions column. Set aggregation to "mean of grades" if it isn't already. As a teacher or admin go the course gradebook. Go to Settings > Course in the gradebook navigation drop down. Look for the Overview report setting "Hide totals if they contain hidden items" and make sure it is set to "hide". As the student go to your overview report and check that the course total is hidden. It should have been replaced with "-". As teacher, set "Hide totals if they contain hidden items" to "show totals excluding hidden items". As student, refresh the overview report. Check that the course total is equal to the grade for the visible activity. As teacher, set "Hide totals if they contain hidden items" to "show totals including hidden items". As student, refresh the overview report. Check that the course total is equal to the average of the two activity grades.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-36680_hidden
    • Rank:
      46181

      Description

      Discovered during testing MDLQA-4744 that hidden items are correctly left out of the User Report but they are being included in the Overview Report course total.

        Issue Links

          Activity

          Hide
          Marina Glancy added a comment -

          I mark it as a blocker by suggestion of MdR
          It seems that this issue was just discovered but is not actually a regression of anything done in 2.4
          It also needs to be tested on previous versions on Moodle

          Show
          Marina Glancy added a comment - I mark it as a blocker by suggestion of MdR It seems that this issue was just discovered but is not actually a regression of anything done in 2.4 It also needs to be tested on previous versions on Moodle
          Hide
          Andrew Davis added a comment -

          I have reproduced this. Even the user user report isn't 100% correct. The average column appears to be including the hidden item.

          Show
          Andrew Davis added a comment - I have reproduced this. Even the user user report isn't 100% correct. The average column appears to be including the hidden item.
          Hide
          Andrew Davis added a comment -

          Its still very much a work in progress but I have begun work on this. https://github.com/andyjdavis/moodle/compare/master...MDL-36680_hidden

          Show
          Andrew Davis added a comment - Its still very much a work in progress but I have begun work on this. https://github.com/andyjdavis/moodle/compare/master...MDL-36680_hidden
          Hide
          Andrew Davis added a comment -

          Ok, I think I've got it. He is a potential fix as well as bunch of unit test code.

          Show
          Andrew Davis added a comment - Ok, I think I've got it. He is a potential fix as well as bunch of unit test code.
          Hide
          Andrew Davis added a comment -

          Ive raised MDL-36793 to look into the user report average column issue. I dont believe thats anywhere near as serious as it effects averages rather than an individual student's grade. Plus averages are not displayed on the user report by default.

          Show
          Andrew Davis added a comment - Ive raised MDL-36793 to look into the user report average column issue. I dont believe thats anywhere near as serious as it effects averages rather than an individual student's grade. Plus averages are not displayed on the user report by default.
          Hide
          Ankit Agarwal added a comment -

          Hi Andrew,
          The tests looks good and makes sense.
          Although are are some missing spaces when creating arrays.

          Please feel free to fix it and submit for integration.

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Andrew, The tests looks good and makes sense. Although are are some missing spaces when creating arrays. Please feel free to fix it and submit for integration. Thanks
          Hide
          Andrew Davis added a comment -

          Adding a 2.3 version. Putting this up for integration. Due to the unit tests this wont go cleanly in to 2.2 plus 2.2 is days away from the end HQ support.

          Show
          Andrew Davis added a comment - Adding a 2.3 version. Putting this up for integration. Due to the unit tests this wont go cleanly in to 2.2 plus 2.2 is days away from the end HQ support.
          Hide
          Dan Poltawski added a comment -

          I was trying to integrate this witht hte release today, but I was getting issues following the testing instructions.

          Show
          Dan Poltawski added a comment - I was trying to integrate this witht hte release today, but I was getting issues following the testing instructions.
          Hide
          Dan Poltawski added a comment -

          In fact the issue seems to be a bug in the user report, but the testing instrucitons are just about the overview report?

          Show
          Dan Poltawski added a comment - In fact the issue seems to be a bug in the user report, but the testing instrucitons are just about the overview report?
          Hide
          Dan Poltawski added a comment -

          Also, we are still needing a 2.2 version of this (withouth the unit tests, I suppose).

          Show
          Dan Poltawski added a comment - Also, we are still needing a 2.2 version of this (withouth the unit tests, I suppose).
          Hide
          Dan Poltawski added a comment -

          Thanks Andrew, I've integrated this now. I see the user report is covered int he linked bug.

          Show
          Dan Poltawski added a comment - Thanks Andrew, I've integrated this now. I see the user report is covered int he linked bug.
          Hide
          Mark Nelson added a comment -

          Failing this.

          This may be because I do not fully understanding the testing instructions. Currently they do not specify that I need to hide any assignment, which seems odd since the name of this issue states 'including hidden item grades'.

          When I did hide the item, chose 'Show totals including hidden items' for the setting 'Hide totals if they contain hidden items', logged in as a student and then viewed the overview report I only saw '-' as the course total, when making the assignment visible again it went back to 90.00, which is the mark I gave the student for their submission.

          Show
          Mark Nelson added a comment - Failing this. This may be because I do not fully understanding the testing instructions. Currently they do not specify that I need to hide any assignment, which seems odd since the name of this issue states 'including hidden item grades'. When I did hide the item, chose 'Show totals including hidden items' for the setting 'Hide totals if they contain hidden items', logged in as a student and then viewed the overview report I only saw '-' as the course total, when making the assignment visible again it went back to 90.00, which is the mark I gave the student for their submission.
          Hide
          Mark Nelson added a comment -

          Also, following the testing instructions w/o making the assignment hidden failed as well. So either way, it did not work as explained.

          Show
          Mark Nelson added a comment - Also, following the testing instructions w/o making the assignment hidden failed as well. So either way, it did not work as explained.
          Hide
          Andrew Davis added a comment -

          Oh, yes, the assignment should be hidden. I've added that to the testing instructions now. I'm retesting this myself to see if I spot any additional problems.

          Show
          Andrew Davis added a comment - Oh, yes, the assignment should be hidden. I've added that to the testing instructions now. I'm retesting this myself to see if I spot any additional problems.
          Hide
          Andrew Davis added a comment -

          Further updates to the testing instructions. The code is fine. The instructions were just vague.

          Show
          Andrew Davis added a comment - Further updates to the testing instructions. The code is fine. The instructions were just vague.
          Hide
          Dan Poltawski added a comment -

          Sending back to testing, please Mark.

          Show
          Dan Poltawski added a comment - Sending back to testing, please Mark.
          Hide
          Dan Poltawski added a comment -

          I didn't wait for the 2.2 version of this. Please can a new issue be created to backport this fix to 2.2.

          Show
          Dan Poltawski added a comment - I didn't wait for the 2.2 version of this. Please can a new issue be created to backport this fix to 2.2.
          Hide
          Mark Nelson added a comment -

          Andrew, I am passing this, but please backport to 2.2 as well - see MDL-37146. Thanks.

          Show
          Mark Nelson added a comment - Andrew, I am passing this, but please backport to 2.2 as well - see MDL-37146 . Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: