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 '';
}
else if ($matchcount > 0 && $perpage < $matchcount) {
- echo '';
+ echo '';
}
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