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

Grader report shows users with expired enrolments as normal, active users

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.5.3, 2.6
    • Fix Version/s: 2.5.4, 2.6.1
    • Component/s: Gradebook
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      Prerequisites:

      • A course with manual enrolments enabled and 8 users enrolled.
      • One user with an enrolment with status 'Active' and no start and end dates enabled.
      • One user with an enrolment with status 'Active', start date only enabled and in the past.
      • One user with an enrolment with status 'Active', end date only enabled and in the future.
      • One user with an enrolment with status 'Active', start and end dates enabled with the start date in the past and the end date in the future.
      • One user with an enrolment with status 'Active', start date only enabled and in the future.
      • One user with an enrolment with status 'Active', end date only enabled and in the past.
      • One user with an enrolment with status 'Active', start and end dates enabled with both dates in the past.
      • One user with an enrolment with status 'Active', start and end dates enabled with both dates in the future.

      Test:

      1. Go to the course's 'Grader report preferences' page.
      2. Set 'Show only active enrolments' to 'Yes' and save changes.
      3. Check that the grader report only displays the active users (i.e. the first 4 users as described above).
      4. Go to the 'Grader report preferences' page again.
      5. Set 'Show only active enrolments' to 'No' and save changes.
      6. Check that the grader report displays the active users normally and the remaining 4 users as inactive (i.e. dimmed).
      7. Change the enrolment status for all users to 'Suspended'.
      8. Check that the grader report displays all users as inactive (dimmed).
      9. Go to the 'Grader report preferences' page again.
      10. Set 'Show only active enrolments' to 'Yes' and save changes.
      11. Check that the grader report displays no users.

      Show
      Prerequisites: A course with manual enrolments enabled and 8 users enrolled. One user with an enrolment with status 'Active' and no start and end dates enabled. One user with an enrolment with status 'Active', start date only enabled and in the past. One user with an enrolment with status 'Active', end date only enabled and in the future. One user with an enrolment with status 'Active', start and end dates enabled with the start date in the past and the end date in the future. One user with an enrolment with status 'Active', start date only enabled and in the future. One user with an enrolment with status 'Active', end date only enabled and in the past. One user with an enrolment with status 'Active', start and end dates enabled with both dates in the past. One user with an enrolment with status 'Active', start and end dates enabled with both dates in the future. Test: 1. Go to the course's 'Grader report preferences' page. 2. Set 'Show only active enrolments' to 'Yes' and save changes. 3. Check that the grader report only displays the active users (i.e. the first 4 users as described above). 4. Go to the 'Grader report preferences' page again. 5. Set 'Show only active enrolments' to 'No' and save changes. 6. Check that the grader report displays the active users normally and the remaining 4 users as inactive (i.e. dimmed). 7. Change the enrolment status for all users to 'Suspended'. 8. Check that the grader report displays all users as inactive (dimmed). 9. Go to the 'Grader report preferences' page again. 10. Set 'Show only active enrolments' to 'Yes' and save changes. 11. Check that the grader report displays no users.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-43408-master

      Description

      Depending on the setting of the 'Show only active enrolments' grader report preference, users with suspended enrolments are either displayed greyed out, or not displayed at all in the grader report. However, this doesn't apply to users whose enrolments have expired (or haven't started yet). Even though their enrolments are inactive, these users are always shown as normal, active users.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            tonybutler Tony Butler added a comment -

            Fix branches for master and MOODLE_25_STABLE provided.

            Show
            tonybutler Tony Butler added a comment - Fix branches for master and MOODLE_25_STABLE provided.
            Hide
            salvetore Michael de Raadt added a comment -

            Hi, Tony.

            I've added you as the assignee for this issue.

            Feel free to push it to peer review when you think it's ready. Testing instructions will be needed first.

            Here's a checklist for what is checked during peer review...

            http://docs.moodle.org/dev/Peer_reviewing_checklist

            Show
            salvetore Michael de Raadt added a comment - Hi, Tony. I've added you as the assignee for this issue. Feel free to push it to peer review when you think it's ready. Testing instructions will be needed first. Here's a checklist for what is checked during peer review... http://docs.moodle.org/dev/Peer_reviewing_checklist
            Hide
            markn Mark Nelson added a comment -

            Hi Tony,

            Thanks a lot for your patch.

            A few points:

            1. The commit message should include the issue number so that we can search for it in the future if needed and those viewing the git history can find the issue related to the patch if they want to better understand it. The message should also include the component so when reading the commit message you know what area it affects. I would recommend - "MDL-43408 gradereport_grader: Display users with expired enrolments as inactive in grader report".
            2. Could you please add spacing before and after '=>' in the array.
            3. round(time(), -2) - the -2 is completely unnecessary.
            4. 'now2'=>$params['now1']) - you have not yet created the $params array, so 'now1' is going to be undefined. You should really create a variable called $time and assign it the value time(), then assign this variable to 'now1' and 'now2'.

            Other than those points it looks good, cheers.

            Regards,

            Mark

            Show
            markn Mark Nelson added a comment - Hi Tony, Thanks a lot for your patch. A few points: The commit message should include the issue number so that we can search for it in the future if needed and those viewing the git history can find the issue related to the patch if they want to better understand it. The message should also include the component so when reading the commit message you know what area it affects. I would recommend - " MDL-43408 gradereport_grader: Display users with expired enrolments as inactive in grader report". Could you please add spacing before and after '=>' in the array. round(time(), -2) - the -2 is completely unnecessary. 'now2'=>$params ['now1'] ) - you have not yet created the $params array, so 'now1' is going to be undefined. You should really create a variable called $time and assign it the value time(), then assign this variable to 'now1' and 'now2'. Other than those points it looks good, cheers. Regards, Mark
            Hide
            markn Mark Nelson added a comment -

            Once you do those changes please let me know and I will view the patch again. Once it all looks good you can then backport it and submit it to integration.

            Show
            markn Mark Nelson added a comment - Once you do those changes please let me know and I will view the patch again. Once it all looks good you can then backport it and submit it to integration.
            Hide
            tonybutler Tony Butler added a comment -

            Hi Mark,

            Thanks for your advice. I've made the suggested changes and updated my MDL-43408-master branch.

            Cheers,
            Tony

            Show
            tonybutler Tony Butler added a comment - Hi Mark, Thanks for your advice. I've made the suggested changes and updated my MDL-43408 -master branch. Cheers, Tony
            Hide
            markn Mark Nelson added a comment -

            Thanks Tony, looks good. Again, I removed the other branch listing to avoid confusion.

            Note to integrator - please cherry-pick this commit to 2.6 and 2.5. Thanks!

            Show
            markn Mark Nelson added a comment - Thanks Tony, looks good. Again, I removed the other branch listing to avoid confusion. Note to integrator - please cherry-pick this commit to 2.6 and 2.5. Thanks!
            Hide
            poltawski Dan Poltawski added a comment -

            Integrated to master, 26 and 25 - thanks Tony.

            Note that the patch did not cherry-pick cleanly to 2.5. I had to resolve a relatively small conflict myself - but we don't normally do this. Please provide branches for all supported branches.

            cheers

            Show
            poltawski Dan Poltawski added a comment - Integrated to master, 26 and 25 - thanks Tony. Note that the patch did not cherry-pick cleanly to 2.5. I had to resolve a relatively small conflict myself - but we don't normally do this. Please provide branches for all supported branches. cheers
            Hide
            markn Mark Nelson added a comment -

            Hi Dan, my fault. I removed the stable branch listings after alterations were done after my review. The reason for this is because the master branch was the only one that was updated. To avoid an incident where code prior to review was integrated I removed them. I should have confirmed that it did cherry-pick successfully, sorry!

            Show
            markn Mark Nelson added a comment - Hi Dan, my fault. I removed the stable branch listings after alterations were done after my review. The reason for this is because the master branch was the only one that was updated. To avoid an incident where code prior to review was integrated I removed them. I should have confirmed that it did cherry-pick successfully, sorry!
            Hide
            abgreeve Adrian Greeve added a comment -

            Tested on the master integration branch.
            Everything behaved as described.
            Test passed.

            Show
            abgreeve Adrian Greeve added a comment - Tested on the master integration branch. Everything behaved as described. Test passed.
            Hide
            damyon Damyon Wiese added a comment -

            David built a framework for behat
            At first just to test this and that
            10000+ steps written
            Sounds like we're all smitten
            And David should be smiling at that

            Thanks for reporting, patching, and testing this issue. It has been released upstream along with 64 others today.

            Show
            damyon Damyon Wiese added a comment - David built a framework for behat At first just to test this and that 10000+ steps written Sounds like we're all smitten And David should be smiling at that Thanks for reporting, patching, and testing this issue. It has been released upstream along with 64 others today.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  13/Jan/14