From 620f6322d95fc311ccff377d2b5bcaeca0887043 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Wed, 29 Mar 2023 10:59:13 +0800 Subject: [PATCH 1/5] MDL-76445 grade: Helper that returns all gradable users in a course --- grade/lib.php | 31 ++++++++++++++++++++ grade/tests/lib_test.php | 63 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/grade/lib.php b/grade/lib.php index 147c0dc9430..f594354f7c6 100644 --- a/grade/lib.php +++ b/grade/lib.php @@ -781,6 +781,37 @@ function grade_get_plugin_info($courseid, $active_type, $active_plugin) { return $plugin_info; } +/** + * 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. + */ +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); + + $course = get_course($courseid); + $gui = new graded_users_iterator($course, null, $groupid); + $gui->require_active_enrolment($onlyactiveenrol); + $gui->init(); + + // Flatten the users. + $users = []; + while ($user = $gui->next_user()) { + $users[$user->user->id] = $user->user; + } + $gui->close(); + + return $users; +} + /** * A simple class containing info about grade plugins. * Can be subclassed for special rules diff --git a/grade/tests/lib_test.php b/grade/tests/lib_test.php index a8173ef80b1..eedff6ddc07 100644 --- a/grade/tests/lib_test.php +++ b/grade/tests/lib_test.php @@ -711,4 +711,67 @@ class lib_test extends \advanced_testcase { $this->assertArrayHasKey('assign', $gradeitems2); $this->assertArrayHasKey('manual', $gradeitems2); } + + /** + * Test get_gradable_users() function. + * + * @covers ::get_gradable_users + */ + public function test_get_gradable_users() { + global $DB; + + $this->setAdminUser(); + $this->resetAfterTest(true); + + $roleteacher = $DB->get_record('role', ['shortname' => 'teacher'], '*', MUST_EXIST); + + // Create a course. + $course = $this->getDataGenerator()->create_course(); + $coursecontext = \context_course::instance($course->id); + // Create groups. + $group1 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + $group2 = $this->getDataGenerator()->create_group(['courseid' => $course->id]); + // Create and enrol a teacher and some students into the course. + $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher'); + $student1 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student2 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + $student3 = $this->getDataGenerator()->create_and_enrol($course, 'student'); + // Add student1 and student2 to group1. + $this->getDataGenerator()->create_group_member(['groupid' => $group1->id, 'userid' => $student1->id]); + $this->getDataGenerator()->create_group_member(['groupid' => $group1->id, 'userid' => $student2->id]); + // Add student3 to group2. + $this->getDataGenerator()->create_group_member(['groupid' => $group2->id, 'userid' => $student3->id]); + + // Perform a regrade before creating the report. + grade_regrade_final_grades($course->id); + // Should return all gradable users (only students). + $gradableusers = get_gradable_users($course->id); + $this->assertEqualsCanonicalizing([$student1->id, $student2->id, $student3->id], array_keys($gradableusers)); + + // 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); + $this->assertEqualsCanonicalizing([$student1->id, $student3->id], array_keys($gradableusers)); + + // Give teacher 'viewsuspendedusers' capability and set a preference to display suspended users. + assign_capability('moodle/course:viewsuspendedusers', CAP_ALLOW, $roleteacher->id, $coursecontext, true); + set_user_preference('grade_report_showonlyactiveenrol', false, $teacher); + accesslib_clear_all_caches_for_unit_testing(); + + $this->setUser($teacher); + // Should return all gradable users (including suspended enrolments). + $gradableusers = 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); + $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); + $this->assertEqualsCanonicalizing([$student3->id], array_keys($gradableusers)); + } } From 6e7ded8feb51a95fe0682400019b84c2c6c9acdf Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Wed, 29 Mar 2023 12:44:39 +0800 Subject: [PATCH 2/5] MDL-76445 gradereport_user: Usability fixes --- grade/report/user/index.php | 38 ++++-- .../user/tests/behat/view_usereport.feature | 112 +++++++++++++++++- 2 files changed, 135 insertions(+), 15 deletions(-) diff --git a/grade/report/user/index.php b/grade/report/user/index.php index b70773de593..1d4e542d6a9 100644 --- a/grade/report/user/index.php +++ b/grade/report/user/index.php @@ -106,6 +106,20 @@ if (has_capability('moodle/grade:viewall', $context)) { $userid = $USER->id; } + // If there is a stored (last viewed) user in a session variable, bypass the user select zero state and display the + // report for that user. + $lastvieweduserid = $SESSION->gradereport_user["useritem-{$context->id}"] ?? null; + if (is_null($userid) && !is_null($lastvieweduserid)) { + $userid = $lastvieweduserid; + } + + $gradableusers = 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))) { + $userid = null; + } + $defaultgradeshowactiveenrol = !empty($CFG->grade_report_showonlyactiveenrol); $showonlyactiveenrol = get_user_preferences('grade_report_showonlyactiveenrol', $defaultgradeshowactiveenrol); $showonlyactiveenrol = $showonlyactiveenrol || !has_capability('moodle/course:viewsuspendedusers', $context); @@ -121,20 +135,22 @@ if (has_capability('moodle/grade:viewall', $context)) { $gui->init(); if (is_null($userid)) { // Zero state. - $report = new gradereport_user\report\user($courseid, $gpr, $context, $USER->id, $viewasuser); - - if (isset($report)) { - // Trigger report viewed event. - $report->viewed(); - } - $actionbar = new \gradereport_user\output\action_bar($context, $userview, null, $currentgroup); // Print header. print_grade_page_head($courseid, 'report', 'user', false, false, null, true, null, null, null, $actionbar); - echo $report->output_report_zerostate(); + if (empty($gradableusers)) { // There are no available gradable users, display a notification. + $message = $currentgroup ? get_string('nostudentsingroup') : get_string('nostudentsyet'); + echo $OUTPUT->notification($message, 'warning', false); + } else { // Otherwise, display the zero state template. + $report = new gradereport_user\report\user($courseid, $gpr, $context, $USER->id, $viewasuser); + echo $report->output_report_zerostate(); + } } else if ($userid == 0) { // Show all reports. + // Store the id of the current user item in a session variable which represents the last viewed item. + $SESSION->gradereport_user["useritem-{$context->id}"] = $userid; + $actionbar = new \gradereport_user\output\action_bar($context, $userview, 0, $currentgroup); print_grade_page_head($courseid, 'report', 'user', false, false, null, true, null, null, null, $actionbar); @@ -152,6 +168,9 @@ if (has_capability('moodle/grade:viewall', $context)) { } $gui->close(); } else { // Show one user's report. + // Store the id of the current user item in a session variable which represents the last viewed item. + $SESSION->gradereport_user["useritem-{$context->id}"] = $userid; + $report = new gradereport_user\report\user($courseid, $gpr, $context, $userid, $viewasuser); $actionbar = new \gradereport_user\output\action_bar($context, $userview, $report->user->id, $currentgroup); @@ -189,9 +208,6 @@ if (has_capability('moodle/grade:viewall', $context)) { if (isset($report)) { // Trigger report viewed event. $report->viewed(); -} else { - echo html_writer::tag('div', '', ['class' => 'clearfix']); - echo $OUTPUT->notification(get_string('nostudentsyet')); } echo $OUTPUT->footer(); diff --git a/grade/report/user/tests/behat/view_usereport.feature b/grade/report/user/tests/behat/view_usereport.feature index 75ee486a29b..7448f4b9bea 100644 --- a/grade/report/user/tests/behat/view_usereport.feature +++ b/grade/report/user/tests/behat/view_usereport.feature @@ -1,17 +1,121 @@ -@core @core_grades @gradereport_user +@core @core_grades @gradereport_user @javascript Feature: We can use the user report As a user I browse to the User report Background: Given the following "courses" exist: - | fullname | shortname | category | - | Course 1 | C1 | 0 | + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | - @javascript Scenario: Verify we can view a user grade report with no users enrolled. Given I log in as "admin" And I am on "Course 1" course homepage And I navigate to "View > User report" in the course gradebook And I click on "All users (0)" in the "user" search widget Then I should see "There are no students enrolled in this course." + + Scenario: Teacher sees his last viewed user report when navigating back to the gradebook user report. + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | teacher2 | Teacher | 2 | teacher2@example.com | + | student1 | Student | 1 | student1@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | teacher2 | C1 | editingteacher | + | student1 | C1 | student | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "View > User report" in the course gradebook + And I should see "Select a user above to view their report" in the "region-main" "region" + And I click on "Student 1" in the "user" search widget + And I should see "Student 1" in the "region-main" "region" + And I am on "Course 1" course homepage + When I navigate to "View > User report" in the course gradebook + Then I should not see "Select a user above to view their report" in the "region-main" "region" + And I should see "Student 1" in the "region-main" "region" + And I log out + And I log in as "teacher2" + And I am on "Course 1" course homepage + And I navigate to "View > User report" in the course gradebook + And I should see "Select a user above to view their report" in the "region-main" "region" + + Scenario: Teacher sees his last viewed user report if the user is a part of the the current group. + Given the following "groups" exist: + | name | course | idnumber | participation | + | Group 1 | C1 | G1 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "group members" exist: + | user | group | + | student2 | G1 | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "View > User report" in the course gradebook + And I click on "Student 2" in the "user" search widget + And I navigate to "View > Grader report" in the course gradebook + And I click on "Group 1" in the "group" search widget + When I navigate to "View > User report" in the course gradebook + Then I should see "Student 2" in the "region-main" "region" + And I should not see "Select a user above to view their report" in the "region-main" "region" + + Scenario: Teacher does not see the last viewed user if the user is not a part of the the current group. + Given the following "groups" exist: + | name | course | idnumber | participation | + | Group 1 | C1 | G1 | 1 | + And the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And the following "group members" exist: + | user | group | + | student2 | G1 | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "View > User report" in the course gradebook + And I click on "Student 1" in the "user" search widget + And I navigate to "View > Grader report" in the course gradebook + And I click on "Group 1" in the "group" search widget + When I navigate to "View > User report" in the course gradebook + Then I should see "Select a user above to view their report" in the "region-main" "region" + And I should not see "Student 1" in the "region-main" "region" + + Scenario: Teacher does not see his last viewed user report if the user is no longer enrolled in the course. + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I navigate to "View > User report" in the course gradebook + And I click on "Student 1" in the "user" search widget + And I should see "Student 1" in the "region-main" "region" + And I navigate to course participants + And I click on "Unenrol" "icon" in the "Student 1" "table_row" + And I click on "Unenrol" "button" in the "Unenrol" "dialogue" + And I am on "Course 1" course homepage + When I navigate to "View > User report" in the course gradebook + Then I should see "Select a user above to view their report" in the "region-main" "region" + And I should not see "Student 1" in the "region-main" "region" From 26f4584200212fa5107610d9d684d4dee568de59 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Thu, 30 Mar 2023 22:34:53 +0800 Subject: [PATCH 3/5] MDL-76445 gradereport_singleview: Usability fixes --- grade/report/singleview/index.php | 128 +++++++++++++----- .../singleview/tests/behat/singleview.feature | 107 ++++++++++++++- 2 files changed, 197 insertions(+), 38 deletions(-) diff --git a/grade/report/singleview/index.php b/grade/report/singleview/index.php index f9867c08c62..4569e3cbf51 100644 --- a/grade/report/singleview/index.php +++ b/grade/report/singleview/index.php @@ -33,32 +33,15 @@ $groupid = optional_param('group', null, PARAM_INT); // Making this work with profile reports. $userid = optional_param('userid', null, PARAM_INT); - -$defaulttype = $userid ? 'user' : 'select'; - $itemid = optional_param('itemid', null, PARAM_INT); -$itemtype = optional_param('item', $defaulttype, PARAM_TEXT); +$itemtype = optional_param('item', null, PARAM_TEXT); $page = optional_param('page', 0, PARAM_INT); $perpage = optional_param('perpage', 100, PARAM_INT); $edit = optional_param('edit', -1, PARAM_BOOL); // Sticky editing mode. -if (empty($itemid) && ($itemtype !== 'user_select' && $itemtype !== 'grade_select')) { - $itemid = $userid; - $itemtype = $defaulttype; -} - $courseparams = ['id' => $courseid]; -$pageparams = [ - 'id' => $courseid, - 'group' => $groupid, - 'userid' => $userid, - 'itemid' => $itemid, - 'item' => $itemtype, - 'page' => $page, - 'perpage' => $perpage, -]; -$PAGE->set_url(new moodle_url('/grade/report/singleview/index.php', $pageparams)); + $PAGE->set_pagelayout('report'); $PAGE->set_other_editing_capability('moodle/grade:edit'); @@ -68,10 +51,6 @@ if (!$course = $DB->get_record('course', $courseparams)) { require_login($course); -if (!in_array($itemtype, gradereport_singleview\report\singleview::valid_screens())) { - throw new \moodle_exception('notvalid', 'gradereport_singleview', '', $itemtype); -} - $context = context_course::instance($course->id); // This is the normal requirements. @@ -85,6 +64,90 @@ $gpr = new grade_plugin_return([ 'courseid' => $courseid ]); +// Last selected report session tracking. +if (!isset($USER->grade_last_report)) { + $USER->grade_last_report = []; +} +$USER->grade_last_report[$course->id] = 'singleview'; +// If the item type is not explicitly defined or not valid, try to use the last viewed one (obtain in from the session) +// or fallback to the user select (zero) state. +if (!$itemtype || !in_array($itemtype, \gradereport_singleview\report\singleview::valid_screens())) { + $itemtype = isset($SESSION->gradereport_singleview["itemtype-{$context->id}"]) ? + $SESSION->gradereport_singleview["itemtype-{$context->id}"] : 'user_select'; +} + +$currentgroup = $gpr->groupid; +// To make some other functions work better later. +if (!$currentgroup) { + $currentgroup = null; +} + +$lastvieweduseritemid = $SESSION->gradereport_singleview["useritem-{$context->id}"] ?? null; +$lastviewedgradeitemid = $SESSION->gradereport_singleview["gradeitem-{$context->id}"] ?? null; + +switch ($itemtype) { + case 'user_select': + // 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))) { + $itemtype = 'user'; + $itemid = $lastvieweduseritemid; + } else { + $itemid = null; + } + break; + case 'user': + if (is_null($itemid)) { + $itemid = $userid ?? $lastvieweduseritemid; + } + // 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))) { + $itemtype = 'user_select'; + } + break; + case 'grade_select': + // If there is a stored grade item (last viewed) in a session variable, bypass the grade item select zero state + // and display this grade item. + if ($lastviewedgradeitemid) { + $itemtype = 'grade'; + $itemid = $lastviewedgradeitemid; + } else { + $itemid = null; + } + break; + case 'grade': + // If there is a stored grade item (last viewed) in a session variable, use it. + if (is_null($itemid) && $lastviewedgradeitemid) { + $itemid = $lastviewedgradeitemid; + } + $gtree = new grade_tree($courseid, false, false, null, !$CFG->enableoutcomes); + $gradeableitems = $gtree->get_items(); + // The item id (grade item id) cannot be defined, display the grade select zero state. + if (is_null($itemid) || !array_key_exists($itemid, $gtree->get_items())) { + $itemtype = 'grade_select'; + } + break; +} + +$report = new gradereport_singleview\report\singleview($courseid, $gpr, $context, $itemtype, $itemid); + +$pageparams = [ + 'id' => $courseid, + 'userid' => $userid, + 'itemid' => $itemid, + 'item' => $itemtype, + 'page' => $page, + 'perpage' => $perpage, +]; + +if (!is_null($groupid)) { + $pageparams['group'] = $groupid; +} + +$PAGE->set_url(new moodle_url('/grade/report/singleview/index.php', $pageparams)); + // Build editing on/off button for themes that need it. $button = ''; if ($PAGE->user_allowed_editing() && !$PAGE->theme->haseditswitch) { @@ -97,14 +160,6 @@ if ($PAGE->user_allowed_editing() && !$PAGE->theme->haseditswitch) { $button = $OUTPUT->edit_button(new moodle_url($PAGE->url, $options), 'get'); } -// Last selected report session tracking. -if (!isset($USER->grade_last_report)) { - $USER->grade_last_report = []; -} -$USER->grade_last_report[$course->id] = 'singleview'; - -$report = new gradereport_singleview\report\singleview($courseid, $gpr, $context, $itemtype, $itemid); - $reportname = $report->screen->heading(); if ($itemtype == 'user' || $itemtype == 'user_select') { @@ -149,6 +204,11 @@ if ($data = data_submitted()) { grade_regrade_final_grades_if_required($course); echo $report->output(); +// Save the screen state in a session variable as last viewed state. +$SESSION->gradereport_singleview["itemtype-{$context->id}"] = $itemtype; +if ($itemid) { + $SESSION->gradereport_singleview["{$itemtype}item-{$context->id}"] = $itemid; +} if (($itemtype !== 'select') && ($itemtype !== 'grade_select') &&($itemtype !== 'user_select')) { $item = (isset($userid)) ? $userid : $itemid; @@ -157,12 +217,6 @@ if (($itemtype !== 'select') && ($itemtype !== 'grade_select') &&($itemtype !== $showonlyactiveenrol = get_user_preferences('grade_report_showonlyactiveenrol', $defaultgradeshowactiveenrol); $showonlyactiveenrol = $showonlyactiveenrol || !has_capability('moodle/course:viewsuspendedusers', $context); - $currentgroup = $gpr->groupid; - - // To make some other functions work better later. - if (!$currentgroup) { - $currentgroup = null; - } $gui = new graded_users_iterator($course, null, $currentgroup); $gui->require_active_enrolment($showonlyactiveenrol); $gui->init(); diff --git a/grade/report/singleview/tests/behat/singleview.feature b/grade/report/singleview/tests/behat/singleview.feature index 6152d518202..78d5a36dbf3 100644 --- a/grade/report/singleview/tests/behat/singleview.feature +++ b/grade/report/singleview/tests/behat/singleview.feature @@ -7,11 +7,12 @@ Feature: We can use Single view Background: Given the following "courses" exist: | fullname | shortname | category | - | Course 1 | C1 | 0 | + | Course 1 | C1 | 0 | And the following "users" exist: | username | firstname | lastname | email | idnumber | middlename | alternatename | firstnamephonetic | lastnamephonetic | | teacher1 | Teacher | 1 | teacher1@example.com | t1 | | fred | | | | teacher2 | No edit | 1 | teacher2@example.com | t2 | | nick | | | + | teacher3 | Teacher | 3 | teacher3@example.com | t3 | | jack | | | | student1 | Grainne | Beauchamp | student1@example.com | s1 | Ann | Jill | Gronya | Beecham | | student2 | Niamh | Cholmondely | student2@example.com | s2 | Jane | Nina | Nee | Chumlee | | student3 | Siobhan | Desforges | student3@example.com | s3 | Sarah | Sev | Shevon | De-forjay | @@ -29,6 +30,7 @@ Feature: We can use Single view | user | course | role | | teacher1 | C1 | editingteacher | | teacher2 | C1 | teacher | + | teacher3 | C1 | teacher | | student1 | C1 | student | | student2 | C1 | student | | student3 | C1 | student | @@ -171,3 +173,106 @@ Feature: We can use Single view And "new grade item 1" "link" should not exist in the "//tbody//tr[position()=1]//td[position()=2]" "xpath_element" Then "Category total" "link" should not exist in the "//tbody//tr[position()=2]//td[position()=2]" "xpath_element" And "Course total" "link" should not exist in the "//tbody//tr[position()=last()]//td[position()=2]" "xpath_element" + + Scenario: Teacher sees his last viewed singleview report type when navigating back to the gradebook singleview report. + Given I navigate to "View > Single view" in the course gradebook + And I should see "Select a user above to view all their grades" in the "region-main" "region" + And I click on "Grade items" "link" + And I should see "Select a grade item above" in the "region-main" "region" + And I am on "Course 1" course homepage + When I navigate to "View > Single view" in the course gradebook + Then I should see "Select a grade item above" in the "region-main" "region" + And I log out + And I log in as "teacher3" + And I am on "Course 1" course homepage + And I navigate to "View > Single view" in the course gradebook + And I should see "Select a user above to view all their grades" in the "region-main" "region" + + Scenario: Teacher sees his last viewed user report when navigating back to the gradebook singleview report. + Given I navigate to "View > Single view" in the course gradebook + And I click on "Gronya,Beecham" in the "user" search widget + And I should see "Gronya,Beecham" in the "region-main" "region" + And I am on "Course 1" course homepage + When I navigate to "View > Single view" in the course gradebook + Then I should not see "Select a user above to view all their grades" in the "region-main" "region" + And I should see "Gronya,Beecham" in the "region-main" "region" + And I log out + And I log in as "teacher3" + And I am on "Course 1" course homepage + And I navigate to "View > Single view" in the course gradebook + And I should see "Select a user above to view all their grades" in the "region-main" "region" + + Scenario: Teacher sees his last viewed grade item report when navigating back to the gradebook singleview report. + Given I navigate to "View > Single view" in the course gradebook + And I click on "Grade items" "link" + And I click on "Test assignment one" in the "grade" search widget + And I should see "Test assignment one" in the "region-main" "region" + And I am on "Course 1" course homepage + When I navigate to "View > Single view" in the course gradebook + Then I should not see "Select a grade item above" in the "region-main" "region" + And I should see "Test assignment one" in the "region-main" "region" + And I log out + And I log in as "teacher3" + And I am on "Course 1" course homepage + And I navigate to "View > Single view" in the course gradebook + And I should see "Select a user above to view all their grades" in the "region-main" "region" + + Scenario: Teacher sees his last viewed user report if the user is a part of the the current group. + Given the following "groups" exist: + | name | course | idnumber | participation | + | Group 1 | C1 | G1 | 1 | + And the following "group members" exist: + | user | group | + | student2 | G1 | + And I am on the "Course 1" "course editing" page + And I expand all fieldsets + And I set the field "Group mode" to "Visible groups" + And I press "Save and display" + And I navigate to "View > Single view" in the course gradebook + And I click on "Nee,Chumlee" in the "user" search widget + And I navigate to "View > Grader report" in the course gradebook + And I click on "Group 1" in the "group" search widget + When I navigate to "View > Single view" in the course gradebook + Then I should see "Nee,Chumlee" in the "region-main" "region" + And I should not see "Select a user above to view all their grades" in the "region-main" "region" + + Scenario: Teacher does not see his last viewed user report if the user is not a part of the the current group. + Given the following "groups" exist: + | name | course | idnumber | participation | + | Group 1 | C1 | G1 | 1 | + And the following "group members" exist: + | user | group | + | student2 | G1 | + And I am on "Course 1" course homepage + And I navigate to "Settings" in current page administration + And I expand all fieldsets + And I set the field "Group mode" to "Visible groups" + And I press "Save and display" + And I navigate to "View > Single view" in the course gradebook + And I click on "Gronya,Beecham" in the "user" search widget + And I navigate to "View > Grader report" in the course gradebook + And I click on "Group 1" in the "group" search widget + When I navigate to "View > Single view" in the course gradebook + Then I should see "Select a user above to view all their grades" in the "region-main" "region" + And I should not see "Gronya,Beecham" in the "region-main" "region" + + Scenario: Teacher does not see his last viewed user report if that user is no longer enrolled in the course. + Given I navigate to "View > Single view" in the course gradebook + And I click on "Gronya,Beecham" in the "user" search widget + And I navigate to course participants + And I click on "Unenrol" "icon" in the "Gronya,Beecham" "table_row" + And I click on "Unenrol" "button" in the "Unenrol" "dialogue" + When I navigate to "View > Single view" in the course gradebook + Then I should see "Select a user above to view all their grades" in the "region-main" "region" + And I should not see "Gronya,Beecham" in the "region-main" "region" + + Scenario: Teacher does not see his last viewed grade item report if the item no longer exists in the course. + Given I navigate to "View > Single view" in the course gradebook + And I click on "Grade items" "link" + And I click on "Test assignment four" in the "grade" search widget + And I am on "Course 1" course homepage with editing mode on + And I delete "Test assignment four" activity + And I run all adhoc tasks + When I navigate to "View > Single view" in the course gradebook + Then I should see "Select a grade item above" in the "region-main" "region" + And I should not see "Test grade item" in the "region-main" "region" From 41ec352b36d628bf6b2fed359c97624831dd3912 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 9 Jun 2023 12:44:01 +0800 Subject: [PATCH 4/5] MDL-76445 gradereport_singleview: Deprecate initial zero state view Deprecates the \gradereport_singleview\local\screen\select class which is used to generate the initial view to select the singleview item type and the strings used by the related mustache template. This view is no longer used as we do not provide direct links to it anymore. --- .../classes/local/screen/select.php | 31 +++++++++++++++++++ .../report/singleview/lang/en/deprecated.txt | 1 + .../lang/en/gradereport_singleview.php | 4 ++- grade/report/upgrade.txt | 6 ++++ 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/grade/report/singleview/classes/local/screen/select.php b/grade/report/singleview/classes/local/screen/select.php index a10f8cc2f84..4a7968d4562 100644 --- a/grade/report/singleview/classes/local/screen/select.php +++ b/grade/report/singleview/classes/local/screen/select.php @@ -32,6 +32,7 @@ defined('MOODLE_INTERNAL') || die; /** * The gradebook simple view - initial view to select your search options * + * @deprecated since Moodle 4.3 * @package gradereport_singleview * @copyright 2014 Moodle Pty Ltd (http://moodle.com) * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later @@ -44,11 +45,15 @@ class select extends screen { /** * Initialise this screen * + * @deprecated since Moodle 4.3 * @param bool $selfitemisempty Has an item been selected (will be false) */ public function init($selfitemisempty = false) { global $DB; + debugging('The function ' . __FUNCTION__ . '() is deprecated as part of the deprecation process of the ' . + '\gradereport_singleview\local\screen\select class which is no longer used.', DEBUG_DEVELOPER); + $roleids = explode(',', get_config('moodle', 'gradebookroles')); $this->items = []; @@ -66,20 +71,28 @@ class select extends screen { /** * Get the type of items on this screen, not valid so return false. * + * @deprecated since Moodle 4.3 * @return string|null */ public function item_type(): ?string { + debugging('The function ' . __FUNCTION__ . '() is deprecated as part of the deprecation process of the ' . + '\gradereport_singleview\local\screen\select class which is no longer used.', DEBUG_DEVELOPER); + return false; } /** * Return the HTML for the page. * + * @deprecated since Moodle 4.3 * @return string */ public function html(): string { global $OUTPUT, $COURSE; + debugging('The function ' . __FUNCTION__ . '() is deprecated as part of the deprecation process of the ' . + '\gradereport_singleview\local\screen\select class which is no longer used.', DEBUG_DEVELOPER); + if ($this->itemid === null) { $userlink = new \moodle_url('/grade/report/singleview/index.php', ['id' => $COURSE->id, 'item' => 'user_select']); $gradelink = new \moodle_url('/grade/report/singleview/index.php', ['id' => $COURSE->id, 'item' => 'grade_select']); @@ -137,17 +150,27 @@ class select extends screen { /** * Should we show the next prev selector? + * + * @deprecated since Moodle 4.3 * @return bool */ public function supports_next_prev(): bool { + debugging('The function ' . __FUNCTION__ . '() is deprecated as part of the deprecation process of the ' . + '\gradereport_singleview\local\screen\select class which is no longer used.', DEBUG_DEVELOPER); + return false; } /** * Should we show the base singlereport group selector? + * + * @deprecated since Moodle 4.3 * @return bool */ public function display_group_selector(): bool { + debugging('The function ' . __FUNCTION__ . '() is deprecated as part of the deprecation process of the ' . + '\gradereport_singleview\local\screen\select class which is no longer used.', DEBUG_DEVELOPER); + if ($this->itemid === null) { return false; } @@ -157,18 +180,26 @@ class select extends screen { /** * Get the heading for the screen. * + * @deprecated since Moodle 4.3 * @return string */ public function heading(): string { + debugging('The function ' . __FUNCTION__ . '() is deprecated as part of the deprecation process of the ' . + '\gradereport_singleview\local\screen\select class which is no longer used.', DEBUG_DEVELOPER); + return ' '; } /** * Does this screen support paging? * + * @deprecated since Moodle 4.3 * @return bool */ public function supports_paging(): bool { + debugging('The function ' . __FUNCTION__ . '() is deprecated as part of the deprecation process of the ' . + '\gradereport_singleview\local\screen\select class which is no longer used.', DEBUG_DEVELOPER); + return false; } } diff --git a/grade/report/singleview/lang/en/deprecated.txt b/grade/report/singleview/lang/en/deprecated.txt index d3a25488b05..ec3b90cad95 100644 --- a/grade/report/singleview/lang/en/deprecated.txt +++ b/grade/report/singleview/lang/en/deprecated.txt @@ -1 +1,2 @@ filtergrades,gradereport_singleview +viewsingleuserorgradeitem,gradereport_singleview diff --git a/grade/report/singleview/lang/en/gradereport_singleview.php b/grade/report/singleview/lang/en/gradereport_singleview.php index a45c8481b8d..b8e31e8d10c 100644 --- a/grade/report/singleview/lang/en/gradereport_singleview.php +++ b/grade/report/singleview/lang/en/gradereport_singleview.php @@ -79,7 +79,6 @@ $string['ariareporttype'] = 'Select a report type to view'; $string['viewby'] = 'View by'; $string['viewsingleuser'] = 'Select a user above to view all their grades'; $string['viewsinglegradeitem'] = 'Select a grade item above'; -$string['viewsingleuserorgradeitem'] = 'View all the grades of a single user or grade item.'; $string['searchgrades'] = 'Search grade items'; $string['selectagrade'] = 'Select a grade item'; $string['selectgradeitemlink'] = 'Select a grade item'; @@ -88,3 +87,6 @@ $string['unsaveddatawarning'] = 'Unsaved data warning'; // Deprecated since Moodle 4.1. $string['filtergrades'] = 'Show grades for {$a}.'; + +// Deprecated since Moodle 4.3. +$string['viewsingleuserorgradeitem'] = 'View all the grades of a single user or grade item.'; diff --git a/grade/report/upgrade.txt b/grade/report/upgrade.txt index 9d11a16f32d..cb533144247 100644 --- a/grade/report/upgrade.txt +++ b/grade/report/upgrade.txt @@ -1,6 +1,12 @@ This files describes API changes in /grade/report/*, information provided here is intended especially for developers. +=== 4.3 === + +* 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. + === 4.2 === * 'Show calculations' setting has been removed from grader report (link is moved to grade action menu) * 'Show activity icons' setting has been removed from grader report From c95c63276470fd7bfa1f4c9961a1c89ed4cca02a Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Fri, 9 Jun 2023 16:14:22 +0800 Subject: [PATCH 5/5] MDL-76445 gradereport_singleview: Deprecate load_users() Replaces current usages of \gradereport_singleview\local\screen::load_users() with get_gradable_users() and deprecates load_users(). The only remaining calls of this method are unit test related. --- .../singleview/classes/local/screen/grade.php | 2 +- .../classes/local/screen/screen.php | 25 +++++-------------- .../singleview/classes/local/screen/user.php | 2 +- grade/report/singleview/tests/screen_test.php | 5 ++++ grade/report/upgrade.txt | 2 ++ 5 files changed, 15 insertions(+), 21 deletions(-) diff --git a/grade/report/singleview/classes/local/screen/grade.php b/grade/report/singleview/classes/local/screen/grade.php index 50648ab5cbc..25d51669ffc 100644 --- a/grade/report/singleview/classes/local/screen/grade.php +++ b/grade/report/singleview/classes/local/screen/grade.php @@ -151,7 +151,7 @@ class grade extends tablelike implements selectable_items, filterable_items { */ public function init($selfitemisempty = false) { - $this->items = $this->load_users(); + $this->items = 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 b26c908dd93..1ef3ab51d50 100644 --- a/grade/report/singleview/classes/local/screen/screen.php +++ b/grade/report/singleview/classes/local/screen/screen.php @@ -405,28 +405,15 @@ abstract class screen { /** * Load a valid list of users for this gradebook as the screen "items". - * @return array $users A list of enroled users. + * + * @deprecated since Moodle 4.3 + * @return array A list of enroled users. */ protected function load_users(): array { - global $CFG; + debugging('The function ' . __FUNCTION__ . '() is deprecated. Please use get_gradable_users() instead.', + DEBUG_DEVELOPER); - // Create a graded_users_iterator because it will properly check the groups etc. - $defaultgradeshowactiveenrol = !empty($CFG->grade_report_showonlyactiveenrol); - $showonlyactiveenrol = get_user_preferences('grade_report_showonlyactiveenrol', $defaultgradeshowactiveenrol); - $showonlyactiveenrol = $showonlyactiveenrol || !has_capability('moodle/course:viewsuspendedusers', $this->context); - - require_once($CFG->dirroot.'/grade/lib.php'); - $gui = new \graded_users_iterator($this->course, null, $this->groupid); - $gui->require_active_enrolment($showonlyactiveenrol); - $gui->init(); - - // Flatten the users. - $users = []; - while ($user = $gui->next_user()) { - $users[$user->user->id] = $user->user; - } - $gui->close(); - return $users; + return 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 6ba69fc7e3c..782e01f16fc 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 = $this->load_users(); + $validusers = 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/tests/screen_test.php b/grade/report/singleview/tests/screen_test.php index 38de9ac63da..5e20de527f4 100644 --- a/grade/report/singleview/tests/screen_test.php +++ b/grade/report/singleview/tests/screen_test.php @@ -63,11 +63,13 @@ 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->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->assertCount(1, $users); // Change the viewsuspendedusers capabilities and set the user preference to display suspended users. @@ -77,6 +79,7 @@ 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->assertCount(2, $users); // Change the capability again, now the user can't see the suspended enrolments. @@ -84,11 +87,13 @@ 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->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->assertCount(2, $users); } } diff --git a/grade/report/upgrade.txt b/grade/report/upgrade.txt index cb533144247..86be947c0e0 100644 --- a/grade/report/upgrade.txt +++ b/grade/report/upgrade.txt @@ -3,6 +3,8 @@ 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. * 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.