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

Inactive enrolments not dimmed when start and end dates defined

    Details

    • 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 'Enrolled users' page.
      2. Check that the enrolment methods for the first 4 users as described above are displayed as active.
      3. Check that the enrolment methods for the remaining 4 users are displayed as inactive (i.e. dimmed).

      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 'Enrolled users' page. 2. Check that the enrolment methods for the first 4 users as described above are displayed as active. 3. Check that the enrolment methods for the remaining 4 users are displayed as inactive (i.e. dimmed).
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-43373-master

      Description

      On the enrolled users page, inactive enrolments (e.g. expired enrolments, those that have not yet begun, or those with a disabled enrolment method) are normally displayed with the 'dimmed_text' class. This works fine in most cases, however if there is both a start date and an end date defined for the enrolment, and the current time is outside of those start and end times, the enrolment is displayed like a normal active enrolment.

      This seems to be due to a logic error requiring the current date to be BOTH before the start date AND after the expiry date in order for the enrolment to be considered inactive. Obviously this could never be the case.

        Gliffy Diagrams

          Activity

          Hide
          tonybutler Tony Butler added a comment -

          Diffs for fix provided.

          Show
          tonybutler Tony Butler added a comment - Diffs for fix 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 -

          Hey Tony,

          Thanks yet again for providing a patch and detailed testing instructions. I'm surprised this issue wasn't detected earlier, it's great you spotted it. The patch looks spot on, the only issue is the commit message not including the tracker issue and component name (which I also pointed out in another issue, so you probably already realise this now). I would suggest "MDL-43373 core_enrol: Display enrolments dimmed if outside of start or end times". I changed your message slightly to use "or" rather than "and" as that is technically what you changed in the code.

          Thanks!

          Show
          markn Mark Nelson added a comment - Hey Tony, Thanks yet again for providing a patch and detailed testing instructions. I'm surprised this issue wasn't detected earlier, it's great you spotted it. The patch looks spot on, the only issue is the commit message not including the tracker issue and component name (which I also pointed out in another issue, so you probably already realise this now). I would suggest " MDL-43373 core_enrol: Display enrolments dimmed if outside of start or end times". I changed your message slightly to use "or" rather than "and" as that is technically what you changed in the code. Thanks!
          Hide
          tonybutler Tony Butler added a comment -

          Hello again Mark, and Happy New Year by the way!

          I've amended the commit message as suggested and updated my MDL-43373-master branch.

          Cheers,
          Tony

          Show
          tonybutler Tony Butler added a comment - Hello again Mark, and Happy New Year by the way! I've amended the commit message as suggested and updated my MDL-43373 -master branch. Cheers, Tony
          Hide
          markn Mark Nelson added a comment -

          Thanks Tony, pushing for integration. I removed the other branch listings to avoid confusion.

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

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

          Integrated to master, 26, 25 & 24 - thanks Tony!

          Show
          poltawski Dan Poltawski added a comment - Integrated to master, 26, 25 & 24 - thanks Tony!
          Hide
          fred Frédéric Massart added a comment -

          Passing, thanks!

          Show
          fred Frédéric Massart added a comment - Passing, thanks!
          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:
              3 Start watching this issue

              Dates

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