Skip to content

Commit

Permalink
MDL-48618 gradebook: Only use individual min/max for aggrigate grades
Browse files Browse the repository at this point in the history
  • Loading branch information
ericmerrill authored and Frederic Massart committed Jun 5, 2015
1 parent 17abbfb commit c07775d
Show file tree
Hide file tree
Showing 11 changed files with 202 additions and 20 deletions.
93 changes: 93 additions & 0 deletions grade/lib.php
Expand Up @@ -467,6 +467,43 @@ function hide_natural_aggregation_upgrade_notice($courseid) {
unset_config('show_sumofgrades_upgrade_' . $courseid);
}

/**
* Hide warning about changed grades during upgrade from 2.8.0-2.8.6 and 2.9.0.
*
* @param int $courseid The current course id.
*/
function hide_min_max_grade_upgrade_notice($courseid) {
unset_config('show_min_max_grades_changed_' . $courseid);
}

/**
* Cause the course to enter a "bug" mode where the buggy computations from 2.8.0 are used.
*
* @param int $courseid The current course id.
*/
function revert_min_max_grade_upgrade($courseid) {
unset_config('show_min_max_grades_changed_' . $courseid);
set_config('use_28_bug_grades_' . $courseid, 1);

grade_force_full_regrading($courseid);
// Do this now, because it probably happened to late in the page load to be happen automatically.
grade_regrade_final_grades($courseid);
}

/**
* Cause fixed grade behaviour to be used.
*
* @param int $courseid The current course id.
*/
function fix_min_max_grade_upgrade($courseid) {
set_config('show_min_max_grades_changed_' . $courseid, 1);
unset_config('use_28_bug_grades_' . $courseid);

grade_force_full_regrading($courseid);
// Do this now, because it probably happened to late in the page load to be happen automatically.
grade_regrade_final_grades($courseid);
}

/**
* Hide warning about changed grades during upgrade to 2.8.
*
Expand Down Expand Up @@ -494,6 +531,11 @@ function print_natural_aggregation_upgrade_notice($courseid, $context, $thispage
$showsubcatswarning = get_config('core', 'show_aggregatesubcats_upgrade_' . $courseid);
$hidenaturalwarning = optional_param('seensumofgradesupgradedgrades', false, PARAM_BOOL) && confirm_sesskey();
$shownaturalwarning = get_config('core', 'show_sumofgrades_upgrade_' . $courseid);
$revertminmax = optional_param('revertminmaxupgradedgrades', false, PARAM_BOOL) && confirm_sesskey();
$hideminmaxwarning = optional_param('seenminmaxupgradedgrades', false, PARAM_BOOL) && confirm_sesskey();
$showminmaxwarning = get_config('core', 'show_min_max_grades_changed_' . $courseid);
$fixminmaxgrades = optional_param('fixminmaxgrades', false, PARAM_BOOL) && confirm_sesskey();
$showminmaxrevertwarning = get_config('core', 'use_28_bug_grades_' . $courseid);

// Do not do anything if they are not a teacher.
if ($hidesubcatswarning || $showsubcatswarning || $hidenaturalwarning || $shownaturalwarning) {
Expand All @@ -511,6 +553,22 @@ function print_natural_aggregation_upgrade_notice($courseid, $context, $thispage
hide_aggregatesubcats_upgrade_notice($courseid);
}

// Hide the warning if the user told it to go away.
if ($hideminmaxwarning) {
hide_min_max_grade_upgrade_notice($courseid);
}

// Revert the 2.8 min/max fix changes.
if ($revertminmax) {
revert_min_max_grade_upgrade($courseid);
}

// Apply the 2.8 min/max fixes.
if ($fixminmaxgrades) {
fix_min_max_grade_upgrade($courseid);
}


if (!$hidenaturalwarning && $shownaturalwarning) {
$message = get_string('sumofgradesupgradedgrades', 'grades');
$hidemessage = get_string('upgradedgradeshidemessage', 'grades');
Expand All @@ -535,6 +593,41 @@ function print_natural_aggregation_upgrade_notice($courseid, $context, $thispage
$html .= $goawaybutton;
}

// Show the message that there were min/max issues that have been resolved.
if (!$hideminmaxwarning && !$revertminmax && ($fixminmaxgrades || $showminmaxwarning)) {
$message = get_string('minmaxupgradedgrades', 'grades');

$revertmessage = get_string('upgradedminmaxrevertmessage', 'grades');
$urlparams = array( 'id' => $courseid,
'revertminmaxupgradedgrades' => true,
'sesskey' => sesskey());
$reverturl = new moodle_url($thispage, $urlparams);
$revertbutton = $OUTPUT->single_button($reverturl, $revertmessage, 'get');

$hidemessage = get_string('upgradedgradeshidemessage', 'grades');
$urlparams = array( 'id' => $courseid,
'seenminmaxupgradedgrades' => true,
'sesskey' => sesskey());
$goawayurl = new moodle_url($thispage, $urlparams);
$goawaybutton = $OUTPUT->single_button($goawayurl, $hidemessage, 'get');
$html .= $OUTPUT->notification($message, 'notifysuccess');
$html .= $revertbutton.$goawaybutton;
}

// Show the warning that there are min/max isseus that have not be resolved.
if ($revertminmax || (!$fixminmaxgrades && $showminmaxrevertwarning)) {
$message = get_string('minmaxupgradewarning', 'grades');

$fixmessage = get_string('minmaxupgradefixbutton', 'grades');
$urlparams = array( 'id' => $courseid,
'fixminmaxgrades' => true,
'sesskey' => sesskey());
$fixurl = new moodle_url($thispage, $urlparams);
$fixbutton = $OUTPUT->single_button($fixurl, $fixmessage, 'get');
$html .= $OUTPUT->notification($message, 'notifywarning');
$html .= $fixbutton;
}

if ($return) {
return $html;
} else {
Expand Down
5 changes: 3 additions & 2 deletions grade/report/grader/lib.php
Expand Up @@ -1086,8 +1086,9 @@ public function get_right_rows($displayaverages) {
} else {
// The max and min for an aggregation may be different to the grade_item.
if (!is_null($gradeval)) {
$item->grademax = $grade->rawgrademax;
$item->grademin = $grade->rawgrademin;
list($min, $max) = $grade->get_grade_min_max();
$item->grademax = $max;
$item->grademin = $min;
}

$itemcell->text .= "<span class='gradevalue{$hidden}{$gradepass}'>" .
Expand Down
5 changes: 3 additions & 2 deletions grade/report/lib.php
Expand Up @@ -437,8 +437,9 @@ protected function blank_hidden_total_and_adjust_bounds($courseid, $course_item,
$grademin = $course_item->grademin;
$grademax = $course_item->grademax;
if ($coursegradegrade) {
$grademin = $coursegradegrade->rawgrademin;
$grademax = $coursegradegrade->rawgrademax;
list($min, $max) = $coursegradegrade->get_grade_min_max();
$grademin = $min;
$grademax = $max;
} else {
$coursegradegrade = new grade_grade(array('userid'=>$this->user->id, 'itemid'=>$course_item->id), false);
}
Expand Down
7 changes: 4 additions & 3 deletions grade/report/overview/lib.php
Expand Up @@ -238,11 +238,12 @@ public function fill_table($activitylink = false, $studentcoursesonly = false) {
$course_item->grademin = $adjustedgrade['grademin'];
}
} else {
// We must use the rawgrademin / rawgrademax because it can be different for
// We must use the specific max/min because it can be different for
// each grade_grade when items are excluded from sum of grades.
if (!is_null($finalgrade)) {
$course_item->grademin = $course_grade->rawgrademin;
$course_item->grademax = $course_grade->rawgrademax;
list($min, $max) = $course_grade->get_grade_min_max();
$course_item->grademin = $min;
$course_item->grademax = $max;
}
}

Expand Down
5 changes: 3 additions & 2 deletions grade/report/user/lib.php
Expand Up @@ -466,8 +466,9 @@ private function fill_table_recursive(&$element) {
} else {
// The max and min for an aggregation may be different to the grade_item.
if (!is_null($gradeval)) {
$grade_grade->grade_item->grademax = $grade_grade->rawgrademax;
$grade_grade->grade_item->grademin = $grade_grade->rawgrademin;
list($min, $max) = $grade_grade->get_grade_min_max();
$grade_grade->grade_item->grademax = $max;
$grade_grade->grade_item->grademin = $min;
}
}

Expand Down
4 changes: 4 additions & 0 deletions lang/en/grades.php
Expand Up @@ -452,6 +452,9 @@
$string['meanselection_help'] = 'This setting determines whether cells with no grade should be included when calculating the average (mean) for each category or grade item.';
$string['median'] = 'Median';
$string['min'] = 'Lowest';
$string['minmaxupgradedgrades'] = 'An internal inconsistency was detected with some grades in this course. They have been corrected, it is recommended that you review your gradebook.';
$string['minmaxupgradefixbutton'] = 'Fix';
$string['minmaxupgradewarning'] = 'An internal inconsistency was detected with some grades in this course. It is recommened that you fix this, although some aggrigate grades may change.';
$string['minimum_show'] = 'Show minimum grade';
$string['minimum_show_help'] = 'Minimum grade is used in calculating grades and weights. If not shown, minimum grade will default to zero and cannot be edited.';
$string['missingscale'] = 'Scale must be selected';
Expand Down Expand Up @@ -722,6 +725,7 @@
$string['typescale_help'] = 'This setting determines the scale used when using the scale grade type. The scale for an activity-based grade item is set on the activity settings page.';
$string['typetext'] = 'Text';
$string['typevalue'] = 'Value';
$string['upgradedminmaxrevertmessage'] = 'Revert';
$string['uncategorised'] = 'Uncategorised';
$string['unenrolledusersinimport'] = 'This import included the following grades for users not currently enrolled in this course: {$a}';
$string['unchangedgrade'] = 'Grade unchanged';
Expand Down
21 changes: 21 additions & 0 deletions lib/db/upgrade.php
Expand Up @@ -4378,5 +4378,26 @@ function xmldb_main_upgrade($oldversion) {
// Moodle v2.9.0 release upgrade line.
// Put any upgrade step following this.

if ($oldversion < 2015060400.01) {
// We only need to do this for sites that have upgraded to 2.8.0 already.
if ($oldversion >= 2014111000.00) {

$sql = "SELECT DISTINCT(gi.courseid)
FROM {grade_items} gi
JOIN {grade_grades} gg
ON gg.itemid = gi.id
WHERE (gi.itemtype <> 'course' AND gi.itemtype <> 'category')
AND (gg.rawgrademax != gi.grademax OR gg.rawgrademin != gi.grademin)";

$rs = $DB->get_recordset_sql($sql);
foreach ($rs as $record) {
set_config('show_min_max_grades_changed_'.$record->courseid, 1);
$DB->set_field('grade_items', 'needsupdate', 1, array('courseid' => $record->courseid));
}
}

upgrade_main_savepoint(true, 2015060400.01);
}

return true;
}
31 changes: 21 additions & 10 deletions lib/grade/grade_category.php
Expand Up @@ -467,6 +467,12 @@ public function generate_grades($userid=null) {
$items = $DB->get_records_sql($sql, $params);
}

if ($items) {
foreach ($items as $id => $item) {
$items[$id] = new grade_item($item);
}
}

$grade_inst = new grade_grade();
$fields = 'g.'.implode(',g.', $grade_inst->required_fields);

Expand Down Expand Up @@ -498,32 +504,37 @@ public function generate_grades($userid=null) {
$grademinoverrides = array();

foreach ($rs as $used) {

if ($used->userid != $prevuser) {
$grade = new grade_grade($used);
if (isset($items[$used->itemid])) {
$grade->grade_item =& $items[$used->itemid];
}
if ($grade->userid != $prevuser) {
$this->aggregate_grades($prevuser,
$items,
$grade_values,
$oldgrade,
$excluded,
$grademinoverrides,
$grademaxoverrides);
$prevuser = $used->userid;
$prevuser = $grade->userid;
$grade_values = array();
$excluded = array();
$oldgrade = null;
$grademaxoverrides = array();
$grademinoverrides = array();
}
$grade_values[$used->itemid] = $used->finalgrade;
$grademaxoverrides[$used->itemid] = $used->rawgrademax;
$grademinoverrides[$used->itemid] = $used->rawgrademin;
$grade_values[$grade->itemid] = $grade->finalgrade;

list($min, $max) = $grade->get_grade_min_max();
$grademaxoverrides[$grade->itemid] = $max;
$grademinoverrides[$grade->itemid] = $min;

if ($used->excluded) {
$excluded[] = $used->itemid;
if ($grade->excluded) {
$excluded[] = $grade->itemid;
}

if ($this->grade_item->id == $used->itemid) {
$oldgrade = $used;
if ($this->grade_item->id == $grade->itemid) {
$oldgrade = $grade;
}
}
$this->aggregate_grades($prevuser,
Expand Down
40 changes: 40 additions & 0 deletions lib/grade/grade_grade.php
Expand Up @@ -337,6 +337,46 @@ public function set_aggregationstatus($aggregationstatus) {
$this->update();
}

/**
* Returns the minimum and maximum number of points this grade is graded with respect to.
*
* @return array A list containing the minimum and maximum number of points
*/
public function get_grade_min_max() {
$this->load_grade_item();

$usequirky = get_config('core', 'use_28_bug_grades_'.$this->grade_item->courseid);

// Only aggregate items use seperate min grades.
if ($usequirky || $this->grade_item->is_aggregate_item()) {
return array($this->rawgrademin, $this->rawgrademax);
} else {
return array($this->grade_item->grademin, $this->grade_item->grademax);
}
}

/**
* Returns the minimum number of points this grade is graded with.
*
* @return float The minimum number of points
*/
public function get_grade_min() {
list($min, $max) = $this->get_grade_min_max();

return $min;
}

/**
* Returns the maximum number of points this grade is graded with respect to.
*
* @return float The maximum number of points
*/
public function get_grade_max() {
list($min, $max) = $this->get_grade_min_max();

return $max;
}

/**
* Returns timestamp when last graded, null if no grade present
*
Expand Down
9 changes: 9 additions & 0 deletions lib/grade/grade_item.php
Expand Up @@ -996,6 +996,15 @@ public function is_raw_used() {
return ($this->is_external_item() and !$this->is_calculated() and !$this->is_outcome_item());
}

/**
* Returns true if the grade item is an aggreggated type grade.
*
* @return bool
*/
public function is_aggregate_item() {
return ($this->is_category_item() || $this->is_course_item());
}

/**
* Returns the grade item associated with the course
*
Expand Down
2 changes: 1 addition & 1 deletion version.php
Expand Up @@ -29,7 +29,7 @@

defined('MOODLE_INTERNAL') || die();

$version = 2015060400.00; // YYYYMMDD = weekly release date of this DEV branch.
$version = 2015060400.01; // YYYYMMDD = weekly release date of this DEV branch.
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.

Expand Down

0 comments on commit c07775d

Please sign in to comment.