MDL-73086 user: account for mixed casing of profile field shortname.

Custom profile fields allow for shortnames containing mixed character
casing, which should also be supported within the user fields API
when retrieving said custom fields.

Given that the DML libs convert all selected columns to lowercased
equivalent, we need to do the same when requesting custom fields to
ensure calling code doesn't try to access `->profile_field_Foo`
property when the DB returns `->profile_field_foo` instead.
This commit is contained in:
Paul Holden 2021-12-20 21:16:43 +00:00
parent f8eb36373d
commit 9e6bbb4fb8
4 changed files with 73 additions and 6 deletions

View File

@ -347,7 +347,8 @@ class fields {
* showing lists of users (in addition to the user's name which is included as standard).
*
* The results include basic field names (columns from the 'user' database table) and, unless
* turned off, custom profile field names in the format 'profile_field_myfield'.
* turned off, custom profile field names in the format 'profile_field_myfield', note these
* fields will always be returned lower cased to match how they are returned by the DML library.
*
* This function does all the required capability checks to see if the current user is allowed
* to see them in the specified context. You can pass context null to get all the fields
@ -421,7 +422,7 @@ class fields {
// Re-index the entries and return.
$extra = array_values($extra);
return $extra;
return array_map([core_text::class, 'strtolower'], $extra);
}
/**
@ -538,7 +539,8 @@ class fields {
$placeholder = '?';
$params[] = $shortname;
}
$joins .= " JOIN {user_info_field} $fieldalias ON $fieldalias.shortname = $placeholder
$joins .= " JOIN {user_info_field} $fieldalias ON " .
$DB->sql_equal($fieldalias . '.shortname', $placeholder, false) . "
LEFT JOIN {user_info_data} $dataalias ON $dataalias.fieldid = $fieldalias.id
AND $dataalias.userid = {$usertable}id";
// For Oracle we need to convert the field into a usable format.
@ -648,7 +650,7 @@ class fields {
// Custom fields have special handling.
if (preg_match(self::PROFILE_FIELD_REGEX, $field, $matches)) {
require_once($CFG->dirroot . '/user/profile/lib.php');
$fieldinfo = profile_get_custom_field_data_by_shortname($matches[1]);
$fieldinfo = profile_get_custom_field_data_by_shortname($matches[1], false);
// Use format_string so it can be translated with multilang filter if necessary.
return $fieldinfo ? format_string($fieldinfo->name) : $field;
}

View File

@ -866,9 +866,11 @@ function profile_save_custom_fields($userid, $profilefields) {
* current request for all fields so that it can be used quickly.
*
* @param string $shortname Shortname of custom profile field
* @param bool $casesensitive Whether to perform case-sensitive matching of shortname. Note current limitations of custom profile
* fields allow the same shortname to exist differing only by it's case
* @return stdClass|null Object with properties id, shortname, name, visible, datatype, categoryid, etc
*/
function profile_get_custom_field_data_by_shortname(string $shortname): ?stdClass {
function profile_get_custom_field_data_by_shortname(string $shortname, bool $casesensitive = true): ?stdClass {
$cache = \cache::make_from_params(cache_store::MODE_REQUEST, 'core_profile', 'customfields',
[], ['simplekeys' => true, 'simpledata' => true]);
$data = $cache->get($shortname);
@ -878,7 +880,13 @@ function profile_get_custom_field_data_by_shortname(string $shortname): ?stdClas
$data = null;
foreach ($fields as $field) {
$cache->set($field->shortname, $field);
if ($field->shortname === $shortname) {
// Perform comparison according to case sensitivity parameter.
$shortnamematch = $casesensitive
? strcmp($field->shortname, $shortname) === 0
: strcasecmp($field->shortname, $shortname) === 0;
if ($shortnamematch) {
$data = $field;
}
}

View File

@ -283,4 +283,59 @@ class core_user_profilelib_testcase extends advanced_testcase {
$this->assertNull(profile_get_custom_field_data_by_shortname('speciality'));
}
/**
* Data provider for {@see test_profile_get_custom_field_data_by_shortname_case_sensitivity}
*
* @return array[]
*/
public function profile_get_custom_field_data_by_shortname_case_sensitivity_provider(): array {
return [
'Matching case, case-sensitive search' => ['hello', 'hello', true, true],
'Matching case, case-insensitive search' => ['hello', 'hello', false, true],
'Non-matching case, case-sensitive search' => ['hello', 'Hello', true, false],
'Non-matching case, case-insensitive search' => ['hello', 'Hello', false, true],
'Non-matching, case-sensitive search' => ['hello', 'hola', true, false],
'Non-matching, case-insensitive search' => ['hello', 'hola', false, false],
];
}
/**
* Test retrieving custom field by shortname, specifying case-sensitivity when matching
*
* @param string $shortname
* @param string $shortnamesearch
* @param bool $casesensitive
* @param bool $expectmatch
*
* @dataProvider profile_get_custom_field_data_by_shortname_case_sensitivity_provider
*/
public function test_profile_get_custom_field_data_by_shortname_case_sensitivity(
string $shortname,
string $shortnamesearch,
bool $casesensitive,
bool $expectmatch
): void {
global $CFG;
require_once("{$CFG->dirroot}/user/profile/lib.php");
$this->resetAfterTest();
$this->getDataGenerator()->create_custom_profile_field([
'datatype' => 'text',
'shortname' => $shortname,
'name' => 'My field',
]);
$customfield = profile_get_custom_field_data_by_shortname($shortnamesearch, $casesensitive);
if ($expectmatch) {
$this->assertInstanceOf(\stdClass::class, $customfield);
$this->assertEquals('text', $customfield->datatype);
$this->assertEquals($shortname, $customfield->shortname);
$this->assertEquals('My field', $customfield->name);
} else {
$this->assertNull($customfield);
}
}
}

View File

@ -7,6 +7,8 @@ This files describes API changes for code that uses the user API.
* External function core_user_external::update_users() now returns an error code and message to why a user update
action failed.
* New method `core_user\fields::get_sql_fullname` for retrieving user fullname format in SQL statement
* The `profile_get_custom_field_data_by_shortname` method now accepts an optional parameter to determine whether
to use case-sensitive matching of the profile field shortname or not (default true)
=== 3.11 ===