MDL-63938 Enrolments: course_enrolment_manager method improvement

course_enrolment_manager method should not always do a count_records and a get_records

enrol/locallib.php

@@ -419,21 +419,50 @@ class course_enrolment_manager {
      * @param int $page which page number of the results to show.
      * @param int $perpage number of users per page.
      * @param int $addedenrollment number of users added to enrollment.
-     * @return array with two elememts:
-     *      int total number of users matching the search.
-     *      array of user objects returned by the query.
-     */
-    protected function execute_search_queries($search, $fields, $countfields, $sql, array $params, $page, $perpage, $addedenrollment=0) {
+     * @param bool $returnexactcount Return the exact total users using count_record or not.
+     * @return array with two or three elements:
+     *      int totalusers Number users matching the search. (This element only exist if $returnexactcount was set to true)
+     *      array users List of user objects returned by the query.
+     *      boolean moreusers True if there are still more users, otherwise is False.
+     * @throws dml_exception
+     */
+    protected function execute_search_queries($search, $fields, $countfields, $sql, array $params, $page, $perpage,
+            $addedenrollment = 0, $returnexactcount = false) {
         global $DB, $CFG;
 
         list($sort, $sortparams) = users_order_by_sql('u', $search, $this->get_context());
         $order = ' ORDER BY ' . $sort;
 
-        $totalusers = $DB->count_records_sql($countfields . $sql, $params);
+        $totalusers = 0;
+        $moreusers = false;
+        $results = [];
+
         $availableusers = $DB->get_records_sql($fields . $sql . $order,
-                array_merge($params, $sortparams), ($page*$perpage) - $addedenrollment, $perpage);
+                array_merge($params, $sortparams), ($page * $perpage) - $addedenrollment, $perpage + 1);
+        if ($availableusers) {
+            $totalusers = count($availableusers);
+            $moreusers = $totalusers > $perpage;
+
+            if ($moreusers) {
+                // We need to discard the last record.
+                array_pop($availableusers);
+            }
+
+            if ($returnexactcount && $moreusers) {
+                // There is more data. We need to do the exact count.
+                $totalusers = $DB->count_records_sql($countfields . $sql, $params);
+            }
+        }
 
-        return array('totalusers' => $totalusers, 'users' => $availableusers);
+        $results['users'] = $availableusers;
+        $results['moreusers'] = $moreusers;
+
+        if ($returnexactcount) {
+            // Include totalusers in result if $returnexactcount flag is true.
+            $results['totalusers'] = $totalusers;
+        }
+
+        return $results;
     }
 
     /**
@@ -446,9 +475,15 @@ class course_enrolment_manager {
      * @param int $page Defaults to 0
      * @param int $perpage Defaults to 25
      * @param int $addedenrollment Defaults to 0
-     * @return array Array(totalusers => int, users => array)
-     */
-    public function get_potential_users($enrolid, $search='', $searchanywhere=false, $page=0, $perpage=25, $addedenrollment=0) {
+     * @param bool $returnexactcount Return the exact total users using count_record or not.
+     * @return array with two or three elements:
+     *      int totalusers Number users matching the search. (This element only exist if $returnexactcount was set to true)
+     *      array users List of user objects returned by the query.
+     *      boolean moreusers True if there are still more users, otherwise is False.
+     * @throws dml_exception
+     */
+    public function get_potential_users($enrolid, $search = '', $searchanywhere = false, $page = 0, $perpage = 25,
+            $addedenrollment = 0, $returnexactcount = false) {
         global $DB;
 
         list($ufields, $params, $wherecondition) = $this->get_basic_search_conditions($search, $searchanywhere);
@@ -461,7 +496,8 @@ class course_enrolment_manager {
                       AND ue.id IS NULL";
         $params['enrolid'] = $enrolid;
 
-        return $this->execute_search_queries($search, $fields, $countfields, $sql, $params, $page, $perpage, $addedenrollment);
+        return $this->execute_search_queries($search, $fields, $countfields, $sql, $params, $page, $perpage, $addedenrollment,
+                $returnexactcount);
     }
 
     /**
@@ -472,9 +508,14 @@ class course_enrolment_manager {
      * @param bool $searchanywhere
      * @param int $page Starting at 0
      * @param int $perpage
-     * @return array
-     */
-    public function search_other_users($search='', $searchanywhere=false, $page=0, $perpage=25) {
+     * @param bool $returnexactcount Return the exact total users using count_record or not.
+     * @return array with two or three elements:
+     *      int totalusers Number users matching the search. (This element only exist if $returnexactcount was set to true)
+     *      array users List of user objects returned by the query.
+     *      boolean moreusers True if there are still more users, otherwise is False.
+     * @throws dml_exception
+     */
+    public function search_other_users($search = '', $searchanywhere = false, $page = 0, $perpage = 25, $returnexactcount = false) {
         global $DB, $CFG;
 
         list($ufields, $params, $wherecondition) = $this->get_basic_search_conditions($search, $searchanywhere);
@@ -487,7 +528,7 @@ class course_enrolment_manager {
                     AND ra.id IS NULL";
         $params['contextid'] = $this->context->id;
 
-        return $this->execute_search_queries($search, $fields, $countfields, $sql, $params, $page, $perpage);
+        return $this->execute_search_queries($search, $fields, $countfields, $sql, $params, $page, $perpage, 0, $returnexactcount);
     }
 
     /**

enrol/renderer.php

@@ -680,6 +680,7 @@ class course_enrolment_other_users_table extends course_enrolment_table {
             $this->manager->get_moodlepage()->requires->strings_for_js(array(
                     'ajaxoneuserfound',
                     'ajaxxusersfound',
+                    'ajaxxmoreusersfound',
                     'ajaxnext25',
                     'enrol',
                     'enrolmentoptions',

enrol/tests/course_enrolment_manager_test.php

@@ -104,6 +104,20 @@ class core_course_enrolment_manager_testcase extends advanced_testcase {
         $this->course = $course;
         $this->users = $users;
         $this->groups = $groups;
+
+        // Make sample users and not enroll to any course.
+        $this->getDataGenerator()->create_user([
+                'username' => 'testapiuser1',
+                'firstname' => 'testapiuser 1'
+        ]);
+        $this->getDataGenerator()->create_user([
+                'username' => 'testapiuser2',
+                'firstname' => 'testapiuser 2'
+        ]);
+        $this->getDataGenerator()->create_user([
+                'username' => 'testapiuser3',
+                'firstname' => 'testapiuser 3'
+        ]);
     }
 
     /**
@@ -239,4 +253,88 @@ class core_course_enrolment_manager_testcase extends advanced_testcase {
         $this->assertCount(1, $users, 'Only suspended users must be returned when suspended users filtering is applied.');
         $this->assertArrayHasKey($this->users['user22']->id, $users);
     }
+
+    /**
+     * Test get_potential_users without returnexactcount param.
+     *
+     * @dataProvider search_users_provider
+     *
+     * @param int $perpage Number of users per page.
+     * @param bool $returnexactcount Return the exact count or not.
+     * @param int $expectedusers Expected number of users return.
+     * @param int $expectedtotalusers Expected total of users in database.
+     * @param bool $expectedmoreusers Expected for more users return or not.
+     */
+    public function test_get_potential_users($perpage, $returnexactcount, $expectedusers, $expectedtotalusers, $expectedmoreusers) {
+        global $DB, $PAGE;
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $enrol = $DB->get_record('enrol', array('courseid' => $this->course->id, 'enrol' => 'manual'));
+        $manager = new course_enrolment_manager($PAGE, $this->course);
+        $users = $manager->get_potential_users($enrol->id,
+                'testapiuser',
+                true,
+                0,
+                $perpage,
+                0,
+                $returnexactcount);
+
+        $this->assertCount($expectedusers, $users['users']);
+        $this->assertEquals($expectedmoreusers, $users['moreusers']);
+        if ($returnexactcount) {
+            $this->assertArrayHasKey('totalusers', $users);
+            $this->assertEquals($expectedtotalusers, $users['totalusers']);
+        } else {
+            $this->assertArrayNotHasKey('totalusers', $users);
+        }
+    }
+
+    /**
+     * Test search_other_users with returnexactcount param.
+     *
+     * @dataProvider search_users_provider
+     *
+     * @param int $perpage Number of users per page.
+     * @param bool $returnexactcount Return the exact count or not.
+     * @param int $expectedusers Expected number of users return.
+     * @param int $expectedtotalusers Expected total of users in database.
+     * @param bool $expectedmoreusers Expected for more users return or not.
+     */
+    public function test_search_other_users($perpage, $returnexactcount, $expectedusers, $expectedtotalusers, $expectedmoreusers) {
+        global $PAGE;
+        $this->resetAfterTest();
+        $this->setAdminUser();
+
+        $manager = new course_enrolment_manager($PAGE, $this->course);
+        $users = $manager->search_other_users(
+                'testapiuser',
+                true,
+                0,
+                $perpage,
+                $returnexactcount);
+
+        $this->assertCount($expectedusers, $users['users']);
+        $this->assertEquals($expectedmoreusers, $users['moreusers']);
+        if ($returnexactcount) {
+            $this->assertArrayHasKey('totalusers', $users);
+            $this->assertEquals($expectedtotalusers, $users['totalusers']);
+        } else {
+            $this->assertArrayNotHasKey('totalusers', $users);
+        }
+    }
+
+    /**
+     * Test case for test_get_potential_users and test_search_other_users tests.
+     *
+     * @return array Dataset
+     */
+    public function search_users_provider() {
+        return [
+                [2, false, 2, 3, true],
+                [5, false, 3, 3, false],
+                [2, true, 2, 3, true],
+                [5, true, 3, 3, false]
+        ];
+    }
 }

enrol/upgrade.txt

 This files describes API changes in /enrol/* - plugins,
 information provided here is intended especially for developers.
 
+=== 3.7 ===
+
+* Functions get_potential_users() and search_other_users() now return more information to avoid extra count query:
+  - 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).
+
 === 3.6 ===
 
 * External function core_enrol_external::get_users_courses now return more information to avoid multiple queries to build the

enrol/yui/otherusersmanager/otherusersmanager.js

@@ -160,8 +160,9 @@ YUI.add('moodle-enrol-otherusersmanager', function(Y) {
             this._loadingNode.addClass(CSS.HIDDEN);
         },
         processSearchResults : function(tid, outcome, args) {
+            var result;
             try {
-                var result = Y.JSON.parse(outcome.responseText);
+                result = Y.JSON.parse(outcome.responseText);
                 if (result.error) {
                     return new M.core.ajaxException(result);
                 }
@@ -186,18 +187,26 @@ YUI.add('moodle-enrol-otherusersmanager', function(Y) {
             }
             this.set(USERCOUNT, count);
             if (!args.append) {
-                var usersstr = (result.response.totalusers == '1')?M.util.get_string('ajaxoneuserfound', 'enrol'):M.util.get_string('ajaxxusersfound','enrol', result.response.totalusers);
+                var usersstr = '';
+                if (this.get(USERCOUNT) === 1) {
+                    usersstr = M.util.get_string('ajaxoneuserfound', 'enrol');
+                } else if (result.response.moreusers) {
+                    usersstr = M.util.get_string('ajaxxmoreusersfound', 'enrol', this.get(USERCOUNT));
+                } else {
+                    usersstr = M.util.get_string('ajaxxusersfound', 'enrol', this.get(USERCOUNT));
+                }
+
                 var content = Y.Node.create('<div class="'+CSS.SEARCHRESULTS+'"></div>')
                     .append(Y.Node.create('<div class="'+CSS.TOTALUSERS+'">'+usersstr+'</div>'))
                     .append(usersnode);
-                if (result.response.totalusers > (this.get(PAGE)+1)*25) {
+                if (result.response.moreusers) {
                     var fetchmore = Y.Node.create('<div class="'+CSS.MORERESULTS+'"><a href="#">'+M.util.get_string('ajaxnext25', 'enrol')+'</a></div>');
                     fetchmore.on('click', this.getUsers, this, true);
                     content.append(fetchmore)
                 }
                 this.setContent(content);
             } else {
-                if (result.response.totalusers <= (this.get(PAGE)+1)*25) {
+                if (!result.response.moreusers) {
                     this.get(BASE).one('.'+CSS.MORERESULTS).remove();
                 }
             }

lang/en/enrol.php

@@ -28,6 +28,7 @@ $string['addinstance'] = 'Add method';
 $string['addinstanceanother'] = 'Add method and create another';
 $string['ajaxoneuserfound'] = '1 user found';
 $string['ajaxxusersfound'] = '{$a} users found';
+$string['ajaxxmoreusersfound'] = 'More than {$a} users found';
 $string['ajaxnext25'] = 'Next 25...';
 $string['assignnotpermitted'] = 'You do not have permission or can not assign roles in this course.';
 $string['bulkuseroperation'] = 'Bulk user operation';