Skip to content

Commit

Permalink
MDL-34657 user sorting: consistent sorting everywhere.
Browse files Browse the repository at this point in the history
This commit coverts everything in the codebase to use the new
users_order_by_sql function when sorting lists of users. More details in
the bug.

Note that this does not change places where users are displayed in a
sortable table, and the sort order comes from the table.
  • Loading branch information
timhunt committed Sep 27, 2012
1 parent 9f82ddd commit 9695ff8
Show file tree
Hide file tree
Showing 29 changed files with 271 additions and 155 deletions.
2 changes: 1 addition & 1 deletion admin/roles/assign.php
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@
foreach ($assignableroles as $roleid => $notused) {
$roleusers = '';
if (0 < $assigncounts[$roleid] && $assigncounts[$roleid] <= MAX_USERS_TO_LIST_PER_ROLE) {
$roleusers = get_role_users($roleid, $context, false, 'u.id, u.lastname, u.firstname');
$roleusers = get_role_users($roleid, $context, false, 'u.id');
if (!empty($roleusers)) {
$strroleusers = array();
foreach ($roleusers as $user) {
Expand Down
30 changes: 21 additions & 9 deletions admin/roles/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -1043,11 +1043,12 @@ public function find_users($search) {
WHERE u.id IN ($enrolsql)
$wherecondition
AND ra.id IS NULL";
$order = ' ORDER BY lastname ASC, firstname ASC';

$params['contextid'] = $this->context->id;
$params['roleid'] = $this->roleid;

list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

// Check to see if there are too many to show sensibly.
if (!$this->is_validating()) {
$potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
Expand All @@ -1057,7 +1058,7 @@ public function find_users($search) {
}

// If not, show them.
$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down Expand Up @@ -1094,7 +1095,9 @@ public function find_users($search) {
FROM {role_assignments} r
WHERE r.contextid = :contextid
AND r.roleid = :roleid)";
$order = ' ORDER BY lastname ASC, firstname ASC';

list($sort, $sortparams) = users_order_by_sql('', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

$params['contextid'] = $this->context->id;
$params['roleid'] = $this->roleid;
Expand All @@ -1106,7 +1109,7 @@ public function find_users($search) {
}
}

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down Expand Up @@ -1140,6 +1143,9 @@ public function find_users($search) {
$params = array_merge($params, $ctxparams);
$params['roleid'] = $this->roleid;

list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
$params = array_merge($params, $sortparams);

$sql = "SELECT ra.id as raid," . $this->required_fields_sql('u') . ",ra.contextid,ra.component
FROM {role_assignments} ra
JOIN {user} u ON u.id = ra.userid
Expand All @@ -1148,7 +1154,7 @@ public function find_users($search) {
$wherecondition AND
ctx.id $ctxcondition AND
ra.roleid = :roleid
ORDER BY ctx.depth DESC, ra.component, u.lastname, u.firstname";
ORDER BY ctx.depth DESC, ra.component, $sort";
$contextusers = $DB->get_records_sql($sql, $params);

// No users at all.
Expand Down Expand Up @@ -1510,9 +1516,12 @@ public function find_users($search) {

$sql = " FROM {user}
WHERE $wherecondition AND mnethostid = :localmnet";
$order = ' ORDER BY lastname ASC, firstname ASC';

$params['localmnet'] = $CFG->mnet_localhost_id; // it could be dangerous to make remote users admins and also this could lead to other problems

list($sort, $sortparams) = users_order_by_sql('', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

// Check to see if there are too many to show sensibly.
if (!$this->is_validating()) {
$potentialcount = $DB->count_records_sql($countfields . $sql, $params);
Expand All @@ -1521,7 +1530,7 @@ public function find_users($search) {
}
}

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down Expand Up @@ -1568,7 +1577,10 @@ public function find_users($search) {
}
$sql = " FROM {user}
WHERE $wherecondition";
$order = ' ORDER BY lastname ASC, firstname ASC';

list($sort, $sortparams) = users_order_by_sql('', $search, $this->accesscontext);
$params = array_merge($params, $sortparams);
$order = ' ORDER BY ' . $sort;

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);

Expand Down
10 changes: 6 additions & 4 deletions admin/webservice/forms.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,16 +194,18 @@ function definition() {
array('deleted' => 0, 'suspended' => 0, 'confirmed' => 1));

if ($usertotal < 500) {
list($sort, $params) = users_order_by_sql('u');
//user searchable selector - get all users (admin and guest included)
//user must be confirmed, not deleted, not suspended, not guest
$sql = "SELECT u.id, u.firstname, u.lastname
FROM {user} u
WHERE u.deleted = 0 AND u.confirmed = 1 AND u.suspended = 0 AND u.id != ?
ORDER BY u.lastname";
$users = $DB->get_records_sql($sql, array($CFG->siteguest));
WHERE u.deleted = 0 AND u.confirmed = 1 AND u.suspended = 0 AND u.id != :siteguestid
ORDER BY $sort";
$params['siteguestid'] = $CFG->siteguest;
$users = $DB->get_records_sql($sql, $params);
$options = array();
foreach ($users as $userid => $user) {
$options[$userid] = $user->firstname . " " . $user->lastname;
$options[$userid] = fullname($user);
}
$mform->addElement('searchableselector', 'user', get_string('user'), $options);
} else {
Expand Down
9 changes: 5 additions & 4 deletions admin/webservice/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
require_once($CFG->dirroot . '/user/selector/lib.php');

/*
* This class displays either all the Moodle users allowed to use a service,
* either all the other Moodle users.
* This class displays either all the Moodle users allowed to use a service,
* either all the other Moodle users.
*/
class service_user_selector extends user_selector_base {
const MAX_USERS_PER_PAGE = 100;
Expand Down Expand Up @@ -81,7 +81,8 @@ public function find_users($search) {
AND esu.userid = u.id)";
}

$order = ' ORDER BY u.lastname ASC, u.firstname ASC';
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

if (!$this->is_validating()) {
$potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
Expand All @@ -90,7 +91,7 @@ public function find_users($search) {
}
}

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down
10 changes: 6 additions & 4 deletions cohort/locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ public function find_users($search) {
LEFT JOIN {cohort_members} cm ON (cm.userid = u.id AND cm.cohortid = :cohortid)
WHERE cm.id IS NULL AND $wherecondition";

$order = ' ORDER BY u.lastname ASC, u.firstname ASC';
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

if (!$this->is_validating()) {
$potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
Expand All @@ -66,7 +67,7 @@ public function find_users($search) {
}
}

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down Expand Up @@ -120,7 +121,8 @@ public function find_users($search) {
JOIN {cohort_members} cm ON (cm.userid = u.id AND cm.cohortid = :cohortid)
WHERE $wherecondition";

$order = ' ORDER BY u.lastname ASC, u.firstname ASC';
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

if (!$this->is_validating()) {
$potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
Expand All @@ -129,7 +131,7 @@ public function find_users($search) {
}
}

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down
3 changes: 2 additions & 1 deletion course/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -2520,10 +2520,11 @@ function print_course($course, $highlightterms = '') {
$rusers = array();

if (!isset($course->managers)) {
list($sort, $sortparams) = users_order_by_sql('u');
$rusers = get_role_users($managerroles, $context, true,
'ra.id AS raid, u.id, u.username, u.firstname, u.lastname, rn.name AS rolecoursealias,
r.name AS rolename, r.sortorder, r.id AS roleid, r.shortname AS roleshortname',
'r.sortorder ASC, u.lastname ASC');
'r.sortorder ASC, ' . $sort, null, '', '', '', '', $sortparams);
} else {
// use the managers array if we have it for perf reasosn
// populate the datastructure like output of get_role_users();
Expand Down
25 changes: 17 additions & 8 deletions enrol/locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -310,11 +310,15 @@ public function get_potential_users($enrolid, $search='', $searchanywhere=false,
LEFT JOIN {user_enrolments} ue ON (ue.userid = u.id AND ue.enrolid = :enrolid)
WHERE $wherecondition
AND ue.id IS NULL";
$order = ' ORDER BY u.lastname ASC, u.firstname ASC';
$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 array('totalusers' => $totalusers, 'users' => $availableusers);
}

/**
Expand All @@ -334,7 +338,7 @@ public function search_other_users($search='', $searchanywhere=false, $page=0, $
$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');
$conditions = array('u.firstname', 'u.lastname');
if ($searchanywhere) {
$searchparam = '%' . $search . '%';
} else {
Expand All @@ -356,11 +360,14 @@ public function search_other_users($search='', $searchanywhere=false, $page=0, $
LEFT JOIN {role_assignments} ra ON (ra.userid = u.id AND ra.contextid = :contextid)
WHERE $wherecondition
AND ra.id IS NULL";
$order = ' ORDER BY lastname ASC, firstname ASC';

$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, $params, $page*$perpage, $perpage);
$availableusers = $DB->get_records_sql($fields . $sql . $order,
array_merge($params, $sortparams), $page*$perpage, $perpage);
return array('totalusers'=>$totalusers, 'users'=>$availableusers);
}

Expand Down Expand Up @@ -1008,14 +1015,16 @@ public function get_users_enrolments(array $userids) {
$userfields = user_picture::fields('u');
list($idsql, $idparams) = $DB->get_in_or_equal($userids, SQL_PARAMS_NAMED, 'userid0000');

list($sort, $sortparams) = users_order_by_sql('u');

$sql = "SELECT ue.id AS ueid, ue.status, ue.enrolid, ue.userid, ue.timestart, ue.timeend, ue.modifierid, ue.timecreated, ue.timemodified, $userfields
FROM {user_enrolments} ue
LEFT JOIN {user} u ON u.id = ue.userid
WHERE ue.enrolid $instancesql AND
u.id $idsql
ORDER BY u.firstname ASC, u.lastname ASC";
ORDER BY $sort";

$rs = $DB->get_recordset_sql($sql, $idparams + $instanceparams);
$rs = $DB->get_recordset_sql($sql, $idparams + $instanceparams + $sortparams);
$users = array();
foreach ($rs as $ue) {
$user = user_picture::unalias($ue);
Expand Down
11 changes: 7 additions & 4 deletions enrol/manual/locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ public function find_users($search) {
LEFT JOIN {user_enrolments} ue ON (ue.userid = u.id AND ue.enrolid = :enrolid)
WHERE $wherecondition
AND ue.id IS NULL";
$order = ' ORDER BY u.lastname ASC, u.firstname ASC';

list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

if (!$this->is_validating()) {
$potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
Expand All @@ -66,7 +68,7 @@ public function find_users($search) {
}
}

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down Expand Up @@ -120,7 +122,8 @@ public function find_users($search) {
JOIN {user_enrolments} ue ON (ue.userid = u.id AND ue.enrolid = :enrolid)
WHERE $wherecondition";

$order = ' ORDER BY u.lastname ASC, u.firstname ASC';
list($sort, $sortparams) = users_order_by_sql('u', $search, $this->accesscontext);
$order = ' ORDER BY ' . $sort;

if (!$this->is_validating()) {
$potentialmemberscount = $DB->count_records_sql($countfields . $sql, $params);
Expand All @@ -129,7 +132,7 @@ public function find_users($search) {
}
}

$availableusers = $DB->get_records_sql($fields . $sql . $order, $params);
$availableusers = $DB->get_records_sql($fields . $sql . $order, array_merge($params, $sortparams));

if (empty($availableusers)) {
return array();
Expand Down
5 changes: 3 additions & 2 deletions enrol/mnet/enrol.php
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,10 @@ public function course_enrolments($courseid, $roles=null) {
$params = array_merge($params, $rparams);
}

$sql .= " ORDER BY u.lastname, u.firstname";
list($sort, $sortparams) = users_order_by_sql('u');
$sql .= " ORDER BY $sort";

$rs = $DB->get_recordset_sql($sql, $params);
$rs = $DB->get_recordset_sql($sql, array_merge($params, $sortparams));
$list = array();
foreach ($rs as $record) {
$list[] = $record;
Expand Down
3 changes: 2 additions & 1 deletion enrol/self/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ protected function email_welcome_message($instance, $user) {
$rusers = array();
if (!empty($CFG->coursecontact)) {
$croles = explode(',', $CFG->coursecontact);
$rusers = get_role_users($croles, $context, true, '', 'r.sortorder ASC, u.lastname ASC');
list($sort, $sortparams) = users_order_by_sql('u');
$rusers = get_role_users($croles, $context, true, '', 'r.sortorder ASC, ' . $sort, null, '', '', '', '', $sortparams);
}
if ($rusers) {
$contact = reset($rusers);
Expand Down
5 changes: 3 additions & 2 deletions grade/import/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ function get_unenrolled_users_in_import($importcode, $courseid) {
//enrolled users
$context = context_course::instance($courseid);
list($enrolledsql, $enrolledparams) = get_enrolled_sql($context);
list($sort, $sortparams) = users_order_by_sql('u');

$sql = "SELECT giv.id, u.firstname, u.lastname, u.idnumber AS useridnumber,
COALESCE(gi.idnumber, gin.itemname) AS gradeidnumber
Expand All @@ -185,8 +186,8 @@ function get_unenrolled_users_in_import($importcode, $courseid) {
ON (giv.userid = ra.userid AND ra.roleid $gradebookrolessql AND ra.contextid $relatedctxcondition)
WHERE giv.importcode = :importcode
AND (ra.id IS NULL OR je.id IS NULL)
ORDER BY gradeidnumber, u.lastname, u.firstname";
$params = array_merge($gradebookrolesparams, $enrolledparams);
ORDER BY gradeidnumber, $sort";
$params = array_merge($gradebookrolesparams, $enrolledparams, $sortparams);
$params['importcode'] = $importcode;

return $DB->get_records_sql($sql, $params);
Expand Down
4 changes: 2 additions & 2 deletions group/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@

case 'ajax_getmembersingroup':
$roles = array();
if ($groupmemberroles = groups_get_members_by_role($groupids[0], $courseid, 'u.id,u.firstname,u.lastname')) {
if ($groupmemberroles = groups_get_members_by_role($groupids[0], $courseid, 'u.id, u.firstname, u.lastname')) {
foreach($groupmemberroles as $roleid=>$roledata) {
$shortroledata = new stdClass();
$shortroledata->name = $roledata->name;
Expand Down Expand Up @@ -248,7 +248,7 @@

$atleastonemember = false;
if ($singlegroup) {
if ($groupmemberroles = groups_get_members_by_role($groupids[0],$courseid,'u.id,u.firstname,u.lastname')) {
if ($groupmemberroles = groups_get_members_by_role($groupids[0], $courseid, 'u.id, u.firstname, u.lastname')) {
foreach($groupmemberroles as $roleid=>$roledata) {
echo '<optgroup label="'.s($roledata->name).'">';
foreach($roledata->users as $member) {
Expand Down
Loading

0 comments on commit 9695ff8

Please sign in to comment.