MDL-53309 grades: Only update when required

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()) {
+        // These are all grade_item ids which grade_grades will NOT end up being 'unknown' (because they are not unknown or
-            case 'mysql':
+        // because we will update them to something different that 'unknown').
-                // Optimize the query for MySQL by using a join rather than a sub-query.
+        $giids = array_keys($usedweights + $novalue + $dropped + $extracredit);
-                $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";
 
-                $sql = "UPDATE {grade_grades}
+        if ($giids) {
-                           SET aggregationstatus = 'unknown',
+            // We include grade items that might not be in categoryid.
-                               aggregationweight = 0
+            list($itemsql, $itemlist) = $DB->get_in_or_equal($giids, SQL_PARAMS_NAMED, 'gg');
-                         WHERE userid = :userid
+            $sql .= ' AND (gi.categoryid = :categoryid OR gi.id ' . $itemsql . ')';
-                           AND itemid IN ($itemssql)";
+            $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.
+        // Update items to 'novalue' status.
-        if (!empty($novalue)) {
+        if (!empty($toupdate['novalue'])) {
-            list($itemsql, $itemlist) = $DB->get_in_or_equal(array_keys($novalue), SQL_PARAMS_NAMED, 'g');
+            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.
+        // Update items to 'dropped' status.
-        if (!empty($dropped)) {
+        if (!empty($toupdate['dropped'])) {
-            list($itemsql, $itemlist) = $DB->get_in_or_equal(array_keys($dropped), SQL_PARAMS_NAMED, 'g');
+            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.
+        // Update items to 'extracredit' status.
-        if (!empty($extracredit)) {
+        if (!empty($toupdate['extracredit'])) {
-            list($itemsql, $itemlist) = $DB->get_in_or_equal(array_keys($extracredit), SQL_PARAMS_NAMED, 'g');
+            list($itemsql, $itemlist) = $DB->get_in_or_equal($toupdate['extracredit'], SQL_PARAMS_NAMED, 'g');
 
             $itemlist['userid'] = $userid;
 

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 = ?",