MDL-80693 core_user: search() fails if no standard identify fields

lib/classes/user.php

@@ -266,29 +266,32 @@ public static function search($query, \context_course $coursecontext = null,
             $index++;
         }
 
-        $identitysystem = has_capability('moodle/site:viewuseridentity', $systemcontext);
         $usingshowidentity = false;
-        if ($identitysystem) {
-            // They have permission everywhere so just add the extra query to the normal query.
-            $where .= ' OR ' . $extrasql;
-            $whereparams = array_merge($whereparams, $extraparams);
-        } else {
-            // Get all courses where user can view full user identity.
-            list($sql, $params) = self::get_enrolled_sql_on_courses_with_capability(
+        // Only do this code if there actually are some identity fields being searched.
+        if ($extrasql) {
+            $identitysystem = has_capability('moodle/site:viewuseridentity', $systemcontext);
+            if ($identitysystem) {
+                // They have permission everywhere so just add the extra query to the normal query.
+                $where .= ' OR ' . $extrasql;
+                $whereparams = array_merge($whereparams, $extraparams);
+            } else {
+                // Get all courses where user can view full user identity.
+                list($sql, $params) = self::get_enrolled_sql_on_courses_with_capability(
                     'moodle/site:viewuseridentity');
-            if ($sql) {
-                // Join that with the user query to get an extra field indicating if we can.
-                $userquery = "
+                if ($sql) {
+                    // Join that with the user query to get an extra field indicating if we can.
+                    $userquery = "
                         SELECT innerusers.id, COUNT(identityusers.id) AS showidentity
                           FROM ($userquery) innerusers
                      LEFT JOIN ($sql) identityusers ON identityusers.id = innerusers.id
                       GROUP BY innerusers.id";
-                $userparams = array_merge($userparams, $params);
-                $usingshowidentity = true;
+                    $userparams = array_merge($userparams, $params);
+                    $usingshowidentity = true;
 
-                // Query on the extra fields only in those places.
-                $where .= ' OR (users.showidentity > 0 AND (' . $extrasql . '))';
-                $whereparams = array_merge($whereparams, $extraparams);
+                    // Query on the extra fields only in those places.
+                    $where .= ' OR (users.showidentity > 0 AND (' . $extrasql . '))';
+                    $whereparams = array_merge($whereparams, $extraparams);
+                }
             }
         }
 

lib/tests/user_test.php

@@ -19,6 +19,7 @@
 /**
  * Test core_user class.
  *
+ * @covers \core_user
  * @package    core
  * @copyright  2013 Rajesh Taneja <[email protected]>
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
@@ -283,6 +284,36 @@ public function test_search() {
         $this->assertCount(1, $result);
     }
 
+    /**
+     * The search function had a bug where it failed if you have no identify fields (or only custom
+     * ones).
+     */
+    public function test_search_no_identity_fields(): void {
+        self::init_search_tests();
+
+        // Set no user identity fields.
+        set_config('showuseridentity', '');
+
+        // Set up course for test with teacher in.
+        $generator = $this->getDataGenerator();
+        $course = $generator->create_course();
+        $teacher = $generator->create_user(['firstname' => 'Alberto', 'lastname' => 'Unwin',
+            'email' => '[email protected]']);
+        $generator->enrol_user($teacher->id, $course->id, 'teacher');
+
+        // Admin user has site-wide permissions, this uses one variant of the query.
+        $this->setAdminUser();
+        $result = \core_user::search('Al');
+        $this->assertCount(1, $result);
+        $this->assertEquals('Alberto', $result[0]->firstname);
+
+        // Teacher has course-wide permissions, this uses another variant.
+        $this->setUser($teacher);
+        $result = \core_user::search('Al');
+        $this->assertCount(1, $result);
+        $this->assertEquals('Alberto', $result[0]->firstname);
+    }
+
     /**
      * Tests the search() function with limits on the number to return.
      */