From 1076e02f29e3003a2a0d298057847855fda8cfd9 Mon Sep 17 00:00:00 2001 From: David Monllao Date: Thu, 31 Mar 2016 16:28:27 +0800 Subject: [PATCH] MDL-53309 grades: Only update when required --- lib/grade/grade_category.php | 136 ++++++++++++++++++------ lib/grade/tests/grade_category_test.php | 4 + 2 files changed, 109 insertions(+), 31 deletions(-) diff --git a/lib/grade/grade_category.php b/lib/grade/grade_category.php index 2723f7079a1..7ad1284ac60 100644 --- a/lib/grade/grade_category.php +++ b/lib/grade/grade_category.php @@ -813,34 +813,108 @@ class grade_category extends grade_object { private function set_usedinaggregation($userid, $usedweights, $novalue, $dropped, $extracredit) { global $DB; - // Reset aggregation to unknown and 0 for all grade items for this user and category. + // We want to know all current user grades so we can decide whether they need to be updated or they already contain the + // expected value. + $sql = "SELECT gi.id, gg.aggregationstatus, gg.aggregationweight FROM {grade_grades} gg + JOIN {grade_items} gi ON (gg.itemid = gi.id) + WHERE gg.userid = :userid"; $params = array('categoryid' => $this->id, 'userid' => $userid); - switch ($DB->get_dbfamily()) { - case 'mysql': - // Optimize the query for MySQL by using a join rather than a sub-query. - $sql = "UPDATE {grade_grades} g - JOIN {grade_items} gi ON (g.itemid = gi.id) - SET g.aggregationstatus = 'unknown', - g.aggregationweight = 0 - WHERE g.userid = :userid - AND gi.categoryid = :categoryid"; - break; - default: - $itemssql = "SELECT id - FROM {grade_items} - WHERE categoryid = :categoryid"; + // These are all grade_item ids which grade_grades will NOT end up being 'unknown' (because they are not unknown or + // because we will update them to something different that 'unknown'). + $giids = array_keys($usedweights + $novalue + $dropped + $extracredit); - $sql = "UPDATE {grade_grades} - SET aggregationstatus = 'unknown', - aggregationweight = 0 - WHERE userid = :userid - AND itemid IN ($itemssql)"; + if ($giids) { + // We include grade items that might not be in categoryid. + list($itemsql, $itemlist) = $DB->get_in_or_equal($giids, SQL_PARAMS_NAMED, 'gg'); + $sql .= ' AND (gi.categoryid = :categoryid OR gi.id ' . $itemsql . ')'; + $params = $params + $itemlist; + } else { + $sql .= ' AND gi.categoryid = :categoryid'; + } + $currentgrades = $DB->get_recordset_sql($sql, $params); + + // We will store here the grade_item ids that need to be updated on db. + $toupdate = array(); + + if ($currentgrades->valid()) { + + // Iterate through the user grades to see if we really need to update any of them. + foreach ($currentgrades as $currentgrade) { + + // Unset $usedweights that we do not need to update. + if (!empty($usedweights) && isset($usedweights[$currentgrade->id]) && $currentgrade->aggregationstatus === 'used') { + // We discard the ones that already have the contribution specified in $usedweights and are marked as 'used'. + if (grade_floats_equal($currentgrade->aggregationweight, $usedweights[$currentgrade->id])) { + unset($usedweights[$currentgrade->id]); + } + // Used weights can be present in multiple set_usedinaggregation arguments. + if (!isset($novalue[$currentgrade->id]) && !isset($dropped[$currentgrade->id]) && + !isset($extracredit[$currentgrade->id])) { + continue; + } + } + + // No value grades. + if (!empty($novalue) && isset($novalue[$currentgrade->id])) { + if ($currentgrade->aggregationstatus !== 'novalue' || + grade_floats_different($currentgrade->aggregationweight, 0)) { + $toupdate['novalue'][] = $currentgrade->id; + } + continue; + } + + // Dropped grades. + if (!empty($dropped) && isset($dropped[$currentgrade->id])) { + if ($currentgrade->aggregationstatus !== 'dropped' || + grade_floats_different($currentgrade->aggregationweight, 0)) { + $toupdate['dropped'][] = $currentgrade->id; + } + continue; + } + + // Extra credit grades. + if (!empty($extracredit) && isset($extracredit[$currentgrade->id])) { + + // If this grade item is already marked as 'extra' and it already has the provided $usedweights value would be + // silly to update to 'used' to later update to 'extra'. + if (!empty($usedweights) && isset($usedweights[$currentgrade->id]) && + grade_floats_equal($currentgrade->aggregationweight, $usedweights[$currentgrade->id])) { + unset($usedweights[$currentgrade->id]); + } + + // Update the item to extra if it is not already marked as extra in the database or if the item's + // aggregationweight will be updated when going through $usedweights items. + if ($currentgrade->aggregationstatus !== 'extra' || + (!empty($usedweights) && isset($usedweights[$currentgrade->id]))) { + $toupdate['extracredit'][] = $currentgrade->id; + } + continue; + } + + // If is not in any of the above groups it should be set to 'unknown', checking that the item is not already + // unknown, if it is we don't need to update it. + if ($currentgrade->aggregationstatus !== 'unknown' || grade_floats_different($currentgrade->aggregationweight, 0)) { + $toupdate['unknown'][] = $currentgrade->id; + } + } + $currentgrades->close(); } - $DB->execute($sql, $params); + // Update items to 'unknown' status. + if (!empty($toupdate['unknown'])) { + list($itemsql, $itemlist) = $DB->get_in_or_equal($toupdate['unknown'], SQL_PARAMS_NAMED, 'g'); - // Included with weights. + $itemlist['userid'] = $userid; + + $sql = "UPDATE {grade_grades} + SET aggregationstatus = 'unknown', + aggregationweight = 0 + WHERE itemid $itemsql AND userid = :userid"; + $DB->execute($sql, $itemlist); + } + + // Update items to 'used' status and setting the proper weight. if (!empty($usedweights)) { // The usedweights items are updated individually to record the weights. foreach ($usedweights as $gradeitemid => $contribution) { @@ -854,9 +928,9 @@ class grade_category extends grade_object { } } - // No value. - if (!empty($novalue)) { - list($itemsql, $itemlist) = $DB->get_in_or_equal(array_keys($novalue), SQL_PARAMS_NAMED, 'g'); + // Update items to 'novalue' status. + if (!empty($toupdate['novalue'])) { + list($itemsql, $itemlist) = $DB->get_in_or_equal($toupdate['novalue'], SQL_PARAMS_NAMED, 'g'); $itemlist['userid'] = $userid; @@ -868,9 +942,9 @@ class grade_category extends grade_object { $DB->execute($sql, $itemlist); } - // Dropped. - if (!empty($dropped)) { - list($itemsql, $itemlist) = $DB->get_in_or_equal(array_keys($dropped), SQL_PARAMS_NAMED, 'g'); + // Update items to 'dropped' status. + if (!empty($toupdate['dropped'])) { + list($itemsql, $itemlist) = $DB->get_in_or_equal($toupdate['dropped'], SQL_PARAMS_NAMED, 'g'); $itemlist['userid'] = $userid; @@ -882,9 +956,9 @@ class grade_category extends grade_object { $DB->execute($sql, $itemlist); } - // Extra credit. - if (!empty($extracredit)) { - list($itemsql, $itemlist) = $DB->get_in_or_equal(array_keys($extracredit), SQL_PARAMS_NAMED, 'g'); + // Update items to 'extracredit' status. + if (!empty($toupdate['extracredit'])) { + list($itemsql, $itemlist) = $DB->get_in_or_equal($toupdate['extracredit'], SQL_PARAMS_NAMED, 'g'); $itemlist['userid'] = $userid; diff --git a/lib/grade/tests/grade_category_test.php b/lib/grade/tests/grade_category_test.php index 5294b9429cb..86201f1f6c0 100644 --- a/lib/grade/tests/grade_category_test.php +++ b/lib/grade/tests/grade_category_test.php @@ -282,6 +282,10 @@ class core_grade_category_testcase extends grade_base_testcase { $DB->set_field('grade_grades', 'finalgrade', null, array('itemid'=>$childcat1itemid)); $parentcat->generate_grades(); + $this->assertFalse($DB->record_exists_select( + 'grade_grades', + "aggregationstatus='dropped' and itemid in (?,?)", + array($childcat1itemid, $childcat2itemid))); $this->assertTrue($DB->record_exists_select( 'grade_grades', "aggregationstatus='novalue' and itemid = ?",