From 405b90bcd2d1b6be1204cb4e2d4244b1c39f2986 Mon Sep 17 00:00:00 2001 From: Adrian Greeve Date: Mon, 9 May 2016 10:33:54 +0800 Subject: [PATCH] MDL-45390 gradebook: Grade boundary fix. Code to upgrade, fix, and freeze the gradebook for courses with letter boundary problems. --- backup/moodle2/restore_stepslib.php | 10 ++ lib/db/install.php | 1 + lib/db/upgrade.php | 16 +++ lib/db/upgradelib.php | 155 ++++++++++++++++++++++++++++ lib/gradelib.php | 10 ++ version.php | 2 +- 6 files changed, 193 insertions(+), 1 deletion(-) diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index 3c1f547310b..163ccf92c35 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -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()); + } + } /** diff --git a/lib/db/install.php b/lib/db/install.php index 466981bd826..1dfc5d20fd3 100644 --- a/lib/db/install.php +++ b/lib/db/install.php @@ -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); diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index b10a8fd2019..3a0c3cc8e10 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -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; } diff --git a/lib/db/upgradelib.php b/lib/db/upgradelib.php index 4c69515f0ba..9708affe34b 100644 --- a/lib/db/upgradelib.php +++ b/lib/db/upgradelib.php @@ -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; +} diff --git a/lib/gradelib.php b/lib/gradelib.php index 8cd7da71f7a..43d5b5e0cf8 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -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); } diff --git a/version.php b/version.php index 461f8d4dc35..912b4b5bbe9 100644 --- a/version.php +++ b/version.php @@ -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.