From eb3ee128525faf98e8ebb4c5e804354787bd2c92 Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Thu, 4 Aug 2022 20:33:57 +0100 Subject: [PATCH] MDL-75381 gradereport_grader: ensure valid paging preference value. Set type of the report paging setting to integer, to ensure usage of it is predictable. Unsupported operated type errors were thrown on PHP8.0 when it's value contained a string or was empty. --- grade/report/grader/lib.php | 4 +-- grade/report/grader/settings.php | 2 +- grade/tests/report_graderlib_test.php | 42 +++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index 9f0217a5625..15d94cfb306 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -1994,7 +1994,7 @@ class grade_report_grader extends grade_report { * * @return int The maximum number of students to display per page */ - public function get_students_per_page() { - return $this->get_pref('studentsperpage'); + public function get_students_per_page(): int { + return (int) $this->get_pref('studentsperpage'); } } diff --git a/grade/report/grader/settings.php b/grade/report/grader/settings.php index 300543a68d3..61fc6d67137 100644 --- a/grade/report/grader/settings.php +++ b/grade/report/grader/settings.php @@ -33,7 +33,7 @@ if ($ADMIN->fulltree) { /// Add settings for this module to the $settings object (it's already defined) $settings->add(new admin_setting_configtext('grade_report_studentsperpage', get_string('studentsperpage', 'grades'), - get_string('studentsperpage_help', 'grades'), 100)); + get_string('studentsperpage_help', 'grades'), 100, PARAM_INT)); $settings->add(new admin_setting_configcheckbox('grade_report_showonlyactiveenrol', get_string('showonlyactiveenrol', 'grades'), get_string('showonlyactiveenrol_help', 'grades'), 1)); diff --git a/grade/tests/report_graderlib_test.php b/grade/tests/report_graderlib_test.php index 6e5351a276c..ab718155d83 100644 --- a/grade/tests/report_graderlib_test.php +++ b/grade/tests/report_graderlib_test.php @@ -29,6 +29,7 @@ require_once($CFG->dirroot.'/grade/report/grader/lib.php'); * Tests grade_report_grader (the grader report) * * @package core_grades + * @covers \grade_report_grader * @category test * @copyright 2012 Andrew Davis * @license http://www.gnu.org/copyleft/gpl.html GNU Public License @@ -518,6 +519,47 @@ class report_graderlib_test extends \advanced_testcase { $this->assertCount(3, $result); } + /** + * Test loading report users when per page preferences are set + */ + public function test_load_users_paging_preference(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + + // The report users will default to sorting by their lastname. + $user1 = $this->getDataGenerator()->create_and_enrol($course, 'student', ['lastname' => 'Apple']); + $user2 = $this->getDataGenerator()->create_and_enrol($course, 'student', ['lastname' => 'Banana']); + $user3 = $this->getDataGenerator()->create_and_enrol($course, 'student', ['lastname' => 'Carrot']); + + // Set to empty string. + $report = $this->create_report($course); + $report->set_pref('studentsperpage', ''); + $users = $report->load_users(); + $this->assertEquals([$user1->id, $user2->id, $user3->id], array_column($users, 'id')); + + // Set to valid value. + $report = $this->create_report($course); + $report->set_pref('studentsperpage', 2); + $users = $report->load_users(); + $this->assertEquals([$user1->id, $user2->id], array_column($users, 'id')); + } + + /** + * Test getting students per page report preference + */ + public function test_get_students_per_page(): void { + $this->resetAfterTest(); + + $course = $this->getDataGenerator()->create_course(); + + $report = $this->create_report($course); + $report->set_pref('studentsperpage', 10); + + $perpage = $report->get_students_per_page(); + $this->assertSame(10, $perpage); + } + private function create_grade_category($course) { static $cnt = 0; $cnt++;