diff --git a/analytics/classes/local/analyser/student_enrolments.php b/analytics/classes/local/analyser/student_enrolments.php index 8e135e6e9ea..a86b019c190 100644 --- 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 d7a3ea7f3a5..61c84cf2db1 100644 --- 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 381e944eb7e..bd4d14fa35e 100644 --- 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.