diff --git a/reportbuilder/classes/local/entities/user.php b/reportbuilder/classes/local/entities/user.php index 195f0efb9d8..881fd02d713 100644 --- a/reportbuilder/classes/local/entities/user.php +++ b/reportbuilder/classes/local/entities/user.php @@ -109,28 +109,30 @@ class user extends base { } /** - * Returns column that corresponds to the given identity field + * Returns column that corresponds to the given identity field, profile field identifiers will be converted to those + * used by the {@see user_profile_fields} helper * - * @param string $identityfield Field from the user table, or the shortname of a custom profile field + * @param string $identityfield Field from the user table, or a custom profile field * @return column */ public function get_identity_column(string $identityfield): column { - if (preg_match("/^profile_field_(?.*)$/", $identityfield, $matches)) { - $identityfield = 'profilefield_' . $matches['shortname']; + if (preg_match(fields::PROFILE_FIELD_REGEX, $identityfield, $matches)) { + $identityfield = 'profilefield_' . $matches[1]; } return $this->get_column($identityfield); } /** - * Returns filter that corresponds to the given identity field + * Returns filter that corresponds to the given identity field, profile field identifiers will be converted to those + * used by the {@see user_profile_fields} helper * - * @param string $identityfield Field from the user table, or the shortname of a custom profile field + * @param string $identityfield Field from the user table, or a custom profile field * @return filter */ public function get_identity_filter(string $identityfield): filter { - if (preg_match("/^profile_field_(?.*)$/", $identityfield, $matches)) { - $identityfield = 'profilefield_' . $matches['shortname']; + if (preg_match(fields::PROFILE_FIELD_REGEX, $identityfield, $matches)) { + $identityfield = 'profilefield_' . $matches[1]; } return $this->get_filter($identityfield); diff --git a/reportbuilder/classes/local/helpers/user_profile_fields.php b/reportbuilder/classes/local/helpers/user_profile_fields.php index 2dc26fc3d44..bed8d1e9c7d 100644 --- a/reportbuilder/classes/local/helpers/user_profile_fields.php +++ b/reportbuilder/classes/local/helpers/user_profile_fields.php @@ -74,8 +74,8 @@ class user_profile_fields { * @return profile_field_base[] */ private function get_user_profile_fields(): array { - return array_filter(profile_get_user_fields_with_data(0), static function($profilefield): bool { - return (int)$profilefield->field->visible === (int)PROFILE_VISIBLE_ALL; + return array_filter(profile_get_user_fields_with_data(0), static function(profile_field_base $profilefield): bool { + return $profilefield->is_visible(); }); } diff --git a/user/classes/fields.php b/user/classes/fields.php index c8495d23876..20c92d20fc6 100644 --- a/user/classes/fields.php +++ b/user/classes/fields.php @@ -377,20 +377,9 @@ class fields { $allowed = false; if ($allowcustom) { require_once($CFG->dirroot . '/user/profile/lib.php'); - $fieldinfo = profile_get_custom_field_data_by_shortname($matches[1]); - switch ($fieldinfo->visible ?? -1) { - case PROFILE_VISIBLE_NONE: - case PROFILE_VISIBLE_PRIVATE: - $allowed = !$context || has_capability('moodle/user:viewalldetails', $context); - break; - case PROFILE_VISIBLE_TEACHERS: - // This is actually defined (in user/profile/lib.php) based on whether - // you have moodle/site:viewuseridentity in context. We already checked - // that, so treat it as visible (fall through). - case PROFILE_VISIBLE_ALL: - $allowed = true; - break; - } + $field = profile_get_custom_field_data_by_shortname($matches[1], false); + $fieldinstance = profile_get_user_field($field->datatype, $field->id, 0, $field); + $allowed = $fieldinstance->is_visible($context); } if (!$allowed) { unset($extra[$key]); diff --git a/user/profile/lib.php b/user/profile/lib.php index ea04d63109e..d27bdf5e9aa 100644 --- a/user/profile/lib.php +++ b/user/profile/lib.php @@ -440,12 +440,16 @@ class profile_field_base { /** * Check if the field data is visible to the current user * @internal This method should not generally be overwritten by child classes. + * + * @param context|null $context * @return bool */ - public function is_visible() { + public function is_visible(?context $context = null): bool { global $USER, $COURSE; - $context = ($this->userid > 0) ? context_user::instance($this->userid) : context_system::instance(); + if ($context === null) { + $context = ($this->userid > 0) ? context_user::instance($this->userid) : context_system::instance(); + } switch ($this->field->visible) { case PROFILE_VISIBLE_TEACHERS: @@ -593,13 +597,32 @@ class profile_field_base { } } +/** + * Return profile field instance for given type + * + * @param string $type + * @param int $fieldid + * @param int $userid + * @param stdClass|null $fielddata + * @return profile_field_base + */ +function profile_get_user_field(string $type, int $fieldid = 0, int $userid = 0, ?stdClass $fielddata = null): profile_field_base { + global $CFG; + + require_once("{$CFG->dirroot}/user/profile/field/{$type}/field.class.php"); + + // Return instance of profile field type. + $profilefieldtype = "profile_field_{$type}"; + return new $profilefieldtype($fieldid, $userid, $fielddata); +} + /** * Returns an array of all custom field records with any defined data (or empty data), for the specified user id. * @param int $userid * @return profile_field_base[] */ function profile_get_user_fields_with_data(int $userid): array { - global $DB, $CFG; + global $DB; // Join any user info data present with each user info field for the user object. $sql = 'SELECT uif.*, uic.name AS categoryname '; @@ -615,11 +638,8 @@ function profile_get_user_fields_with_data(int $userid): array { $fields = $DB->get_records_sql($sql, ['userid' => $userid]); $data = []; foreach ($fields as $field) { - require_once($CFG->dirroot . '/user/profile/field/' . $field->datatype . '/field.class.php'); - $classname = 'profile_field_' . $field->datatype; $field->hasuserdata = !empty($field->hasuserdata); - /** @var profile_field_base $fieldobject */ - $fieldobject = new $classname($field->id, $userid, $field); + $fieldobject = profile_get_user_field($field->datatype, $field->id, $userid, $field); $fieldobject->set_category_name($field->categoryname); unset($field->categoryname); $data[] = $fieldobject; diff --git a/user/tests/fields_test.php b/user/tests/fields_test.php index bd4be5649d8..d046ee1bf15 100644 --- a/user/tests/fields_test.php +++ b/user/tests/fields_test.php @@ -51,7 +51,7 @@ class fields_test extends \advanced_testcase { * Tests getting the identity fields. */ public function test_get_identity_fields() { - global $DB, $CFG; + global $DB, $CFG, $COURSE; $this->resetAfterTest(); @@ -81,9 +81,8 @@ class fields_test extends \advanced_testcase { $usercontext = \context_user::instance($anotheruser->id); $generator->enrol_user($user->id, $course->id, 'student'); - // When no context is provided, it does no access checks and should return all specified. - $this->assertEquals(['email', 'department', 'profile_field_a', 'profile_field_b', - 'profile_field_c', 'profile_field_d'], + // When no context is provided, it does no access checks and should return all specified (other than non-visible). + $this->assertEquals(['email', 'department', 'profile_field_a', 'profile_field_b', 'profile_field_d'], fields::get_identity_fields(null)); // If you turn off custom profile fields, you don't get those. @@ -104,6 +103,7 @@ class fields_test extends \advanced_testcase { // Give the student the basic identity fields permission (also makes them count as 'teacher' // for the teacher-restricted field). + $COURSE = $course; // Horrible hack, because PROFILE_VISIBLE_TEACHERS relies on this global. $roleid = $DB->get_field('role', 'id', ['shortname' => 'student']); role_change_permission($roleid, $coursecontext, 'moodle/site:viewuseridentity', CAP_ALLOW); $this->assertEquals(['department', 'profile_field_a', 'profile_field_d'], diff --git a/user/tests/profilelib_test.php b/user/tests/profilelib_test.php index dc0978ce789..e8609d71056 100644 --- a/user/tests/profilelib_test.php +++ b/user/tests/profilelib_test.php @@ -24,14 +24,20 @@ namespace core_user; * @licensehttp://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class profilelib_test extends \advanced_testcase { + + /** + * Load required test libraries + */ + public static function setUpBeforeClass(): void { + global $CFG; + require_once("{$CFG->dirroot}/user/profile/lib.php"); + } + /** * Tests profile_get_custom_fields function and checks it is consistent * with profile_user_record. */ public function test_get_custom_fields() { - global $CFG; - require_once($CFG->dirroot . '/user/profile/lib.php'); - $this->resetAfterTest(); $user = $this->getDataGenerator()->create_user(); @@ -235,8 +241,7 @@ class profilelib_test extends \advanced_testcase { * Tests the profile_get_custom_field_data_by_shortname function when working normally. */ public function test_profile_get_custom_field_data_by_shortname_normal() { - global $DB, $CFG; - require_once($CFG->dirroot . '/user/profile/lib.php'); + global $DB; $this->resetAfterTest(); @@ -268,9 +273,6 @@ class profilelib_test extends \advanced_testcase { * Tests the profile_get_custom_field_data_by_shortname function with a field that doesn't exist. */ public function test_profile_get_custom_field_data_by_shortname_missing() { - global $CFG; - require_once($CFG->dirroot . '/user/profile/lib.php'); - $this->assertNull(profile_get_custom_field_data_by_shortname('speciality')); } @@ -306,10 +308,6 @@ class profilelib_test extends \advanced_testcase { bool $casesensitive, bool $expectmatch ): void { - global $CFG; - - require_once("{$CFG->dirroot}/user/profile/lib.php"); - $this->resetAfterTest(); $this->getDataGenerator()->create_custom_profile_field([ @@ -328,4 +326,26 @@ class profilelib_test extends \advanced_testcase { $this->assertNull($customfield); } } + + /** + * Test profile field loading via profile_get_user_field helper + * + * @covers ::profile_get_user_field + */ + public function test_profile_get_user_field(): void { + $this->resetAfterTest(); + + $profilefield = $this->getDataGenerator()->create_custom_profile_field([ + 'shortname' => 'fruit', + 'name' => 'Fruit', + 'datatype' => 'text', + ]); + $user = $this->getDataGenerator()->create_user(['profile_field_fruit' => 'Apple']); + + $fieldinstance = profile_get_user_field('text', $profilefield->id, $user->id); + $this->assertInstanceOf(\profile_field_text::class, $fieldinstance); + $this->assertEquals($profilefield->id, $fieldinstance->fieldid); + $this->assertEquals($user->id, $fieldinstance->userid); + $this->assertEquals('Apple', $fieldinstance->data); + } } diff --git a/user/upgrade.txt b/user/upgrade.txt index 6041942b042..290a5d92d64 100644 --- a/user/upgrade.txt +++ b/user/upgrade.txt @@ -3,6 +3,8 @@ This files describes API changes for code that uses the user API. === 4.1.2 === * New method `core_user::is_current_user`, useful for components implementing permission callbacks for their preferences +* New `profile_get_user_field` method for returning profile field instance of given type +* The `profile_field_base::is_visible` method now accepts an optional `$context` argument === 4.1 ===