MDL-74676 course: Speed up course search if using limittoenrolled

Co-authored-by: Huong Nguyen <[email protected]>
ingoj · Feb 26, 2024 · 2a494f0 · 2a494f0
1 parent f30110b
commit 2a494f0
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 14 deletions.
diff --git a/course/classes/category.php b/course/classes/category.php
@@ -1566,6 +1566,7 @@ public function get_view_link() {
      *     - modulelist - name of module (if we are searching for courses containing specific module
      *     - tagid - id of tag
      *     - onlywithcompletion - set to true if we only need courses with completion enabled
+     *     - limittoenrolled - set to true if we only need courses where user is enrolled
      * @param array $options display options, same as in get_courses() except 'recursive' is ignored -
      *                       search is always category-independent
      * @param array $requiredcapabilities List of capabilities required to see return course.
@@ -1622,6 +1623,15 @@ public static function search_courses($search, $options = array(), $requiredcapa
             $search['search'] = '';
         }
 
+        $courseidsearch = '';
+        $courseidparams = [];
+
+        if (!empty($search['limittoenrolled'])) {
+            $enrolled = enrol_get_my_courses(['id']);
+            list($sql, $courseidparams) = $DB->get_in_or_equal(array_keys($enrolled), SQL_PARAMS_NAMED, 'courseid', true, 0);
+            $courseidsearch = "c.id " . $sql;
+        }
+
         if (empty($search['blocklist']) && empty($search['modulelist']) && empty($search['tagid'])) {
             // Search courses that have specified words in their names/summaries.
             $searchterms = preg_split('|\s+|', trim($search['search']), 0, PREG_SPLIT_NO_EMPTY);
@@ -1630,6 +1640,10 @@ public static function search_courses($search, $options = array(), $requiredcapa
                 $searchcond = ['c.enablecompletion = :p1'];
                 $searchcondparams = ['p1' => 1];
             }
+            if (!empty($courseidsearch)) {
+                $searchcond[] = $courseidsearch;
+                $searchcondparams = array_merge($searchcondparams, $courseidparams);
+            }
             $courselist = get_courses_search($searchterms, 'c.sortorder ASC', 0, 9999999, $totalcount,
                 $requiredcapabilities, $searchcond, $searchcondparams);
             self::sort_records($courselist, $sortfields);
@@ -1676,6 +1690,11 @@ public static function search_courses($search, $options = array(), $requiredcapa
                 debugging('No criteria is specified while searching courses', DEBUG_DEVELOPER);
                 return array();
             }
+            if (!empty($courseidsearch)) {
+                $where .= ' AND ' . $courseidsearch;
+                $params = array_merge($params, $courseidparams);
+            }
+
             $courselist = self::get_course_records($where, $params, $options, true);
             if (!empty($requiredcapabilities)) {
                 foreach ($courselist as $key => $course) {

diff --git a/course/externallib.php b/course/externallib.php
@@ -2660,6 +2660,9 @@ public static function search_courses($criterianame,
         if ($params['onlywithcompletion']) {
             $searchcriteria['onlywithcompletion'] = true;
         }
+        if ($params['limittoenrolled']) {
+            $searchcriteria['limittoenrolled'] = true;
+        }
 
         $options = array();
         if ($params['perpage'] != 0) {
@@ -2671,23 +2674,10 @@ public static function search_courses($criterianame,
         $courses = core_course_category::search_courses($searchcriteria, $options, $params['requiredcapabilities']);
         $totalcount = core_course_category::search_courses_count($searchcriteria, $options, $params['requiredcapabilities']);
 
-        if (!empty($limittoenrolled)) {
-            // Get the courses where the current user has access.
-            $enrolled = enrol_get_my_courses(array('id', 'cacherev'));
-        }
-
         $finalcourses = array();
         $categoriescache = array();
 
         foreach ($courses as $course) {
-            if (!empty($limittoenrolled)) {
-                // Filter out not enrolled courses.
-                if (!isset($enrolled[$course->id])) {
-                    $totalcount--;
-                    continue;
-                }
-            }
-
             $coursecontext = context_course::instance($course->id);
 
             $finalcourses[] = self::get_course_public_information($course, $coursecontext);

diff --git a/course/tests/category_test.php b/course/tests/category_test.php
@@ -513,6 +513,8 @@ public function test_resort_courses() {
     }
 
     public function test_get_search_courses() {
+        global $DB;
+
         $cat1 = core_course_category::create(array('name' => 'Cat1'));
         $cat2 = core_course_category::create(array('name' => 'Cat2', 'parent' => $cat1->id));
         $c1 = $this->getDataGenerator()->create_course(array('category' => $cat1->id, 'fullname' => 'Test 3', 'summary' => ' ', 'idnumber' => 'ID3'));
@@ -578,7 +580,8 @@ public function test_get_search_courses() {
         $this->assertEquals(array($c3->id, $c6->id), array_keys($res));
         $this->assertEquals(2, core_course_category::search_courses_count(array('search' => 'Математика'), array()));
 
-        $this->setUser($this->getDataGenerator()->create_user());
+        $user = $this->getDataGenerator()->create_user();
+        $this->setUser($user);
 
         // Add necessary capabilities.
         $this->assign_capability('moodle/course:create', CAP_ALLOW, \context_coursecat::instance($cat2->id));
@@ -587,6 +590,36 @@ public function test_get_search_courses() {
         $res = core_course_category::search_courses(array('search' => 'test'), array(), $reqcaps);
         $this->assertEquals(array($c8->id, $c5->id), array_keys($res));
         $this->assertEquals(2, core_course_category::search_courses_count(array('search' => 'test'), array(), $reqcaps));
+
+        // We should get no courses here as user is not enrolled to any courses.
+        $res = core_course_category::search_courses([
+            'search' => '',
+            'limittoenrolled' => 1,
+        ]);
+        $this->assertEquals([], $res);
+        $this->assertEquals(0, core_course_category::search_courses_count([
+            'search' => '',
+            'limittoenrolled' => 1,
+        ]));
+
+        $manual = enrol_get_plugin('manual');
+        $teacherrole = $DB->get_record('role', ['shortname' => 'editingteacher']);
+        $enrol = $DB->get_record('enrol', ['courseid' => $c5->id, 'enrol' => 'manual'], '*', MUST_EXIST);
+        $manual->enrol_user($enrol, $user->id, $teacherrole->id);
+
+        // Avoid using the cached values from previous method call.
+        \cache::make('core', 'coursecat')->purge();
+
+        // As the user is now enrolled, we should get this one course.
+        $res = core_course_category::search_courses([
+            'search' => '',
+            'limittoenrolled' => 1,
+        ]);
+        $this->assertEquals([$c5->id], array_keys($res));
+        $this->assertEquals(1, core_course_category::search_courses_count([
+            'search' => '',
+            'limittoenrolled' => 1,
+        ]));
     }
 
     public function test_course_contacts() {