Skip to content

Commit

Permalink
MDL-6041 - Proper fix that eliminates the magic number 99999 when get…
Browse files Browse the repository at this point in the history 
…ting lists of studnets. Now, there is no arbitrary upper limit in the datalib functions, and sensible upper limits on pages that display lists of users. However, if you try to get all the site students, then get_students prints a warning in debug mode, telling you that you need to rethink your code.

Also a few more ISNULL()s eliminated.

And a typo role_assignment -> role_assignments.
  • Loading branch information
tjhunt committed Sep 15, 2006
1 parent 4b10f08 commit 36075e0
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 55 deletions.
10 changes: 3 additions & 7 deletions admin/admin.php
View file
Expand Up @@ -91,25 +91,21 @@

unset($adminarray);

$usercount = get_users(false, '', true, $adminlist);

/// Get search results excluding any current admins
if (!empty($frm->searchtext) and $previoussearch) {
$searchusers = get_users(true, $frm->searchtext, true, $adminlist, 'firstname ASC, lastname ASC',
'', '', 0, 99999, 'id, firstname, lastname, email');
$usercount = get_users(false, '', true, $adminlist);
'', '', 0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email');
}

/// If no search results then get potential users excluding current admins
if (empty($searchusers)) {

$usercount = get_users(false, '', true, $adminlist, 'firstname ASC, lastname ASC', '', '',
0, 99999, 'id, firstname, lastname, email');

$users = array();

if ($usercount <= MAX_USERS_PER_PAGE) {
if (!$users = get_users(true, '', true, $adminlist, 'firstname ASC, lastname ASC', '', '',
0, 99999, 'id, firstname, lastname, email') ) {
0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email') ) {
$users = array();
}
}
Expand Down
16 changes: 9 additions & 7 deletions admin/creators.php
View file
Expand Up @@ -5,7 +5,7 @@

require_once('../config.php');

define("MAX_USERS_PER_PAGE", 50);
define("MAX_USERS_PER_PAGE", 5000);

if (! $site = get_site()) {
redirect("$CFG->wwwroot/$CFG->admin/index.php");
Expand Down Expand Up @@ -86,21 +86,23 @@

unset($creatorarray);

$usercount = get_users(false, '', true, $creatorlist);

/// Get search results excluding any current admins
if (!empty($frm->searchtext) and $previoussearch) {
$searchusers = get_users(true, $frm->searchtext, true, $creatorlist, 'firstname ASC, lastname ASC',
'', '', 0, 99999, 'id, firstname, lastname, email');
$usercount = get_users(false, '', true, $creatorlist);
'', '', 0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email');
}

/// If no search results then get potential users excluding current creators
if (empty($searchusers)) {
if (!$users = get_users(true, '', true, $creatorlist, 'firstname ASC, lastname ASC', '', '',
0, 99999, 'id, firstname, lastname, email') ) {
$users = array();
$users = array();
if ($usercount <= MAX_USERS_PER_PAGE) {
if (!$users = get_users(true, '', true, $creatorlist, 'firstname ASC, lastname ASC', '', '',
0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email') ) {
$users = array();
}
}
$usercount = count($users);
}

$searchtext = (isset($frm->searchtext)) ? $frm->searchtext : "";
Expand Down
17 changes: 8 additions & 9 deletions admin/roles/assign.php
View file
Expand Up @@ -133,23 +133,22 @@
$existinguserlist = implode(',', $existinguserarray);
unset($existinguserarray);

$usercount = get_users(false, '', true, $existinguserlist);

/// Get search results excluding any users already in this course
if (($searchtext != '') and $previoussearch) {
$searchusers = get_users(true, $searchtext, true, $existinguserlist, 'firstname ASC, lastname ASC',
'', '', 0, 99999, 'id, firstname, lastname, email');
$usercount = get_users(false, '', true, $existinguserlist);
'', '', 0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email');
}

/// If no search results then get potential students for this course excluding users already in course
if (empty($searchusers)) {

$usercount = get_users(false, '', true, $existinguserlist, 'firstname ASC, lastname ASC', '', '',
0, 99999, 'id, firstname, lastname, email') ;
$users = array();

if ($usercount <= MAX_USERS_PER_PAGE) {
$users = get_users(true, '', true, $existinguserlist, 'firstname ASC, lastname ASC', '', '',
0, 99999, 'id, firstname, lastname, email');
if (!$users = get_users(true, '', true, $existinguserlist, 'firstname ASC, lastname ASC', '', '',
0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email')) {
$users = array();
}
}

}
Expand Down Expand Up @@ -195,4 +194,4 @@

print_footer($course);

?>
?>
5 changes: 3 additions & 2 deletions course/report/participation/index.php
View file
Expand Up @@ -3,6 +3,7 @@
require_once('../../../config.php');

define('DEFAULT_PAGE_SIZE', 20);
define('SHOW_ALL_PAGE_SIZE', 5000);

$id = required_param('id', PARAM_INT); // course id.
$moduleid = optional_param('moduleid', 0, PARAM_INT); // module id.
Expand Down Expand Up @@ -338,11 +339,11 @@ function checknos() {

$table->print_html();

if ($perpage == 99999) {
if ($perpage == SHOW_ALL_PAGE_SIZE) {
echo '<div id="showall"><a href="'.$baseurl.'&amp;perpage='.DEFAULT_PAGE_SIZE.'">'.get_string('showperpage', '', DEFAULT_PAGE_SIZE).'</a></div>';
}
else if ($matchcount > 0 && $perpage < $matchcount) {
echo '<div id="showall"><a href="'.$baseurl.'&amp;perpage=99999">'.get_string('showall', '', $matchcount).'</a></div>';
echo '<div id="showall"><a href="'.$baseurl.'&amp;perpage='.SHOW_ALL_PAGE_SIZE.'">'.get_string('showall', '', $matchcount).'</a></div>';
}

echo '<br /><center>';
Expand Down
16 changes: 7 additions & 9 deletions course/student.php
View file
Expand Up @@ -98,7 +98,7 @@


/// Get all existing students and teachers for this course.
if (!$students = get_course_students($course->id, "u.firstname ASC, u.lastname ASC", "", 0, 99999,
if (!$students = get_course_students($course->id, "u.firstname ASC, u.lastname ASC", "", 0, '',
'', '', NULL, '', 'u.id,u.firstname,u.lastname,u.email')) {
$students = array();
}
Expand All @@ -116,24 +116,22 @@

unset($existinguserarray);

$usercount = get_users(false, '', true, $existinguserlist);

/// Get search results excluding any users already in this course
if (($searchtext != '') and $previoussearch) {
$searchusers = get_users(true, $searchtext, true, $existinguserlist, 'firstname ASC, lastname ASC',
'', '', 0, 99999, 'id, firstname, lastname, email');
$usercount = get_users(false, '', true, $existinguserlist);
'', '', 0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email');
}

/// If no search results then get potential students for this course excluding users already in course
if (empty($searchusers)) {

$usercount = get_users(false, '', true, $existinguserlist, 'firstname ASC, lastname ASC', '', '',
0, 99999, 'id, firstname, lastname, email') ;
$users = array();

if ($usercount <= MAX_USERS_PER_PAGE) {
$users = get_users(true, '', true, $existinguserlist, 'firstname ASC, lastname ASC', '', '',
0, 99999, 'id, firstname, lastname, email');
if (!$users = get_users(true, '', true, $existinguserlist, 'firstname ASC, lastname ASC', '', '',
0, MAX_USERS_PER_PAGE, 'id, firstname, lastname, email')) {
$users = array();
}
}

}
Expand Down
30 changes: 17 additions & 13 deletions lib/datalib.php
View file
Expand Up @@ -194,11 +194,17 @@ function get_site_users($sort='u.lastaccess DESC', $fields='*', $exceptions='')
* @return object|false|int {@link $USER} records unless get is false in which case the integer count of the records found is returned. False is returned if an error is encountered.
*/
function get_users($get=true, $search='', $confirmed=false, $exceptions='', $sort='firstname ASC',
$firstinitial='', $lastinitial='', $page=0, $recordsperpage=99999, $fields='*') {
$firstinitial='', $lastinitial='', $page='', $recordsperpage='', $fields='*') {

global $CFG;

if ($get && !$recordsperpage) {
debugging('Call to get_users with $get = true no $recordsperpage limit. ' .
'On large installations, this will probably cause an out of memory error. ' .
'Please think again and change your code so that it does not try to ' .
'load so much data into memory.', E_USER_WARNING);
}

$limit = sql_paging_limit($page, $recordsperpage);
$LIKE = sql_ilike();
$fullname = sql_fullname();

Expand All @@ -224,16 +230,10 @@ function get_users($get=true, $search='', $confirmed=false, $exceptions='', $sor
$select .= ' AND lastname '. $LIKE .' \''. $lastinitial .'%\'';
}

if ($sort and $get) {
$sort = ' ORDER BY '. $sort .' ';
} else {
$sort = '';
}

if ($get) {
return get_records_select('user', $select .' '. $sort .' '. $limit, '', $fields);
return get_records_select('user', $select, $sort, $fields, $page, $recordsperpage);
} else {
return count_records_select('user', $select .' '. $sort .' '. $limit);
return count_records_select('user', $select);
}
}

Expand All @@ -255,12 +255,16 @@ function get_users($get=true, $search='', $confirmed=false, $exceptions='', $sor
* @todo Finish documenting this function
*/

function get_users_listing($sort='lastaccess', $dir='ASC', $page=0, $recordsperpage=99999,
function get_users_listing($sort='lastaccess', $dir='ASC', $page=0, $recordsperpage=0,
$search='', $firstinitial='', $lastinitial='') {

global $CFG;

$limit = sql_paging_limit($page, $recordsperpage);
if ($recordsperpage) {
$limit = sql_paging_limit($page, $recordsperpage);
} else {
$limit = '';
}
$LIKE = sql_ilike();
$fullname = sql_fullname();

Expand Down Expand Up @@ -1536,4 +1540,4 @@ function category_parent_visible($parent = 0) {
}

// vim:autoindent:expandtab:shiftwidth=4:tabstop=4:tw=140:
?>
?>
9 changes: 4 additions & 5 deletions lib/deprecatedlib.php
View file
Expand Up @@ -641,7 +641,7 @@ function get_recent_enrolments($courseid, $timestart) {
* @return object
* @todo Finish documenting this function
*/
function get_course_students($courseid, $sort='ul.timeaccess', $dir='', $page=0, $recordsperpage=99999,
function get_course_students($courseid, $sort='ul.timeaccess', $dir='', $page='', $recordsperpage='',
$firstinitial='', $lastinitial='', $group=NULL, $search='', $fields='', $exceptions='') {

global $CFG;
Expand Down Expand Up @@ -671,15 +671,14 @@ function get_course_students($courseid, $sort='ul.timeaccess', $dir='', $page=0,
$page, $recordsperpage, $fields ? $fields : '*');
}

$limit = sql_paging_limit($page, $recordsperpage);
$LIKE = sql_ilike();
$fullname = sql_fullname('u.firstname','u.lastname');

$groupmembers = '';

// make sure it works on the site course
$context = get_context_instance(CONTEXT_COURSE, $courseid);
$select = "(ul.courseid = '$courseid' OR ISNULL(ul.courseid)) AND ";
$select = "(ul.courseid = '$courseid' OR ul.courseid IS NULL) AND ";
if ($courseid == SITEID) {
$select = '';
}
Expand Down Expand Up @@ -723,10 +722,10 @@ function get_course_students($courseid, $sort='ul.timeaccess', $dir='', $page=0,

$students = get_records_sql("SELECT $fields
FROM {$CFG->prefix}user u INNER JOIN
{$CFG->prefix}role_assignment ra on u.id=ra.userid LEFT OUTER JOIN
{$CFG->prefix}role_assignments ra on u.id=ra.userid LEFT OUTER JOIN
{$CFG->prefix}user_lastaccess ul on ul.userid=ra.userid
$groupmembers
WHERE $select $search $sort $dir $limit");
WHERE $select $search $sort $dir", $page, $recordsperpage);

//if ($courseid != SITEID) {
return $students;
Expand Down
7 changes: 4 additions & 3 deletions user/index.php
View file
Expand Up @@ -8,6 +8,7 @@
define('USER_SMALL_CLASS', 20); // Below this is considered small
define('USER_LARGE_CLASS', 200); // Above this is considered large
define('DEFAULT_PAGE_SIZE', 20);
define('SHOW_ALL_PAGE_SIZE', 5000);

$group = optional_param('group', -1, PARAM_INT); // Group to show
$page = optional_param('page', 0, PARAM_INT); // which page to show
Expand Down Expand Up @@ -387,7 +388,7 @@ function checkchecked(form) {
$where = "WHERE (r.contextid = $context->id OR r.contextid in $listofcontexts)
AND u.deleted = 0
AND r.roleid = $roleid
AND (ul.courseid = $course->id OR ISNULL(ul.courseid))";
AND (ul.courseid = $course->id OR ul.courseid IS NULL)";
$where .= get_lastaccess_sql($accesssince);

$wheresearch = '';
Expand Down Expand Up @@ -584,11 +585,11 @@ function checkchecked(form) {
echo '<input type="text" name="search" value="'.$search.'" />&nbsp;<input type="submit" value="'.get_string('search').'" /></p></form>'."\n";
}

if ($perpage == 99999) {
if ($perpage == SHOW_ALL_PAGE_SIZE) {
echo '<div id="showall"><a href="'.$baseurl.'&amp;perpage='.DEFAULT_PAGE_SIZE.'">'.get_string('showperpage', '', DEFAULT_PAGE_SIZE).'</a></div>';
}
else if ($matchcount > 0 && $perpage < $matchcount) {
echo '<div id="showall"><a href="'.$baseurl.'&amp;perpage=99999">'.get_string('showall', '', $matchcount).'</a></div>';
echo '<div id="showall"><a href="'.$baseurl.'&amp;perpage='.SHOW_ALL_PAGE_SIZE.'">'.get_string('showall', '', $matchcount).'</a></div>';
}
} // end of if ($roleid);

Expand Down

0 comments on commit 36075e0

Please sign in to comment.