Commit e48f0ef1 authored by Marina Glancy's avatar Marina Glancy
Browse files

MDL-49257 grades: fix bugs with extra credit weights

When we adjust the weights in natural grading (setup screen), the extra credit items
should not depend on other overrides. If extra credit item's weight was overriden, it stays as it is.
Otherwise it is calculated as itemgrademax / totalgrademax (total excludes extra credit items)
parent 4a841774
......@@ -476,6 +476,12 @@ 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];
// Extra credits need adjustments only for backups made between 2.8 release (20141110) and the fix release (20150619).
if (!$gradebookcalculationsfreeze && $backupbuild >= 20141110 && $backupbuild < 20150619) {
require_once($CFG->libdir . '/db/upgradelib.php');
upgrade_extra_credit_weightoverride($this->get_courseid());
}
}
}
......
......@@ -131,6 +131,7 @@ function xmldb_main_install() {
'filterall' => 0, // setting page, so have to be initialised here.
'texteditors' => 'atto,tinymce,textarea',
'upgrade_minmaxgradestepignored' => 1, // New installs should not run this upgrade step.
'upgrade_extracreditweightsstepignored' => 1, // New installs should not run this upgrade step.
);
foreach($defaults as $key => $value) {
set_config($key, $value);
......
......@@ -4094,5 +4094,25 @@ function xmldb_main_upgrade($oldversion) {
upgrade_main_savepoint(true, 2014111006.05);
}
if ($oldversion < 2014111006.08) {
// MDL-49257. Changed the algorithm of calculating automatic weights of extra credit items.
// Before the change, in case when grade category (in "Natural" agg. method) had items with
// overridden weights, the automatic weight of extra credit items was illogical.
// In order to prevent grades changes after the upgrade we need to freeze gradebook calculation
// for the affected courses.
// This script in included in each major version upgrade process so make sure we don't run it twice.
if (empty($CFG->upgrade_extracreditweightsstepignored)) {
upgrade_extra_credit_weightoverride();
// To skip running the same script on the upgrade to the next major release.
set_config('upgrade_extracreditweightsstepignored', 1);
}
// Main savepoint reached.
upgrade_main_savepoint(true, 2014111006.08);
}
return true;
}
......@@ -529,4 +529,39 @@ function upgrade_mimetypes($filetypes) {
array($extension)
);
}
}
/**
* Marks all courses with changes in extra credit weight calculation
*
* Used during upgrade and in course restore process
*
* This upgrade script is needed because we changed the algorithm for calculating the automatic weights of extra
* credit items and want to prevent changes in the existing student grades.
*
* @param int $onlycourseid
*/
function upgrade_extra_credit_weightoverride($onlycourseid = 0) {
global $DB;
// Find all courses that have categories in Natural aggregation method where there is at least one extra credit
// item and at least one item with overridden weight.
$courses = $DB->get_fieldset_sql(
"SELECT DISTINCT gc.courseid
FROM {grade_categories} gc
INNER JOIN {grade_items} gi ON gc.id = gi.categoryid AND gi.weightoverride = :weightoverriden
INNER JOIN {grade_items} gie ON gc.id = gie.categoryid AND gie.aggregationcoef = :extracredit
WHERE gc.aggregation = :naturalaggmethod" . ($onlycourseid ? " AND gc.courseid = :onlycourseid" : ''),
array('naturalaggmethod' => 13,
'weightoverriden' => 1,
'extracredit' => 1,
'onlycourseid' => $onlycourseid,
)
);
foreach ($courses as $courseid) {
$gradebookfreeze = get_config('core', 'gradebook_calculations_freeze_' . $courseid);
if (!$gradebookfreeze) {
set_config('gradebook_calculations_freeze_' . $courseid, 20150619);
}
}
}
\ No newline at end of file
......@@ -1166,6 +1166,12 @@ class grade_category extends grade_object {
$this->load_grade_item();
$num = count($grade_values);
$sum = 0;
// This setting indicates if we should use algorithm prior to MDL-49257 fix for calculating extra credit weights.
// Even though old algorith has bugs in it, we need to preserve existing grades.
$gradebookcalculationfreeze = (int)get_config('core', 'gradebook_calculations_freeze_' . $this->courseid);
$oldextracreditcalculation = $gradebookcalculationfreeze && ($gradebookcalculationfreeze <= 20150619);
$sumweights = 0;
$grademin = 0;
$grademax = 0;
......@@ -1205,7 +1211,11 @@ class grade_category extends grade_object {
$userweights[$itemid] = 0;
continue;
}
$userweights[$itemid] = $items[$itemid]->aggregationcoef2 / $sumweights;
$userweights[$itemid] = $sumweights ? ($items[$itemid]->aggregationcoef2 / $sumweights) : 0;
if (!$oldextracreditcalculation && isset($extracredititems[$itemid])) {
// Extra credit items do not affect totals.
continue;
}
$totaloverriddenweight += $userweights[$itemid];
$usergrademax = $items[$itemid]->grademax;
if (isset($grademaxoverrides[$itemid])) {
......@@ -1216,9 +1226,9 @@ class grade_category extends grade_object {
}
$nonoverriddenpoints = $grademax - $totaloverriddengrademax;
// Then we need to recalculate the automatic weights.
// Then we need to recalculate the automatic weights except for extra credit items.
foreach ($grade_values as $itemid => $gradevalue) {
if (!$items[$itemid]->weightoverride) {
if (!$items[$itemid]->weightoverride && ($oldextracreditcalculation || !isset($extracredititems[$itemid]))) {
$usergrademax = $items[$itemid]->grademax;
if (isset($grademaxoverrides[$itemid])) {
$usergrademax = $grademaxoverrides[$itemid];
......@@ -1236,6 +1246,19 @@ class grade_category extends grade_object {
}
}
// Now when we finally know the grademax we can adjust the automatic weights of extra credit items.
if (!$oldextracreditcalculation) {
foreach ($grade_values as $itemid => $gradevalue) {
if (!$items[$itemid]->weightoverride && isset($extracredititems[$itemid])) {
$usergrademax = $items[$itemid]->grademax;
if (isset($grademaxoverrides[$itemid])) {
$usergrademax = $grademaxoverrides[$itemid];
}
$userweights[$itemid] = $grademax ? ($usergrademax / $grademax) : 0;
}
}
}
// We can use our freshly corrected weights below.
foreach ($grade_values as $itemid => $gradevalue) {
if (isset($extracredititems[$itemid])) {
......@@ -1517,6 +1540,11 @@ class grade_category extends grade_object {
$totalnonoverriddengrademax = $totalgrademax - $totaloverriddengrademax;
// This setting indicates if we should use algorithm prior to MDL-49257 fix for calculating extra credit weights.
// Even though old algorith has bugs in it, we need to preserve existing grades.
$gradebookcalculationfreeze = (int)get_config('core', 'gradebook_calculations_freeze_' . $this->courseid);
$oldextracreditcalculation = $gradebookcalculationfreeze && ($gradebookcalculationfreeze <= 20150619);
reset($children);
foreach ($children as $sortorder => $child) {
$gradeitem = null;
......@@ -1539,6 +1567,16 @@ class grade_category extends grade_object {
continue;
}
if (!$oldextracreditcalculation && $gradeitem->aggregationcoef > 0) {
// For an item with extra credit ignore other weigths and overrides.
// Do not change anything at all if it's weight was already overridden.
if (!$gradeitem->weightoverride) {
$gradeitem->aggregationcoef2 = $totalgrademax ? ($gradeitem->grademax / $totalgrademax) : 0;
$gradeitem->update();
}
continue;
}
if (!$gradeitem->weightoverride) {
// Calculations with a grade maximum of zero will cause problems. Just set the weight to zero.
if ($totaloverriddenweight >= 1 || $totalnonoverriddengrademax == 0 || $gradeitem->grademax == 0) {
......
......@@ -482,4 +482,67 @@ class core_upgradelib_testcase extends advanced_testcase {
// Restore value.
$CFG->grade_minmaxtouse = $initialminmax;
}
public function test_upgrade_extra_credit_weightoverride() {
global $DB, $CFG;
$this->resetAfterTest(true);
$c = array();
$a = array();
$gi = array();
for ($i=0; $i<5; $i++) {
$c[$i] = $this->getDataGenerator()->create_course();
$a[$i] = array();
$gi[$i] = array();
for ($j=0;$j<3;$j++) {
$a[$i][$j] = $this->getDataGenerator()->create_module('assign', array('course' => $c[$i], 'grade' => 100));
$giparams = array('itemtype' => 'mod', 'itemmodule' => 'assign', 'iteminstance' => $a[$i][$j]->id,
'courseid' => $c[$i]->id, 'itemnumber' => 0);
$gi[$i][$j] = grade_item::fetch($giparams);
}
}
// Case 1: Course $c[0] has aggregation method different from natural.
$coursecategory = grade_category::fetch_course_category($c[0]->id);
$coursecategory->aggregation = GRADE_AGGREGATE_WEIGHTED_MEAN;
$coursecategory->update();
$gi[0][1]->aggregationcoef = 1;
$gi[0][1]->update();
$gi[0][2]->weightoverride = 1;
$gi[0][2]->update();
// Case 2: Course $c[1] has neither extra credits nor overrides
// Case 3: Course $c[2] has extra credits but no overrides
$gi[2][1]->aggregationcoef = 1;
$gi[2][1]->update();
// Case 4: Course $c[3] has no extra credits and has overrides
$gi[3][2]->weightoverride = 1;
$gi[3][2]->update();
// Case 5: Course $c[4] has both extra credits and overrides
$gi[4][1]->aggregationcoef = 1;
$gi[4][1]->update();
$gi[4][2]->weightoverride = 1;
$gi[4][2]->update();
// Run the upgrade script and make sure only course $c[4] was marked as needed to be fixed.
upgrade_extra_credit_weightoverride();
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $c[0]->id}));
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $c[1]->id}));
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $c[2]->id}));
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $c[3]->id}));
$this->assertEquals(20150619, $CFG->{'gradebook_calculations_freeze_' . $c[4]->id});
set_config('gradebook_calculations_freeze_' . $c[4]->id, null);
// Run the upgrade script for a single course only.
upgrade_extra_credit_weightoverride($c[0]->id);
$this->assertTrue(empty($CFG->{'gradebook_calculations_freeze_' . $c[0]->id}));
upgrade_extra_credit_weightoverride($c[4]->id);
$this->assertEquals(20150619, $CFG->{'gradebook_calculations_freeze_' . $c[4]->id});
}
}
......@@ -29,7 +29,7 @@
defined('MOODLE_INTERNAL') || die();
$version = 2014111006.07; // 20141110 = branching date YYYYMMDD - do not modify!
$version = 2014111006.08; // 20141110 = branching date YYYYMMDD - do not modify!
// RR = release increments - 00 in DEV branches.
// .XX = incremental changes.
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment