diff --git a/grade/report/grader/lib.php b/grade/report/grader/lib.php index 089b1686ea393..2890159a44e72 100644 --- a/grade/report/grader/lib.php +++ b/grade/report/grader/lib.php @@ -181,7 +181,7 @@ public function process_data($data) { // Were any changes made? $changedgrades = false; - foreach ($data as $varname => $postedvalue) { + foreach ($data as $varname => $students) { $needsupdate = false; @@ -194,113 +194,116 @@ public function process_data($data) { continue; } - $gradeinfo = explode("_", $varname); - $userid = clean_param($gradeinfo[1], PARAM_INT); - $itemid = clean_param($gradeinfo[2], PARAM_INT); + foreach ($students as $userid => $items) { + $userid = clean_param($userid, PARAM_INT); + foreach ($items as $itemid => $postedvalue) { + $itemid = clean_param($itemid, PARAM_INT); + + // Was change requested? + $oldvalue = $this->grades[$userid][$itemid]; + if ($datatype === 'grade') { + // If there was no grade and there still isn't + if (is_null($oldvalue->finalgrade) && $postedvalue == -1) { + // -1 means no grade + continue; + } - // Was change requested? - $oldvalue = $this->grades[$userid][$itemid]; - if ($datatype === 'grade') { - // If there was no grade and there still isn't - if (is_null($oldvalue->finalgrade) && $postedvalue == -1) { - // -1 means no grade - continue; - } + // If the grade item uses a custom scale + if (!empty($oldvalue->grade_item->scaleid)) { - // If the grade item uses a custom scale - if (!empty($oldvalue->grade_item->scaleid)) { + if ((int)$oldvalue->finalgrade === (int)$postedvalue) { + continue; + } + } else { + // The grade item uses a numeric scale - if ((int)$oldvalue->finalgrade === (int)$postedvalue) { - continue; - } - } else { - // The grade item uses a numeric scale + // Format the finalgrade from the DB so that it matches the grade from the client + if ($postedvalue === format_float($oldvalue->finalgrade, $oldvalue->grade_item->get_decimals())) { + continue; + } + } - // Format the finalgrade from the DB so that it matches the grade from the client - if ($postedvalue === format_float($oldvalue->finalgrade, $oldvalue->grade_item->get_decimals())) { - continue; + $changedgrades = true; + + } else if ($datatype === 'feedback') { + if (($oldvalue->feedback === $postedvalue) or ($oldvalue->feedback === NULL and empty($postedvalue))) { + continue; + } } - } - $changedgrades = true; + if (!$gradeitem = grade_item::fetch(array('id'=>$itemid, 'courseid'=>$this->courseid))) { + print_error('invalidgradeitemid'); + } - } else if ($datatype === 'feedback') { - if (($oldvalue->feedback === $postedvalue) or ($oldvalue->feedback === NULL and empty($postedvalue))) { - continue; - } - } + // Pre-process grade + if ($datatype == 'grade') { + $feedback = false; + $feedbackformat = false; + if ($gradeitem->gradetype == GRADE_TYPE_SCALE) { + if ($postedvalue == -1) { // -1 means no grade + $finalgrade = null; + } else { + $finalgrade = $postedvalue; + } + } else { + $finalgrade = unformat_float($postedvalue); + } - if (!$gradeitem = grade_item::fetch(array('id'=>$itemid, 'courseid'=>$this->courseid))) { // we must verify course id here! - print_error('invalidgradeitemid'); - } + $errorstr = ''; + // Warn if the grade is out of bounds. + if (is_null($finalgrade)) { + // ok + } else { + $bounded = $gradeitem->bounded_grade($finalgrade); + if ($bounded > $finalgrade) { + $errorstr = 'lessthanmin'; + } else if ($bounded < $finalgrade) { + $errorstr = 'morethanmax'; + } + } + if ($errorstr) { + $user = $DB->get_record('user', array('id' => $userid), 'id, firstname, lastname'); + $gradestr = new stdClass(); + $gradestr->username = fullname($user); + $gradestr->itemname = $gradeitem->get_name(); + $warnings[] = get_string($errorstr, 'grades', $gradestr); + } - // Pre-process grade - if ($datatype == 'grade') { - $feedback = false; - $feedbackformat = false; - if ($gradeitem->gradetype == GRADE_TYPE_SCALE) { - if ($postedvalue == -1) { // -1 means no grade - $finalgrade = null; - } else { - $finalgrade = $postedvalue; + } else if ($datatype == 'feedback') { + $finalgrade = false; + $trimmed = trim($postedvalue); + if (empty($trimmed)) { + $feedback = NULL; + } else { + $feedback = $postedvalue; + } } - } else { - $finalgrade = unformat_float($postedvalue); - } - $errorstr = ''; - // Warn if the grade is out of bounds. - if (is_null($finalgrade)) { - // ok - } else { - $bounded = $gradeitem->bounded_grade($finalgrade); - if ($bounded > $finalgrade) { - $errorstr = 'lessthanmin'; - } else if ($bounded < $finalgrade) { - $errorstr = 'morethanmax'; + // group access control + if ($separategroups) { + // note: we can not use $this->currentgroup because it would fail badly + // when having two browser windows each with different group + $sharinggroup = false; + foreach($mygroups as $groupid) { + if (groups_is_member($groupid, $userid)) { + $sharinggroup = true; + break; + } + } + if (!$sharinggroup) { + // either group membership changed or somebody is hacking grades of other group + $warnings[] = get_string('errorsavegrade', 'grades'); + continue; + } } - } - if ($errorstr) { - $user = $DB->get_record('user', array('id' => $userid), 'id, firstname, lastname'); - $gradestr = new stdClass(); - $gradestr->username = fullname($user); - $gradestr->itemname = $gradeitem->get_name(); - $warnings[] = get_string($errorstr, 'grades', $gradestr); - } - } else if ($datatype == 'feedback') { - $finalgrade = false; - $trimmed = trim($postedvalue); - if (empty($trimmed)) { - $feedback = NULL; - } else { - $feedback = $postedvalue; - } - } + $gradeitem->update_final_grade($userid, $finalgrade, 'gradebook', $feedback, FORMAT_MOODLE); - // group access control - if ($separategroups) { - // note: we can not use $this->currentgroup because it would fail badly - // when having two browser windows each with different group - $sharinggroup = false; - foreach($mygroups as $groupid) { - if (groups_is_member($groupid, $userid)) { - $sharinggroup = true; - break; + // We can update feedback without reloading the grade item as it doesn't affect grade calculations + if ($datatype === 'feedback') { + $this->grades[$userid][$itemid]->feedback = $feedback; } } - if (!$sharinggroup) { - // either group membership changed or somebody is hacking grades of other group - $warnings[] = get_string('errorsavegrade', 'grades'); - continue; - } - } - - $gradeitem->update_final_grade($userid, $finalgrade, 'gradebook', $feedback, FORMAT_MOODLE); - - // We can update feedback without reloading the grade item as it doesn't affect grade calculations - if ($datatype === 'feedback') { - $this->grades[$userid][$itemid]->feedback = $feedback; } } @@ -1018,7 +1021,7 @@ public function get_right_rows() { } $attributes = array('tabindex' => $tabindices[$item->id]['grade'], 'id'=>'grade_'.$userid.'_'.$item->id); $itemcell->text .= html_writer::label(get_string('typescale', 'grades'), $attributes['id'], false, array('class' => 'accesshide')); - $itemcell->text .= html_writer::select($scaleopt, 'grade_'.$userid.'_'.$item->id, $gradeval, array(-1=>$nogradestr), $attributes); + $itemcell->text .= html_writer::select($scaleopt, 'grade['.$userid.']['.$item->id.']', $gradeval, array(-1=>$nogradestr), $attributes); } elseif(!empty($scale)) { $scales = explode(",", $scale->scale); @@ -1040,8 +1043,8 @@ public function get_right_rows() { $itemcell->text .= ''; $itemcell->text .= ''; + . '" type="text" class="text" title="'. $strgrade .'" name="grade[' + .$userid.'][' .$item->id.']" id="grade_'.$userid.'_'.$item->id.'" value="'.$value.'" />'; } else { $itemcell->text .= html_writer::tag('span', format_float($gradeval, $decimalpoints), array('class'=>"gradevalue$hidden$gradepass")); } @@ -1054,7 +1057,7 @@ public function get_right_rows() { $itemcell->text .= ''; $itemcell->text .= ''; + . '" size="6" title="' . $strfeedback . '" type="text" name="feedback['.$userid.']['.$item->id.']" value="' . s($grade->feedback) . '" />'; } } else { // Not editing @@ -1738,25 +1741,21 @@ public function get_students_per_page() { // Will this number of students result in more fields that we are allowed? $maxinputvars = ini_get('max_input_vars'); if ($maxinputvars !== false) { - $fieldspergradeitem = 0; // The number of fields output per grade item for each student + // We can't do anything about there being more grade items than max_input_vars, + // but we can decrease number of students per page if there are >= max_input_vars + $fieldsperstudent = 0; // The number of fields output per student - if ($this->get_pref('quickgrading')) { - // One grade field - $fieldspergradeitem ++; - } - if ($this->get_pref('showquickfeedback')) { - // One feedback field - $fieldspergradeitem ++; + if ($this->get_pref('quickgrading') || $this->get_pref('showquickfeedback')) { + // Each array (grade, feedback) will gain one element + $fieldsperstudent ++; } - $fieldsperstudent = $fieldspergradeitem * count($this->gtree->get_items()); $fieldsrequired = $studentsperpage * $fieldsperstudent; - if ($fieldsrequired > $maxinputvars) { - $studentsperpage = floor($maxinputvars / $fieldsperstudent); + if ($fieldsrequired >= $maxinputvars) { + $studentsperpage = $maxinputvars - 1; // Subtract one to be on the safe side if ($studentsperpage<1) { - // Make sure students per page doesn't fall below 1 - // PHP max_input_vars could potentially be reached with 1 student - // if there are >500 grade items and quickgrading and showquickfeedback are on + // Make sure students per page doesn't fall below 1, though if your + // max_input_vars is only 1 you've got bigger problems! $studentsperpage = 1; }