From d64e545a36756fb338d76e16bf83d22b7c7b992b Mon Sep 17 00:00:00 2001 From: Eric Merrill Date: Tue, 22 Jan 2019 21:40:12 -0500 Subject: [PATCH] MDL-64609 gradebook: Prevent infinite loop in regrading --- lib/gradelib.php | 6 +++-- lib/tests/gradelib_test.php | 47 +++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/lib/gradelib.php b/lib/gradelib.php index f0cd4c8e3937f..2e67318badfd9 100644 --- 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 index 178783969998c..0e6eebcb6f871 100644 --- 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])); + } }