diff --git a/lib/classes/user.php b/lib/classes/user.php index 576e9800eeb..2e929bd8a83 100644 --- a/lib/classes/user.php +++ b/lib/classes/user.php @@ -266,29 +266,32 @@ class core_user { $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); + } } } diff --git a/lib/tests/user_test.php b/lib/tests/user_test.php index de55016b7cc..60851556cd3 100644 --- a/lib/tests/user_test.php +++ b/lib/tests/user_test.php @@ -19,6 +19,7 @@ namespace core; /** * Test core_user class. * + * @covers \core_user * @package core * @copyright 2013 Rajesh Taneja * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later @@ -283,6 +284,36 @@ class user_test extends \advanced_testcase { $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' => 'a.unwin@x.x']); + $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. */