Merge branch 'MDL-71186-master' of https://github.com/clransom/moodle

This commit is contained in:
Andrew Nicols 2021-04-16 07:52:35 +08:00 committed by Víctor Déniz
commit 1528661e68
8 changed files with 269 additions and 40 deletions

View File

@ -81,12 +81,15 @@ switch ($action) {
case 'ajax_getmembersingroup':
$roles = array();
// TODO Does not support custom user profile fields (MDL-70456).
$userfieldsapi = \core_user\fields::for_identity($context, false)->with_userpic();
$userfields = $userfieldsapi->get_sql('u', false, '', '', false)->selects;
$userfieldsapi = \core_user\fields::for_identity($context)->with_userpic();
[
'selects' => $userfieldsselects,
'joins' => $userfieldsjoin,
'params' => $userfieldsparams
] = (array)$userfieldsapi->get_sql('u', true, '', '', false);
$extrafields = $userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]);
if ($groupmemberroles = groups_get_members_by_role($groupids[0], $courseid,
'u.id, ' . $userfields)) {
'u.id, ' . $userfieldsselects, null, '', $userfieldsparams, $userfieldsjoin)) {
$viewfullnames = has_capability('moodle/site:viewfullnames', $context);
@ -205,12 +208,15 @@ if ($groups) {
// Get list of group members to render if there is a single selected group.
$members = array();
if ($singlegroup) {
// TODO Does not support custom user profile fields (MDL-70456).
$userfieldsapi = \core_user\fields::for_identity($context, false)->with_userpic();
$userfields = $userfieldsapi->get_sql('u', false, '', '', false)->selects;
$userfieldsapi = \core_user\fields::for_identity($context)->with_userpic();
[
'selects' => $userfieldsselects,
'joins' => $userfieldsjoin,
'params' => $userfieldsparams
] = (array)$userfieldsapi->get_sql('u', true, '', '', false);
$extrafields = $userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]);
if ($groupmemberroles = groups_get_members_by_role(reset($groupids), $courseid,
'u.id, ' . $userfields)) {
'u.id, ' . $userfieldsselects, null, '', $userfieldsparams, $userfieldsjoin)) {
$viewfullnames = has_capability('moodle/site:viewfullnames', $context);

View File

@ -971,14 +971,15 @@ function groups_unassign_grouping($groupingid, $groupid, $invalidatecache = true
*
* @param int $groupid
* @param int $courseid Course ID (should match the group's course)
* @param string $fields List of fields from user table prefixed with u, default 'u.*'
* @param string $sort SQL ORDER BY clause, default (when null passed) is what comes from users_order_by_sql.
* @param string $fields List of fields from user table (prefixed with u) and joined tables, default 'u.*'
* @param string|null $sort SQL ORDER BY clause, default (when null passed) is what comes from users_order_by_sql.
* @param string $extrawheretest extra SQL conditions ANDed with the existing where clause.
* @param array $whereorsortparams any parameters required by $extrawheretest (named parameters).
* @param array $whereorsortparams any parameters required by $extrawheretest or $joins (named parameters).
* @param string $joins any joins required to get the specified fields.
* @return array Complex array as described above
*/
function groups_get_members_by_role($groupid, $courseid, $fields='u.*',
$sort=null, $extrawheretest='', $whereorsortparams=array()) {
function groups_get_members_by_role(int $groupid, int $courseid, string $fields = 'u.*',
?string $sort = null, string $extrawheretest = '', array $whereorsortparams = [], string $joins = '') {
global $DB;
// Retrieve information about all users and their roles on the course or
@ -1002,6 +1003,7 @@ function groups_get_members_by_role($groupid, $courseid, $fields='u.*',
JOIN {user} u ON u.id = gm.userid
LEFT JOIN {role_assignments} ra ON (ra.userid = u.id AND ra.contextid $relatedctxsql)
LEFT JOIN {role} r ON r.id = ra.roleid
$joins
WHERE gm.groupid=:mgroupid
".$extrawheretest."
ORDER BY r.sortorder, $sort";

View File

@ -110,21 +110,25 @@ if ($groupingid) {
list($sort, $sortparams) = users_order_by_sql('u');
// TODO Does not support custom user profile fields (MDL-70456).
$userfieldsapi = \core_user\fields::for_identity($context, false)->with_userpic();
$userfields = $userfieldsapi->get_sql('u', false, '', '', false)->selects;
$userfieldsapi = \core_user\fields::for_identity($context)->with_userpic();
[
'selects' => $userfieldsselects,
'joins' => $userfieldsjoin,
'params' => $userfieldsparams
] = (array)$userfieldsapi->get_sql('u', true);
$extrafields = $userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]);
$allnames = 'u.id, ' . $userfields;
$allnames = 'u.id ' . $userfieldsselects;
$sql = "SELECT g.id AS groupid, gg.groupingid, u.id AS userid, $allnames, u.idnumber, u.username
FROM {groups} g
LEFT JOIN {groupings_groups} gg ON g.id = gg.groupid
LEFT JOIN {groups_members} gm ON g.id = gm.groupid
LEFT JOIN {user} u ON gm.userid = u.id
$userfieldsjoin
WHERE g.courseid = :courseid $groupwhere $groupingwhere
ORDER BY g.name, $sort";
$rs = $DB->get_recordset_sql($sql, array_merge($params, $sortparams));
$rs = $DB->get_recordset_sql($sql, array_merge($params, $sortparams, $userfieldsparams));
foreach ($rs as $row) {
$user = username_load_fields_from_object((object) [], $row, null,
array_merge(['id' => 'userid', 'username', 'idnumber'], $extrafields));
@ -171,10 +175,11 @@ if ($groupid <= 0 && $groupingid <= 0) {
JOIN {groups} g ON g.id = gm.groupid
WHERE g.courseid = :courseid
) grouped ON grouped.userid = u.id
$userfieldsjoin
WHERE grouped.userid IS NULL";
$params['courseid'] = $courseid;
$nogroupusers = $DB->get_records_sql($sql, $params);
$nogroupusers = $DB->get_records_sql($sql, array_merge($params, $userfieldsparams));
if ($nogroupusers) {
$members[OVERVIEW_GROUPING_NO_GROUP][OVERVIEW_NO_GROUP] = $nogroupusers;

View File

@ -0,0 +1,74 @@
@core @core_group
Feature: Custom profile fields in groups
In order to organize participants into groups
As a teacher
I need to be able to view and search on custom profile fields
Background:
Given the following "custom profile fields" exist:
| datatype | shortname | name | param2 |
| text | species | Species | 255 |
And the following "users" exist:
| username | firstname | lastname | profile_field_species | email |
| user1 | Robin | Hood | fox | email1@example.org |
| user2 | Little | John | bear | email2@example.org |
And the following "courses" exist:
| shortname | fullname |
| C1 | Course 1 |
And the following "course enrolments" exist:
| user | course | role |
| user1 | C1 | manager |
| user2 | C1 | manager |
And the following "groups" exist:
| name | course | idnumber |
| Canines | C1 | G1 |
And the following "group members" exist:
| user | group |
| user1 | G1 |
Given the following config values are set as admin:
| showuseridentity | username,profile_field_species |
@javascript
Scenario: Check the custom profile fields show up and can be searched on
When I am on the "C1" "Course" page logged in as "admin"
And I navigate to "Users > Groups" in current page administration
# Check the Overview page.
And I follow "Overview"
And "Robin Hood (user1, fox)" "text" should exist in the "Canines" "table_row"
And "Little John (user2, bear)" "text" should exist in the "No group" "table_row"
# Check the groups page.
And I follow "Groups"
And I set the field "groups" to "Canines"
And I should see "Robin Hood (user1, fox)"
And I should not see "Little John (user2, bear)"
# Check the members page.
And I press "Add/remove users"
And I should see "Robin Hood (user1, fox)"
And I should see "Little John (user2, bear)"
And I set the field "addselect" to "Little John (user2, bear)"
And I press "Add"
And I should see "Robin Hood (user1, fox)"
And I should see "Little John (user2, bear)"
And I set the field "Search" in the "#existingcell" "css_element" to "fox"
And I wait "1" seconds
And I should see "Robin Hood (user1, fox)"
And I should not see "Little John (user2, bear)"
And I set the field "Search" in the "#existingcell" "css_element" to ""
And I wait "1" seconds
And I set the field "removeselect" to "Little John (user2, bear)"
And I press "Remove"
And I set the field "removeselect" to "Robin Hood (user1, fox)"
And I press "Remove"
And I should see "Robin Hood (user1, fox)"
And I should see "Little John (user2, bear)"
And I set the field "Search" in the "#potentialcell" "css_element" to "bear"
And I wait "1" seconds
And I should see "Little John (user2, bear)"
And I should not see "Robin Hood (user1, fox)"

View File

@ -764,4 +764,66 @@ class core_group_lib_testcase extends advanced_testcase {
$this->assertEquals(3, $DB->count_records('message_conversation_members', ['conversationid' => $conversation->id]));
}
public function test_groups_get_members_by_role(): void {
$this->resetAfterTest();
$this->setAdminUser();
$course1 = $this->getDataGenerator()->create_course();
$user1 = $this->getDataGenerator()->create_user(['username' => 'user1', 'idnumber' => 1]);
$user2 = $this->getDataGenerator()->create_user(['username' => 'user2', 'idnumber' => 2]);
$user3 = $this->getDataGenerator()->create_user(['username' => 'user3', 'idnumber' => 3]);
$this->getDataGenerator()->enrol_user($user1->id, $course1->id, 0);
$this->getDataGenerator()->enrol_user($user2->id, $course1->id, 1);
$this->getDataGenerator()->enrol_user($user3->id, $course1->id, 1);
$group1 = $this->getDataGenerator()->create_group(['courseid' => $course1->id]);
$this->getDataGenerator()->create_group_member(['groupid' => $group1->id, 'userid' => $user1->id]);
$this->getDataGenerator()->create_group_member(['groupid' => $group1->id, 'userid' => $user2->id]);
$this->getDataGenerator()->create_group_member(['groupid' => $group1->id, 'userid' => $user3->id]);
// Test basic usage.
$result = groups_get_members_by_role($group1->id, $course1->id);
$this->assertEquals(1, count($result[0]->users));
$this->assertEquals(2, count($result[1]->users));
$this->assertEquals($user1->firstname, reset($result[0]->users)->firstname);
$this->assertEquals($user1->username, reset($result[0]->users)->username);
// Test with specified fields.
$result = groups_get_members_by_role($group1->id, $course1->id, 'u.firstname, u.lastname');
$this->assertEquals(1, count($result[0]->users));
$this->assertEquals($user1->firstname, reset($result[0]->users)->firstname);
$this->assertEquals($user1->lastname, reset($result[0]->users)->lastname);
$this->assertEquals(false, isset(reset($result[0]->users)->username));
// Test with sorting.
$result = groups_get_members_by_role($group1->id, $course1->id, 'u.username', 'u.username DESC');
$this->assertEquals(1, count($result[0]->users));
$this->assertEquals($user3->username, reset($result[1]->users)->username);
$result = groups_get_members_by_role($group1->id, $course1->id, 'u.username', 'u.username ASC');
$this->assertEquals(1, count($result[0]->users));
$this->assertEquals($user2->username, reset($result[1]->users)->username);
// Test with extra WHERE.
$result = groups_get_members_by_role(
$group1->id,
$course1->id,
'u.username',
null,
'u.idnumber > :number',
['number' => 2]);
$this->assertEquals(1, count($result));
$this->assertEquals(1, count($result[1]->users));
$this->assertEquals($user3->username, reset($result[1]->users)->username);
// Test with join.
set_user_preference('reptile', 'snake', $user1);
$result = groups_get_members_by_role($group1->id, $course1->id, 'u.username, up.value', null, 'up.name = :prefname',
['prefname' => 'reptile'], 'JOIN {user_preferences} up ON up.userid = u.id');
$this->assertEquals('snake', reset($result[0]->users)->value);
}
}

View File

@ -218,7 +218,8 @@ function search_users($courseid, $groupid, $searchtext, $sort='', array $excepti
* built. May be ''.
* @param bool $searchanywhere If true (default), searches in the middle of
* names, otherwise only searches at start
* @param array $extrafields Array of extra user fields to include in search
* @param array $extrafields Array of extra user fields to include in search, must be prefixed with table alias if they are not in
* the user table.
* @param array $exclude Array of user ids to exclude (empty = don't exclude)
* @param array $includeonly If specified, only returns users that have ids
* incldued in this array (empty = don't restrict)
@ -226,8 +227,8 @@ function search_users($courseid, $groupid, $searchtext, $sort='', array $excepti
* where clause the query, and an associative array containing any required
* parameters (using named placeholders).
*/
function users_search_sql($search, $u = 'u', $searchanywhere = true, array $extrafields = array(),
array $exclude = null, array $includeonly = null) {
function users_search_sql(string $search, string $u = 'u', bool $searchanywhere = true, array $extrafields = [],
array $exclude = null, array $includeonly = null): array {
global $DB, $CFG;
$params = array();
$tests = array();
@ -243,7 +244,8 @@ function users_search_sql($search, $u = 'u', $searchanywhere = true, array $extr
$conditions[] = $u . 'lastname'
);
foreach ($extrafields as $field) {
$conditions[] = $u . $field;
// Add the table alias for the user table if the field doesn't already have an alias.
$conditions[] = strpos($field, '.') !== false ? $field : $u . $field;
}
if ($searchanywhere) {
$searchparam = '%' . $search . '%';
@ -305,7 +307,7 @@ function users_search_sql($search, $u = 'u', $searchanywhere = true, array $extr
* - firstname
* - lastname
* - $DB->sql_fullname
* - those returned by \core_user\fields::get_identity_fields
* - those returned by \core_user\fields::get_identity_fields or those included in $customfieldmappings
*
* If named parameters are used (which is the default, and highly recommended),
* then the parameter names are like :usersortexactN, where N is an int.
@ -334,13 +336,15 @@ function users_search_sql($search, $u = 'u', $searchanywhere = true, array $extr
* @param string $usertablealias (optional) any table prefix for the {users} table. E.g. 'u'.
* @param string $search (optional) a current search string. If given,
* any exact matches to this string will be sorted first.
* @param context $context the context we are in. Used by \core_user\fields::get_identity_fields.
* @param context|null $context the context we are in. Used by \core_user\fields::get_identity_fields.
* Defaults to $PAGE->context.
* @param array $customfieldmappings associative array of mappings for custom fields returned by \core_user\fields::get_sql.
* @return array with two elements:
* string SQL fragment to use in the ORDER BY clause. For example, "firstname, lastname".
* array of parameters used in the SQL fragment.
*/
function users_order_by_sql($usertablealias = '', $search = null, context $context = null) {
function users_order_by_sql(string $usertablealias = '', string $search = null, context $context = null,
array $customfieldmappings = []) {
global $DB, $PAGE;
if ($usertablealias) {
@ -368,10 +372,17 @@ function users_order_by_sql($usertablealias = '', $search = null, context $conte
$params[$paramkey] = $search;
$paramkey++;
// TODO Does not support custom user profile fields (MDL-70456).
$fieldstocheck = array_merge(array('firstname', 'lastname'), \core_user\fields::get_identity_fields($context, false));
if ($customfieldmappings) {
$fieldstocheck = array_merge([$tableprefix . 'firstname', $tableprefix . 'lastname'], array_values($customfieldmappings));
} else {
$fieldstocheck = array_merge(['firstname', 'lastname'], \core_user\fields::get_identity_fields($context, false));
$fieldstocheck = array_map(function($field) use ($tableprefix) {
return $tableprefix . $field;
}, $fieldstocheck);
}
foreach ($fieldstocheck as $key => $field) {
$exactconditions[] = 'LOWER(' . $tableprefix . $field . ') = LOWER(:' . $paramkey . ')';
$exactconditions[] = 'LOWER(' . $field . ') = LOWER(:' . $paramkey . ')';
$params[$paramkey] = $search;
$paramkey++;
}

View File

@ -153,6 +153,21 @@ class core_datalib_testcase extends advanced_testcase {
foreach ($results as $record) {
$this->assertSame('frog', $record->value);
}
// Join with another table and include other table fields in search.
set_user_preference('reptile', 'snake', $user1);
set_user_preference('reptile', 'lizard', $user2);
list($sql, $params) = users_search_sql('snake', 'qq', true, ['up.value']);
$results = $DB->get_records_sql("
SELECT up.id, up.value
FROM {user} qq
JOIN {user_preferences} up ON up.userid = qq.id
WHERE up.name = :prefname
AND $sql", array_merge(array('prefname' => 'reptile'), $params));
$this->assertEquals(1, count($results));
foreach ($results as $record) {
$this->assertSame('snake', $record->value);
}
}
public function test_users_order_by_sql_simple() {
@ -202,6 +217,25 @@ class core_datalib_testcase extends advanced_testcase {
'usersortexact3' => 'search', 'usersortexact4' => 'search', 'usersortexact5' => 'search'), $params);
}
public function test_users_order_by_sql_search_with_custom_fields(): void {
global $CFG, $DB;
$this->resetAfterTest();
$CFG->showuseridentity = 'email,idnumber';
$this->setAdminUser();
list($sort, $params) =
users_order_by_sql('u', 'search', context_system::instance(), ['profile_field_customfield' => 'x.customfield']);
$this->assert_same_sql('CASE WHEN
' . $DB->sql_fullname('u.firstname', 'u.lastname') . ' = :usersortexact1 OR
LOWER(u.firstname) = LOWER(:usersortexact2) OR
LOWER(u.lastname) = LOWER(:usersortexact3) OR
LOWER(x.customfield) = LOWER(:usersortexact4)
THEN 0 ELSE 1 END, u.lastname, u.firstname, u.id', $sort);
$this->assertEquals(array('usersortexact1' => 'search', 'usersortexact2' => 'search',
'usersortexact3' => 'search', 'usersortexact4' => 'search'), $params);
}
public function test_get_admin() {
global $CFG, $DB;
$this->resetAfterTest();

View File

@ -85,6 +85,17 @@ abstract class user_selector_base {
/** @var boolean Whether to override fullname() */
public $viewfullnames = false;
/** @var boolean Whether to include custom user profile fields */
protected $includecustomfields = false;
/** @var string User fields selects for custom fields. */
protected $userfieldsselects = '';
/** @var string User fields join for custom fields. */
protected $userfieldsjoin = '';
/** @var array User fields params for custom fields. */
protected $userfieldsparams = [];
/** @var array User fields mappings for custom fields. */
protected $userfieldsmappings = [];
/**
* Constructor. Each subclass must have a constructor with this signature.
*
@ -113,9 +124,25 @@ abstract class user_selector_base {
unset($options['extrafields']);
}
if (isset($options['includecustomfields'])) {
$this->includecustomfields = $options['includecustomfields'];
} else {
$this->includecustomfields = false;
}
// Populate the list of additional user identifiers to display.
// TODO Does not support custom user profile fields (MDL-70456).
$this->extrafields = \core_user\fields::get_identity_fields($this->accesscontext, false);
if ($this->includecustomfields) {
$userfieldsapi = \core_user\fields::for_identity($this->accesscontext)->with_name();
$this->extrafields = $userfieldsapi->get_required_fields([\core_user\fields::PURPOSE_IDENTITY]);
[
'selects' => $this->userfieldsselects,
'joins' => $this->userfieldsjoin,
'params' => $this->userfieldsparams,
'mappings' => $this->userfieldsmappings
] = (array) $userfieldsapi->get_sql('u', true, '', '', false);
} else {
$this->extrafields = \core_user\fields::get_identity_fields($this->accesscontext, false);
}
if (isset($options['exclude']) && is_array($options['exclude'])) {
$this->exclude = $options['exclude'];
@ -434,8 +461,14 @@ abstract class user_selector_base {
* @param string $u the table alias for the user table in the query being
* built. May be ''.
* @return string fragment of SQL to go in the select list of the query.
* @throws coding_exception if used when includecustomfields is true
*/
protected function required_fields_sql($u) {
protected function required_fields_sql(string $u) {
if ($this->includecustomfields) {
throw new coding_exception('required_fields_sql() is not needed when includecustomfields is true, '.
'use $userfieldsselects instead.');
}
// Raw list of fields.
$fields = array('id');
// Add additional name fields.
@ -460,8 +493,8 @@ abstract class user_selector_base {
* where clause the query, and an array containing any required parameters.
* this uses ? style placeholders.
*/
protected function search_sql($search, $u) {
return users_search_sql($search, $u, $this->searchanywhere, $this->extrafields,
protected function search_sql(string $search, string $u): array {
return users_search_sql($search, $u, $this->searchanywhere, array_values($this->userfieldsmappings),
$this->exclude, $this->validatinguserids);
}
@ -691,6 +724,7 @@ abstract class groups_user_selector_base extends user_selector_base {
public function __construct($name, $options) {
global $CFG;
$options['accesscontext'] = context_course::instance($options['courseid']);
$options['includecustomfields'] = true;
parent::__construct($name, $options);
$this->groupid = $options['groupid'];
$this->courseid = $options['courseid'];
@ -761,11 +795,11 @@ class group_members_selector extends groups_user_selector_base {
public function find_users($search) {
list($wherecondition, $params) = $this->search_sql($search, 'u');
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext, $this->userfieldsmappings);
$roles = groups_get_members_by_role($this->groupid, $this->courseid,
$this->required_fields_sql('u') . ', gm.component',
$sort, $wherecondition, array_merge($params, $sortparams));
$this->userfieldsselects . ', gm.component',
$sort, $wherecondition, array_merge($params, $sortparams, $this->userfieldsparams), $this->userfieldsjoin);
return $this->convert_array_format($roles, $search);
}
@ -902,7 +936,7 @@ class group_non_members_selector extends groups_user_selector_base {
$wheres .= ' AND ' . $searchcondition;
$fields = "SELECT r.id AS roleid, u.id AS userid,
" . $this->required_fields_sql('u') . ",
" . $this->userfieldsselects . ",
(SELECT count(igm.groupid)
FROM {groups_members} igm
JOIN {groups} ig ON igm.groupid = ig.id
@ -912,12 +946,13 @@ class group_non_members_selector extends groups_user_selector_base {
LEFT JOIN {role_assignments} ra ON (ra.userid = u.id AND ra.contextid $relatedctxsql AND ra.roleid $roleids)
LEFT JOIN {role} r ON r.id = ra.roleid
LEFT JOIN {groups_members} gm ON (gm.userid = u.id AND gm.groupid = :groupid)
$this->userfieldsjoin
WHERE $wheres";
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext, $this->userfieldsmappings);
$orderby = ' ORDER BY ' . $sort;
$params = array_merge($searchparams, $roleparams, $relatedctxparams, $enrolledjoin->params);
$params = array_merge($searchparams, $roleparams, $relatedctxparams, $enrolledjoin->params, $this->userfieldsparams);
$params['courseid'] = $this->courseid;
$params['groupid'] = $this->groupid;