Commit dbc09f74 authored by sam marshall's avatar sam marshall
Browse files

MDL-45242 Admin: User list supports custom profile fields

parent 558cc1b8
......@@ -5,12 +5,16 @@ Feature: An administrator can filter user accounts by role, cohort and other pro
I need to filter the users account list using different filter
Background:
Given the following "users" exist:
| username | firstname | lastname | email | auth | confirmed | lastip | institution | department |
| user1 | User | One | one@example.com | manual | 0 | 127.0.1.1 | moodle | red |
| user2 | User | Two | two@example.com | ldap | 1 | 0.0.0.0 | moodle | blue |
| user3 | User | Three | three@example.com | manual | 1 | 0.0.0.0 | | |
| user4 | User | Four | four@example.com | ldap | 0 | 127.0.1.2 | | |
Given the following "custom profile fields" exist:
| datatype | shortname | name |
| text | frog | Favourite frog |
| text | undead | Type of undead |
And the following "users" exist:
| username | firstname | lastname | email | auth | confirmed | lastip | institution | department | profile_field_frog | profile_field_undead |
| user1 | User | One | one@example.com | manual | 0 | 127.0.1.1 | moodle | red | Kermit | |
| user2 | User | Two | two@example.com | ldap | 1 | 0.0.0.0 | moodle | blue | Mr Toad | Zombie |
| user3 | User | Three | three@example.com | manual | 1 | 0.0.0.0 | | | | |
| user4 | User | Four | four@example.com | ldap | 0 | 127.0.1.2 | | | | |
And the following "cohorts" exist:
| name | idnumber |
| Cohort 1 | CH1 |
......@@ -116,3 +120,20 @@ Feature: An administrator can filter user accounts by role, cohort and other pro
And I press "Add filter"
And I should see "User One"
And I should not see "User Two"
Scenario: Filter users by custom profile field (specific or any)
When I set the field "id_profile_fld" to "Favourite frog"
And I set the field "id_profile" to "Kermit"
And I press "Add filter"
Then I should see "User One"
And I should not see "User Two"
And I should not see "User Three"
And I should not see "User Four"
And I press "Remove all filters"
And I set the field "id_profile_fld" to "any field"
And I set the field "id_profile" to "Zombie"
And I press "Add filter"
And I should see "User Two"
And I should not see "User One"
And I should not see "User Three"
And I should not see "User Four"
......@@ -183,12 +183,10 @@
// These columns are always shown in the users list.
$requiredcolumns = array('city', 'country', 'lastaccess');
// Extra columns containing the extra user fields, excluding the required columns (city and country, to be specific).
// TODO Does not support custom user profile fields (MDL-70456).
$userfields = \core\user_fields::for_identity($context, false)->excluding(...$requiredcolumns);
$userfields = \core\user_fields::for_identity($context, true)->with_name()->excluding(...$requiredcolumns);
$extracolumns = $userfields->get_required_fields();
// Get all user name fields as an array, but with firstname and lastname first.
$allusernamefields = \core\user_fields::get_name_fields(true);
$columns = array_merge($allusernamefields, $extracolumns, $requiredcolumns);
// Get all user name fields as an array.
$columns = array_merge($extracolumns, $requiredcolumns);
foreach ($columns as $column) {
$string[$column] = \core\user_fields::get_display_name($column);
......@@ -228,7 +226,7 @@
}
// Order in string will ensure that the name columns are in the correct order.
$usernames = order_in_string($allusernamefields, $fullnamesetting);
$usernames = order_in_string($extracolumns, $fullnamesetting);
$fullnamedisplay = array();
foreach ($usernames as $name) {
// Use the link from $$column for sorting on the user's name.
......
......@@ -480,7 +480,7 @@ function get_users_listing($sort='lastaccess', $dir='ASC', $page=0, $recordsperp
$fullname = $DB->sql_fullname();
$select = "deleted <> 1 AND id <> :guestid";
$select = "deleted <> 1 AND u.id <> :guestid";
$params = array('guestid' => $CFG->siteguest);
if (!empty($search)) {
......@@ -503,6 +503,10 @@ function get_users_listing($sort='lastaccess', $dir='ASC', $page=0, $recordsperp
}
if ($extraselect) {
// The extra WHERE clause may refer to the 'id' column which can now be ambiguous because we
// changed the query to include joins, so replace any 'id' that is on its own (no alias)
// with 'u.id'.
$extraselect = preg_replace('~([ =]|^)id([ =]|$)~', '$1u.id$2', $extraselect);
$select .= " AND $extraselect";
$params = $params + (array)$extraparams;
}
......@@ -512,18 +516,21 @@ function get_users_listing($sort='lastaccess', $dir='ASC', $page=0, $recordsperp
}
// If a context is specified, get extra user fields that the current user
// is supposed to see.
// TODO Does not support custom user profile fields (MDL-70456).
$userfieldsapi = \core\user_fields::for_identity($extracontext, false)->with_name()
->excluding('id', 'username', 'email', 'firstname', 'lastname', 'city', 'country',
'lastaccess', 'confirmed', 'mnethostid');
$extrafields = $userfields->get_sql()->selects;
// is supposed to see, otherwise just get the name fields.
$userfields = \core\user_fields::for_name();
if ($extracontext) {
$userfields->with_identity($extracontext, true);
}
$userfields->excluding('id', 'username', 'email', 'city', 'country', 'lastaccess', 'confirmed', 'mnethostid');
['selects' => $selects, 'joins' => $joins, 'params' => $joinparams] =
(array)$userfields->get_sql('u', true);
// warning: will return UNCONFIRMED USERS
return $DB->get_records_sql("SELECT id, username, email, city, country, lastaccess, confirmed, mnethostid, suspended $extrafields
FROM {user}
return $DB->get_records_sql("SELECT u.id, username, email, city, country, lastaccess, confirmed, mnethostid, suspended $selects
FROM {user} u
$joins
WHERE $select
$sort", $params, $page, $recordsperpage);
$sort", array_merge($params, $joinparams), $page, $recordsperpage);
}
......
......@@ -746,4 +746,100 @@ class core_datalib_testcase extends advanced_testcase {
"Please also make sure \$CFG->maxcoursesincategory * MAX_COURSE_CATEGORIES less than max integer. " .
"See tracker issues: MDL-25669 and MDL-69573");
}
/**
* Tests the get_users_listing function.
*/
public function test_get_users_listing(): void {
global $DB;
$this->resetAfterTest();
$generator = $this->getDataGenerator();
// Set up profile field.
$generator->create_custom_profile_field(['datatype' => 'text',
'shortname' => 'specialid', 'name' => 'Special user id']);
// Set up the show user identity option.
set_config('showuseridentity', 'department,profile_field_specialid');
// Get all the existing user ids (we're going to remove these from test results).
$existingids = array_fill_keys($DB->get_fieldset_select('user', 'id', '1 = 1'), true);
// Create some test user accounts.
$userids = [];
foreach (['a', 'b', 'c', 'd'] as $key) {
$record = [
'username' => 'user_' . $key,
'firstname' => $key . '_first',
'lastname' => 'last_' . $key,
'department' => 'department_' . $key,
'profile_field_specialid' => 'special_' . $key,
'lastaccess' => ord($key)
];
$user = $generator->create_user($record);
$userids[] = $user->id;
}
// Check default result with no parameters.
$results = get_users_listing();
$results = array_diff_key($results, $existingids);
// It should return all the results in order.
$this->assertEquals($userids, array_keys($results));
// Results should have some general fields and name fields, check some samples.
$this->assertEquals('user_a', $results[$userids[0]]->username);
$this->assertEquals('user_a@example.com', $results[$userids[0]]->email);
$this->assertEquals(1, $results[$userids[0]]->confirmed);
$this->assertEquals('a_first', $results[$userids[0]]->firstname);
$this->assertObjectHasAttribute('firstnamephonetic', $results[$userids[0]]);
// Should not have the custom field or department because no context specified.
$this->assertObjectNotHasAttribute('department', $results[$userids[0]]);
$this->assertObjectNotHasAttribute('profile_field_specialid', $results[$userids[0]]);
// Check sorting.
$results = get_users_listing('username', 'DESC');
$results = array_diff_key($results, $existingids);
$this->assertEquals([$userids[3], $userids[2], $userids[1], $userids[0]], array_keys($results));
// Add the options to showuseridentity and check it returns those fields but only if you
// specify a context AND have permissions.
$results = get_users_listing('lastaccess', 'asc', 0, 0, '', '', '', '', null,
\context_system::instance());
$this->assertObjectNotHasAttribute('department', $results[$userids[0]]);
$this->assertObjectNotHasAttribute('profile_field_specialid', $results[$userids[0]]);
$this->setAdminUser();
$results = get_users_listing('lastaccess', 'asc', 0, 0, '', '', '', '', null,
\context_system::instance());
$this->assertEquals('department_a', $results[$userids[0]]->department);
$this->assertEquals('special_a', $results[$userids[0]]->profile_field_specialid);
// Check search (full name, email, username).
$results = get_users_listing('lastaccess', 'asc', 0, 0, 'b_first last_b');
$this->assertEquals([$userids[1]], array_keys($results));
$results = get_users_listing('lastaccess', 'asc', 0, 0, 'c@example');
$this->assertEquals([$userids[2]], array_keys($results));
$results = get_users_listing('lastaccess', 'asc', 0, 0, 'user_d');
$this->assertEquals([$userids[3]], array_keys($results));
// Check first and last initial restriction (all the test ones have same last initial).
$results = get_users_listing('lastaccess', 'asc', 0, 0, '', 'C');
$this->assertEquals([$userids[2]], array_keys($results));
$results = get_users_listing('lastaccess', 'asc', 0, 0, '', '', 'L');
$results = array_diff_key($results, $existingids);
$this->assertEquals($userids, array_keys($results));
// Check the extra where clause, either with the 'u.' prefix or not.
$results = get_users_listing('lastaccess', 'asc', 0, 0, '', '', '', 'id IN (:x,:y)',
['x' => $userids[1], 'y' => $userids[3]]);
$results = array_diff_key($results, $existingids);
$this->assertEquals([$userids[1], $userids[3]], array_keys($results));
$results = get_users_listing('lastaccess', 'asc', 0, 0, '', '', '', 'u.id IN (:x,:y)',
['x' => $userids[1], 'y' => $userids[3]]);
$results = array_diff_key($results, $existingids);
$this->assertEquals([$userids[1], $userids[3]], array_keys($results));
}
}
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment