MDL-59174 analytics: Analyser queries to enrollib

Part of MDL-57791 epic.

diff --git a/analytics/classes/local/analyser/student_enrolments.php b/analytics/classes/local/analyser/student_enrolments.php
index 8e135e6..a86b019 100644 (file)
--- a/analytics/classes/local/analyser/student_enrolments.php
+++ b/analytics/classes/local/analyser/student_enrolments.php
@@ -45,7 +45,7 @@ class student_enrolments extends by_course {
     }
 
     public function sample_access_context($sampleid) {
-        return \context_course::instance($this->get_sample_course($sampleid));
+        return \context_course::instance($this->get_sample_courseid($sampleid));
     }
 
     public function get_sample_analysable($sampleid) {
@@ -64,17 +64,8 @@ class student_enrolments extends by_course {
      * @return void
      */
     protected function get_all_samples(\core_analytics\analysable $course) {
-        global $DB;
 
-        // Using a custom SQL query because we want to include all course enrolments.
-        // TODO Review this is future as does not look ideal
-        // Although we load all the course users data in memory anyway, using recordsets we will
-        // not use the double of the memory required by the end of the iteration.
-        $sql = "SELECT ue.id AS enrolmentid, u.* FROM {user_enrolments} ue
-                  JOIN {enrol} e ON e.id = ue.enrolid
-                  JOIN {user} u ON ue.userid = u.id
-                  WHERE e.courseid = :courseid";
-        $enrolments = $DB->get_recordset_sql($sql, array('courseid' => $course->get_id()));
+        $enrolments = enrol_get_course_users($course->get_id());
 
         // We fetch all enrolments, but we are only interested in students.
         $studentids = $course->get_students();
@@ -97,7 +88,6 @@ class student_enrolments extends by_course {
             // Fill the cache.
             $this->samplecourses[$sampleid] = $course->get_id();
         }
-        $enrolments->close();
 
         $enrolids = array_keys($samplesdata);
         return array(array_combine($enrolids, $enrolids), $samplesdata);
@@ -124,7 +114,7 @@ class student_enrolments extends by_course {
 
             // Enrolment samples are grouped by the course they belong to, so all $sampleids belong to the same
             // course, $courseid and $coursemodinfo will only query the DB once and cache the course data in memory.
-            $courseid = $this->get_sample_course($sampleid);
+            $courseid = $this->get_sample_courseid($sampleid);
             $coursemodinfo = get_fast_modinfo($courseid);
             $coursecontext = \context_course::instance($courseid);
 
@@ -141,22 +131,31 @@ class student_enrolments extends by_course {
         return array(array_combine($enrolids, $enrolids), $samplesdata);
     }
 
-    protected function get_sample_course($sampleid) {
+    /**
+     * Returns the student enrolment course id.
+     *
+     * @param int $sampleid
+     * @return int
+     */
+    protected function get_sample_courseid($sampleid) {
         global $DB;
 
         if (empty($this->samplecourses[$sampleid])) {
-            // TODO New function in enrollib.php.
-            $sql = "SELECT e.courseid
-                      FROM {enrol} e
-                      JOIN {user_enrolments} ue ON ue.enrolid = e.id
-                     WHERE ue.id = :userenrolmentid";
-
-            $this->samplecourses[$sampleid] = $DB->get_field_sql($sql, array('userenrolmentid' => $sampleid));
+            $course = enrol_get_course_by_user_enrolment_id($sampleid);
+            $this->samplecourses[$sampleid] = $course->id;
         }
 
         return $this->samplecourses[$sampleid];
     }
 
+    /**
+     * Returns the visible name of a sample + a renderable to display as sample picture.
+     *
+     * @param int $sampleid
+     * @param int $contextid
+     * @param array $sampledata
+     * @return array
+     */
     public function sample_description($sampleid, $contextid, $sampledata) {
         $description = fullname($sampledata['user'], true, array('context' => $contextid));
         return array($description, new \user_picture($sampledata['user']));

diff --git a/enrol/tests/enrollib_test.php b/enrol/tests/enrollib_test.php
index d7a3ea7..61c84cf 100644 (file)
--- a/enrol/tests/enrollib_test.php
+++ b/enrol/tests/enrollib_test.php
@@ -577,4 +577,33 @@ class core_enrollib_testcase extends advanced_testcase {
         $this->assertEquals($course1->id, $courses[$course1->id]->id);
         $this->assertEquals($course2->id, $courses[$course2->id]->id);
     }
+
+    /**
+     * test_course_users
+     *
+     * @return void
+     */
+    public function test_course_users() {
+        $this->resetAfterTest();
+
+        $user1 = $this->getDataGenerator()->create_user();
+        $user2 = $this->getDataGenerator()->create_user();
+        $course1 = $this->getDataGenerator()->create_course();
+        $course2 = $this->getDataGenerator()->create_course();
+
+        $this->getDataGenerator()->enrol_user($user1->id, $course1->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course1->id);
+        $this->getDataGenerator()->enrol_user($user2->id, $course2->id);
+
+        $this->assertCount(2, enrol_get_course_users($course1->id));
+        $this->assertCount(2, enrol_get_course_users($course1->id, true));
+
+        $instances = enrol_get_instances($course1->id, true);
+        $manualinstance = reset($instances);
+
+        $manualplugin = enrol_get_plugin('manual');
+        $manualplugin->update_user_enrol($manualinstance, $user1->id, ENROL_USER_SUSPENDED);
+        $this->assertCount(2, enrol_get_course_users($course1->id, false));
+        $this->assertCount(1, enrol_get_course_users($course1->id, true));
+    }
 }

diff --git a/lib/enrollib.php b/lib/enrollib.php
index 381e944..bd4d14f 100644 (file)
--- a/lib/enrollib.php
+++ b/lib/enrollib.php
@@ -1429,6 +1429,35 @@ function enrol_get_course_by_user_enrolment_id($ueid) {
     return $DB->get_record_sql($sql, array('ueid' => $ueid));
 }
 
+/**
+ * Return all users enrolled in a course.
+ *
+ * @param int $courseid
+ * @param bool $onlyactive consider only active enrolments in enabled plugins and time restrictions
+ * @return stdClass
+ */
+function enrol_get_course_users($courseid, $onlyactive = false) {
+    global $DB;
+
+    $sql = "SELECT ue.id AS enrolmentid, u.* FROM {user_enrolments} ue
+              JOIN {enrol} e ON e.id = ue.enrolid
+              JOIN {user} u ON ue.userid = u.id
+              WHERE e.courseid = :courseid";
+    $params = array('courseid' => $courseid);
+
+    if ($onlyactive) {
+        $subwhere = "AND ue.status = :active AND e.status = :enabled AND ue.timestart < :now1 AND (ue.timeend = 0 OR ue.timeend > :now2)";
+        $params['now1']    = round(time(), -2); // improves db caching
+        $params['now2']    = $params['now1'];
+        $params['active']  = ENROL_USER_ACTIVE;
+        $params['enabled'] = ENROL_INSTANCE_ENABLED;
+    } else {
+        $subwhere = "";
+    }
+    $sql = $sql . $subwhere;
+    return $DB->get_records_sql($sql, $params);
+}
+
 /**
  * All enrol plugins should be based on this class,
  * this is also the main source of documentation.