Merge branch 'MDL-64886_master' of https://github.com/marxjohnson/moodle

mvaraujo · Mar 5, 2019 · 264fdc2 · 264fdc2
2 parents 96ec5f0 + 7d4c345
commit 264fdc2
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 13 deletions.
diff --git 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,18 +294,21 @@ 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');
         require_once($CFG->libdir . '/completionlib.php');
 
         // 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
@@ -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
@@ -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 ===