MDL-48618 grades: Handling of inconsistencies due to min/max grades

admin/settings/grades.php

@@ -76,6 +76,8 @@
 
         $temp->add(new admin_setting_special_gradepointdefault());
 
+        $temp->add(new admin_setting_special_grademinmaxtouse());
+
         $temp->add(new admin_setting_my_grades_report());
 
         $temp->add(new admin_setting_configtext('gradereport_mygradeurl', new lang_string('externalurl', 'grades'),

grade/edit/settings/form.php

@@ -65,6 +65,22 @@ function definition() {
         $mform->addElement('select', 'aggregationposition', get_string('aggregationposition', 'grades'), $options);
         $mform->addHelpButton('aggregationposition', 'aggregationposition', 'grades');
 
+        if ($CFG->grade_minmaxtouse == GRADE_MIN_MAX_FROM_GRADE_ITEM) {
+            $default = get_string('gradeitemminmax', 'grades');
+        } else if ($CFG->grade_minmaxtouse == GRADE_MIN_MAX_FROM_GRADE_GRADE) {
+            $default = get_string('gradegrademinmax', 'grades');
+        } else {
+            throw new coding_exception('Invalid $CFG->grade_minmaxtouse value.');
+        }
+
+        $options = array(
+            -1 => get_string('defaultprev', 'grades', $default),
+            GRADE_MIN_MAX_FROM_GRADE_ITEM => get_string('gradeitemminmax', 'grades'),
+            GRADE_MIN_MAX_FROM_GRADE_GRADE => get_string('gradegrademinmax', 'grades')
+        );
+        $mform->addElement('select', 'minmaxtouse', get_string('minmaxtouse', 'grades'), $options);
+        $mform->addHelpButton('minmaxtouse', 'minmaxtouse', 'grades');
+
         // Grade item settings
         $mform->addElement('header', 'grade_item_settings', get_string('gradeitemsettings', 'grades'));
         $mform->setExpanded('grade_item_settings');

grade/edit/settings/index.php

@@ -58,7 +58,7 @@
 
 } else if ($data = $mform->get_data()) {
     $data = (array)$data;
-    $general = array('displaytype', 'decimalpoints', 'aggregationposition');
+    $general = array('displaytype', 'decimalpoints', 'aggregationposition', 'minmaxtouse');
     foreach ($data as $key=>$value) {
         if (!in_array($key, $general) and strpos($key, 'report_') !== 0
                                       and strpos($key, 'import_') !== 0
@@ -69,13 +69,23 @@
             $value = null;
         }
         grade_set_setting($course->id, $key, $value);
+
+        $previousvalue = isset($settings->{$key}) ? $settings->{$key} : null;
+        if ($key == 'minmaxtouse' && $previousvalue != $value) {
+            // The min max has changed, we need to regrade the grades.
+            grade_force_full_regrading($courseid);
+        }
     }
 
     redirect($returnurl);
 }
 
 print_grade_page_head($courseid, 'settings', 'coursesettings', get_string('coursegradesettings', 'grades'));
 
+// The settings could have been changed due to a notice shown in print_grade_page_head, we need to refresh them.
+$settings = grade_get_settings($course->id);
+$mform->set_data($settings);
+
 echo $OUTPUT->box_start('generalbox boxaligncenter boxwidthnormal centerpara');
 echo get_string('coursesettingsexplanation', 'grades');
 echo $OUTPUT->box_end();

grade/lib.php

@@ -472,32 +472,34 @@ function hide_natural_aggregation_upgrade_notice($courseid) {
  *
  * @param int $courseid The current course id.
  */
-function hide_min_max_grade_upgrade_notice($courseid) {
+function grade_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.
+ * Use the grade min and max from the grade_grade.
+ *
+ * This is reserved for core use after an upgrade.
  *
  * @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);
+function grade_upgrade_use_min_max_from_grade_grade($courseid) {
+    grade_set_setting($courseid, 'minmaxtouse', GRADE_MIN_MAX_FROM_GRADE_GRADE);
 
     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.
+ * Use the grade min and max from the grade_item.
+ *
+ * This is reserved for core use after an upgrade.
  *
  * @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);
+function grade_upgrade_use_min_max_from_grade_item($courseid) {
+    grade_set_setting($courseid, 'minmaxtouse', GRADE_MIN_MAX_FROM_GRADE_ITEM);
 
     grade_force_full_regrading($courseid);
     // Do this now, because it probably happened to late in the page load to be happen automatically.
@@ -524,25 +526,26 @@ function hide_aggregatesubcats_upgrade_notice($courseid) {
  * @return nothing or string if $return true
  */
 function print_natural_aggregation_upgrade_notice($courseid, $context, $thispage, $return=false) {
-    global $OUTPUT;
+    global $CFG, $OUTPUT;
     $html = '';
 
+    // Do not do anything if they cannot manage the grades of this course.
+    if (!has_capability('moodle/grade:manage', $context)) {
+        return $html;
+    }
+
     $hidesubcatswarning = optional_param('seenaggregatesubcatsupgradedgrades', false, PARAM_BOOL) && confirm_sesskey();
     $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) {
-        if (!has_capability('moodle/grade:manage', $context)) {
-            return '';
-        }
-    }
+    $useminmaxfromgradeitem = optional_param('useminmaxfromgradeitem', false, PARAM_BOOL) && confirm_sesskey();
+    $useminmaxfromgradegrade = optional_param('useminmaxfromgradegrade', false, PARAM_BOOL) && confirm_sesskey();
+
+    $minmaxtouse = grade_get_setting($courseid, 'minmaxtouse', $CFG->grade_minmaxtouse);
 
     // Hide the warning if the user told it to go away.
     if ($hidenaturalwarning) {
@@ -553,19 +556,23 @@ 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.
+    // Hide the min/max warning if the user told it to go away.
     if ($hideminmaxwarning) {
-        hide_min_max_grade_upgrade_notice($courseid);
+        grade_hide_min_max_grade_upgrade_notice($courseid);
+        $showminmaxwarning = false;
     }
 
-    // Revert the 2.8 min/max fix changes.
-    if ($revertminmax) {
-        revert_min_max_grade_upgrade($courseid);
-    }
+    if ($useminmaxfromgradegrade) {
+        // Revert to the new behaviour, we now use the grade_grade for min/max.
+        grade_upgrade_use_min_max_from_grade_grade($courseid);
+        grade_hide_min_max_grade_upgrade_notice($courseid);
+        $showminmaxwarning = false;
 
-    // Apply the 2.8 min/max fixes.
-    if ($fixminmaxgrades) {
-        fix_min_max_grade_upgrade($courseid);
+    } else if ($useminmaxfromgradeitem) {
+        // Apply the new logic, we now use the grade_item for min/max.
+        grade_upgrade_use_min_max_from_grade_item($courseid);
+        grade_hide_min_max_grade_upgrade_notice($courseid);
+        $showminmaxwarning = false;
     }
 
 
@@ -593,39 +600,49 @@ 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');
-
+    if ($showminmaxwarning) {
         $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;
-    }
+        $hideminmaxbutton = $OUTPUT->single_button($goawayurl, $hidemessage, 'get');
+        $moreinfo = html_writer::link(get_docs_url(get_string('minmaxtouse_link', 'grades')), get_string('moreinfo'),
+            array('target' => '_blank'));
 
-    // Show the warning that there are min/max isseus that have not be resolved.
-    if ($revertminmax || (!$fixminmaxgrades && $showminmaxrevertwarning)) {
-        $message = get_string('minmaxupgradewarning', 'grades');
+        if ($minmaxtouse == GRADE_MIN_MAX_FROM_GRADE_ITEM) {
+            // Show the message that there were min/max issues that have been resolved.
+            $message = get_string('minmaxupgradedgrades', 'grades') . ' ' . $moreinfo;
 
-        $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;
+            $revertmessage = get_string('upgradedminmaxrevertmessage', 'grades');
+            $urlparams = array('id' => $courseid,
+                               'useminmaxfromgradegrade' => true,
+                               'sesskey' => sesskey());
+            $reverturl = new moodle_url($thispage, $urlparams);
+            $revertbutton = $OUTPUT->single_button($reverturl, $revertmessage, 'get');
+
+            $html .= $OUTPUT->notification($message, 'notifywarning');
+            $html .= $revertbutton . $hideminmaxbutton;
+
+        } else if ($minmaxtouse == GRADE_MIN_MAX_FROM_GRADE_GRADE) {
+            // Show the warning that there are min/max issues that have not be resolved.
+            $message = get_string('minmaxupgradewarning', 'grades') . ' ' . $moreinfo;
+
+            $fixmessage = get_string('minmaxupgradefixbutton', 'grades');
+            $urlparams = array('id' => $courseid,
+                               'useminmaxfromgradeitem' => true,
+                               'sesskey' => sesskey());
+            $fixurl = new moodle_url($thispage, $urlparams);
+            $fixbutton = $OUTPUT->single_button($fixurl, $fixmessage, 'get');
+
+            $html .= $OUTPUT->notification($message, 'notifywarning');
+            $html .= $fixbutton . $hideminmaxbutton;
+        }
+    }
+
+    if (!empty($html)) {
+        $html = html_writer::tag('div', $html, array('class' => 'core_grades_notices'));
     }
 
     if ($return) {

grade/report/grader/lib.php

@@ -1086,9 +1086,8 @@ 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)) {
-                            list($min, $max) = $grade->get_grade_min_max();
-                            $item->grademax = $max;
-                            $item->grademin = $min;
+                            $item->grademax = $grade->get_grade_max();
+                            $item->grademin = $grade->get_grade_min();
                         }
 
                         $itemcell->text .= "<span class='gradevalue{$hidden}{$gradepass}'>" .

grade/report/lib.php

@@ -437,9 +437,8 @@ protected function blank_hidden_total_and_adjust_bounds($courseid, $course_item,
         $grademin = $course_item->grademin;
         $grademax = $course_item->grademax;
         if ($coursegradegrade) {
-            list($min, $max) = $coursegradegrade->get_grade_min_max();
-            $grademin = $min;
-            $grademax = $max;
+            $grademin = $coursegradegrade->get_grade_min();
+            $grademax = $coursegradegrade->get_grade_max();
         } else {
             $coursegradegrade = new grade_grade(array('userid'=>$this->user->id, 'itemid'=>$course_item->id), false);
         }

grade/report/overview/lib.php

@@ -241,9 +241,8 @@ public function fill_table($activitylink = false, $studentcoursesonly = false) {
                     // 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)) {
-                        list($min, $max) = $course_grade->get_grade_min_max();
-                        $course_item->grademin = $min;
-                        $course_item->grademax = $max;
+                        $course_item->grademin = $course_grade->get_grade_min();
+                        $course_item->grademax = $course_grade->get_grade_max();
                     }
                 }
 

grade/report/user/lib.php

@@ -466,9 +466,8 @@ 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)) {
-                    list($min, $max) = $grade_grade->get_grade_min_max();
-                    $grade_grade->grade_item->grademax = $max;
-                    $grade_grade->grade_item->grademin = $min;
+                    $grade_grade->grade_item->grademax = $grade_grade->get_grade_max();
+                    $grade_grade->grade_item->grademin = $grade_grade->get_grade_min();
                 }
             }
 

lang/en/grades.php

@@ -268,6 +268,7 @@
 $string['gradeexportuserprofilefields'] = 'Grade export user profile fields';
 $string['gradeexportuserprofilefields_desc'] = 'Include these user profile fields in the grade export, separated by commas.';
 $string['gradeforstudent'] = '{$a->student}<br />{$a->item}{$a->feedback}';
+$string['gradegrademinmax'] = 'Initial min and max grades';
 $string['gradehelp'] = 'Grade help';
 $string['gradehistorylifetime'] = 'Grade history lifetime';
 $string['gradehistorylifetime_help'] = 'This specifies the length of time you want to keep history of changes in grade related tables. It is recommended to keep it as long as possible. If you experience performance problems or have limited database space, try to set lower value.';
@@ -279,6 +280,7 @@
 $string['gradeitemadvanced_help'] = 'Select all elements that should be displayed as advanced when editing grade items.';
 $string['gradeitemislocked'] = 'This activity is locked in the gradebook. Changes that are made to grades in this activity will not be copied to the gradebook until it is unlocked.';
 $string['gradeitemlocked'] = 'Grading locked';
+$string['gradeitemminmax'] = 'Min and max grades as specified in grade item settings';
 $string['gradeitemmembersselected'] = 'Excluded from grading';
 $string['gradeitemnonmembers'] = 'Included in grading';
 $string['gradeitemremovemembers'] = 'Include in grading';
@@ -452,9 +454,13 @@
 $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['minmaxtouse'] = 'Min and max grades used in calculation';
+$string['minmaxtouse_desc'] = 'This setting determines whether to use the initial minimum and maximum grades from when the grade was given, or the minimum and maximum grades as specified in the settings for the grade item, when calculating the grade displayed in the gradebook. It is recommended that this setting is modified at an off-peak time, as all grades will be recalculated, which may result in a high server load.';
+$string['minmaxtouse_help'] = 'This setting determines whether to use the initial minimum and maximum grades from when the grade was given, or the minimum and maximum grades as specified in the settings for the grade item, when calculating the grade displayed in the gradebook.';
+$string['minmaxtouse_link'] = 'Grades_min_max';
+$string['minmaxupgradedgrades'] = 'Note: Some grades have been changed in order to resolve an inconsistency in the gradebook caused by a change in the minimum and maximum grades used when calculating the grade displayed. It is recommended that the changes are reviewed and accepted.';
+$string['minmaxupgradefixbutton'] = 'Resolve inconsistencies';
+$string['minmaxupgradewarning'] = 'Note: An inconsistency has been detected with some grades due to a change in the minimum and maximum grades used when calculating the grade displayed in the gradebook. It is recommended that the inconsistency is resolved by clicking the button below, though this will result in some grades being changed.';
 $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';
@@ -710,7 +716,7 @@
 $string['submittedon'] = 'Submitted: {$a}';
 $string['sumofgradesupgradedgrades'] = 'Note: The aggregation method "Sum of grades" has been changed to "Natural" as part of a site upgrade. Since "Sum of grades" was previously used in this course, it is recommended that you review this change in the gradebook.';
 $string['aggregatesubcatsupgradedgrades'] = 'Note: The aggregation setting "Aggregate including subcategories" has been removed as part of a site upgrade. Since "Aggregate including subcategories" was previously used in this course, it is recommended that you review this change in the gradebook.';
-$string['upgradedgradeshidemessage'] = 'OK';
+$string['upgradedgradeshidemessage'] = 'Dismiss notice';
 $string['switchtofullview'] = 'Switch to full view';
 $string['switchtosimpleview'] = 'Switch to simple view';
 $string['tabs'] = 'Tabs';
@@ -725,7 +731,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['upgradedminmaxrevertmessage'] = 'Revert the changes';
 $string['uncategorised'] = 'Uncategorised';
 $string['unenrolledusersinimport'] = 'This import included the following grades for users not currently enrolled in this course: {$a}';
 $string['unchangedgrade'] = 'Grade unchanged';

lang/en/moodle.php

@@ -1185,6 +1185,7 @@
 $string['moodlerelease'] = 'Moodle release';
 $string['more'] = 'more';
 $string['morehelp'] = 'More help';
+$string['moreinfo'] = 'More info';
 $string['moreinformation'] = 'More information about this error';
 $string['moreprofileinfoneeded'] = 'Please tell us more about yourself';
 $string['mostrecently'] = 'most recently';