Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

MDL-35074: grader report - Ease limit on number of students per page

The limit imposed by max_input_vars "applies only to each nesting level of
a multi-dimensional input array"
(http://www.php.net/manual/en/info.configuration.php#ini.max-input-vars).

Turning the grader report fields into arrays (they were all top-level form
fields, i.e. elements of the top-level array) makes the number of students
become the only limiting factor, thereby allowing us to relax the limit on
number of students per page greatly (to 1 less than max_input_vars, for
safety).
  • Loading branch information...
commit 30793ea3bd2a162365e363641efd7f2b5d22128a 1 parent 9da506c
@pauln pauln authored
Showing with 109 additions and 110 deletions.
  1. +109 −110 grade/report/grader/lib.php
View
219 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 .= '<label class="accesshide" for="grade_'.$userid.'_'.$item->id.'">'
.get_string('useractivitygrade', 'gradereport_grader', $gradelabel).'</label>';
$itemcell->text .= '<input size="6" tabindex="' . $tabindices[$item->id]['grade']
- . '" type="text" class="text" title="'. $strgrade .'" name="grade_'
- .$userid.'_' .$item->id.'" id="grade_'.$userid.'_'.$item->id.'" value="'.$value.'" />';
+ . '" 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 .= '<label class="accesshide" for="feedback_'.$userid.'_'.$item->id.'">'
.get_string('useractivityfeedback', 'gradereport_grader', $feedbacklabel).'</label>';
$itemcell->text .= '<input class="quickfeedback" tabindex="' . $tabindices[$item->id]['feedback'].'" id="feedback_'.$userid.'_'.$item->id
- . '" size="6" title="' . $strfeedback . '" type="text" name="feedback_'.$userid.'_'.$item->id.'" value="' . s($grade->feedback) . '" />';
+ . '" 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;
}
Please sign in to comment.
Something went wrong with that request. Please try again.