diff --git a/mod/assign/feedback/comments/locallib.php b/mod/assign/feedback/comments/locallib.php index 96dde23d982..e0ea32e42ab 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 fe5f1e66ce8..c4242511cff 100644 --- a/mod/assign/gradingtable.php +++ b/mod/assign/gradingtable.php @@ -759,6 +759,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 fc62b564e74..0ab485fd6b0 100644 --- a/mod/assign/locallib.php +++ b/mod/assign/locallib.php @@ -1251,12 +1251,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 { @@ -1295,9 +1291,6 @@ class assign { $o .= ''; } $o .= ''; - $o .= ''; return $o; } else { $scaleid = (int)$grade; @@ -4945,8 +4938,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; @@ -4984,6 +4977,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) { @@ -4991,7 +4986,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; @@ -5002,6 +4999,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'); @@ -5022,10 +5021,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; @@ -5050,6 +5053,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) { @@ -5063,11 +5067,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); @@ -5082,8 +5096,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; } }