diff --git a/mod/assign/feedback/comments/locallib.php b/mod/assign/feedback/comments/locallib.php index 2d6e92d4069..0c19e9cc465 100644 --- a/mod/assign/feedback/comments/locallib.php +++ b/mod/assign/feedback/comments/locallib.php @@ -94,7 +94,10 @@ class assign_feedback_comments extends assign_feedback_plugin { $commenttext = $feedbackcomments->commenttext; } } - return optional_param('quickgrade_comments_' . $userid, '', PARAM_TEXT) != $commenttext; + // Note that this handles the difference between empty and not in the quickgrading + // form at all (hidden column). + $newvalue = optional_param('quickgrade_comments_' . $userid, false, PARAM_TEXT); + return ($newvalue !== false) && ($newvalue != $commenttext); } @@ -173,6 +176,11 @@ class assign_feedback_comments extends assign_feedback_plugin { public function save_quickgrading_changes($userid, $grade) { global $DB; $feedbackcomment = $this->get_feedback_comments($grade->id); + $feedbackpresent = optional_param('quickgrade_comments_' . $userid, false, PARAM_TEXT) !== false; + if (!$feedbackpresent) { + // Nothing to save (e.g. hidden column). + return true; + } if ($feedbackcomment) { $feedbackcomment->commenttext = optional_param('quickgrade_comments_' . $userid, '', PARAM_TEXT); return $DB->update_record('assignfeedback_comments', $feedbackcomment); diff --git a/mod/assign/gradingtable.php b/mod/assign/gradingtable.php index 87d0389be8b..b7e32c9cee3 100644 --- a/mod/assign/gradingtable.php +++ b/mod/assign/gradingtable.php @@ -754,6 +754,9 @@ class assign_grading_table extends table_sql implements renderable { id="selectuser_' . $row->userid . '" name="selectedusers" value="' . $row->userid . '"/>'; + $selectcol .= ''; return $selectcol; } diff --git a/mod/assign/locallib.php b/mod/assign/locallib.php index 21356829bbd..067b6d50178 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -1230,12 +1230,8 @@ class assign { maxlength="10" class="quickgrade"/>'; $o .= ' / ' . format_float($this->get_instance()->grade, 2); - $o .= ''; return $o; } else { - $o .= ''; if ($grade == -1 || $grade === null) { $o .= '-'; } else { @@ -1274,9 +1270,6 @@ class assign { $o .= ''; } $o .= ''; - $o .= ''; return $o; } else { $scaleid = (int)$grade; @@ -4768,8 +4761,8 @@ class assign { $record->userid = $userid; if ($modified >= 0) { $record->grade = unformat_float(optional_param('quickgrade_' . $record->userid, -1, PARAM_TEXT)); - $record->workflowstate = optional_param('quickgrade_' . $record->userid.'_workflowstate', '', PARAM_TEXT); - $record->allocatedmarker = optional_param('quickgrade_' . $record->userid.'_allocatedmarker', '', PARAM_INT); + $record->workflowstate = optional_param('quickgrade_' . $record->userid.'_workflowstate', false, PARAM_TEXT); + $record->allocatedmarker = optional_param('quickgrade_' . $record->userid.'_allocatedmarker', false, PARAM_INT); } else { // This user was not in the grading table. continue; @@ -4807,6 +4800,8 @@ class assign { foreach ($currentgrades as $current) { $modified = $users[(int)$current->userid]; $grade = $this->get_user_grade($modified->userid, false); + // Check to see if the grade column was even visible. + $gradecolpresent = optional_param('quickgrade_' . $modified->userid, false, PARAM_INT) !== false; // Check to see if the outcomes were modified. if ($CFG->enableoutcomes) { @@ -4814,7 +4809,9 @@ class assign { $oldoutcome = $outcome->grades[$modified->userid]->grade; $paramname = 'outcome_' . $outcomeid . '_' . $modified->userid; $newoutcome = optional_param($paramname, -1, PARAM_FLOAT); - if ($oldoutcome != $newoutcome) { + // Check to see if the outcome column was even visible. + $outcomecolpresent = optional_param($paramname, false, PARAM_FLOAT) !== false; + if ($outcomecolpresent && ($oldoutcome != $newoutcome)) { // Can't check modified time for outcomes because it is not reported. $modifiedusers[$modified->userid] = $modified; continue; @@ -4825,6 +4822,8 @@ class assign { // Let plugins participate. foreach ($this->feedbackplugins as $plugin) { if ($plugin->is_visible() && $plugin->is_enabled() && $plugin->supports_quickgrading()) { + // The plugins must handle is_quickgrading_modified correctly - ie + // handle hidden columns. if ($plugin->is_quickgrading_modified($modified->userid, $grade)) { if ((int)$current->lastmodified > (int)$modified->lastmodified) { return get_string('errorrecordmodified', 'assign'); @@ -4845,10 +4844,14 @@ class assign { if ($current->grade !== null) { $current->grade = floatval($current->grade); } - if ($current->grade !== $modified->grade || - ($this->get_instance()->markingallocation && $current->allocatedmarker != $modified->allocatedmarker ) || - ($this->get_instance()->markingworkflow && $current->workflowstate !== $modified->workflowstate )) { - + $gradechanged = $gradecolpresent && $current->grade !== $modified->grade; + $markingallocationchanged = $this->get_instance()->markingallocation && + ($modified->allocatedmarker !== false) && + ($current->allocatedmarker != $modified->allocatedmarker); + $workflowstatechanged = $this->get_instance()->markingworkflow && + ($modified->workflowstate !== false) && + ($current->workflowstate != $modified->workflowstate); + if ($gradechanged || $markingallocationchanged || $workflowstatechanged) { // Grade changed. if ($this->grading_disabled($modified->userid)) { continue; @@ -4873,6 +4876,7 @@ class assign { $flags = $this->get_user_flags($userid, true); $grade->grade= grade_floatval(unformat_float($modified->grade)); $grade->grader= $USER->id; + $gradecolpresent = optional_param('quickgrade_' . $userid, false, PARAM_INT) !== false; // Save plugins data. foreach ($this->feedbackplugins as $plugin) { @@ -4886,11 +4890,21 @@ class assign { } } - if ($flags->workflowstate != $modified->workflowstate || - $flags->allocatedmarker != $modified->allocatedmarker) { + // These will be set to false if they are not present in the quickgrading + // form (e.g. column hidden). + $workflowstatemodified = ($modified->workflowstate !== false) && + ($flags->workflowstate != $modified->workflowstate); + $allocatedmarkermodified = ($modified->allocatedmarker !== false) && + ($flags->allocatedmarker != $modified->allocatedmarker); + + if ($workflowstatemodified) { $flags->workflowstate = $modified->workflowstate; + } + if ($allocatedmarkermodified) { $flags->allocatedmarker = $modified->allocatedmarker; + } + if ($workflowstatemodified || $allocatedmarkermodified) { $this->update_user_flags($flags); } $this->update_grade($grade); @@ -4902,8 +4916,10 @@ class assign { foreach ($modified->gradinginfo->outcomes as $outcomeid => $outcome) { $oldoutcome = $outcome->grades[$modified->userid]->grade; $paramname = 'outcome_' . $outcomeid . '_' . $modified->userid; - $newoutcome = optional_param($paramname, -1, PARAM_INT); - if ($oldoutcome != $newoutcome) { + // This will be false if the input was not in the quickgrading + // form (e.g. column hidden). + $newoutcome = optional_param($paramname, false, PARAM_INT); + if ($newoutcome !== false && ($oldoutcome != $newoutcome)) { $data[$outcomeid] = $newoutcome; } } diff --git a/mod/assign/tests/behat/quickgrading.feature b/mod/assign/tests/behat/quickgrading.feature new file mode 100644 index 00000000000..75bf99c662c --- /dev/null +++ b/mod/assign/tests/behat/quickgrading.feature @@ -0,0 +1,143 @@ +@mod @mod_assign +Feature: In an assignment, teachers grade multiple students on one page + In order to quickly give students grades and feedback + As a teacher + I need to grade multiple students on one page + + @javascript + Scenario: Grade multiple students on one page + Given the following "courses" exists: + | fullname | shortname | category | groupmode | + | Course 1 | C1 | 0 | 1 | + And the following "users" exists: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@asd.com | + | student1 | Student | 1 | student1@asd.com | + | student2 | Student | 2 | student2@asd.com | + And the following "course enrolments" exists: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + When I log in as "admin" + And I set the following administration settings values: + | Enable outcomes | 1 | + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Outcomes" + And I follow "Edit outcomes" + And I press "Add a new outcome" + And I press "Continue" + And I fill the moodle form with: + | Name | 1337dom scale | + | Scale | Noob, Nub, 1337, HaXor | + And I press "Save changes" + And I follow "Course 1" + And I follow "Outcomes" + And I follow "Edit outcomes" + And I press "Add a new outcome" + And I fill the moodle form with: + | Full name | M8d skillZ! | + | Short name | skillZ! | + | Scale | 1337dom scale | + And I press "Save changes" + And I follow "Course 1" + And I turn 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 | + | M8d skillZ! | 1 | + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test assignment name" + And I press "Add submission" + And I fill the moodle form with: + | Online text | I'm the student1 submission | + And I press "Save changes" + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test assignment name" + When I press "Add submission" + And I fill the moodle form with: + | Online text | I'm the student2 submission | + And I press "Save changes" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Test assignment name" + And I follow "View/grade all submissions" + And I click on "Grade Student 1" "link" in the "Student 1" "table_row" + And I fill the moodle form with: + | Grade out of 100 | 50.0 | + | M8d skillZ! | 1337 | + | Feedback comments | I'm the teacher first feedback | + And I press "Save changes" + And I press "Continue" + Then I click on "Quick grading" "checkbox" + And I fill in "User grade" with "60.0" + And I press "Save all quick grading changes" + And I should see "The grade changes were saved" + And I press "Continue" + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test assignment name" + And I should see "I'm the teacher first feedback" + And I should see "60.0" + And I follow "Course 1" + And I follow "Grades" + And I should see "1337" + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test assignment name" + And I should not see "I'm the teacher first feedback" + And I should not see "60.0" + And I follow "Course 1" + And I follow "Grades" + And I should not see "1337" + And I log out + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Test assignment name" + And I follow "View/grade all submissions" + And I click on "Hide User picture" "link" + And I click on "Hide Full name" "link" + And I click on "Hide Email address" "link" + And I click on "Hide Status" "link" + And I click on "Hide Grade" "link" + And I click on "Hide Edit" "link" + And I click on "Hide Last modified (submission)" "link" + And I click on "Hide Online text" "link" + And I click on "Hide Submission comments" "link" + And I click on "Hide Last modified (grade)" "link" + And I click on "Hide Feedback comments" "link" + And I click on "Hide Annotate PDF" "link" + And I click on "Hide Final grade" "link" + And I click on "Hide Outcomes" "link" + And I press "Save all quick grading changes" + And I should see "The grade changes were saved" + And I press "Continue" + And I log out + And I log in as "student1" + And I follow "Course 1" + And I follow "Test assignment name" + And I should see "I'm the teacher first feedback" + And I should see "60.0" + And I follow "Course 1" + And I follow "Grades" + And I should see "1337" + And I log out + And I log in as "student2" + And I follow "Course 1" + And I follow "Test assignment name" + And I should not see "I'm the teacher first feedback" + And I should not see "60.0" + And I follow "Course 1" + And I follow "Grades" + And I should not see "1337"