From bfb8ad5735026e8e1e9f0c1949ade238e310a472 Mon Sep 17 00:00:00 2001 From: Ilya Tregubov Date: Thu, 26 Oct 2023 12:55:04 +0800 Subject: [PATCH] MDL-68652 core_grades: Refactor grades functions. get_gradable_users now has extra param to retrieve only active users. Grade reports hase their own grade_report::get_gradable_users since they decided whether to inlcude active users from report preferences --- grade/lib.php | 11 ++--------- grade/report/lib.php | 20 ++++++++++++++++++++ .../report/singleview/classes/local/screen/grade.php | 3 ++- .../singleview/classes/local/screen/screen.php | 6 ++++-- .../report/singleview/classes/local/screen/user.php | 2 +- grade/report/singleview/index.php | 6 ++++-- grade/report/singleview/tests/screen_test.php | 15 ++++++++++----- grade/report/upgrade.txt | 2 +- grade/report/user/index.php | 2 +- grade/tests/lib_test.php | 8 ++++---- grade/upgrade.txt | 1 + 11 files changed, 50 insertions(+), 26 deletions(-) diff --git a/grade/lib.php b/grade/lib.php index 4a514cffc0e..720f35e9f93 100644 --- a/grade/lib.php +++ b/grade/lib.php @@ -786,17 +786,10 @@ function grade_get_plugin_info($courseid, $active_type, $active_plugin) { * * @param int $courseid The course ID. * @param int|null $groupid The group ID (optional). + * @param bool $onlyactiveenrol Include only active enrolments. * @return array $users A list of enrolled gradable users. */ -function get_gradable_users(int $courseid, ?int $groupid = null): array { - global $CFG; - - $context = context_course::instance($courseid); - // Create a graded_users_iterator because it will properly check the groups etc. - $defaultgradeshowactiveenrol = !empty($CFG->grade_report_showonlyactiveenrol); - $onlyactiveenrol = get_user_preferences('grade_report_showonlyactiveenrol', $defaultgradeshowactiveenrol) || - !has_capability('moodle/course:viewsuspendedusers', $context); - +function get_gradable_users(int $courseid, ?int $groupid = null, bool $onlyactiveenrol = false): array { $course = get_course($courseid); $gui = new graded_users_iterator($course, null, $groupid); $gui->require_active_enrolment($onlyactiveenrol); diff --git a/grade/report/lib.php b/grade/report/lib.php index 96272646b93..084a18989aa 100644 --- a/grade/report/lib.php +++ b/grade/report/lib.php @@ -956,6 +956,26 @@ abstract class grade_report { } /** + * Load a valid list of gradable users in a course. + * + * @param int $courseid The course ID. + * @param int|null $groupid The group ID (optional). + * @return array $users A list of enrolled gradable users. + */ + public static function get_gradable_users(int $courseid, ?int $groupid = null): array { + global $CFG; + require_once($CFG->dirroot . '/grade/lib.php'); + + $context = context_course::instance($courseid); + // Create a graded_users_iterator because it will properly check the groups etc. + $defaultgradeshowactiveenrol = !empty($CFG->grade_report_showonlyactiveenrol); + $onlyactiveenrol = get_user_preferences('grade_report_showonlyactiveenrol', $defaultgradeshowactiveenrol) || + !has_capability('moodle/course:viewsuspendedusers', $context); + + return get_gradable_users($courseid, $groupid, $onlyactiveenrol); + } + + /** * Returns a row of grade items averages * * @param array $ungradedcounts Ungraded grade items counts with report preferences. diff --git a/grade/report/singleview/classes/local/screen/grade.php b/grade/report/singleview/classes/local/screen/grade.php index 0391f99d408..039eaf965c3 100644 --- a/grade/report/singleview/classes/local/screen/grade.php +++ b/grade/report/singleview/classes/local/screen/grade.php @@ -24,6 +24,7 @@ namespace gradereport_singleview\local\screen; +use grade_report; use gradereport_singleview\local\ui\range; use gradereport_singleview\local\ui\bulk_insert; use grade_grade; @@ -151,7 +152,7 @@ class grade extends tablelike implements selectable_items, filterable_items { */ public function init($selfitemisempty = false) { - $this->items = get_gradable_users($this->courseid, $this->groupid); + $this->items = grade_report::get_gradable_users($this->courseid, $this->groupid); $this->totalitemcount = count($this->items); if ($selfitemisempty) { diff --git a/grade/report/singleview/classes/local/screen/screen.php b/grade/report/singleview/classes/local/screen/screen.php index 47c16c839aa..2958d9660e7 100644 --- a/grade/report/singleview/classes/local/screen/screen.php +++ b/grade/report/singleview/classes/local/screen/screen.php @@ -25,6 +25,7 @@ namespace gradereport_singleview\local\screen; use context_course; +use grade_report; use moodle_url; use html_writer; use grade_structure; @@ -33,6 +34,7 @@ use grade_item; use stdClass; defined('MOODLE_INTERNAL') || die; +require_once($CFG->dirroot . '/grade/report/lib.php'); /** * Abstract class used as a base for the 3 screens. @@ -415,10 +417,10 @@ abstract class screen { * @return array A list of enroled users. */ protected function load_users(): array { - debugging('The function ' . __FUNCTION__ . '() is deprecated. Please use get_gradable_users() instead.', + debugging('The function ' . __FUNCTION__ . '() is deprecated. Please use grade_report::get_gradable_users() instead.', DEBUG_DEVELOPER); - return get_gradable_users($this->courseid, $this->groupid); + return grade_report::get_gradable_users($this->courseid, $this->groupid); } /** diff --git a/grade/report/singleview/classes/local/screen/user.php b/grade/report/singleview/classes/local/screen/user.php index dd1bdfee246..0cbc3b490c0 100644 --- a/grade/report/singleview/classes/local/screen/user.php +++ b/grade/report/singleview/classes/local/screen/user.php @@ -102,7 +102,7 @@ class user extends tablelike implements selectable_items { public function init($selfitemisempty = false) { if (!$selfitemisempty) { - $validusers = get_gradable_users($this->courseid, $this->groupid); + $validusers = \grade_report::get_gradable_users($this->courseid, $this->groupid); if (!isset($validusers[$this->itemid])) { // If the passed user id is not valid, show the first user from the list instead. $this->item = reset($validusers); diff --git a/grade/report/singleview/index.php b/grade/report/singleview/index.php index f7876235d69..47b9f930225 100644 --- a/grade/report/singleview/index.php +++ b/grade/report/singleview/index.php @@ -27,6 +27,7 @@ define('NO_OUTPUT_BUFFERING', true); require_once('../../../config.php'); require_once($CFG->dirroot.'/lib/gradelib.php'); require_once($CFG->dirroot.'/grade/lib.php'); +require_once($CFG->dirroot.'/grade/report/lib.php'); $courseid = required_param('id', PARAM_INT); $groupid = optional_param('group', null, PARAM_INT); @@ -90,7 +91,8 @@ switch ($itemtype) { // If there is a stored user item (last viewed) in a session variable, bypass the user select zero state // and display this user item. Also, make sure that the stored last viewed user is part of the current // list of gradable users in this course. - if ($lastvieweduseritemid && array_key_exists($lastvieweduseritemid, get_gradable_users($courseid, $currentgroup))) { + if ($lastvieweduseritemid && + array_key_exists($lastvieweduseritemid, grade_report::get_gradable_users($courseid, $currentgroup))) { $itemtype = 'user'; $itemid = $lastvieweduseritemid; } else { @@ -103,7 +105,7 @@ switch ($itemtype) { } // If the item id (user id) cannot be defined or the user id is not part of the list of gradable users, // display the user select zero state. - if (is_null($itemid) || !array_key_exists($itemid, get_gradable_users($courseid, $currentgroup))) { + if (is_null($itemid) || !array_key_exists($itemid, grade_report::get_gradable_users($courseid, $currentgroup))) { $itemtype = 'user_select'; } break; diff --git a/grade/report/singleview/tests/screen_test.php b/grade/report/singleview/tests/screen_test.php index 5e20de527f4..9a98cac97c2 100644 --- a/grade/report/singleview/tests/screen_test.php +++ b/grade/report/singleview/tests/screen_test.php @@ -63,13 +63,15 @@ class screen_test extends \advanced_testcase { grade_regrade_final_grades($course->id); $screentest = new gradereport_singleview_screen_testable($course->id, 0, $group->id); $groupusers = $screentest->test_load_users(); - $this->assertDebuggingCalled('The function load_users() is deprecated. Please use get_gradable_users() instead.'); + $this->assertDebuggingCalled('The function load_users() is deprecated. ' . + 'Please use grade_report::get_gradable_users() instead.'); $this->assertCount(2, $groupusers); // Now, let's suspend the enrolment of a user. Should return only one user. $this->getDataGenerator()->enrol_user($user2->id, $course->id, $roleteacher->id, 'manual', 0, 0, ENROL_USER_SUSPENDED); $users = $screentest->test_load_users(); - $this->assertDebuggingCalled('The function load_users() is deprecated. Please use get_gradable_users() instead.'); + $this->assertDebuggingCalled('The function load_users() is deprecated. ' . + 'Please use grade_report::get_gradable_users() instead.'); $this->assertCount(1, $users); // Change the viewsuspendedusers capabilities and set the user preference to display suspended users. @@ -79,7 +81,8 @@ class screen_test extends \advanced_testcase { $this->setUser($teacher); $screentest = new gradereport_singleview_screen_testable($course->id, 0, $group->id); $users = $screentest->test_load_users(); - $this->assertDebuggingCalled('The function load_users() is deprecated. Please use get_gradable_users() instead.'); + $this->assertDebuggingCalled('The function load_users() is deprecated. ' . + 'Please use grade_report::get_gradable_users() instead.'); $this->assertCount(2, $users); // Change the capability again, now the user can't see the suspended enrolments. @@ -87,13 +90,15 @@ class screen_test extends \advanced_testcase { set_user_preference('grade_report_showonlyactiveenrol', false, $teacher); accesslib_clear_all_caches_for_unit_testing(); $users = $screentest->test_load_users(); - $this->assertDebuggingCalled('The function load_users() is deprecated. Please use get_gradable_users() instead.'); + $this->assertDebuggingCalled('The function load_users() is deprecated. ' . + 'Please use grade_report::get_gradable_users() instead.'); $this->assertCount(1, $users); // Now, activate the user enrolment again. We shall get 2 users now. $this->getDataGenerator()->enrol_user($user2->id, $course->id, $roleteacher->id, 'manual', 0, 0, ENROL_USER_ACTIVE); $users = $screentest->test_load_users(); - $this->assertDebuggingCalled('The function load_users() is deprecated. Please use get_gradable_users() instead.'); + $this->assertDebuggingCalled('The function load_users() is deprecated. ' . + 'Please use grade_report::get_gradable_users() instead.'); $this->assertCount(2, $users); } } diff --git a/grade/report/upgrade.txt b/grade/report/upgrade.txt index 55bdb2e12b6..d0e91b3512b 100644 --- a/grade/report/upgrade.txt +++ b/grade/report/upgrade.txt @@ -9,7 +9,7 @@ information provided here is intended especially for developers. === 4.3 === * The load_users() method in the gradereport_singleview\local\screen class has been deprecated. Please use - get_gradable_users() instead. + grade_report::get_gradable_users() instead. * The \gradereport_singleview\local\screen\select has been deprecated. This class generates the output for the initial view to select the single view item type (user or grade item) which is no longer actively used as we do not provide direct links to it. diff --git a/grade/report/user/index.php b/grade/report/user/index.php index cc052cff5ae..891c3558fea 100644 --- a/grade/report/user/index.php +++ b/grade/report/user/index.php @@ -117,7 +117,7 @@ if (has_capability('moodle/grade:viewall', $context)) { $userid = $lastvieweduserid; } - $gradableusers = get_gradable_users($courseid, $currentgroup); + $gradableusers = grade_report::get_gradable_users($courseid, $currentgroup); // Validate whether the requested user is a valid gradable user in this course. If, not display the user select // zero state. if (empty($gradableusers) || ($userid && !array_key_exists($userid, $gradableusers))) { diff --git a/grade/tests/lib_test.php b/grade/tests/lib_test.php index 54a0793c17b..19417763c5b 100644 --- a/grade/tests/lib_test.php +++ b/grade/tests/lib_test.php @@ -1010,7 +1010,7 @@ class lib_test extends \advanced_testcase { // Now, let's suspend the enrolment of student2. $this->getDataGenerator()->enrol_user($student2->id, $course->id, 'student', 'manual', 0, 0, ENROL_USER_SUSPENDED); // Should return only the active gradable users (student1 and student3). - $gradableusers = get_gradable_users($course->id); + $gradableusers = \grade_report::get_gradable_users($course->id); $this->assertEqualsCanonicalizing([$student1->id, $student3->id], array_keys($gradableusers)); // Give teacher 'viewsuspendedusers' capability and set a preference to display suspended users. @@ -1020,17 +1020,17 @@ class lib_test extends \advanced_testcase { $this->setUser($teacher); // Should return all gradable users (including suspended enrolments). - $gradableusers = get_gradable_users($course->id); + $gradableusers = \grade_report::get_gradable_users($course->id); $this->assertEqualsCanonicalizing([$student1->id, $student2->id, $student3->id], array_keys($gradableusers)); // Reactivate the course enrolment of student2. $this->getDataGenerator()->enrol_user($student2->id, $course->id, 'student', 'manual', 0, 0, ENROL_USER_ACTIVE); $this->setAdminUser(); // Should return all gradable users from group1 (student1 and student2). - $gradableusers = get_gradable_users($course->id, $group1->id); + $gradableusers = \grade_report::get_gradable_users($course->id, $group1->id); $this->assertEqualsCanonicalizing([$student1->id, $student2->id], array_keys($gradableusers)); // Should return all gradable users from group2 (student3). - $gradableusers = get_gradable_users($course->id, $group2->id); + $gradableusers = \grade_report::get_gradable_users($course->id, $group2->id); $this->assertEqualsCanonicalizing([$student3->id], array_keys($gradableusers)); } } diff --git a/grade/upgrade.txt b/grade/upgrade.txt index f26cb9b2721..f9fc3620b16 100644 --- a/grade/upgrade.txt +++ b/grade/upgrade.txt @@ -6,6 +6,7 @@ Information provided here is intended especially for developers. * The grade_structure::get_element_type_string() function has been deprecated. Please use grade_helper::get_element_type_string() instead. * The grade_structure::get_element_header() function has been deprecated. Please use grade_helper::get_element_header() instead. * The grade_structure::get_activity_link() functions has been deprecated. Please use grade_helper::get_activity_link() instead. +* The function get_gradable_users() in grade/lib.php has extra param now to retrieve only active enrolments. === 4.3 === * The $showtitle parameter in the print_grade_page_head function located inside grade/lib.php has been deprecated and is not used anymore. -- 2.11.4.GIT