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

          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