From 7d4c345820cd37aa8c46e9adc05d2d00b50814ee Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Wed, 27 Feb 2019 10:08:41 +0000 Subject: [PATCH] MDL-64886 enrol: Make enrolledusercount optional in enrol_get_users_courses --- enrol/externallib.php | 32 +++++++++++++++++++++++--------- enrol/tests/externallib_test.php | 17 +++++++++++++---- enrol/upgrade.txt | 2 ++ 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/enrol/externallib.php b/enrol/externallib.php index 0f10bba4814e0..d76f06eefc7c8 100644 --- a/enrol/externallib.php +++ b/enrol/externallib.php @@ -280,6 +280,11 @@ public static function get_users_courses_parameters() { return new external_function_parameters( array( 'userid' => new external_value(PARAM_INT, 'user id'), + 'returnusercount' => new external_value(PARAM_BOOL, + 'Include count of enrolled users for each course? This can add several seconds to the response time' + . ' if a user is on several large courses, so set this to false if the value will not be used to' + . ' improve performance.', + VALUE_DEFAULT, true), ) ); } @@ -289,9 +294,10 @@ public static function get_users_courses_parameters() { * Please note the current user must be able to access the course, otherwise the course is not included. * * @param int $userid + * @param bool $returnusercount * @return array of courses */ - public static function get_users_courses($userid) { + public static function get_users_courses($userid, $returnusercount = true) { global $CFG, $USER, $DB; require_once($CFG->dirroot . '/course/lib.php'); @@ -299,8 +305,10 @@ public static function get_users_courses($userid) { // Do basic automatic PARAM checks on incoming data, using params description // If any problems are found then exceptions are thrown with helpful error messages - $params = self::validate_parameters(self::get_users_courses_parameters(), array('userid'=>$userid)); + $params = self::validate_parameters(self::get_users_courses_parameters(), + ['userid' => $userid, 'returnusercount' => $returnusercount]); $userid = $params['userid']; + $returnusercount = $params['returnusercount']; $courses = enrol_get_users_courses($userid, true, '*'); $result = array(); @@ -337,9 +345,11 @@ function($favourite) { continue; } - list($enrolledsqlselect, $enrolledparams) = get_enrolled_sql($context); - $enrolledsql = "SELECT COUNT('x') FROM ($enrolledsqlselect) enrolleduserids"; - $enrolledusercount = $DB->count_records_sql($enrolledsql, $enrolledparams); + if ($returnusercount) { + list($enrolledsqlselect, $enrolledparams) = get_enrolled_sql($context); + $enrolledsql = "SELECT COUNT('x') FROM ($enrolledsqlselect) enrolleduserids"; + $enrolledusercount = $DB->count_records_sql($enrolledsql, $enrolledparams); + } $displayname = external_format_string(get_course_display_name_for_list($course), $context->id); list($course->summary, $course->summaryformat) = @@ -395,14 +405,13 @@ function($favourite) { ); } - $result[] = array( + $courseresult = [ 'id' => $course->id, 'shortname' => $course->shortname, 'fullname' => $course->fullname, 'displayname' => $displayname, 'idnumber' => $course->idnumber, 'visible' => $course->visible, - 'enrolledusercount' => $enrolledusercount, 'summary' => $course->summary, 'summaryformat' => $course->summaryformat, 'format' => $course->format, @@ -420,7 +429,11 @@ function($favourite) { 'isfavourite' => isset($favouritecourseids[$course->id]), 'hidden' => $hidden, 'overviewfiles' => $overviewfiles, - ); + ]; + if ($returnusercount) { + $courseresult['enrolledusercount'] = $enrolledusercount; + } + $result[] = $courseresult; } return $result; @@ -439,7 +452,8 @@ public static function get_users_courses_returns() { 'shortname' => new external_value(PARAM_RAW, 'short name of course'), 'fullname' => new external_value(PARAM_RAW, 'long name of course'), 'displayname' => new external_value(PARAM_TEXT, 'course display name for lists.', VALUE_OPTIONAL), - 'enrolledusercount' => new external_value(PARAM_INT, 'Number of enrolled users in this course'), + 'enrolledusercount' => new external_value(PARAM_INT, 'Number of enrolled users in this course', + VALUE_OPTIONAL), 'idnumber' => new external_value(PARAM_RAW, 'id number of course'), 'visible' => new external_value(PARAM_INT, '1 means visible, 0 means not yet visible course'), 'summary' => new external_value(PARAM_RAW, 'summary', VALUE_OPTIONAL), diff --git a/enrol/tests/externallib_test.php b/enrol/tests/externallib_test.php index f945dc39a455c..526169feb5c8a 100644 --- a/enrol/tests/externallib_test.php +++ b/enrol/tests/externallib_test.php @@ -425,7 +425,7 @@ public function test_get_users_courses() { $this->setUser($student); // Call the external function. - $enrolledincourses = core_enrol_external::get_users_courses($student->id); + $enrolledincourses = core_enrol_external::get_users_courses($student->id, true); // We need to execute the return values cleaning process to simulate the web service server. $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); @@ -455,6 +455,7 @@ public function test_get_users_courses() { $this->assertTrue($courseenrol['completionhascriteria']); $this->assertTrue($courseenrol['hidden']); $this->assertTrue($courseenrol['isfavourite']); + $this->assertEquals(2, $courseenrol['enrolledusercount']); } else { // Check language pack. Should be empty since an incorrect one was used when creating the course. $this->assertEmpty($courseenrol['lang']); @@ -466,13 +467,21 @@ public function test_get_users_courses() { $this->assertFalse($courseenrol['completionhascriteria']); $this->assertFalse($courseenrol['hidden']); $this->assertFalse($courseenrol['isfavourite']); + $this->assertEquals(1, $courseenrol['enrolledusercount']); } } + // Check that returnusercount works correctly. + $enrolledincourses = core_enrol_external::get_users_courses($student->id, false); + $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); + foreach ($enrolledincourses as $courseenrol) { + $this->assertFalse(isset($courseenrol['enrolledusercount'])); + } + // Now check that admin users can see all the info. $this->setAdminUser(); - $enrolledincourses = core_enrol_external::get_users_courses($student->id); + $enrolledincourses = core_enrol_external::get_users_courses($student->id, true); $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); $this->assertEquals(2, count($enrolledincourses)); foreach ($enrolledincourses as $courseenrol) { @@ -493,7 +502,7 @@ public function test_get_users_courses() { // Check other users can't see private info. $this->setUser($otherstudent); - $enrolledincourses = core_enrol_external::get_users_courses($student->id); + $enrolledincourses = core_enrol_external::get_users_courses($student->id, true); $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); $this->assertEquals(1, count($enrolledincourses)); @@ -502,7 +511,7 @@ public function test_get_users_courses() { // Change some global profile visibility fields. $CFG->hiddenuserfields = 'lastaccess'; - $enrolledincourses = core_enrol_external::get_users_courses($student->id); + $enrolledincourses = core_enrol_external::get_users_courses($student->id, true); $enrolledincourses = external_api::clean_returnvalue(core_enrol_external::get_users_courses_returns(), $enrolledincourses); $this->assertEquals(0, $enrolledincourses[0]['lastaccess']); // I can't see this, hidden by global setting. diff --git a/enrol/upgrade.txt b/enrol/upgrade.txt index df24f1f5e5483..54434675300e2 100644 --- a/enrol/upgrade.txt +++ b/enrol/upgrade.txt @@ -7,6 +7,8 @@ information provided here is intended especially for developers. - users: List of user objects returned by the query. - moreusers: True if there are still more users, otherwise is False. - totalusers: Number users matching the search. (This element only exists if the function is called with $returnexactcount param set to true). +* enrolledusercount is now optional in the return value of get_users_courses() for performance reasons. This is controlled with the new + optional returnusercount parameter (default true). === 3.6 ===