MDL-70099 enrol: increase accuracy of current enrolments by date.

By rounding the current time it was possible that the most recently created user enrolments (e.g. self enrolments) were being excluded. This would manifest itself in a user being enrolled on a course, but it not appearing under "My courses" navigation or on their own Dashboard until the rounded time had caught up with the current time.
moodle · Apr 5, 2021 · ebfe1d5 · ebfe1d5
1 parent ef7fcf4
commit ebfe1d5
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 4 deletions.
diff --git a/enrol/self/tests/behat/self_enrolment.feature b/enrol/self/tests/behat/self_enrolment.feature
@@ -98,6 +98,7 @@ Feature: Users can auto-enrol themself in courses where self enrolment is allowe
     And I log in as "student1"
     And I am on "Course 1" course homepage
     And I press "Enrol me"
+    And I should see "You are enrolled in the course"
     And I log out
     And I log in as "teacher1"
     And I am on "Course 1" course homepage
@@ -118,6 +119,7 @@ Feature: Users can auto-enrol themself in courses where self enrolment is allowe
     And I log in as "student1"
     And I am on "Course 1" course homepage
     And I press "Enrol me"
+    And I should see "You are enrolled in the course"
     And I log out
     And I log in as "teacher1"
     And I am on "Course 1" course homepage

diff --git a/enrol/tests/enrollib_test.php b/enrol/tests/enrollib_test.php
@@ -1036,6 +1036,59 @@ public function test_enrol_get_my_courses_all_accessible() {
         $this->assertEquals([$course1->id, $course2->id, $course3->id, $course4->id], array_keys($courses));
     }
 
+    /**
+     * Data provider for {@see test_enrol_get_my_courses_by_time}
+     *
+     * @return array
+     */
+    public function enrol_get_my_courses_by_time_provider(): array {
+        return [
+            'No start or end time' =>
+                [null, null, true],
+            'Start time now, no end time' =>
+                [0, null, true],
+            'Start time now, end time in the future' =>
+                [0, MINSECS, true],
+            'Start time in the past, no end time' =>
+                [-MINSECS, null, true],
+            'Start time in the past, end time in the future' =>
+                [-MINSECS, MINSECS, true],
+            'Start time in the past, end time in the past' =>
+                [-DAYSECS, -HOURSECS, false],
+            'Start time in the future' =>
+                [MINSECS, null, false],
+        ];
+    }
+
+    /**
+     * Test that expected course enrolments are returned when they have timestart / timeend specified
+     *
+     * @param int|null $timestartoffset Null for 0, otherwise offset from current time
+     * @param int|null $timeendoffset Null for 0, otherwise offset from current time
+     * @param bool $expectreturn
+     *
+     * @dataProvider enrol_get_my_courses_by_time_provider
+     */
+    public function test_enrol_get_my_courses_by_time(?int $timestartoffset, ?int $timeendoffset, bool $expectreturn): void {
+        $this->resetAfterTest();
+
+        $time = time();
+        $timestart = $timestartoffset === null ? 0 : $time + $timestartoffset;
+        $timeend = $timeendoffset === null ? 0 : $time + $timeendoffset;
+
+        $course = $this->getDataGenerator()->create_course();
+        $user = $this->getDataGenerator()->create_and_enrol($course, 'student', null, 'manual', $timestart, $timeend);
+        $this->setUser($user);
+
+        $courses = enrol_get_my_courses();
+        if ($expectreturn) {
+            $this->assertCount(1, $courses);
+            $this->assertEquals($course->id, reset($courses)->id);
+        } else {
+            $this->assertEmpty($courses);
+        }
+    }
+
     /**
      * test_course_users
      *

diff --git a/lib/enrollib.php b/lib/enrollib.php
@@ -664,13 +664,12 @@ function enrol_get_my_courses($fields = null, $sort = null, $limit = 0, $coursei
                 SELECT DISTINCT e.courseid
                   FROM {enrol} e
                   JOIN {user_enrolments} ue ON (ue.enrolid = e.id AND ue.userid = :userid1)
-                 WHERE ue.status = :active AND e.status = :enabled AND ue.timestart < :now1
+                 WHERE ue.status = :active AND e.status = :enabled AND ue.timestart <= :now1
                        AND (ue.timeend = 0 OR ue.timeend > :now2)";
         $params['userid1'] = $USER->id;
         $params['active'] = ENROL_USER_ACTIVE;
         $params['enabled'] = ENROL_INSTANCE_ENABLED;
-        $params['now1'] = round(time(), -2); // Improves db caching.
-        $params['now2'] = $params['now1'];
+        $params['now1'] = $params['now2'] = time();
 
         if ($sorttimeaccess) {
             $params['userid2'] = $USER->id;

diff --git a/lib/navigationlib.php b/lib/navigationlib.php
@@ -3670,7 +3670,8 @@ private function get_course_categories() {
         }
 
         // Don't show the 'course' node if enrolled in this course.
-        if (!is_enrolled(context_course::instance($this->page->course->id, null, '', true))) {
+        $coursecontext = context_course::instance($this->page->course->id);
+        if (!is_enrolled($coursecontext, null, '', true)) {
             $courses = $this->page->navigation->get('courses');
             if (!$courses) {
                 // Courses node may not be present.