From c3b4e76768d4642d70ebd8d6f175bcdedadb253f Mon Sep 17 00:00:00 2001
From: Paul Holden <paulh@moodle.com>
Date: Tue, 3 Nov 2020 08:16:07 +0000
Subject: [PATCH] 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.
---
 enrol/self/tests/behat/self_enrolment.feature |  2 +
 enrol/tests/enrollib_test.php                 | 53 +++++++++++++++++++
 lib/enrollib.php                              |  5 +-
 lib/navigationlib.php                         |  3 +-
 4 files changed, 59 insertions(+), 4 deletions(-)

diff --git a/enrol/self/tests/behat/self_enrolment.feature b/enrol/self/tests/behat/self_enrolment.feature
index 91551f027a2..c21729f6fc0 100644
--- 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
index acd3e6086f1..34917c0f27b 100644
--- a/enrol/tests/enrollib_test.php
+++ b/enrol/tests/enrollib_test.php
@@ -1036,6 +1036,59 @@ class core_enrollib_testcase extends advanced_testcase {
         $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
index 7b40ca068ce..286eb63e169 100644
--- 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
index 531056ef798..90ab2956786 100644
--- a/lib/navigationlib.php
+++ b/lib/navigationlib.php
@@ -3670,7 +3670,8 @@ class navbar extends navigation_node {
         }
 
         // 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.