MDL-48618 gradebook: Only use individual min/max for aggrigate grades

2025-01-17 21:49:15 +01:00 · 2015-06-02 16:25:06 -04:00 · 2015-06-02 16:25:06 -04:00 · c07775dfff
commit c07775dfff
parent 17abbfba1f
11 changed files with 202 additions and 20 deletions
--- a/grade/lib.php
+++ b/grade/lib.php
@ -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.
 *
@ -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) {
@ -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');
@ -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 {
--- a/grade/report/grader/lib.php
+++ b/grade/report/grader/lib.php
@ -1086,8 +1086,9 @@ class grade_report_grader extends grade_report {
                    } 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}'>" .
--- a/grade/report/lib.php
+++ b/grade/report/lib.php
@ -437,8 +437,9 @@ abstract class grade_report {
        $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);
        }
--- a/grade/report/overview/lib.php
+++ b/grade/report/overview/lib.php
@ -238,11 +238,12 @@ class grade_report_overview extends grade_report {
                        $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;
                    }
                }

--- a/grade/report/user/lib.php
+++ b/grade/report/user/lib.php
@ -466,8 +466,9 @@ class grade_report_user extends grade_report {
            } 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;
                }
            }

--- a/lang/en/grades.php
+++ b/lang/en/grades.php
@ -452,6 +452,9 @@ $string['meanselection'] = 'Grades selected for column averages';
 $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';
@ -722,6 +725,7 @@ $string['typescale'] = 'Scale';
 $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';
--- a/lib/db/upgrade.php
+++ b/lib/db/upgrade.php
@ -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;
 }
--- a/lib/grade/grade_category.php
+++ b/lib/grade/grade_category.php
@ -467,6 +467,12 @@ class grade_category extends grade_object {
            $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);

@ -498,8 +504,11 @@ class grade_category extends grade_object {
            $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,
@ -507,23 +516,25 @@ class grade_category extends grade_object {
                                            $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;

-                if ($used->excluded) {
-                    $excluded[] = $used->itemid;
+                list($min, $max) = $grade->get_grade_min_max();
+                $grademaxoverrides[$grade->itemid] = $max;
+                $grademinoverrides[$grade->itemid] = $min;
+
+                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,
--- a/lib/grade/grade_grade.php
+++ b/lib/grade/grade_grade.php
@ -337,6 +337,46 @@ class grade_grade extends grade_object {
        $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
     *
--- a/lib/grade/grade_item.php
+++ b/lib/grade/grade_item.php
@ -996,6 +996,15 @@ class grade_item extends grade_object {
        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
     *
--- a/version.php
+++ b/version.php
@ -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.