From 49953fd4cadad9fbfee0cf0bdf7970bb390532a5 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Mon, 25 Nov 2019 10:49:21 +0800 Subject: [PATCH 1/2] MDL-65956 mod_assign: Add proper capability checks and error handling --- mod/assign/gradingbatchoperationsform.php | 2 +- mod/assign/lang/en/assign.php | 1 + mod/assign/locallib.php | 35 +++++++++++++++++++++-- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/mod/assign/gradingbatchoperationsform.php b/mod/assign/gradingbatchoperationsform.php index 5ea1b0128a5..d31b8bfb754 100644 --- a/mod/assign/gradingbatchoperationsform.php +++ b/mod/assign/gradingbatchoperationsform.php @@ -50,7 +50,7 @@ class mod_assign_grading_batch_operations_form extends moodleform { if ($instance['submissiondrafts']) { $options['reverttodraft'] = get_string('reverttodraft', 'assign'); } - if (has_capability('mod/assign:grade', $instance['context'])) { + if (has_capability('mod/assign:editothersubmission', $instance['context'])) { $options['removesubmission'] = get_string('removesubmission', 'assign'); } if ($instance['duedate'] && has_capability('mod/assign:grantextension', $instance['context'])) { diff --git a/mod/assign/lang/en/assign.php b/mod/assign/lang/en/assign.php index 426730864e1..6741cd2a5ff 100644 --- a/mod/assign/lang/en/assign.php +++ b/mod/assign/lang/en/assign.php @@ -597,6 +597,7 @@ $string['userextensiondate'] = 'Extension granted until: {$a}'; $string['useridlistnotcached'] = 'The grade changes were NOT saved, as it was not possible to determine which submission they were for.'; $string['useroverrides'] = 'User overrides'; $string['useroverridesdeleted'] = 'User overrides deleted'; +$string['usersubmissioncannotberemoved'] = 'The submission of {$a} cannot be removed.'; $string['usersnone'] = 'No students have access to this assignment.'; $string['userswhoneedtosubmit'] = 'Users who need to submit: {$a}'; $string['usergrade'] = 'User grade'; diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 20a30a1d530..311932cc4d0 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -176,6 +176,9 @@ class assign { */ private $mostrecentteamsubmission = null; + /** @var array Array of error messages encountered during the execution of assignment related operations. */ + private $errors = array(); + /** * Constructor for the base assign class. * @@ -311,6 +314,24 @@ class assign { $this->course = $course; } + /** + * Set error message. + * + * @param string $message The error message + */ + protected function set_error_message(string $message) { + $this->errors[] = $message; + } + + /** + * Get error messages. + * + * @return array The array of error messages + */ + protected function get_error_messages(): array { + return $this->errors; + } + /** * Get list of feedback plugins installed. * @@ -594,7 +615,14 @@ class assign { // Now show the right view page. if ($action == 'redirect') { $nextpageurl = new moodle_url('/mod/assign/view.php', $nextpageparams); - redirect($nextpageurl); + $messages = ''; + $messagetype = \core\output\notification::NOTIFY_INFO; + $errors = $this->get_error_messages(); + if (!empty($errors)) { + $messages = html_writer::alist($errors, ['class' => 'mb-1 mt-1']); + $messagetype = \core\output\notification::NOTIFY_ERROR; + } + redirect($nextpageurl, $messages, null, $messagetype); return; } else if ($action == 'savegradingresult') { $message = get_string('gradingchangessaved', 'assign'); @@ -7882,7 +7910,10 @@ class assign { global $USER; if (!$this->can_edit_submission($userid, $USER->id)) { - print_error('nopermission'); + $user = core_user::get_user($userid); + $message = get_string('usersubmissioncannotberemoved', 'assign', fullname($user)); + $this->set_error_message($message); + return false; } if ($this->get_instance()->teamsubmission) { From d522d06f07fcd636a6bee03125d27da9a8f1b882 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Mon, 25 Nov 2019 10:49:36 +0800 Subject: [PATCH 2/2] MDL-65956 mod_assign: Add behat tests --- .../behat/bulk_remove_submissions.feature | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 mod/assign/tests/behat/bulk_remove_submissions.feature diff --git a/mod/assign/tests/behat/bulk_remove_submissions.feature b/mod/assign/tests/behat/bulk_remove_submissions.feature new file mode 100644 index 00000000000..8994e295741 --- /dev/null +++ b/mod/assign/tests/behat/bulk_remove_submissions.feature @@ -0,0 +1,207 @@ +@mod @mod_assign +Feature: Bulk remove submissions + In order to reset the assignment submission of multiple students + As a teacher with the capability to edit submissions + I need to be able to remove student submissions by bulk + + Background: + Given the following "courses" exist: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 0 | + 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 "groups" exist: + | name | course | idnumber | + | Group 1 | C1 | G1 | + + @javascript + Scenario: Bulk remove submissions should remove the data that was submitted + Given I log in as "admin" + And I set the following system permissions of "Teacher" role: + | capability | permission | + | mod/assign:editothersubmission | Allow | + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add a "Assignment" to section "1" and I fill the form with: + | Assignment name | Test assignment name | + | Description | Submit your online text | + | assignsubmission_onlinetext_enabled | 1 | + | assignsubmission_file_enabled | 0 | + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student1 submission | + And I press "Save changes" + And I log out + And I log in as "student2" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student2 submission | + And I press "Save changes" + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I navigate to "View all submissions" in current page administration + And I should see "I'm the student1 submission" + And I should see "I'm the student2 submission" + And I set the field "selectall" to "1" + When I set the field "operation" to "Remove submission" + And I click on "Go" "button" confirming the dialogue + Then I should not see "I'm the student1 submission" + And I should not see "I'm the student2 submission" + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I should not see "I'm the student1 submission" + And I log out + And I log in as "student2" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I should not see "I'm the student2 submission1" + + @javascript + Scenario: Bulk remove submissions should be unavailable if the user is missing the editing submission capability + Given I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add a "Assignment" to section "1" and I fill the form with: + | Assignment name | Test assignment name | + | Description | Submit your online text | + | assignsubmission_onlinetext_enabled | 1 | + | assignsubmission_file_enabled | 0 | + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student1 submission | + And I press "Save changes" + And I log out + And I log in as "student2" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student2 submission | + And I press "Save changes" + And I log out + When I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I navigate to "View all submissions" in current page administration + And I should see "I'm the student1 submission" + And I should see "I'm the student2 submission" + And I set the field "selectall" to "1" + Then I should not see "Remove submission" in the "Choose operation" "select" + + @javascript + Scenario: Notification should be displayed when non-group users are selected for submission bulk removal + in separate group mode + Given I log in as "admin" + And I set the following system permissions of "Teacher" role: + | capability | permission | + | mod/assign:editothersubmission | Allow | + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add a "Assignment" to section "1" and I fill the form with: + | Assignment name | Test assignment name | + | Description | Submit your online text | + | assignsubmission_onlinetext_enabled | 1 | + | assignsubmission_file_enabled | 0 | + | groupmode | 1 | + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student1 submission | + And I press "Save changes" + And I log out + And I log in as "student2" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student2 submission | + And I press "Save changes" + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I navigate to "View all submissions" in current page administration + And I should see "I'm the student1 submission" + And I should see "I'm the student2 submission" + And I set the field "selectall" to "1" + When I set the field "operation" to "Remove submission" + And I click on "Go" "button" confirming the dialogue + Then I should see "I'm the student1 submission" + And I should see "I'm the student2 submission" + And I should see "The submission of Student 1 cannot be removed" + And I should see "The submission of Student 2 cannot be removed" + + @javascript + Scenario: Bulk remove submission when group users are added to the bulk + removing submissions process in separate group mode + Given the following "group members" exist: + | user | group | + | student1 | G1 | + | student2 | G1 | + And I log in as "admin" + And I set the following system permissions of "Teacher" role: + | capability | permission | + | mod/assign:editothersubmission | Allow | + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage with editing mode on + And I add a "Assignment" to section "1" and I fill the form with: + | Assignment name | Test assignment name | + | Description | Submit your online text | + | assignsubmission_onlinetext_enabled | 1 | + | assignsubmission_file_enabled | 0 | + | groupmode | 1 | + And I log out + And I log in as "student1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student1 submission | + And I press "Save changes" + And I log out + And I log in as "student2" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I press "Add submission" + And I set the following fields to these values: + | Online text | I'm the student2 submission | + And I press "Save changes" + And I log out + And I log in as "teacher1" + And I am on "Course 1" course homepage + And I follow "Test assignment name" + And I navigate to "View all submissions" in current page administration + And I should see "I'm the student1 submission" + And I should see "I'm the student2 submission" + And I set the field "selectall" to "1" + When I set the field "operation" to "Remove submission" + And I click on "Go" "button" confirming the dialogue + Then I should not see "I'm the student1 submission" + And I should not see "I'm the student2 submission"