Skip to content

Commit

Permalink
MDL-82545 gradereport_grader: Limit grades per page
Browse files Browse the repository at this point in the history
This replaces the limit on the number of students per page with a number
of grades per page, to take into account the number of grade items on a
course.
  • Loading branch information
marxjohnson committed Jul 22, 2024
1 parent d7cb2a6 commit bdc8394
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 5 deletions.
11 changes: 8 additions & 3 deletions grade/report/grader/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
require_once($CFG->dirroot.'/grade/lib.php');
require_once($CFG->dirroot.'/grade/report/grader/lib.php');

// This report may require a lot of memory and time on large courses.
raise_memory_limit(MEMORY_HUGE);
set_time_limit(120);

$courseid = required_param('id', PARAM_INT); // course id
$page = optional_param('page', 0, PARAM_INT); // active page
$edit = optional_param('edit', -1, PARAM_BOOL); // sticky editting mode
Expand Down Expand Up @@ -191,8 +195,9 @@
$pagingoptions = array_unique($pagingoptions);
sort($pagingoptions);
$pagingoptions = array_combine($pagingoptions, $pagingoptions);
if ($numusers > grade_report_grader::MAX_STUDENTS_PER_PAGE) {
$pagingoptions['0'] = grade_report_grader::MAX_STUDENTS_PER_PAGE;
$maxusers = $report->get_max_students_per_page();
if ($numusers > $maxusers) {
$pagingoptions['0'] = $maxusers;
} else {
$pagingoptions['0'] = get_string('all');
}
Expand All @@ -215,7 +220,7 @@
);

// The number of students per page is always limited even if it is claimed to be unlimited.
$studentsperpage = $studentsperpage ?: grade_report_grader::MAX_STUDENTS_PER_PAGE;
$studentsperpage = $studentsperpage ?: $maxusers;
$footercontent .= html_writer::div(
$OUTPUT->paging_bar($numusers, $report->page, $studentsperpage, $report->pbarurl),
'col'
Expand Down
24 changes: 22 additions & 2 deletions grade/report/grader/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ class grade_report_grader extends grade_report {
/** @var int Maximum number of students that can be shown on one page */
public const MAX_STUDENTS_PER_PAGE = 5000;

/**
* @var int The maximum number of grades that can be shown on one page.
*
* More than this causes issues for the browser due to the size of the page.
*/
public const MAX_GRADES_PER_PAGE = 200000;

/** @var int[] List of available options on the pagination dropdown */
public const PAGINATION_OPTIONS = [20, 100];

Expand Down Expand Up @@ -466,9 +473,10 @@ public function load_users(bool $allusers = false) {
$this->userwheresql
$this->groupwheresql
ORDER BY $sort";
// We never work with unlimited result. Limit the number of records by MAX_STUDENTS_PER_PAGE if no other limit is specified.
// We never work with unlimited result. Limit the number of records by $this->get_max_students_per_page() if no other limit
// is specified.
$studentsperpage = ($this->get_students_per_page() && !$allusers) ?
$this->get_students_per_page() : static::MAX_STUDENTS_PER_PAGE;
$this->get_students_per_page() : $this->get_max_students_per_page();
$this->users = $DB->get_records_sql($sql, $params, $studentsperpage * $this->page, $studentsperpage);

if (empty($this->users)) {
Expand Down Expand Up @@ -526,6 +534,18 @@ protected function get_allgradeitems() {
return $this->allgradeitems;
}

/**
* Return the maximum number of students we can display per page.
*
* This is based on the number of grade items on the course, to limit the overall number of grades displayed on a single page.
* Trying to display too many grades causes browser issues.
*
* @return int
*/
public function get_max_students_per_page(): int {
return round(static::MAX_GRADES_PER_PAGE / count($this->get_allgradeitems()));
}

/**
* we supply the userids in this query, and get all the grades
* pulls out all the grades, this does not need to worry about paging
Expand Down

0 comments on commit bdc8394

Please sign in to comment.