Merge branch 'MDL-76873' of https://github.com/paulholden/moodle

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_(?<shortname>.*)$/", $identityfield, $matches)) {
+        if (preg_match(fields::PROFILE_FIELD_REGEX, $identityfield, $matches)) {
-            $identityfield = 'profilefield_' . $matches['shortname'];
+            $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_(?<shortname>.*)$/", $identityfield, $matches)) {
+        if (preg_match(fields::PROFILE_FIELD_REGEX, $identityfield, $matches)) {
-            $identityfield = 'profilefield_' . $matches['shortname'];
+            $identityfield = 'profilefield_' . $matches[1];
         }
 
         return $this->get_filter($identityfield);

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 array_filter(profile_get_user_fields_with_data(0), static function(profile_field_base $profilefield): bool {
-            return (int)$profilefield->field->visible === (int)PROFILE_VISIBLE_ALL;
+            return $profilefield->is_visible();
         });
     }
 

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]);
+                    $field = profile_get_custom_field_data_by_shortname($matches[1], false);
-                    switch ($fieldinfo->visible ?? -1) {
+                    $fieldinstance = profile_get_user_field($field->datatype, $field->id, 0, $field);
-                        case PROFILE_VISIBLE_NONE:
+                    $allowed = $fieldinstance->is_visible($context);
-                        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;
-                    }
                 }
                 if (!$allowed) {
                     unset($extra[$key]);

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 = profile_get_user_field($field->datatype, $field->id, $userid, $field);
-        $fieldobject = new $classname($field->id, $userid, $field);
         $fieldobject->set_category_name($field->categoryname);
         unset($field->categoryname);
         $data[] = $fieldobject;

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.
+        // 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',
+        $this->assertEquals(['email', 'department', 'profile_field_a', 'profile_field_b', 'profile_field_d'],
-                'profile_field_c', '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'],

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;
+        global $DB;
-        require_once($CFG->dirroot . '/user/profile/lib.php');
 
         $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);
+    }
 }

user/upgrade.txt

@@ -12,6 +12,8 @@ This files describes API changes for code that uses the user API.
   The get_profile_field_list() returns the profile fields
   in a format that can be used for choices in a group select menu.
 * 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 ===