From e059c033d9fa317753b28f88b2290c791eb663c3 Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Fri, 5 Oct 2012 11:59:00 +0100 Subject: [PATCH] MDL-35802 enrol other users: should use Show user identity setting. As part of fixing this, I refactored some common code out of get_potential_users and search_other_users. Previously, only one of those bits of code had been updated. --- enrol/ajax.php | 13 ++- enrol/locallib.php | 109 ++++++++++-------- enrol/manual/ajax.php | 6 +- .../assets/skins/sam/quickenrolment.css | 2 +- .../assets/skins/sam/otherusersmanager.css | 2 +- .../otherusersmanager/otherusersmanager.js | 8 +- 6 files changed, 84 insertions(+), 56 deletions(-) diff --git a/enrol/ajax.php b/enrol/ajax.php index ede3c5a377c..aa8da35bd8f 100644 --- a/enrol/ajax.php +++ b/enrol/ajax.php @@ -59,6 +59,8 @@ $outcome->success = true; $outcome->response = new stdClass; $outcome->error = ''; +$searchanywhere = get_user_preferences('userselector_searchanywhere', false); + switch ($action) { case 'unenrol': $ue = $DB->get_record('user_enrolments', array('id'=>required_param('ue', PARAM_INT)), '*', MUST_EXIST); @@ -90,13 +92,20 @@ switch ($action) { $outcome->response = array_reverse($manager->get_assignable_roles($otheruserroles), true); break; case 'searchotherusers': - $search = optional_param('search', '', PARAM_RAW); + $search = optional_param('search', '', PARAM_RAW); $page = optional_param('page', 0, PARAM_INT); - $outcome->response = $manager->search_other_users($search, false, $page); + $outcome->response = $manager->search_other_users($search, $searchanywhere, $page); + $extrafields = get_extra_user_fields($context); foreach ($outcome->response['users'] as &$user) { $user->userId = $user->id; $user->picture = $OUTPUT->user_picture($user); $user->fullname = fullname($user); + $fieldvalues = array(); + foreach ($extrafields as $field) { + $fieldvalues[] = s($user->{$field}); + unset($user->{$field}); + } + $user->extrafields = implode(', ', $fieldvalues); unset($user->id); } $outcome->success = true; diff --git a/enrol/locallib.php b/enrol/locallib.php index 66a2381ef9c..2ee9a7bb63c 100644 --- a/enrol/locallib.php +++ b/enrol/locallib.php @@ -265,17 +265,16 @@ class course_enrolment_manager { } /** - * Gets an array of the users that can be enrolled in this course. + * Helper method used by {@link get_potential_users()} and {@link search_other_users()}. * - * @global moodle_database $DB - * @param int $enrolid - * @param string $search - * @param bool $searchanywhere - * @param int $page Defaults to 0 - * @param int $perpage Defaults to 25 - * @return array Array(totalusers => int, users => array) + * @param string $search the search term, if any. + * @param bool $searchanywhere Can the search term be anywhere, or must it be at the start. + * @return array with three elements: + * string list of fields to SELECT, + * string contents of SQL WHERE clause, + * array query params. Note that the SQL snippets use named parameters. */ - public function get_potential_users($enrolid, $search='', $searchanywhere=false, $page=0, $perpage=25) { + protected function get_basic_search_conditions($search, $searchanywhere) { global $DB, $CFG; // Add some additional sensible conditions @@ -283,15 +282,17 @@ class course_enrolment_manager { $params = array('guestid' => $CFG->siteguest); if (!empty($search)) { $conditions = get_extra_user_fields($this->get_context()); - $conditions[] = $DB->sql_concat('u.firstname', "' '", 'u.lastname'); + $conditions[] = 'u.firstname'; + $conditions[] = 'u.lastname'; + $conditions[] = $DB->sql_fullname('u.firstname', 'u.lastname'); if ($searchanywhere) { $searchparam = '%' . $search . '%'; } else { $searchparam = $search . '%'; } $i = 0; - foreach ($conditions as $key=>$condition) { - $conditions[$key] = $DB->sql_like($condition,":con{$i}00", false); + foreach ($conditions as $key => $condition) { + $conditions[$key] = $DB->sql_like($condition, ":con{$i}00", false); $params["con{$i}00"] = $searchparam; $i++; } @@ -304,6 +305,52 @@ class course_enrolment_manager { $extrafields[] = 'lastaccess'; $ufields = user_picture::fields('u', $extrafields); + return array($ufields, $params, $wherecondition); + } + + /** + * Helper method used by {@link get_potential_users()} and {@link search_other_users()}. + * + * @param string $search the search string, if any. + * @param string $fields the first bit of the SQL when returning some users. + * @param string $countfields fhe first bit of the SQL when counting the users. + * @param string $sql the bulk of the SQL statement. + * @param array $params query parameters. + * @param int $page which page number of the results to show. + * @param int $perpage number of users per page. + * @return array with two elememts: + * int total number of users matching the search. + * array of user objects returned by the query. + */ + protected function execute_search_queries($search, $fields, $countfields, $sql, array $params, $page, $perpage) { + global $DB, $CFG; + + list($sort, $sortparams) = users_order_by_sql('u', $search, $this->get_context()); + $order = ' ORDER BY ' . $sort; + + $totalusers = $DB->count_records_sql($countfields . $sql, $params); + $availableusers = $DB->get_records_sql($fields . $sql . $order, + array_merge($params, $sortparams), $page*$perpage, $perpage); + + return array('totalusers' => $totalusers, 'users' => $availableusers); + } + + /** + * Gets an array of the users that can be enrolled in this course. + * + * @global moodle_database $DB + * @param int $enrolid + * @param string $search + * @param bool $searchanywhere + * @param int $page Defaults to 0 + * @param int $perpage Defaults to 25 + * @return array Array(totalusers => int, users => array) + */ + public function get_potential_users($enrolid, $search='', $searchanywhere=false, $page=0, $perpage=25) { + global $DB; + + list($ufields, $params, $wherecondition) = $this->get_basic_search_conditions($search, $searchanywhere); + $fields = 'SELECT '.$ufields; $countfields = 'SELECT COUNT(1)'; $sql = " FROM {user} u @@ -312,13 +359,7 @@ class course_enrolment_manager { AND ue.id IS NULL"; $params['enrolid'] = $enrolid; - list($sort, $sortparams) = users_order_by_sql('u', $search, $this->get_context()); - $order = ' ORDER BY ' . $sort; - $params += $sortparams; - - $totalusers = $DB->count_records_sql($countfields . $sql, $params); - $availableusers = $DB->get_records_sql($fields . $sql . $order, $params, $page*$perpage, $perpage); - return array('totalusers' => $totalusers, 'users' => $availableusers); + return $this->execute_search_queries($search, $fields, $countfields, $sql, $params, $page, $perpage); } /** @@ -334,27 +375,9 @@ class course_enrolment_manager { public function search_other_users($search='', $searchanywhere=false, $page=0, $perpage=25) { global $DB, $CFG; - // Add some additional sensible conditions - $tests = array("u.id <> :guestid", 'u.deleted = 0', 'u.confirmed = 1'); - $params = array('guestid'=>$CFG->siteguest); - if (!empty($search)) { - $conditions = array('u.firstname', 'u.lastname'); - if ($searchanywhere) { - $searchparam = '%' . $search . '%'; - } else { - $searchparam = $search . '%'; - } - $i = 0; - foreach ($conditions as $key=>$condition) { - $conditions[$key] = $DB->sql_like($condition, ":con{$i}00", false); - $params["con{$i}00"] = $searchparam; - $i++; - } - $tests[] = '(' . implode(' OR ', $conditions) . ')'; - } - $wherecondition = implode(' AND ', $tests); + list($ufields, $params, $wherecondition) = $this->get_basic_search_conditions($search, $searchanywhere); - $fields = 'SELECT '.user_picture::fields('u', array('username','lastaccess')); + $fields = 'SELECT ' . $ufields; $countfields = 'SELECT COUNT(u.id)'; $sql = " FROM {user} u LEFT JOIN {role_assignments} ra ON (ra.userid = u.id AND ra.contextid = :contextid) @@ -362,13 +385,7 @@ class course_enrolment_manager { AND ra.id IS NULL"; $params['contextid'] = $this->context->id; - list($sort, $sortparams) = users_order_by_sql('u', $search, $this->context); - $order = ' ORDER BY ' . $sort; - - $totalusers = $DB->count_records_sql($countfields . $sql, $params); - $availableusers = $DB->get_records_sql($fields . $sql . $order, - array_merge($params, $sortparams), $page*$perpage, $perpage); - return array('totalusers'=>$totalusers, 'users'=>$availableusers); + return $this->execute_search_queries($search, $fields, $countfields, $sql, $params, $page, $perpage); } /** diff --git a/enrol/manual/ajax.php b/enrol/manual/ajax.php index faf8ac02025..6d15b3abeae 100644 --- a/enrol/manual/ajax.php +++ b/enrol/manual/ajax.php @@ -56,6 +56,8 @@ $outcome->success = true; $outcome->response = new stdClass; $outcome->error = ''; +$searchanywhere = get_user_preferences('userselector_searchanywhere', false); + switch ($action) { case 'getassignable': $otheruserroles = optional_param('otherusers', false, PARAM_BOOL); @@ -63,9 +65,9 @@ switch ($action) { break; case 'searchusers': $enrolid = required_param('enrolid', PARAM_INT); - $search = optional_param('search', '', PARAM_RAW); + $search = optional_param('search', '', PARAM_RAW); $page = optional_param('page', 0, PARAM_INT); - $outcome->response = $manager->get_potential_users($enrolid, $search, true, $page); + $outcome->response = $manager->get_potential_users($enrolid, $search, $searchanywhere, $page); $extrafields = get_extra_user_fields($context); foreach ($outcome->response['users'] as &$user) { $user->picture = $OUTPUT->user_picture($user); diff --git a/enrol/manual/yui/quickenrolment/assets/skins/sam/quickenrolment.css b/enrol/manual/yui/quickenrolment/assets/skins/sam/quickenrolment.css index f9c1ac46ea2..ff3543f69ff 100644 --- a/enrol/manual/yui/quickenrolment/assets/skins/sam/quickenrolment.css +++ b/enrol/manual/yui/quickenrolment/assets/skins/sam/quickenrolment.css @@ -15,7 +15,7 @@ Structure of the user enroller panel .picture .details .fullname - .email + .extrafields .options .enrol .uep-more-results diff --git a/enrol/yui/otherusersmanager/assets/skins/sam/otherusersmanager.css b/enrol/yui/otherusersmanager/assets/skins/sam/otherusersmanager.css index b19cb3efed0..db39b132ca1 100644 --- a/enrol/yui/otherusersmanager/assets/skins/sam/otherusersmanager.css +++ b/enrol/yui/otherusersmanager/assets/skins/sam/otherusersmanager.css @@ -16,7 +16,7 @@ Structure of the other user role assignment panel .oump-user-picture .oump-user-specifics .oump-user-fullname - .oump-user-email + .oump-user-extrafields .oump-role-options .label .oump-assignable-role diff --git a/enrol/yui/otherusersmanager/otherusersmanager.js b/enrol/yui/otherusersmanager/otherusersmanager.js index 32b1ad87476..1df94ce6042 100644 --- a/enrol/yui/otherusersmanager/otherusersmanager.js +++ b/enrol/yui/otherusersmanager/otherusersmanager.js @@ -11,7 +11,7 @@ YUI.add('moodle-enrol-otherusersmanager', function(Y) { USERCOUNT = 'userCount', PICTURE = 'picture', FULLNAME = 'fullname', - EMAIL = 'email', + EXTRAFIELDS = 'extrafields', ASSIGNABLEROLES = 'assignableRoles', USERS = 'users', URL = 'url', @@ -36,7 +36,7 @@ YUI.add('moodle-enrol-otherusersmanager', function(Y) { PICTURE : 'oump-user-picture', DETAILS : 'oump-user-specifics', FULLNAME : 'oump-user-fullname', - EMAIL : 'oump-user-email', + EXTRAFIELDS : 'oump-user-extrafields', OPTIONS : 'oump-role-options', ROLEOPTION : 'oump-assignable-role', ODD : 'odd', @@ -310,7 +310,7 @@ YUI.add('moodle-enrol-otherusersmanager', function(Y) { ) .append(Y.Node.create('
') .append(Y.Node.create('
'+this.get(FULLNAME)+'
')) - .append(Y.Node.create('
'+this.get(EMAIL)+'
')) + .append(Y.Node.create('
'+this.get(EXTRAFIELDS)+'
')) ) .append(Y.Node.create('
'+M.str.role.assignrole+':
')) ); @@ -374,7 +374,7 @@ YUI.add('moodle-enrol-otherusersmanager', function(Y) { fullname : { validator : Y.Lang.isString }, - email : { + extrafields : { validator : Y.Lang.isString }, picture : {