MDL-45390 gradebook: Grade boundary fix.
authorAdrian Greeve <adrian@moodle.com>
Mon, 9 May 2016 02:33:54 +0000 (10:33 +0800)
committerAdrian Greeve <adrian@moodle.com>
Wed, 18 May 2016 01:56:00 +0000 (09:56 +0800)
Code to upgrade, fix, and freeze the gradebook for
courses with letter boundary problems.

backup/moodle2/restore_stepslib.php
lib/db/install.php
lib/db/upgrade.php
lib/db/upgradelib.php
lib/gradelib.php
version.php

index 3c1f547..163ccf9 100644 (file)
@@ -493,6 +493,8 @@ class restore_gradebook_structure_step extends restore_structure_step {
         $gradebookcalculationsfreeze = get_config('core', 'gradebook_calculations_freeze_' . $this->get_courseid());
         preg_match('/(\d{8})/', $this->get_task()->get_info()->moodle_release, $matches);
         $backupbuild = (int)$matches[1];
+        // The function floatval will return a float even if there is text mixed with the release number.
+        $backuprelease = floatval($this->get_task()->get_info()->backup_release);
 
         // Extra credits need adjustments only for backups made between 2.8 release (20141110) and the fix release (20150619).
         if (!$gradebookcalculationsfreeze && $backupbuild >= 20141110 && $backupbuild < 20150619) {
@@ -504,6 +506,14 @@ class restore_gradebook_structure_step extends restore_structure_step {
             require_once($CFG->libdir . '/db/upgradelib.php');
             upgrade_calculated_grade_items($this->get_courseid());
         }
+        // Courses from before 3.1 (20160511) may have a letter boundary problem and should be checked for this issue.
+        // Backups from before and including 2.9 could have a build number that is greater than 20160511 and should
+        // be checked for this problem.
+        if (!$gradebookcalculationsfreeze && ($backupbuild < 20160511 || $backuprelease <= 2.9)) {
+            require_once($CFG->libdir . '/db/upgradelib.php');
+            upgrade_course_letter_boundary($this->get_courseid());
+        }
+
     }
 
     /**
index 466981b..1dfc5d2 100644 (file)
@@ -134,6 +134,7 @@ function xmldb_main_install() {
         'upgrade_minmaxgradestepignored' => 1, // New installs should not run this upgrade step.
         'upgrade_extracreditweightsstepignored' => 1, // New installs should not run this upgrade step.
         'upgrade_calculatedgradeitemsignored' => 1, // New installs should not run this upgrade step.
+        'upgrade_letterboundarycourses' => 1, // New installs should not run this upgrade step.
     );
     foreach($defaults as $key => $value) {
         set_config($key, $value);
index b10a8fd..3a0c3cc 100644 (file)
@@ -2053,5 +2053,21 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2016051300.00);
     }
 
+    if ($oldversion < 2016051700.01) {
+        // This script is included in each major version upgrade process (3.0, 3.1) so make sure we don't run it twice.
+        if (empty($CFG->upgrade_letterboundarycourses)) {
+            // MDL-21746. If a grade is being displayed with letters and the grade boundaries are not being adhered to properly
+            // then this course will also be frozen.
+            // If the changes are accepted then the display of some grades may change.
+            // This is here to freeze the gradebook in affected courses.
+            upgrade_course_letter_boundary();
+
+            // To skip running the same script on the upgrade to the next major version release.
+            set_config('upgrade_letterboundarycourses', 1);
+        }
+        // Main savepoint reached.
+        upgrade_main_savepoint(true, 2016051700.01);
+    }
+
     return true;
 }
index 4c69515..9708aff 100644 (file)
@@ -357,3 +357,158 @@ function make_competence_scale() {
     $defaultscale->id = $DB->insert_record('scale', $defaultscale);
     return $defaultscale;
 }
+
+/**
+ * Marks all courses that require rounded grade items be updated.
+ *
+ * Used during upgrade and in course restore process.
+ *
+ * This upgrade script is needed because it has been decided that if a grade is rounded up, and it will changed a letter
+ * grade or satisfy a course completion grade criteria, then it should be set as so, and the letter will be awarded and or
+ * the course completion grade will be awarded.
+ *
+ * @param int $courseid Specify a course ID to run this script on just one course.
+ */
+function upgrade_course_letter_boundary($courseid = null) {
+    global $DB, $CFG;
+
+    $coursesql = '';
+    $params = array('contextlevel' => CONTEXT_COURSE);
+    if (!empty($courseid)) {
+        $coursesql = 'AND c.id = :courseid';
+        $params['courseid'] = $courseid;
+    }
+
+    $contextselect = context_helper::get_preload_record_columns_sql('ctx');
+
+    // 3, 13, 23, 31, and 32 are the grade display types that incorporate showing letters. See lib/grade/constants/php.
+    if (isset($CFG->grade_displaytype) && in_array($CFG->grade_displaytype, array(3, 13, 23, 31, 32))) {
+        // Check to see if the system letter boundaries are borked.
+        $systemcontext = context_system::instance();
+        if (upgrade_letter_boundary_needs_freeze($systemcontext)) {
+            // Select courses with no grade setting for display and a grade item that is using the default display,
+            // but have not altered the course letter boundary configuration. These courses are definitely affected.
+            $sql = "SELECT DISTINCT c.id AS courseid
+                      FROM {grade_items} gi
+                      JOIN {course} c ON c.id = gi.courseid
+                 LEFT JOIN {grade_settings} gs ON gs.courseid = c.id AND name = 'displaytype'
+                 LEFT JOIN (SELECT DISTINCT c.id
+                              FROM {grade_letters} gl
+                              JOIN {context} ctx ON gl.contextid = ctx.id
+                              JOIN {course} c ON ctx.instanceid = c.id
+                             WHERE ctx.contextlevel = :contextlevel) gl ON gl.id = c.id
+                     WHERE (gi.display = 0 AND (gs.value is NULL))
+                     AND gl.id is NULL $coursesql";
+            $affectedcourseids = $DB->get_recordset_sql($sql, $params);
+            foreach ($affectedcourseids as $courseid) {
+                set_config('gradebook_calculations_freeze_' . $courseid->courseid, 20160511);
+            }
+            $affectedcourseids->close();
+        }
+        // If the system letter boundary is okay proceed to check grade item and course grade display settings.
+        $params['contextlevel2'] = CONTEXT_COURSE;
+        $sql = "SELECT DISTINCT c.id AS courseid, $contextselect
+                  FROM {course} c
+                  JOIN {context} ctx ON ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel
+                  JOIN {grade_items} gi ON c.id = gi.courseid
+             LEFT JOIN {grade_settings} gs ON c.id = gs.courseid AND name = 'displaytype'
+             LEFT JOIN (SELECT DISTINCT c.id
+                          FROM {grade_letters} gl
+                          JOIN {context} ctx ON gl.contextid = ctx.id
+                          JOIN {course} c ON ctx.instanceid = c.id
+                         WHERE ctx.contextlevel = :contextlevel2) gl ON gl.id = c.id
+                 WHERE (gi.display IN (3, 13, 23, 31, 32)
+                    OR (" . $DB->sql_compare_text('gs.value') . " IN ('3', '13', '23', '31', '32'))
+                    OR gl.id is NOT NULL)
+                       $coursesql";
+    } else {
+        // There is no site setting for letter grades. Just check the modified letter boundaries.
+        $sql = "SELECT DISTINCT c.id AS courseid, $contextselect
+                  FROM {grade_letters} l, {course} c
+                  JOIN {context} ctx ON ctx.instanceid = c.id AND ctx.contextlevel = :contextlevel
+                 WHERE l.contextid = ctx.id
+                   AND ctx.instanceid = c.id
+                       $coursesql";
+    }
+
+    $potentialcourses = $DB->get_recordset_sql($sql, $params);
+
+    foreach ($potentialcourses as $value) {
+        $gradebookfreeze = 'gradebook_calculations_freeze_' . $value->courseid;
+
+        // Check also if this course id has already been frozen.
+        // If we already have this course ID then move on to the next record.
+        if (!property_exists($CFG, $gradebookfreeze)) {
+            // Check for 57 letter grade issue.
+            context_helper::preload_from_record($value);
+            $coursecontext = context_course::instance($value->courseid);
+            if (upgrade_letter_boundary_needs_freeze($coursecontext)) {
+                // We have a course with a possible score standardisation problem. Flag for freeze.
+                // Flag this course as being frozen.
+                set_config('gradebook_calculations_freeze_' . $value->courseid, 20160511);
+            }
+        }
+    }
+    $potentialcourses->close();
+}
+
+/**
+ * Checks the letter boundary of the provided context to see if it needs freezing.
+ * Each letter boundary is tested to see if receiving that boundary number will
+ * result in achieving the cosponsoring letter.
+ *
+ * @param object $context Context object
+ * @return bool if the letter boundary for this context should be frozen.
+ */
+function upgrade_letter_boundary_needs_freeze($context) {
+    global $DB;
+
+    $contexts = $context->get_parent_context_ids();
+    array_unshift($contexts, $context->id);
+
+    foreach ($contexts as $ctxid) {
+
+        $letters = $DB->get_records_menu('grade_letters', array('contextid' => $ctxid), 'lowerboundary DESC',
+                'lowerboundary, letter');
+
+        if (!empty($letters)) {
+            foreach ($letters as $boundary => $notused) {
+                $standardisedboundary = upgrade_standardise_score($boundary, 0, 100, 0, 100);
+                if ($boundary != $standardisedboundary) {
+                    return true;
+                }
+            }
+            // We found letters but we have no boundary problem.
+            return false;
+        }
+    }
+    return false;
+}
+
+/**
+ * Given a float value situated between a source minimum and a source maximum, converts it to the
+ * corresponding value situated between a target minimum and a target maximum. Thanks to Darlene
+ * for the formula :-)
+ *
+ * @param float $rawgrade
+ * @param float $sourcemin
+ * @param float $sourcemax
+ * @param float $targetmin
+ * @param float $targetmax
+ * @return float Converted value
+ */
+function upgrade_standardise_score($rawgrade, $sourcemin, $sourcemax, $targetmin, $targetmax) {
+    if (is_null($rawgrade)) {
+        return null;
+    }
+
+    if ($sourcemax == $sourcemin or $targetmin == $targetmax) {
+        // Prevent division by 0.
+        return $targetmax;
+    }
+
+    $factor = ($rawgrade - $sourcemin) / ($sourcemax - $sourcemin);
+    $diff = $targetmax - $targetmin;
+    $standardisedvalue = $factor * $diff + $targetmin;
+    return $standardisedvalue;
+}
index 8cd7da7..43d5b5e 100644 (file)
@@ -846,6 +846,7 @@ function grade_format_gradevalue_percentage($value, $grade_item, $decimals, $loc
  * @return string
  */
 function grade_format_gradevalue_letter($value, $grade_item) {
+    global $CFG;
     $context = context_course::instance($grade_item->courseid, IGNORE_MISSING);
     if (!$letters = grade_get_letters($context)) {
         return ''; // no letters??
@@ -857,7 +858,16 @@ function grade_format_gradevalue_letter($value, $grade_item) {
 
     $value = grade_grade::standardise_score($value, $grade_item->grademin, $grade_item->grademax, 0, 100);
     $value = bounded_number(0, $value, 100); // just in case
+
+    $gradebookcalculationsfreeze = 'gradebook_calculations_freeze_' . $grade_item->courseid;
+
     foreach ($letters as $boundary => $letter) {
+        if (property_exists($CFG, $gradebookcalculationsfreeze) && (int)$CFG->{$gradebookcalculationsfreeze} <= 20160511) {
+            // Do nothing.
+        } else {
+            // The boundary is a percentage out of 100 so use 0 as the min and 100 as the max.
+            $boundary = grade_grade::standardise_score($boundary, 0, 100, 0, 100);
+        }
         if ($value >= $boundary) {
             return format_string($letter);
         }
index 461f8d4..912b4b5 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2016051700.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2016051700.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.