From 36075e092d619d659b832f7d949f608f8fdae177 Mon Sep 17 00:00:00 2001 From: tjhunt Date: Fri, 15 Sep 2006 14:32:35 +0000 Subject: [PATCH] MDL-6041 - Proper fix that eliminates the magic number 99999 when getting 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. --- admin/admin.php | 10 +++------ admin/creators.php | 16 +++++++------- admin/roles/assign.php | 17 +++++++-------- course/report/participation/index.php | 5 +++-- course/student.php | 16 +++++++------- lib/datalib.php | 30 +++++++++++++++------------ lib/deprecatedlib.php | 9 ++++---- user/index.php | 7 ++++--- 8 files changed, 55 insertions(+), 55 deletions(-) diff --git a/admin/admin.php b/admin/admin.php index 2bcb86cb3d..5841ad1ec7 100644 --- a/admin/admin.php +++ b/admin/admin.php @@ -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(); } } diff --git a/admin/creators.php b/admin/creators.php index 23388005a1..567a3662bc 100755 --- a/admin/creators.php +++ b/admin/creators.php @@ -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"); @@ -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 : ""; diff --git a/admin/roles/assign.php b/admin/roles/assign.php index 6dc894dde4..c868d56782 100755 --- a/admin/roles/assign.php +++ b/admin/roles/assign.php @@ -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(); + } } } @@ -195,4 +194,4 @@ print_footer($course); -?> +?> \ No newline at end of file diff --git a/course/report/participation/index.php b/course/report/participation/index.php index 972144b293..ab92f96dd6 100644 --- a/course/report/participation/index.php +++ b/course/report/participation/index.php @@ -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. @@ -338,11 +339,11 @@ function checknos() { $table->print_html(); - if ($perpage == 99999) { + if ($perpage == SHOW_ALL_PAGE_SIZE) { echo '
'.get_string('showperpage', '', DEFAULT_PAGE_SIZE).'
'; } else if ($matchcount > 0 && $perpage < $matchcount) { - echo '
'.get_string('showall', '', $matchcount).'
'; + echo '
'.get_string('showall', '', $matchcount).'
'; } echo '
'; diff --git a/course/student.php b/course/student.php index 094a6b68d1..08387e5cc1 100644 --- a/course/student.php +++ b/course/student.php @@ -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(); } @@ -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(); + } } } diff --git a/lib/datalib.php b/lib/datalib.php index ee0d6fd866..338f9ff15a 100644 --- a/lib/datalib.php +++ b/lib/datalib.php @@ -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(); @@ -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); } } @@ -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(); @@ -1536,4 +1540,4 @@ function category_parent_visible($parent = 0) { } // vim:autoindent:expandtab:shiftwidth=4:tabstop=4:tw=140: -?> +?> \ No newline at end of file diff --git a/lib/deprecatedlib.php b/lib/deprecatedlib.php index 24bee40748..2bc9c69524 100644 --- a/lib/deprecatedlib.php +++ b/lib/deprecatedlib.php @@ -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; @@ -671,7 +671,6 @@ 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'); @@ -679,7 +678,7 @@ function get_course_students($courseid, $sort='ul.timeaccess', $dir='', $page=0, // 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 = ''; } @@ -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; diff --git a/user/index.php b/user/index.php index 2c26ace1c8..7ade006948 100644 --- a/user/index.php +++ b/user/index.php @@ -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 @@ -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 = ''; @@ -584,11 +585,11 @@ function checkchecked(form) { echo ' 

'."\n"; } - if ($perpage == 99999) { + if ($perpage == SHOW_ALL_PAGE_SIZE) { echo ''; } else if ($matchcount > 0 && $perpage < $matchcount) { - echo ''; + echo ''; } } // end of if ($roleid); -- 2.39.5