From 0ac237d542384268e273debb56672326fd91bd26 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Mon, 18 Dec 2023 12:56:31 +0000 Subject: [PATCH 1/2] MDL-74500 user: method to determine whether to show profile fields. The new API replaces identical behaviour in existing calling code, but allows for profile field types to override/separate the logic used to show the field and determine whether it's empty --- lib/myprofilelib.php | 2 +- user/lib.php | 3 +-- user/profile/lib.php | 10 ++++++++++ user/upgrade.txt | 5 +++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/myprofilelib.php b/lib/myprofilelib.php index eb6baaa394e..7917898a0f3 100644 --- a/lib/myprofilelib.php +++ b/lib/myprofilelib.php @@ -333,7 +333,7 @@ function core_myprofile_navigation(core_user\output\myprofile\tree $tree, $user, $categories = profile_get_user_fields_with_data_by_category($user->id); foreach ($categories as $categoryid => $fields) { foreach ($fields as $formfield) { - if ($formfield->is_visible() and !$formfield->is_empty()) { + if ($formfield->show_field_content()) { $node = new core_user\output\myprofile\node('contact', 'custom_field_' . $formfield->field->shortname, format_string($formfield->field->name), null, null, $formfield->display_data()); $tree->add_node($node); diff --git a/user/lib.php b/user/lib.php index 75d316a3900..d799acb3518 100644 --- a/user/lib.php +++ b/user/lib.php @@ -399,8 +399,7 @@ function user_get_user_details($user, $course = null, array $userfields = array( $userdetails['customfields'] = array(); foreach ($categories as $categoryid => $fields) { foreach ($fields as $formfield) { - if ($formfield->is_visible() and !$formfield->is_empty()) { - + if ($formfield->show_field_content()) { $userdetails['customfields'][] = [ 'name' => $formfield->field->name, 'value' => $formfield->data, diff --git a/user/profile/lib.php b/user/profile/lib.php index 117cb967503..41ce7d92a2b 100644 --- a/user/profile/lib.php +++ b/user/profile/lib.php @@ -587,6 +587,16 @@ class profile_field_base { return array(PARAM_RAW, NULL_NOT_ALLOWED); } + /** + * Whether to display the field and content to the user + * + * @param context|null $context + * @return bool + */ + public function show_field_content(?context $context = null): bool { + return $this->is_visible($context) && !$this->is_empty(); + } + /** * Check if the field should convert the raw data into user-friendly data when exporting * diff --git a/user/upgrade.txt b/user/upgrade.txt index e20f6c07300..34ac80b9253 100644 --- a/user/upgrade.txt +++ b/user/upgrade.txt @@ -1,5 +1,10 @@ This files describes API changes for code that uses the user API. +=== 4.4 === + +* The `profile_field_base` class now contains a `show_field_content` method to determine whether the field and + content should be shown to the user. Can be overridden in child classes as required + === 4.3 === * Added new methods: From 597a8ac878a518668c1712b43f213922d56099a8 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 19 Dec 2023 11:36:13 +0000 Subject: [PATCH 2/2] MDL-74500 profilefield_checkbox: override show/empty class methods. --- user/profile/field/checkbox/field.class.php | 25 ++++ .../tests/profile_field_checkbox_test.php | 122 ++++++++++++++++++ 2 files changed, 147 insertions(+) create mode 100644 user/profile/field/checkbox/tests/profile_field_checkbox_test.php diff --git a/user/profile/field/checkbox/field.class.php b/user/profile/field/checkbox/field.class.php index a6e0885f8ec..6ddfc03a5b7 100644 --- a/user/profile/field/checkbox/field.class.php +++ b/user/profile/field/checkbox/field.class.php @@ -39,6 +39,31 @@ class profile_field_checkbox extends profile_field_base { } } + /** + * Override parent {@see profile_field_base::is_empty} check + * + * We can't check the "data" property, because if not set by the user then it's populated by "defaultdata" of the field, + * which can also be 0 (false) therefore ensuring the parent class check could never return true for this comparison + * + * @return bool + */ + public function is_empty() { + return ($this->userid && !$this->field->hasuserdata); + } + + /** + * Override parent {@see profile_field_base::show_field_content} check + * + * We only need to determine whether the field is visible, because we also want to show the "defaultdata" of the field, + * even if the user hasn't explicitly filled it in + * + * @param context|null $context + * @return bool + */ + public function show_field_content(?context $context = null): bool { + return $this->is_visible($context); + } + /** * Display the data for this field * diff --git a/user/profile/field/checkbox/tests/profile_field_checkbox_test.php b/user/profile/field/checkbox/tests/profile_field_checkbox_test.php new file mode 100644 index 00000000000..d008e859519 --- /dev/null +++ b/user/profile/field/checkbox/tests/profile_field_checkbox_test.php @@ -0,0 +1,122 @@ +. + +namespace profilefield_checkbox; + +use advanced_testcase; +use profile_field_checkbox; + +/** + * Unit tests for the field class + * + * @package profilefield_checkbox + * @covers \profile_field_checkbox + * @copyright 2024 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class profile_field_checkbox_test extends advanced_testcase { + + /** + * Load required test libraries + */ + public static function setUpBeforeClass(): void { + global $CFG; + require_once("{$CFG->dirroot}/user/profile/lib.php"); + } + + /** + * Data provider for {@see test_is_empty} + * + * @return array[] + */ + public static function is_empty_provider(): array { + return [ + 'No value' => [ + [], + true, + ], + 'Value equals 0' => [ + ['profile_field_check' => 0], + false, + ], + 'Value equals 1' => [ + ['profile_field_check' => 1], + false, + ], + ]; + } + + /** + * Test field empty state + * + * @param array $userrecord + * @param bool $expected + * + * @dataProvider is_empty_provider + */ + public function test_is_empty(array $userrecord, bool $expected): void { + $this->resetAfterTest(); + + $this->getDataGenerator()->create_custom_profile_field([ + 'datatype' => 'checkbox', + 'name' => 'My check', + 'shortname' => 'check', + ]); + + $user = $this->getDataGenerator()->create_user($userrecord); + + /** @var profile_field_checkbox[] $fields */ + $fields = profile_get_user_fields_with_data($user->id); + $fieldinstance = reset($fields); + + $this->assertEquals($expected, $fieldinstance->is_empty()); + } + + /** + * Test whether to show field content + */ + public function test_show_field_content(): void { + $this->resetAfterTest(); + + $this->getDataGenerator()->create_custom_profile_field([ + 'datatype' => 'checkbox', + 'name' => 'My check', + 'shortname' => 'check', + 'visible' => PROFILE_VISIBLE_PRIVATE, + ]); + + // User can view their own value. + $userwith = $this->getDataGenerator()->create_user(['profile_field_check' => 1]); + $this->setUser($userwith); + + /** @var profile_field_checkbox[] $fields */ + $fields = profile_get_user_fields_with_data($userwith->id); + $fieldinstance = reset($fields); + + $this->assertTrue($fieldinstance->show_field_content()); + + // Another user cannot view the value. + $userview = $this->getDataGenerator()->create_user(); + $this->setUser($userview); + + $this->assertFalse($fieldinstance->show_field_content()); + + // Another user with appropriate access can view the value. + $this->setAdminUser(); + $this->assertTrue($fieldinstance->show_field_content()); + + } +}