MDL-64609 gradebook: Prevent infinite loop in regrading

robjoyce · Jan 23, 2019 · d64e545 · d64e545
1 parent c092f75
commit d64e545
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 2 deletions.
diff --git a/lib/gradelib.php b/lib/gradelib.php
@@ -1207,6 +1207,7 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null,
     $failed = 0;
 
     while (count($finalids) < count($gids)) { // work until all grades are final or error found
+        $count = 0;
         foreach ($gids as $gid) {
             if (in_array($gid, $finalids)) {
                 continue; // already final
@@ -1261,7 +1262,7 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null,
                 if ($updateddependencies === false) {
                     // If no direct descendants are marked as updated, then we don't need to update this grade item. We then mark it
                     // as final.
-
+                    $count++;
                     $finalids[] = $gid;
                     continue;
                 }
@@ -1280,6 +1281,7 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null,
                 } else {
                     $grade_items[$gid]->needsupdate = 0;
                 }
+                $count++;
                 $finalids[] = $gid;
                 $updatedids[] = $gid;
 
@@ -1289,7 +1291,7 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null,
             }
         }
 
-        if (count($finalids) == 0) {
+        if ($count == 0) {
             $failed++;
         } else {
             $failed = 0;

diff --git a/lib/tests/gradelib_test.php b/lib/tests/gradelib_test.php
@@ -104,4 +104,51 @@ public function test_grade_course_category_delete() {
         // Confirm grade letter was deleted.
         $this->assertEquals(0, $DB->count_records('grade_letters'));
     }
+
+    /**
+     * Tests the function grade_regrade_final_grades().
+     */
+    public function test_grade_regrade_final_grades() {
+        global $DB;
+
+        $this->resetAfterTest();
+
+        // Setup some basics.
+        $course = $this->getDataGenerator()->create_course();
+        $user = $this->getDataGenerator()->create_user();
+        $this->getDataGenerator()->enrol_user($user->id, $course->id, 'student');
+
+        // We need two grade items.
+        $params = ['idnumber' => 'g1', 'courseid' => $course->id];
+        $g1 = new grade_item($this->getDataGenerator()->create_grade_item($params));
+        unset($params['idnumber']);
+        $g2 = new grade_item($this->getDataGenerator()->create_grade_item($params));
+
+        $category = new grade_category($this->getDataGenerator()->create_grade_category($params));
+        $catitem = $category->get_grade_item();
+
+        // Now set a calculation.
+        $catitem->set_calculation('=[[g1]]');
+
+        $catitem->update();
+
+        // Everything needs updating.
+        $this->assertEquals(4, $DB->count_records('grade_items', ['courseid' => $course->id, 'needsupdate' => 1]));
+
+        grade_regrade_final_grades($course->id);
+
+        // See that everything has been updated.
+        $this->assertEquals(0, $DB->count_records('grade_items', ['courseid' => $course->id, 'needsupdate' => 1]));
+
+        $g1->delete();
+
+        // Now there is one that needs updating.
+        $this->assertEquals(1, $DB->count_records('grade_items', ['courseid' => $course->id, 'needsupdate' => 1]));
+
+        // This can cause an infinite loop if things go... poorly.
+        grade_regrade_final_grades($course->id);
+
+        // Now because of the failure, two things need updating.
+        $this->assertEquals(2, $DB->count_records('grade_items', ['courseid' => $course->id, 'needsupdate' => 1]));
+    }
 }