From e60edbf0cb75fadec8e621dec693eb8915744a17 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Wed, 29 Mar 2023 12:44:39 +0800 Subject: [PATCH] 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 ffd16cac34a..d772779613b 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, 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, 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"