Skip to content

Commit 462e97c

Browse files
marxjohnsonmarxjohnson
committed
MDL-81714 grades: Make large regrades asynchronous
Currently, large courses can take a long time to perform a full regrade. This is currently handled with a progress bar to prevent frontend timeouts while the regrade takes place. However, because it can take so long a teacher may not want to wait with the page open for several minutes, particularly if they are performing several operations that trigger a regrade. This adds a new async flag to grade_regrade_final_grades which is true by default. Instead of performing the regrade immediately, this queues an instance of \core\task\regrade_final_grades for the course, which will be executed in the background. It is advisable to always leave the async flag set true, except in the following scenarios: - Automated tests. - The regrade_final_grades task which actually wants to do the calculations immediately. - When you have performed a check to determine that the regrade process is unlikely to take a long time, for example there are only a small number of grade items.
1 parent 6a49950 commit 462e97c

File tree

18 files changed

+205
-67
lines changed

18 files changed

+205
-67
lines changed

‎course/classes/task/regrade_final_grades.php

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php
2+
// This file is part of Moodle - http://moodle.org/
3+
//
4+
// Moodle is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// Moodle is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
16+
namespace core_course\task;
17+
18+
defined('MOODLE_INTERNAL') || die();
19+
20+
use core\task\adhoc_task;
21+
22+
require_once($CFG->libdir . '/gradelib.php');
23+
24+
/**
25+
* Asynchronously regrade a course.
26+
*
27+
* @copyright 2024 onwards Catalyst IT Europe Ltd.
28+
* @author Mark Johnson <[email protected]>
29+
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
30+
*/
31+
class regrade_final_grades extends adhoc_task {
32+
33+
use \core\task\logging_trait;
34+
35+
use \core\task\stored_progress_task_trait;
36+
37+
public static function create(int $courseid): self {
38+
$task = new regrade_final_grades();
39+
$task->set_custom_data((object)['courseid' => $courseid]);
40+
$task->set_component('core');
41+
return $task;
42+
}
43+
44+
/**
45+
* Run regrade_final_grades for the provided course id.
46+
*
47+
* @return void
48+
* @throws \dml_exception
49+
*/
50+
public function execute(): void {
51+
$data = $this->get_custom_data();
52+
$this->start_stored_progress();
53+
$this->log_start("Recalculating grades for course ID {$data->courseid}");
54+
// Ensure the course exists.
55+
$course = get_course($data->courseid);
56+
$this->log("Found course {$course->shortname}. Starting regrade.");
57+
$results = grade_regrade_final_grades($course->id, progress: $this->get_progress(), async: false);
58+
if (is_array($results)) {
59+
$this->log('Errors reported during regrade:');
60+
foreach ($results as $id => $result) {
61+
$this->log("Grade item {$id}: {$result}", 2);
62+
}
63+
}
64+
$this->log_finish('Regrade complete.');
65+
}
66+
}

‎course/modedit.php

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,6 @@
200200
$url = course_get_url($course, $cw->section, $options);
201201
}
202202

203-
// If we need to regrade the course with a progress bar as a result of updating this module,
204-
// redirect first to the page that will do this.
205-
if (isset($fromform->needsfrontendregrade)) {
206-
$url = new moodle_url('/course/modregrade.php', ['id' => $fromform->coursemodule,
207-
'url' => $url->out_as_local_url(false)]);
208-
}
209-
210203
redirect($url);
211204
exit;
212205

‎course/modlib.php

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -398,13 +398,17 @@ function edit_module_post_actions($moduleinfo, $course) {
398398
// And if it actually needs regrading...
399399
$courseitem = grade_item::fetch_course_item($course->id);
400400
if ($courseitem->needsupdate) {
401-
// Then don't do it as part of this form save, do it on an extra web request with a
402-
// progress bar.
403-
$moduleinfo->needsfrontendregrade = true;
401+
// Queue an asynchronous regrade.
402+
grade_regrade_final_grades($course->id);
404403
}
405404
} else {
406405
// Regrade now.
407-
grade_regrade_final_grades($course->id);
406+
$result = grade_regrade_final_grades($course->id, async: false);
407+
if (is_array($result)) {
408+
foreach ($result as $error) {
409+
\core\notification::add($error, \core\output\notification::NOTIFY_ERROR);
410+
}
411+
}
408412
}
409413
}
410414

‎grade/classes/privacy/provider.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,7 @@ public static function export_user_data(approved_contextlist $contextlist) {
510510

511511
// Ensure that the grades are final and do not need regrading.
512512
array_walk($courseids, function($courseid) {
513-
grade_regrade_final_grades($courseid);
513+
grade_regrade_final_grades($courseid, async: false);
514514
});
515515

516516
// Export own grades.

‎grade/report/overview/tests/lib_test.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ public function test_regrade_all_courses_if_needed(bool $frontend): void {
113113
$gpr = new \stdClass();
114114
$report = new \grade_report_overview($user->id, $gpr, '');
115115
$report->regrade_all_courses_if_needed($frontend);
116+
if (!$frontend) {
117+
$this->expectOutputRegex("~^Recalculating grades for course ID {$course->id}~");
118+
$this->run_all_adhoc_tasks();
119+
}
116120

117121
// This should have regraded courses 1 and 3, but not 2 (because the user doesn't belong).
118122
$this->assertEqualsWithDelta(50.0, $DB->get_field('grade_grades', 'finalgrade',

‎grade/report/singleview/tests/screen_test.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public function test_load_users() {
6060
$this->getDataGenerator()->create_group_member(['groupid' => $group->id, 'userid' => $user2->id]);
6161

6262
// Perform a regrade before creating the report.
63-
grade_regrade_final_grades($course->id);
63+
grade_regrade_final_grades($course->id, async: false);
6464
$screentest = new gradereport_singleview_screen_testable($course->id, 0, $group->id);
6565
$groupusers = $screentest->test_load_users();
6666
$this->assertDebuggingCalled('The function load_users() is deprecated. ' .

‎grade/tests/behat/grade_async_regrade.feature

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
@core @core_grades @javascript
2+
Feature: Asynchronous regrade on a large course
3+
4+
Background:
5+
Given the following "courses" exist:
6+
| shortname | fullname | idnumber |
7+
| C1 | Test course 1 | C1 |
8+
And the following "users" exist:
9+
| username |
10+
| teacher1 |
11+
And the following "course enrolments" exist:
12+
| user | course | role |
13+
| teacher1 | C1 | editingteacher |
14+
And "100" "users" exist with the following data:
15+
| username | student[count] |
16+
| firstname | Student |
17+
| lastname | [count] |
18+
| email | student[count]@example.com |
19+
And "100" "course enrolments" exist with the following data:
20+
| user | student[count] |
21+
| course | C1 |
22+
| role | student |
23+
And the following "activity" exists:
24+
| activity | assign |
25+
| course | C1 |
26+
| idnumber | a1 |
27+
| name | Test assignment 1 |
28+
| grade | 100 |
29+
| intro | Submit your online text |
30+
31+
Scenario: Edit a course module without any grades saved
32+
And I am on the "Test assignment 1" "assign activity editing" page logged in as teacher1
33+
And I expand all fieldsets
34+
And I set the field "Maximum grade" to "50"
35+
And I press "Save and return to course"
36+
And I am on the "Test course 1" "grades > Grader report > View" page
37+
Then I should not see "Grade recalculations are being performed in the background."
38+
39+
Scenario: Edit a course module with 100 grades saved
40+
And "100" "grade grades" exist with the following data:
41+
| gradeitem | Test assignment 1 |
42+
| user | student[count] |
43+
| grade | 80.00 |
44+
And I am on the "Test assignment 1" "assign activity editing" page logged in as teacher1
45+
And I expand all fieldsets
46+
And I set the field "Rescale existing grades" to "Yes"
47+
And I set the field "Maximum grade" to "50"
48+
When I press "Save and return to course"
49+
And I am on the "Test course 1" "grades > Grader report > View" page
50+
And I should see "Grade recalculations are being performed in the background."
51+
And I should see "Task pending"
52+
And I should see "0.0%"
53+
And "40.00" "text" should not exist in the "[email protected]" "table_row"
54+
When I run all adhoc tasks
55+
Then I should not see "Task pending"
56+
And I should see "100%"
57+
And I reload the page
58+
And I should not see "Grade recalculations are being performed in the background."
59+
And "40.00" "text" should exist in the "[email protected]" "table_row"

‎grade/tests/lib_test.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,8 @@ public function test_ungraded_counts_count_sumgrades() {
280280
// Trigger a regrade.
281281
grade_force_full_regrading($course1->id);
282282
grade_force_full_regrading($course2->id);
283-
grade_regrade_final_grades($course1->id);
284-
grade_regrade_final_grades($course2->id);
283+
grade_regrade_final_grades($course1->id, async: false);
284+
grade_regrade_final_grades($course2->id, async: false);
285285

286286
// Initialise reports.
287287
$context1 = \context_course::instance($course1->id);
@@ -411,7 +411,7 @@ public function test_ungraded_counts_hidden_grades(bool $hidden, array $expected
411411

412412
// Trigger a regrade.
413413
grade_force_full_regrading($course->id);
414-
grade_regrade_final_grades($course->id);
414+
grade_regrade_final_grades($course->id, async: false);
415415

416416
// Initialise reports.
417417
$context = \context_course::instance($course->id);
@@ -532,7 +532,7 @@ public function test_ungraded_count_sumgrades_groups() {
532532

533533
// Trigger a regrade.
534534
grade_force_full_regrading($course->id);
535-
grade_regrade_final_grades($course->id);
535+
grade_regrade_final_grades($course->id, async: false);
536536

537537
// Initialise report.
538538
$context = \context_course::instance($course->id);
@@ -683,7 +683,7 @@ public function test_ungraded_counts_only_active_enrol(bool $onlyactive,
683683

684684
// Trigger a regrade.
685685
grade_force_full_regrading($course->id);
686-
grade_regrade_final_grades($course->id);
686+
grade_regrade_final_grades($course->id, async: false);
687687

688688
// Initialise reports.
689689
$context = \context_course::instance($course->id);
@@ -1002,7 +1002,7 @@ public function test_get_gradable_users() {
10021002
$this->getDataGenerator()->create_group_member(['groupid' => $group2->id, 'userid' => $student3->id]);
10031003

10041004
// Perform a regrade before creating the report.
1005-
grade_regrade_final_grades($course->id);
1005+
grade_regrade_final_grades($course->id, async: false);
10061006
// Should return all gradable users (only students).
10071007
$gradableusers = get_gradable_users($course->id);
10081008
$this->assertEqualsCanonicalizing([$student1->id, $student2->id, $student3->id], array_keys($gradableusers));

‎grade/tests/report_graderlib_test.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,9 @@ public function test_get_right_rows() {
480480
// Set the grade for the second one to 0 (note, you have to do this after creating it,
481481
// otherwise it doesn't create an ungraded grade item).
482482
quiz_settings::create($ungradedquiz->id)->get_grade_calculator()->update_quiz_maximum_grade(0);
483+
ob_start();
484+
$this->run_all_adhoc_tasks();
485+
ob_end_clean();
483486

484487
// Set current user.
485488
$this->setUser($manager);

‎lang/en/grades.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,7 @@
711711
$string['realletter'] = 'Real (letter)';
712712
$string['realpercentage'] = 'Real (percentage)';
713713
$string['recalculatinggrades'] = 'Recalculating grades';
714+
$string['recalculatinggradesadhoc'] = 'Grade recalculations are being performed in the background. Displayed grades may be incorrect until the process is complete.';
714715
$string['recovergradesdefault'] = 'Recover grades default';
715716
$string['recovergradesdefault_help'] = 'By default recover old grades when re-enrolling a user in a course.';
716717
$string['refreshpreview'] = 'Refresh preview';

‎lib/classes/stored_progress_bar.php

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,13 @@ public function export_for_template(\renderer_base $output): array {
231231
* @return int ID of the DB record
232232
*/
233233
public function start(): int {
234-
global $DB;
234+
global $DB, $OUTPUT;
235+
236+
// If we are running in an non-interactive CLI environment, call the progress bar renderer to avoid warnings
237+
// when we do an update.
238+
if (defined('STDOUT') && !stream_isatty(STDOUT)) {
239+
$OUTPUT->render_progress_bar($this);
240+
}
235241

236242
$record = $DB->get_record('stored_progress', ['idnumber' => $this->idnumber]);
237243
if ($record) {

‎lib/grade/tests/grade_category_test.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ protected function sub_test_grade_category_set_locked() {
789789
$this->assertTrue($category->set_locked(1));
790790
// The category should not be locked yet as we are waiting for a recalculation.
791791
$this->assertFalse($category->is_locked());
792-
grade_regrade_final_grades($this->courseid);
792+
grade_regrade_final_grades($this->courseid, async: false);
793793

794794
// Get the category from the db again.
795795
$category = new \grade_category($this->grade_categories[0]);

‎lib/grade/tests/grade_grade_test.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,7 +687,7 @@ public function test_category_get_hiding_affected() {
687687
$this->assertTrue(in_array($gradeitem1a->id, array_keys($hidingaffectedunlocked['altered'])));
688688

689689
// This creates all the grade_grades we need.
690-
grade_regrade_final_grades($course1->id);
690+
grade_regrade_final_grades($course1->id, async: false);
691691

692692
// Set grade override.
693693
$gradegrade = \grade_grade::fetch([

‎lib/gradelib.php

Lines changed: 32 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -383,45 +383,18 @@ function grade_needs_regrade_progress_bar($courseid) {
383383
* @return moodle_url|false The URL to redirect to if redirecting
384384
*/
385385
function grade_regrade_final_grades_if_required($course, callable $callback = null) {
386-
global $PAGE, $OUTPUT;
386+
global $PAGE;
387387

388388
if (!grade_needs_regrade_final_grades($course->id)) {
389389
return false;
390390
}
391391

392392
if (grade_needs_regrade_progress_bar($course->id)) {
393-
if ($PAGE->state !== moodle_page::STATE_IN_BODY) {
394-
$PAGE->set_heading($course->fullname);
395-
echo $OUTPUT->header();
396-
}
397-
echo $OUTPUT->heading(get_string('recalculatinggrades', 'grades'));
398-
$progress = new \core\progress\display(true);
399-
$status = grade_regrade_final_grades($course->id, null, null, $progress);
400-
401-
// Show regrade errors and set the course to no longer needing regrade (stop endless loop).
402-
if (is_array($status)) {
403-
foreach ($status as $error) {
404-
$errortext = new \core\output\notification($error, \core\output\notification::NOTIFY_ERROR);
405-
echo $OUTPUT->render($errortext);
406-
}
407-
$courseitem = grade_item::fetch_course_item($course->id);
408-
$courseitem->regrading_finished();
409-
}
410-
411-
if ($callback) {
412-
//
413-
$url = call_user_func($callback);
414-
}
415-
416-
if (empty($url)) {
417-
$url = $PAGE->url;
418-
}
419-
420-
echo $OUTPUT->continue_button($url);
421-
echo $OUTPUT->footer();
422-
die();
393+
// Queue ad-hoc task and redirect.
394+
grade_regrade_final_grades($course->id);
395+
return $callback ? call_user_func($callback) : $PAGE->url;
423396
} else {
424-
$result = grade_regrade_final_grades($course->id);
397+
$result = grade_regrade_final_grades($course->id, async: false);
425398
if ($callback) {
426399
call_user_func($callback);
427400
}
@@ -1148,9 +1121,11 @@ function grade_recover_history_grades($userid, $courseid) {
11481121
* @param int $userid If specified try to do a quick regrading of the grades of this user only
11491122
* @param object $updated_item Optional grade item to be marked for regrading. It is required if $userid is set.
11501123
* @param \core\progress\base $progress If provided, will be used to update progress on this long operation.
1124+
* @param bool $async If true, and we are recalculating an entire course's grades, defer processing to an ad-hoc task.
11511125
* @return array|true true if ok, array of errors if problems found. Grade item id => error message
11521126
*/
1153-
function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null, $progress=null) {
1127+
function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null, $progress=null, bool $async = true) {
1128+
global $DB;
11541129
// This may take a very long time and extra memory.
11551130
\core_php_time_limit::raise();
11561131
raise_memory_limit(MEMORY_EXTRA);
@@ -1176,6 +1151,30 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null,
11761151
// nothing to do :-)
11771152
return true;
11781153
}
1154+
// Defer recalculation to an ad-hoc task.
1155+
if ($async) {
1156+
$regradecache = cache::make_from_params(
1157+
mode: cache_store::MODE_REQUEST,
1158+
component: 'core',
1159+
area: 'grade_regrade_final_grades',
1160+
options: [
1161+
'simplekeys' => true,
1162+
'simpledata' => true,
1163+
],
1164+
);
1165+
// If the courseid already exists in the cache, return so we don't do this multiple times per request.
1166+
if ($regradecache->get($courseid)) {
1167+
return true;
1168+
}
1169+
$task = \core_course\task\regrade_final_grades::create($courseid);
1170+
$taskid = \core\task\manager::queue_adhoc_task($task, true);
1171+
if ($taskid) {
1172+
$task->set_id($taskid);
1173+
$task->initialise_stored_progress();
1174+
}
1175+
$regradecache->set($courseid, true);
1176+
return true;
1177+
}
11791178
}
11801179

11811180
// Categories might have to run some processing before we fetch the grade items.

0 commit comments

Comments
 (0)