From d6b81786eed2e879eaf82951ea0f8c32163e96cc Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 15 Apr 2025 15:26:49 +0100 Subject: [PATCH 1/2] MDL-82132 user: re-factor code for generating dummy user fullname. --- .upgradenotes/MDL-82132-2025041514493372.yml | 7 +++++ lib/classes/user.php | 22 +++++++++++++--- lib/tests/user_test.php | 26 +++++++++++++++++++ reportbuilder/classes/local/entities/user.php | 5 ++-- user/classes/fields.php | 6 ++--- 5 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 .upgradenotes/MDL-82132-2025041514493372.yml diff --git a/.upgradenotes/MDL-82132-2025041514493372.yml b/.upgradenotes/MDL-82132-2025041514493372.yml new file mode 100644 index 00000000000..625a9e21756 --- /dev/null +++ b/.upgradenotes/MDL-82132-2025041514493372.yml @@ -0,0 +1,7 @@ +issueNumber: MDL-82132 +notes: + core_user: + - message: >- + New method `\core_user::get_dummy_fullname(...)` for returning dummy + user fullname comprised of configured name fields only + type: improved diff --git a/lib/classes/user.php b/lib/classes/user.php index 06fa5f4c158..065692233d3 100644 --- a/lib/classes/user.php +++ b/lib/classes/user.php @@ -1518,6 +1518,22 @@ class user { return $displayname; } + /** + * Return fullname of a dummy user comprised of configured name fields only + * + * @param context|null $context + * @param array $options + * @return string + */ + public static function get_dummy_fullname(?context $context = null, array $options = []): string { + + // Create a dummy user object containing all name fields. + $namefields = \core_user\fields::get_name_fields(); + $user = (object) array_combine($namefields, $namefields); + + return static::get_fullname($user, $context, $options); + } + /** * Return profile url depending on context. * @@ -1588,10 +1604,10 @@ class user { public static function get_initials(stdClass $user): string { // Get the available name fields. $namefields = \core_user\fields::get_name_fields(); - // Build a dummy user to determine the name format. - $dummyuser = array_combine($namefields, $namefields); + // Determine the name format by using fullname() and passing the dummy user. - $nameformat = fullname((object) $dummyuser); + $nameformat = static::get_dummy_fullname(); + // Fetch all the available username fields. $availablefields = order_in_string($namefields, $nameformat); // We only want the first and last name fields. diff --git a/lib/tests/user_test.php b/lib/tests/user_test.php index 5230bbfab6d..7ce0c0989da 100644 --- a/lib/tests/user_test.php +++ b/lib/tests/user_test.php @@ -876,6 +876,32 @@ final class user_test extends \advanced_testcase { $this->assertEquals('John Doe', \core_user::get_fullname($user, $context, $options)); } + /** + * Test retrieving dummy user fullname + * + * @covers \core_user::get_dummy_fullname + */ + public function test_get_dummy_fullname(): void { + $context = \context_system::instance(); + + // Show real name as the force names config are not set. + $this->assertEquals('firstname lastname', \core_user::get_dummy_fullname($context)); + + // With override, still show real name. + $options = ['override' => true]; + $this->assertEquals('firstname lastname', \core_user::get_dummy_fullname($context, $options)); + + // Set the alternative names config. + set_config('alternativefullnameformat', 'alternatename lastname firstname'); + + // Show default name format. + $this->assertEquals('firstname lastname', \core_user::get_dummy_fullname($context)); + + // With override, show alternative name format. + $options = ['override' => true]; + $this->assertEquals('alternatename lastname firstname', \core_user::get_dummy_fullname($context, $options)); + } + /** * Test for function to get user details. * diff --git a/reportbuilder/classes/local/entities/user.php b/reportbuilder/classes/local/entities/user.php index 8b7a9106f67..bcf319268a5 100644 --- a/reportbuilder/classes/local/entities/user.php +++ b/reportbuilder/classes/local/entities/user.php @@ -24,6 +24,7 @@ use context_user; use core\context; use core_component; use core_date; +use core_user; use html_writer; use lang_string; use moodle_url; @@ -397,10 +398,8 @@ class user extends base { $namefields = fields::get_name_fields(true); - // Create a dummy user object containing all name fields. - $dummyuser = (object) array_combine($namefields, $namefields); $viewfullnames = has_capability('moodle/site:viewfullnames', context_system::instance()); - $dummyfullname = fullname($dummyuser, $viewfullnames); + $dummyfullname = core_user::get_dummy_fullname(null, ['override' => $viewfullnames]); // Extract any name fields from the fullname format in the order that they appear. $matchednames = array_values(order_in_string($namefields, $dummyfullname)); diff --git a/user/classes/fields.php b/user/classes/fields.php index c4154d83308..4da49487e38 100644 --- a/user/classes/fields.php +++ b/user/classes/fields.php @@ -17,6 +17,7 @@ namespace core_user; use core_text; +use core_user; /** * Class for retrieving information about user fields that are needed for displaying user identity. @@ -585,10 +586,7 @@ class fields { $unique = self::$uniqueidentifier++; $namefields = self::get_name_fields(); - - // Create a dummy user object containing all name fields. - $dummyuser = (object) array_combine($namefields, $namefields); - $dummyfullname = fullname($dummyuser, $override); + $dummyfullname = core_user::get_dummy_fullname(null, ['override' => $override]); // Extract any name fields from the fullname format in the order that they appear. $matchednames = array_values(order_in_string($namefields, $dummyfullname)); From f09a2fc6ec26c405c73fefe2624b9f7cc8427f2c Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Fri, 7 Jun 2024 13:20:38 +0100 Subject: [PATCH 2/2] MDL-82132 user: less restrictive returning first/lastname details. The `viewfullnames` capability check was overeager in restricting the inclusion of user first and lastname properties in the returned structure. Especially given that the same data was almost always present in the fullname property of the same structure. --- user/lib.php | 27 ++++++++++++++++----------- user/tests/userlib_test.php | 24 +++++++++++++++++++++++- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/user/lib.php b/user/lib.php index 96ab8c28ea9..6f82000dd40 100644 --- a/user/lib.php +++ b/user/lib.php @@ -373,23 +373,28 @@ function user_get_user_details($user, $course = null, array $userfields = array( return null; } - $userdetails = array(); - $userdetails['id'] = $user->id; + // User ID and fullname are always included. + $userdetails = [ + 'id' => $user->id, + 'fullname' => fullname($user, $canviewfullnames), + ]; + + // User first/lastname included if capability check passes, or the same is present in fullname. + $dummyusername = core_user::get_dummy_fullname($context, ['override' => $canviewfullnames]); + if (in_array('firstname', $userfields) && + ($canviewfullnames || core_text::strrpos($dummyusername, 'firstname') !== false)) { + $userdetails['firstname'] = $user->firstname; + } + if (in_array('lastname', $userfields) && + ($canviewfullnames || core_text::strrpos($dummyusername, 'lastname') !== false)) { + $userdetails['lastname'] = $user->lastname; + } if (in_array('username', $userfields)) { if ($currentuser or has_capability('moodle/user:viewalldetails', $context)) { $userdetails['username'] = $user->username; } } - if ($isadmin or $canviewfullnames) { - if (in_array('firstname', $userfields)) { - $userdetails['firstname'] = $user->firstname; - } - if (in_array('lastname', $userfields)) { - $userdetails['lastname'] = $user->lastname; - } - } - $userdetails['fullname'] = fullname($user, $canviewfullnames); if (in_array('customfields', $userfields)) { $categories = profile_get_user_fields_with_data_by_category($user->id); diff --git a/user/tests/userlib_test.php b/user/tests/userlib_test.php index 337ecf7fadb..edf1de5c185 100644 --- a/user/tests/userlib_test.php +++ b/user/tests/userlib_test.php @@ -850,16 +850,21 @@ final class userlib_test extends \advanced_testcase { accesslib_clear_all_caches_for_unit_testing(); // Get student details as a user with super system capabilities. + $this->setAdminUser(); $result = user_get_user_details($student, $course1); $this->assertEquals($student->id, $result['id']); $this->assertEquals($studentfullname, $result['fullname']); + $this->assertEquals($student->firstname, $result['firstname']); + $this->assertEquals($student->lastname, $result['lastname']); $this->assertEquals($course1->id, $result['enrolledcourses'][0]['id']); - $this->setUser($teacher); // Get student details as a user who can only see this user in a course. + $this->setUser($teacher); $result = user_get_user_details($student, $course1); $this->assertEquals($student->id, $result['id']); $this->assertEquals($studentfullname, $result['fullname']); + $this->assertEquals($student->firstname, $result['firstname']); + $this->assertEquals($student->lastname, $result['lastname']); $this->assertEquals($course1->id, $result['enrolledcourses'][0]['id']); // Get student details with required fields. @@ -867,6 +872,23 @@ final class userlib_test extends \advanced_testcase { $this->assertCount(2, $result); $this->assertEquals($student->id, $result['id']); $this->assertEquals($studentfullname, $result['fullname']); + $this->assertArrayNotHasKey('firstname', $result); + $this->assertArrayNotHasKey('lastname', $result); + $this->assertArrayNotHasKey('enrolledcourses', $result); + + // Change fullname display format for a user with viewfullnames capability. + set_config('fullnamedisplay', 'firstname'); + $result = user_get_user_details($student, $course1); + $this->assertEquals($studentfullname, $result['fullname']); + $this->assertEquals($student->firstname, $result['firstname']); + $this->assertEquals($student->lastname, $result['lastname']); + + // Now check for a user without viewfullnames capability. + $this->setUser($student); + $result = user_get_user_details($teacher, $course1); + $this->assertEquals($teacher->firstname, $result['fullname']); + $this->assertEquals($teacher->firstname, $result['firstname']); + $this->assertArrayNotHasKey('lastname', $result); // Get exception for invalid required fields. $this->expectException('moodle_exception');