From 91e69cee7f8c707298a5bf445a04bce79ef2aa51 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Wed, 3 Apr 2024 14:28:11 +0100 Subject: [PATCH 1/7] MDL-81714 progress: Add new \core\progress\stored class This progress class updates a stored_progress_bar in a similar way to how \core\progress\display updates a regular progress_bar, but it does not produce any output unless the progress bar does. --- lib/classes/progress/stored.php | 60 +++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 lib/classes/progress/stored.php diff --git a/lib/classes/progress/stored.php b/lib/classes/progress/stored.php new file mode 100644 index 00000000000..baeb53c3ccd --- /dev/null +++ b/lib/classes/progress/stored.php @@ -0,0 +1,60 @@ +. + +namespace core\progress; + +use core\output\stored_progress_bar; + +/** + * Progress handler which updates a stored progress bar. + * + * @package core + * @copyright 2024 Catalyst IT Europe Ltd. + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class stored extends base { + /** + * Constructs the progress reporter. + * + * @param stored_progress_bar $bar The stored progress bar to update. + */ + public function __construct( + /** + * @var stored_progress_bar $bar The stored progress bar to update. + */ + protected stored_progress_bar $bar, + ) { + } + + /** + * Updates the progress in the database. + * Database update frequency is set by $interval. + * + * @see \core\progress\base::update_progress() + */ + public function update_progress() { + // Get progress. + [$min] = $this->get_progress_proportion_range(); + + $message = ''; + if ($this->is_in_progress_section()) { + $message = $this->get_current_description(); + } + // Update progress bar. + $this->bar->update_full($min * 100, $message); + } +} From b94b1ed5ff76b6ba4410274e980c548ff2cc8212 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 14 May 2024 16:26:41 +0100 Subject: [PATCH 2/7] MDL-81714 output: Update stored progress bar APIs Expand the methods available in the stored_progress_bar output component and stored_progress_task_trait to allow a progress bar to be created in a "pending" state before the associated task is executed. --- lib/classes/output/stored_progress_bar.php | 83 ++++++++++++++++--- .../task/stored_progress_task_trait.php | 54 +++++++++--- lib/templates/progress_bar.mustache | 2 +- 3 files changed, 114 insertions(+), 25 deletions(-) diff --git a/lib/classes/output/stored_progress_bar.php b/lib/classes/output/stored_progress_bar.php index 41fd66c0c32..18f2e5a988a 100644 --- a/lib/classes/output/stored_progress_bar.php +++ b/lib/classes/output/stored_progress_bar.php @@ -29,6 +29,9 @@ class stored_progress_bar extends progress_bar { /** @var bool Can use output buffering. */ protected static $supportsoutputbuffering = true; + /** @var bool Flag to indicate the Javascript module has been initialised already. */ + protected static $jsloaded = false; + /** @var int DB record ID */ protected $recordid; @@ -41,15 +44,19 @@ class stored_progress_bar extends progress_bar { /** * This overwrites the progress_bar::__construct method. * + * The stored progress bar does not need to check NO_OUTPUT_BUFFERING since it outputs to the page + * then polls for updates asynchronously, rather than waiting for synchronous updates in later output. + * * @param string $idnumber + * @param int $width The suggested width. + * @param bool $autostart Whether to start the progress bar right away. */ - public function __construct($idnumber) { + public function __construct(string $idnumber, int $width = 0, bool $autostart = true) { $this->clock = \core\di::get(\core\clock::class); // Construct from the parent. - parent::__construct($idnumber, 0, true); - + parent::__construct($idnumber, $width, $autostart); } /** @@ -125,10 +132,10 @@ class stored_progress_bar extends progress_bar { /** * Set the time we started the process. * - * @param int $value + * @param ?int $value * @return void */ - protected function set_time_started(int $value): void { + protected function set_time_started(?int $value): void { $this->timestart = $value; } @@ -185,17 +192,33 @@ class stored_progress_bar extends progress_bar { return $this->message; } + /** + * Initialise Javascript for stored progress bars. + * + * The javascript polls the status of all progress bars on the page, so it only needs to be initialised once. + * + * @return void + */ + public function init_js(): void { + global $PAGE; + if (self::$jsloaded) { + return; + } + $PAGE->requires->js_call_amd('core/stored_progress', 'init', [ + self::get_timeout(), + ]); + self::$jsloaded = true; + } + /** * Get the content to display the progress bar and start polling via AJAX * * @return string */ public function get_content(): string { - global $CFG, $PAGE, $OUTPUT; + global $OUTPUT; - $PAGE->requires->js_call_amd('core/stored_progress', 'init', [ - self::get_timeout(), - ]); + $this->init_js(); $context = $this->export_for_template($OUTPUT); return $OUTPUT->render_from_template('core/progress_bar', $context); @@ -208,11 +231,15 @@ class stored_progress_bar extends progress_bar { * @return array */ public function export_for_template(\renderer_base $output): array { + $class = 'stored-progress-bar'; + if (empty($this->timestart)) { + $class .= ' stored-progress-notstarted'; + } return [ 'id' => $this->recordid, 'idnumber' => $this->idnumber, 'width' => $this->width, - 'class' => 'stored-progress-bar', + 'class' => $class, 'value' => $this->percent, 'message' => $this->message, 'error' => $this->haserrored, @@ -233,8 +260,19 @@ class stored_progress_bar extends progress_bar { $OUTPUT->render_progress_bar($this); } - // Delete any existing records for this. - $this->clear_records(); + $record = $DB->get_record('stored_progress', ['idnumber' => $this->idnumber]); + if ($record) { + if ($record->timestart == 0) { + // Set the timestart now and return. + $record->timestart = $this->timestart; + $DB->update_record('stored_progress', $record); + $this->recordid = $record->id; + return $this->recordid; + } else { + // Delete any existing records for this. + $this->clear_records(); + } + } // Create new progress record. $this->recordid = $DB->insert_record('stored_progress', [ @@ -362,4 +400,25 @@ class stored_progress_bar extends progress_bar { return $CFG->progresspollinterval ?? 5; } + /** + * Store a progress bar record in a pending state. + * + * @return int ID of the DB record + */ + public function store_pending(): int { + global $DB; + + // Delete any existing records for this. + $this->clear_records(); + + // Create new progress record. + $this->recordid = $DB->insert_record('stored_progress', [ + 'idnumber' => $this->idnumber, + 'timestart' => $this->timestart, + 'message' => '', + ]); + + return $this->recordid; + } + } diff --git a/lib/classes/task/stored_progress_task_trait.php b/lib/classes/task/stored_progress_task_trait.php index ae79ccf913e..2fc835f5554 100644 --- a/lib/classes/task/stored_progress_task_trait.php +++ b/lib/classes/task/stored_progress_task_trait.php @@ -16,6 +16,10 @@ namespace core\task; +use core\progress\db_updater; +use core\progress\stored; +use core\output\stored_progress_bar; + /** * Trait to use in tasks to automatically add stored progress functionality. * @@ -25,10 +29,44 @@ namespace core\task; * @author Conn Warwicker */ trait stored_progress_task_trait { - - /** @var \core\output\stored_progress_bar|null $progress */ + /** @var ?stored_progress_bar $progress */ protected $progress = null; + /** + * Construct a unique name for the progress bar. + * + * For adhoc tasks, this will need the ID in it. For scheduled tasks just the class name. + * + * @return string + */ + protected function get_progress_name(): string { + if (method_exists($this, 'get_id')) { + return get_class($this) . '_' . $this->get_id(); + } else { + return get_class($this); + } + } + + /** + * Initialise a stored progress record. + */ + public function initialise_stored_progress(): void { + $this->progress = new stored_progress_bar( + stored_progress_bar::convert_to_idnumber($this->get_progress_name()), + autostart: false, + ); + $this->progress->store_pending(); + } + + /** + * Get a stored object for the stored progress record. + * + * @return stored + */ + public function get_progress(): stored { + return new stored($this->progress); + } + /** * Start a stored progress bar implementation for the task this trait is used in. * @@ -40,16 +78,8 @@ trait stored_progress_task_trait { // To get around the issue in MDL-80770, we are manually setting the renderer to cli. $OUTPUT = $PAGE->get_renderer('core', null, 'cli'); - // Construct a unique name for the progress bar. - // For adhoc tasks, this will need the ID in it. For scheduled tasks just the class name. - if (method_exists($this, 'get_id')) { - $name = get_class($this) . '_' . $this->get_id(); - } else { - $name = get_class($this); - } - - $this->progress = new \core\output\stored_progress_bar( - \core\output\stored_progress_bar::convert_to_idnumber($name) + $this->progress = new stored_progress_bar( + stored_progress_bar::convert_to_idnumber($this->get_progress_name()) ); // Start the progress. diff --git a/lib/templates/progress_bar.mustache b/lib/templates/progress_bar.mustache index 13190c416f3..984a4024773 100644 --- a/lib/templates/progress_bar.mustache +++ b/lib/templates/progress_bar.mustache @@ -31,7 +31,7 @@
-
 
+
{{message}}
  From b746bcd186b2a352d252f038a8655652b83f0dd9 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 14 May 2024 16:35:23 +0100 Subject: [PATCH 3/7] 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. --- .../tests/behat/backup_user_data.feature | 1 + backup/moodle2/restore_stepslib.php | 2 +- course/classes/task/regrade_final_grades.php | 77 +++++++++++++++++++ course/modedit.php | 7 -- course/modlib.php | 12 ++- grade/report/overview/lib.php | 2 +- grade/report/overview/tests/lib_test.php | 4 + grade/tests/report_graderlib_test.php | 3 + lang/en/grades.php | 1 + lib/grade/tests/grade_category_test.php | 2 +- lib/gradelib.php | 64 ++++++++------- lib/moodlelib.php | 2 +- .../behat/h5pactivity_reset_grades.feature | 1 + mod/h5pactivity/tests/local/grader_test.php | 2 + 14 files changed, 132 insertions(+), 48 deletions(-) create mode 100644 course/classes/task/regrade_final_grades.php diff --git a/admin/tool/recyclebin/tests/behat/backup_user_data.feature b/admin/tool/recyclebin/tests/behat/backup_user_data.feature index 0eccc5944df..cdd9b075041 100644 --- a/admin/tool/recyclebin/tests/behat/backup_user_data.feature +++ b/admin/tool/recyclebin/tests/behat/backup_user_data.feature @@ -54,6 +54,7 @@ Feature: Backup user data And I navigate to "Recycle bin" in current page administration And I should see "Quiz 1" And I click on "Restore" "link" in the "region-main" "region" + And I run all adhoc tasks When I am on the "Course 1" "grades > User report > View" page logged in as "student1" Then "Quiz 1" row "Grade" column of "user-grade" table should contain "50" And "Quiz 1" row "Percentage" column of "user-grade" table should contain "50" diff --git a/backup/moodle2/restore_stepslib.php b/backup/moodle2/restore_stepslib.php index c1c749322a4..065fda8c50b 100644 --- a/backup/moodle2/restore_stepslib.php +++ b/backup/moodle2/restore_stepslib.php @@ -490,7 +490,7 @@ class restore_gradebook_structure_step extends restore_structure_step { rebuild_course_cache($this->get_courseid(), true); // Restore marks items as needing update. Update everything now. - grade_regrade_final_grades($this->get_courseid()); + grade_regrade_final_grades($this->get_courseid(), async: true); } /** diff --git a/course/classes/task/regrade_final_grades.php b/course/classes/task/regrade_final_grades.php new file mode 100644 index 00000000000..3777072b8a8 --- /dev/null +++ b/course/classes/task/regrade_final_grades.php @@ -0,0 +1,77 @@ +. + +namespace core_course\task; + +defined('MOODLE_INTERNAL') || die(); + +use core\task\adhoc_task; + +require_once($CFG->libdir . '/gradelib.php'); + +/** + * Asynchronously regrade a course. + * + * @copyright 2024 onwards Catalyst IT Europe Ltd. + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package core_course + */ +class regrade_final_grades extends adhoc_task { + use \core\task\logging_trait; + use \core\task\stored_progress_task_trait; + + /** + * Create and return an instance of this task for a given course ID. + * + * @param int $courseid + * @return self + */ + public static function create(int $courseid): self { + $task = new regrade_final_grades(); + $task->set_custom_data((object)['courseid' => $courseid]); + $task->set_component('core'); + return $task; + } + + /** + * Run regrade_final_grades for the provided course id. + * + * @return void + * @throws \dml_exception + */ + public function execute(): void { + $data = $this->get_custom_data(); + $this->start_stored_progress(); + $this->log_start("Recalculating grades for course ID {$data->courseid}"); + // Ensure the course exists. + try { + $course = get_course($data->courseid); + } catch (\dml_missing_record_exception $e) { + $this->log("Course with id {$data->courseid} not found. It may have been deleted. Skipping regrade."); + return; + } + $this->log("Found course {$course->shortname}. Starting regrade."); + $results = grade_regrade_final_grades($course->id, progress: $this->get_progress()); + if (is_array($results)) { + $this->log('Errors reported during regrade:'); + foreach ($results as $id => $result) { + $this->log("Grade item {$id}: {$result}", 2); + } + } + $this->log_finish('Regrade complete.'); + } +} diff --git a/course/modedit.php b/course/modedit.php index 32a6a34a315..087a1379a37 100644 --- a/course/modedit.php +++ b/course/modedit.php @@ -204,13 +204,6 @@ if ($mform->is_cancelled()) { $url = course_get_url($course, $cw->section, $options); } - // If we need to regrade the course with a progress bar as a result of updating this module, - // redirect first to the page that will do this. - if (isset($fromform->needsfrontendregrade)) { - $url = new moodle_url('/course/modregrade.php', ['id' => $fromform->coursemodule, - 'url' => $url->out_as_local_url(false)]); - } - redirect($url); exit; diff --git a/course/modlib.php b/course/modlib.php index d3bbee28aec..5d431f3b8cb 100644 --- a/course/modlib.php +++ b/course/modlib.php @@ -404,13 +404,17 @@ function edit_module_post_actions($moduleinfo, $course) { // And if it actually needs regrading... $courseitem = grade_item::fetch_course_item($course->id); if ($courseitem->needsupdate) { - // Then don't do it as part of this form save, do it on an extra web request with a - // progress bar. - $moduleinfo->needsfrontendregrade = true; + // Queue an asynchronous regrade. + grade_regrade_final_grades($course->id, async: true); } } else { // Regrade now. - grade_regrade_final_grades($course->id); + $result = grade_regrade_final_grades($course->id); + if (is_array($result)) { + foreach ($result as $error) { + \core\notification::add($error, \core\output\notification::NOTIFY_ERROR); + } + } } } diff --git a/grade/report/overview/lib.php b/grade/report/overview/lib.php index 736d3752ccb..9b5dd767f4f 100644 --- a/grade/report/overview/lib.php +++ b/grade/report/overview/lib.php @@ -137,7 +137,7 @@ class grade_report_overview extends grade_report { if ($frontend) { grade_regrade_final_grades_if_required($course); } else { - grade_regrade_final_grades($course->id); + grade_regrade_final_grades($course->id, async: true); } } } diff --git a/grade/report/overview/tests/lib_test.php b/grade/report/overview/tests/lib_test.php index 5a292457e03..e85046f98f5 100644 --- a/grade/report/overview/tests/lib_test.php +++ b/grade/report/overview/tests/lib_test.php @@ -114,6 +114,10 @@ final class lib_test extends \advanced_testcase { $gpr = new \stdClass(); $report = new \grade_report_overview($user->id, $gpr, ''); $report->regrade_all_courses_if_needed($frontend); + if (!$frontend) { + $this->expectOutputRegex("~^Recalculating grades for course ID {$course->id}~"); + $this->run_all_adhoc_tasks(); + } // This should have regraded courses 1 and 3, but not 2 (because the user doesn't belong). $this->assertEqualsWithDelta(50.0, $DB->get_field('grade_grades', 'finalgrade', diff --git a/grade/tests/report_graderlib_test.php b/grade/tests/report_graderlib_test.php index 3ab7fa6670b..79e647ace91 100644 --- a/grade/tests/report_graderlib_test.php +++ b/grade/tests/report_graderlib_test.php @@ -480,6 +480,9 @@ final class report_graderlib_test extends \advanced_testcase { // Set the grade for the second one to 0 (note, you have to do this after creating it, // otherwise it doesn't create an ungraded grade item). quiz_settings::create($ungradedquiz->id)->get_grade_calculator()->update_quiz_maximum_grade(0); + ob_start(); + $this->run_all_adhoc_tasks(); + ob_end_clean(); // Set current user. $this->setUser($manager); diff --git a/lang/en/grades.php b/lang/en/grades.php index 69aebb4be73..109cbfd8157 100644 --- a/lang/en/grades.php +++ b/lang/en/grades.php @@ -707,6 +707,7 @@ $string['real'] = 'Real'; $string['realletter'] = 'Real (letter)'; $string['realpercentage'] = 'Real (percentage)'; $string['recalculatinggrades'] = 'Recalculating grades'; +$string['recalculatinggradesadhoc'] = 'Grade recalculations are being performed in the background. Displayed grades may be incorrect until the process is complete.'; $string['recovergradesdefault'] = 'Recover grades default'; $string['recovergradesdefault_help'] = 'By default recover old grades when re-enrolling a user in a course.'; $string['refreshpreview'] = 'Refresh preview'; diff --git a/lib/grade/tests/grade_category_test.php b/lib/grade/tests/grade_category_test.php index 85c46c8d6f6..48539480236 100644 --- a/lib/grade/tests/grade_category_test.php +++ b/lib/grade/tests/grade_category_test.php @@ -444,7 +444,7 @@ final class grade_category_test extends \grade_base_testcase { $category_grade_item = $grade_category->get_grade_item(); // This creates all the grade_grades we need. - grade_regrade_final_grades($this->courseid); + grade_regrade_final_grades($this->courseid, async: true); $grade = $DB->get_record('grade_grades', array('itemid'=>$category_grade_item->id, 'userid'=>$this->userid)); $this->assertWithinMargin($grade->rawgrade, $grade->rawgrademin, $grade->rawgrademax); diff --git a/lib/gradelib.php b/lib/gradelib.php index eeb2212344a..b271f105864 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -383,43 +383,16 @@ function grade_needs_regrade_progress_bar($courseid) { * @return moodle_url|false The URL to redirect to if redirecting */ function grade_regrade_final_grades_if_required($course, ?callable $callback = null) { - global $PAGE, $OUTPUT; + global $PAGE; if (!grade_needs_regrade_final_grades($course->id)) { return false; } if (grade_needs_regrade_progress_bar($course->id)) { - if ($PAGE->state !== moodle_page::STATE_IN_BODY) { - $PAGE->set_heading($course->fullname); - echo $OUTPUT->header(); - } - echo $OUTPUT->heading(get_string('recalculatinggrades', 'grades')); - $progress = new \core\progress\display(true); - $status = grade_regrade_final_grades($course->id, null, null, $progress); - - // Show regrade errors and set the course to no longer needing regrade (stop endless loop). - if (is_array($status)) { - foreach ($status as $error) { - $errortext = new \core\output\notification($error, \core\output\notification::NOTIFY_ERROR); - echo $OUTPUT->render($errortext); - } - $courseitem = grade_item::fetch_course_item($course->id); - $courseitem->regrading_finished(); - } - - if ($callback) { - // - $url = call_user_func($callback); - } - - if (empty($url)) { - $url = $PAGE->url; - } - - echo $OUTPUT->continue_button($url); - echo $OUTPUT->footer(); - die(); + // Queue ad-hoc task and redirect. + grade_regrade_final_grades($course->id, async: true); + return $callback ? call_user_func($callback) : $PAGE->url; } else { $result = grade_regrade_final_grades($course->id); if ($callback) { @@ -1153,9 +1126,10 @@ function grade_recover_history_grades($userid, $courseid) { * @param int $userid If specified try to do a quick regrading of the grades of this user only * @param object $updated_item Optional grade item to be marked for regrading. It is required if $userid is set. * @param \core\progress\base $progress If provided, will be used to update progress on this long operation. + * @param bool $async If true, and we are recalculating an entire course's grades, defer processing to an ad-hoc task. * @return array|true true if ok, array of errors if problems found. Grade item id => error message */ -function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null, $progress=null) { +function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null, $progress=null, bool $async = false) { // This may take a very long time and extra memory. \core_php_time_limit::raise(); raise_memory_limit(MEMORY_EXTRA); @@ -1181,6 +1155,30 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null, // nothing to do :-) return true; } + // Defer recalculation to an ad-hoc task. + if ($async) { + $regradecache = cache::make_from_params( + mode: cache_store::MODE_REQUEST, + component: 'core', + area: 'grade_regrade_final_grades', + options: [ + 'simplekeys' => true, + 'simpledata' => true, + ], + ); + // If the courseid already exists in the cache, return so we don't do this multiple times per request. + if ($regradecache->get($courseid)) { + return true; + } + $task = \core_course\task\regrade_final_grades::create($courseid); + $taskid = \core\task\manager::queue_adhoc_task($task, true); + if ($taskid) { + $task->set_id($taskid); + $task->initialise_stored_progress(); + } + $regradecache->set($courseid, true); + return true; + } } // Categories might have to run some processing before we fetch the grade items. @@ -1601,7 +1599,7 @@ function grade_course_reset($courseid) { grade_grab_course_grades($courseid); // recalculate all grades - grade_regrade_final_grades($courseid); + grade_regrade_final_grades($courseid, async: true); return true; } diff --git a/lib/moodlelib.php b/lib/moodlelib.php index 73f98edf641..93b65d14dd3 100644 --- a/lib/moodlelib.php +++ b/lib/moodlelib.php @@ -5327,7 +5327,7 @@ function reset_course_userdata($data) { if (!empty($data->reset_gradebook_items)) { remove_course_grades($data->courseid, false); grade_grab_course_grades($data->courseid); - grade_regrade_final_grades($data->courseid); + grade_regrade_final_grades($data->courseid, async: true); $status[] = array('component' => $componentstr, 'item' => get_string('removeallcourseitems', 'grades'), 'error' => false); } else if (!empty($data->reset_gradebook_grades)) { diff --git a/mod/h5pactivity/tests/behat/h5pactivity_reset_grades.feature b/mod/h5pactivity/tests/behat/h5pactivity_reset_grades.feature index ce0cbeec31d..823ae8c418f 100644 --- a/mod/h5pactivity/tests/behat/h5pactivity_reset_grades.feature +++ b/mod/h5pactivity/tests/behat/h5pactivity_reset_grades.feature @@ -45,6 +45,7 @@ Feature: Teacher can reset H5P activity grades And I click on "Reset course" "button" in the "Reset course?" "dialogue" Then I should see "Done" in the "Gradebook" "table_row" And I press "Continue" + And I run all adhoc tasks # Confirm that previously saved grades are gone And I navigate to "View > Grader report" in the course gradebook And I should not see "7.00" in the "First Student" "table_row" diff --git a/mod/h5pactivity/tests/local/grader_test.php b/mod/h5pactivity/tests/local/grader_test.php index 9598223c409..de2f5e121f3 100644 --- a/mod/h5pactivity/tests/local/grader_test.php +++ b/mod/h5pactivity/tests/local/grader_test.php @@ -139,6 +139,7 @@ final class grader_test extends \advanced_testcase { $grader->grade_item_update($param); // Check new grade item and grades. + grade_regrade_final_grades($course->id); $gradeinfo = grade_get_grades($course->id, 'mod', 'h5pactivity', $activity->id, $user->id); $item = array_shift($gradeinfo->items); $this->assertEquals($scaleid, $item->scaleid); @@ -245,6 +246,7 @@ final class grader_test extends \advanced_testcase { $grader->update_grades($userid); // Check new grade item and grades. + grade_regrade_final_grades($course->id); $gradeinfo = grade_get_grades($course->id, 'mod', 'h5pactivity', $activity->id, [$user1->id, $user2->id]); $item = array_shift($gradeinfo->items); $this->assertArrayHasKey($user1->id, $item->grades); From f96965b9306e859a68da4298db652e0e0b2f24f9 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Wed, 15 May 2024 16:35:37 +0100 Subject: [PATCH 4/7] MDL-81714 tool_recyclebin: Remove unnecessary task runs in tests I assume that it was necessary at some point to run the ad-hoc tasks in these tests, however the only task now being run is regrade_final_grades, which causes the tests to fail due as they produce output. Whether the regrade is performed or not has no impact on the result of the test, so removing the ad-hoc task run seems appropriate. --- admin/tool/recyclebin/tests/course_bin_test.php | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/admin/tool/recyclebin/tests/course_bin_test.php b/admin/tool/recyclebin/tests/course_bin_test.php index 965441bd3d6..60cf0d5ead7 100644 --- a/admin/tool/recyclebin/tests/course_bin_test.php +++ b/admin/tool/recyclebin/tests/course_bin_test.php @@ -67,9 +67,6 @@ final class course_bin_test extends \advanced_testcase { // Delete the course module. course_delete_module($this->quiz->cmid); - // Now, run the course module deletion adhoc task. - \phpunit_util::run_all_adhoc_tasks(); - // Check the course module is now in the recycle bin. $this->assertEquals(1, $DB->count_records('tool_recyclebin_course')); @@ -111,9 +108,6 @@ final class course_bin_test extends \advanced_testcase { // Delete the course module. course_delete_module($this->quiz->cmid); - // Now, run the course module deletion adhoc task. - \phpunit_util::run_all_adhoc_tasks(); - // Try purging. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); foreach ($recyclebin->get_items() as $item) { @@ -136,9 +130,6 @@ final class course_bin_test extends \advanced_testcase { // Delete the quiz. course_delete_module($this->quiz->cmid); - // Now, run the course module deletion adhoc task. - \phpunit_util::run_all_adhoc_tasks(); - // Set deleted date to the distant past. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); foreach ($recyclebin->get_items() as $item) { @@ -152,9 +143,6 @@ final class course_bin_test extends \advanced_testcase { course_delete_module($book->cmid); - // Now, run the course module deletion adhoc task. - \phpunit_util::run_all_adhoc_tasks(); - // Should have 2 items now. $this->assertEquals(2, count($recyclebin->get_items())); @@ -224,7 +212,6 @@ final class course_bin_test extends \advanced_testcase { // Delete quiz. $cm = get_coursemodule_from_instance('quiz', $this->quiz->id); course_delete_module($cm->id); - \phpunit_util::run_all_adhoc_tasks(); $quizzes = get_coursemodules_in_course('quiz', $this->course->id); $this->assertEquals(0, count($quizzes)); @@ -261,9 +248,6 @@ final class course_bin_test extends \advanced_testcase { // Delete the course module. course_delete_module($this->quiz->cmid); - // Now, run the course module deletion adhoc task. - \phpunit_util::run_all_adhoc_tasks(); - // Check there is no items in the recycle bin. $recyclebin = new \tool_recyclebin\course_bin($this->course->id); $this->assertEquals(0, count($recyclebin->get_items())); @@ -294,7 +278,6 @@ final class course_bin_test extends \advanced_testcase { // Delete quiz. $cm = get_coursemodule_from_instance('quiz', $this->quiz->id); course_delete_module($cm->id); - \phpunit_util::run_all_adhoc_tasks(); $quizzes = get_coursemodules_in_course('quiz', $this->course->id); $this->assertEquals(0, count($quizzes)); From 10de32939759240a228f673b86f7f999afd9d4a1 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 14 May 2024 16:36:11 +0100 Subject: [PATCH 5/7] MDL-81714 grades: Add regrading progress indicator to grade reports --- .../moodle/components/task-indicator.md | 93 +++++++++++ grade/report/grader/index.php | 39 +++-- grade/report/outcomes/index.php | 18 ++- grade/report/overview/index.php | 34 ++++ grade/report/singleview/index.php | 33 +++- grade/report/summary/index.php | 19 ++- grade/report/user/classes/external/user.php | 2 +- grade/report/user/index.php | 24 ++- grade/report/user/lib.php | 13 +- grade/tests/behat/grade_async_regrade.feature | 147 ++++++++++++++++++ lang/en/grades.php | 2 +- lib/amd/build/task_indicator.min.js | 14 ++ lib/amd/build/task_indicator.min.js.map | 1 + lib/amd/src/task_indicator.js | 43 +++++ lib/classes/output/task_indicator.php | 112 +++++++++++++ lib/classes/task/manager.php | 2 +- lib/gradelib.php | 7 +- lib/templates/task_indicator.mustache | 61 ++++++++ pix/i/timer.svg | 11 ++ theme/boost/scss/moodle.scss | 1 + theme/boost/scss/moodle/task-indicator.scss | 25 +++ theme/boost/style/moodle.css | 22 +++ theme/classic/style/moodle.css | 22 +++ 23 files changed, 714 insertions(+), 31 deletions(-) create mode 100644 admin/tool/componentlibrary/content/moodle/components/task-indicator.md create mode 100644 grade/tests/behat/grade_async_regrade.feature create mode 100644 lib/amd/build/task_indicator.min.js create mode 100644 lib/amd/build/task_indicator.min.js.map create mode 100644 lib/amd/src/task_indicator.js create mode 100644 lib/classes/output/task_indicator.php create mode 100644 lib/templates/task_indicator.mustache create mode 100644 pix/i/timer.svg create mode 100644 theme/boost/scss/moodle/task-indicator.scss diff --git a/admin/tool/componentlibrary/content/moodle/components/task-indicator.md b/admin/tool/componentlibrary/content/moodle/components/task-indicator.md new file mode 100644 index 00000000000..4216f49c3ef --- /dev/null +++ b/admin/tool/componentlibrary/content/moodle/components/task-indicator.md @@ -0,0 +1,93 @@ +--- +layout: docs +title: "Task indicator" +description: "A progress indicator for background tasks" +date: 2024-08-21T00:00:00+01:00 +draft: false +tags: +- MDL-81714 +- 4.5 +--- + +{{< mustache template="core/task_indicator" >}} +{{< /mustache >}} + +## How to use + +The task indicator component is used to display on any page the status and progress of an ad-hoc or scheduled task running in the +background. If a task is running that will update the content on the page, it can be displayed in place of the content to inform +the user that the current content is out-of-date, and will be updated when the task is complete. + +## Source files + +* lib/amd/src/task_indicator.js +* lib/classes/output/task_indicator.php +* lib/templates/task_indicator.mustache + +## Usage + +The task indicator can only be used to display the progress of a task if its class uses `core\task\stored_progress_task_trait`. + +When the task is queued, you must call the `initialise_stored_progress()` method to store the progress record in a pending state, +for the indicator to display while the task is queued. + +{{< php >}} +$task = new \core\task\mytask($id); +$taskid = \core\task\manager::queue_adhoc_task($task, true); +if ($taskid) { + $task->set_id($taskid); + $task->initialise_stored_progress(); +} +{{< /php >}} + +When the task runs, it must start, progress and complete its stored progress bar. +See `core_course\task\regrade_final_grades` for a real-life example. + +{{< php >}} + +class mytask extends adhoc_task { + use \core\task\stored_progress_task_trait; + + public function execute(): void { + $this->start_stored_progress(); + $storedprogress = $this->get_progress(); + foreach ($this->get_records() as $record) { + $this->process_record($record); + $storedprogress->progress(); + } + $storedprogress->end_progress(); + } +} + +{{< /php >}} + +Any page that wishes to display the status of the task must create an instance of the task object with the same parameters, +and pass it to a `task_indicator`. + +{{< php >}} + +$task = new mytask($id); +$taskindicator = new \core\output\task_indicator( + task: $task, + heading: 'Task processing', + message: get_string('recalculatinggradesadhoc', 'grades'), + icon: new \core\output\pix_icon('i/grades', ''), + redirecturl: $PAGE->url, + extraclasses: ['mytask'], +); + +{{< /php >}} + +If there is currently a queued instance of the task, `$taskindicator->has_task_record()` will return true. We can use this to +decide whether we display the indicator. See `grade/report/summary/index.php` for a real-life example. + +{{< php >}} + +if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); +} + +{{< /php >}} + +If the optional `redirecturl` parameter is set when creating the indicator, the page will automatically reload or redirect to +this URL when the progress bar completes. diff --git a/grade/report/grader/index.php b/grade/report/grader/index.php index 534dd7ca75f..27817798e3d 100644 --- a/grade/report/grader/index.php +++ b/grade/report/grader/index.php @@ -53,10 +53,6 @@ $baseurl = new moodle_url('/grade/report/grader/index.php', ['id' => $courseid]) $PAGE->set_url(new moodle_url('/grade/report/grader/index.php', array('id'=>$courseid))); $PAGE->set_pagelayout('report'); -$PAGE->requires->js_call_amd('gradereport_grader/stickycolspan', 'init'); -$PAGE->requires->js_call_amd('gradereport_grader/user', 'init', [$baseurl->out(false)]); -$PAGE->requires->js_call_amd('gradereport_grader/feedback_modal', 'init'); -$PAGE->requires->js_call_amd('core_grades/gradebooksetup_forms', 'init'); // basic access checks if (!$course = $DB->get_record('course', array('id' => $courseid))) { @@ -129,9 +125,14 @@ if (!empty($target) && !empty($action) && confirm_sesskey()) { } $reportname = get_string('pluginname', 'gradereport_grader'); - -// Do this check just before printing the grade header (and only do it once). -grade_regrade_final_grades_if_required($course); +$regradetask = \core_course\task\regrade_final_grades::create($courseid); +$indicatorheading = get_string('recalculatinggrades', 'grades'); +$indicatormessage = get_string('recalculatinggradesadhoc', 'grades'); +$taskindicator = new \core\output\task_indicator($regradetask, $indicatorheading, $indicatormessage, $PAGE->url); +if (!$taskindicator->has_task_record()) { + // Do this check just before printing the grade header (and only do it once). + grade_regrade_final_grades_if_required($course); +} //Initialise the grader report object that produces the table //the class grade_report_grader_ajax was removed as part of MDL-21562 @@ -143,13 +144,6 @@ $sort = strtoupper($sort); $report = new grade_report_grader($courseid, $gpr, $context, $page, $sortitemid, $sort); -// We call this a little later since we need some info from the grader report. -$PAGE->requires->js_call_amd('gradereport_grader/collapse', 'init', [ - 'userID' => $USER->id, - 'courseID' => $courseid, - 'defaultSort' => $report->get_default_sortable() -]); - $numusers = $report->get_numusers(true, true); $actionbar = new \gradereport_grader\output\action_bar($context, $report, $numusers); @@ -170,6 +164,23 @@ if ($isediting && ($data = data_submitted()) && confirm_sesskey()) { $warnings = $report->process_data($data); } +if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; +} + +// We call this a little later since we need some info from the grader report. +$PAGE->requires->js_call_amd('gradereport_grader/collapse', 'init', [ + 'userID' => $USER->id, + 'courseID' => $courseid, + 'defaultSort' => $report->get_default_sortable(), +]); +$PAGE->requires->js_call_amd('gradereport_grader/stickycolspan', 'init'); +$PAGE->requires->js_call_amd('gradereport_grader/user', 'init', [$baseurl->out(false)]); +$PAGE->requires->js_call_amd('gradereport_grader/feedback_modal', 'init'); +$PAGE->requires->js_call_amd('core_grades/gradebooksetup_forms', 'init'); + // Final grades MUST be loaded after the processing. $report->load_users(); $report->load_final_grades(); diff --git a/grade/report/outcomes/index.php b/grade/report/outcomes/index.php index f88635685c1..6c10cd3a0a3 100644 --- a/grade/report/outcomes/index.php +++ b/grade/report/outcomes/index.php @@ -44,7 +44,13 @@ if (empty($CFG->enableoutcomes)) { } // First make sure we have proper final grades. -grade_regrade_final_grades_if_required($course); +$regradetask = \core_course\task\regrade_final_grades::create($courseid); +$indicatormessage = get_string('recalculatinggradesadhoc', 'grades'); +$indicatorheading = get_string('recalculatinggrades', 'grades'); +$taskindicator = new \core\output\task_indicator($regradetask, $indicatorheading, $indicatormessage, $PAGE->url); +if (!$taskindicator->has_task_record()) { + grade_regrade_final_grades_if_required($course); +} // Grab all outcomes used in course. $report_info = array(); @@ -96,6 +102,14 @@ foreach ($outcomes as $outcomeid => $outcome) { } } +print_grade_page_head($courseid, 'report', 'outcomes'); + +if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; +} + $html = '' . "\n"; $html .= ''; $html .= ''; @@ -176,8 +190,6 @@ foreach ($report_info as $outcomeid => $outcomedata) { $html .= '
' . get_string('outcomeshortname', 'grades') . '' . get_string('courseavg', 'grades') . '
'; -print_grade_page_head($courseid, 'report', 'outcomes'); - echo $html; $event = \gradereport_outcomes\event\grade_report_viewed::create( diff --git a/grade/report/overview/index.php b/grade/report/overview/index.php index 5fcce755556..e7215069b28 100644 --- a/grade/report/overview/index.php +++ b/grade/report/overview/index.php @@ -90,6 +90,13 @@ $USER->grade_last_report[$course->id] = 'overview'; $actionbar = new \core_grades\output\general_action_bar($context, new moodle_url('/grade/report/overview/index.php', ['id' => $courseid]), 'report', 'overview'); +$taskindicator = new \core\output\task_indicator( + \core_course\task\regrade_final_grades::create($courseid), + get_string('recalculatinggrades', 'grades'), + get_string('recalculatinggradesadhoc', 'grades'), + $PAGE->url, +); + if (has_capability('moodle/grade:viewall', $context) && $courseid != SITEID) { // Please note this would be extremely slow if we wanted to implement this properly for all teachers. $groupmode = groups_get_course_groupmode($course); // Groups are being used @@ -115,6 +122,12 @@ if (has_capability('moodle/grade:viewall', $context) && $courseid != SITEID) { groups_print_course_menu($course, $gpr->get_return_url('index.php?id='.$courseid, array('userid'=>0))); + if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; + } + if ($user_selector) { $renderer = $PAGE->get_renderer('gradereport_overview'); echo $renderer->graded_users_selector('overview', $course, $userid, $currentgroup, false); @@ -131,6 +144,12 @@ if (has_capability('moodle/grade:viewall', $context) && $courseid != SITEID) { $report->user, $actionbar); groups_print_course_menu($course, $gpr->get_return_url('index.php?id='.$courseid, array('userid'=>0))); + if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; + } + if ($user_selector) { $renderer = $PAGE->get_renderer('gradereport_overview'); echo $renderer->graded_users_selector('overview', $course, $userid, $currentgroup, false); @@ -168,6 +187,11 @@ if (has_capability('moodle/grade:viewall', $context) && $courseid != SITEID) { } echo $OUTPUT->header(); + if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; + } if ($report->fill_table(true, true)) { echo html_writer::tag('h3', get_string('coursesiamtaking', 'grades')); echo '
' . $report->print_table(true); @@ -176,6 +200,11 @@ if (has_capability('moodle/grade:viewall', $context) && $courseid != SITEID) { print_grade_page_head($courseid, 'report', 'overview', get_string('pluginname', 'gradereport_overview') . ' - ' . fullname($report->user), false, false, true, null, null, $report->user, $actionbar); + if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; + } if ($report->fill_table()) { echo '
' . $report->print_table(true); } @@ -189,6 +218,11 @@ if (has_capability('moodle/grade:viewall', $context) && $courseid != SITEID) { } if (count($report->teachercourses)) { + if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; + } echo html_writer::tag('h3', get_string('coursesiamteaching', 'grades')); $report->print_teacher_table(); } diff --git a/grade/report/singleview/index.php b/grade/report/singleview/index.php index 50fc0f5bc31..118035fc438 100644 --- a/grade/report/singleview/index.php +++ b/grade/report/singleview/index.php @@ -133,8 +133,6 @@ switch ($itemtype) { break; } -$report = new gradereport_singleview\report\singleview($courseid, $gpr, $context, $itemtype, $itemid); - $pageparams = [ 'id' => $courseid, 'userid' => $userid, @@ -150,6 +148,31 @@ if (!is_null($groupid)) { $PAGE->set_url(new moodle_url('/grade/report/singleview/index.php', $pageparams)); +// Make sure we have proper final grades. +$taskindicator = new \core\output\task_indicator( + \core_course\task\regrade_final_grades::create($courseid), + get_string('recalculatinggrades', 'grades'), + get_string('recalculatinggradesadhoc', 'grades'), + $PAGE->url, +); + +if ($taskindicator->has_task_record()) { + // We need to bail out early as the report requires recalculations to be complete, so just display a basic header + // with navigation, and the indicator. + $actionbar = new \core_grades\output\general_action_bar( + $context, + new moodle_url('/grade/report/singleview/index.php', ['id' => $courseid]), + 'report', + 'singleview' + ); + print_grade_page_head($course->id, 'report', 'singleview', actionbar: $actionbar); + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; +} + +$report = new gradereport_singleview\report\singleview($courseid, $gpr, $context, $itemtype, $itemid); + // Build editing on/off button for themes that need it. $button = ''; if ($PAGE->user_allowed_editing() && !$PAGE->theme->haseditswitch) { @@ -211,6 +234,12 @@ if ($data = data_submitted()) { // Make sure we have proper final grades. grade_regrade_final_grades_if_required($course); +if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; +} + // Save the screen state in a session variable as last viewed state. $SESSION->gradereport_singleview["itemtype-{$context->id}"] = $itemtype; if ($itemid) { diff --git a/grade/report/summary/index.php b/grade/report/summary/index.php index 854cc909d02..8ef2c6cd6f3 100644 --- a/grade/report/summary/index.php +++ b/grade/report/summary/index.php @@ -45,11 +45,20 @@ $PAGE->add_body_class('limitedwidth'); require_capability('gradereport/summary:view', $context); require_capability('moodle/grade:viewall', $context); -print_grade_page_head($courseid, 'report', 'summary', false, - false, false, true, null, null, - null, null); +$taskindicator = new \core\output\task_indicator( + \core_course\task\regrade_final_grades::create($courseid), + get_string('recalculatinggrades', 'grades'), + get_string('recalculatinggradesadhoc', 'grades'), + $PAGE->url, +); -$report = system_report_factory::create(summary::class, context_course::instance($courseid)); +print_grade_page_head($courseid, 'report', 'summary'); + +if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); +} else { + $report = system_report_factory::create(summary::class, context_course::instance($courseid)); + echo $report->output(); +} -echo $report->output(); echo $OUTPUT->footer(); diff --git a/grade/report/user/classes/external/user.php b/grade/report/user/classes/external/user.php index 564aade0155..bb6b983b3bb 100644 --- a/grade/report/user/classes/external/user.php +++ b/grade/report/user/classes/external/user.php @@ -161,7 +161,7 @@ class user extends external_api { require_once($CFG->dirroot . '/grade/report/user/lib.php'); // Force regrade to update items marked as 'needupdate'. - grade_regrade_final_grades($course->id); + grade_regrade_final_grades($course->id, async: false); $gpr = new grade_plugin_return( [ diff --git a/grade/report/user/index.php b/grade/report/user/index.php index b481c4faf58..f2a644137d1 100644 --- a/grade/report/user/index.php +++ b/grade/report/user/index.php @@ -85,12 +85,30 @@ if (!isset($USER->grade_last_report)) { $USER->grade_last_report[$course->id] = 'user'; // First make sure we have proper final grades. -grade_regrade_final_grades_if_required($course); +$taskindicator = new \core\output\task_indicator( + \core_course\task\regrade_final_grades::create($courseid), + get_string('recalculatinggrades', 'grades'), + get_string('recalculatinggradesadhoc', 'grades'), + $PAGE->url, +); +if (!$taskindicator->has_task_record()) { + grade_regrade_final_grades_if_required($course); +} $gradesrenderer = $PAGE->get_renderer('core_grades'); // Teachers will see all student reports. if (has_capability('moodle/grade:viewall', $context)) { + if ($taskindicator->has_task_record()) { + // We need to bail out early as getting the gradeable users requires calculations to be complete, + // so just display a basic header with navigation, and the indicator. + $actionbar = new \core_grades\output\general_action_bar($context, $PAGE->url, 'report', 'user'); + print_grade_page_head($course->id, 'report', 'user', actionbar: $actionbar); + echo $OUTPUT->render($taskindicator); + echo $OUTPUT->footer(); + exit; + } + // Verify if we are using groups or not. $groupmode = groups_get_course_groupmode($course); $currentgroup = $gpr->groupid; @@ -203,7 +221,9 @@ if (has_capability('moodle/grade:viewall', $context)) { // Print the page. print_grade_page_head($courseid, 'report', 'user', false, false, false, true, null, null, $report->user); - if ($report->fill_table()) { + if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + } else if ($report->fill_table()) { echo $report->print_table(true); } } diff --git a/grade/report/user/lib.php b/grade/report/user/lib.php index a1436f68ff5..5c854a40fee 100644 --- a/grade/report/user/lib.php +++ b/grade/report/user/lib.php @@ -174,7 +174,18 @@ function grade_report_user_settings_definition(&$mform) { * @param boolean $viewasuser True when we are viewing this as the targetted user sees it. */ function grade_report_user_profilereport(object $course, object $user, bool $viewasuser = false) { - if (!empty($course->showgrades)) { + global $OUTPUT; + + $taskindicator = new \core\output\task_indicator( + \core_course\task\regrade_final_grades::create($course->id), + get_string('recalculatinggrades', 'grades'), + get_string('recalculatinggradesadhoc', 'grades'), + new \core\url('/course/user.php', ['mode' => 'grade', 'id' => $course->id, 'user' => $user->id]), + ); + + if ($taskindicator->has_task_record()) { + echo $OUTPUT->render($taskindicator); + } else if (!empty($course->showgrades)) { $context = context_course::instance($course->id); diff --git a/grade/tests/behat/grade_async_regrade.feature b/grade/tests/behat/grade_async_regrade.feature new file mode 100644 index 00000000000..a7a0f6efd8f --- /dev/null +++ b/grade/tests/behat/grade_async_regrade.feature @@ -0,0 +1,147 @@ +@core @core_grades @javascript +Feature: Asynchronous regrade on a large course + + Background: + Given the following "courses" exist: + | shortname | fullname | idnumber | + | C1 | Test course 1 | C1 | + | C2 | Test course 2 | C2 | + And the following "users" exist: + | username | + | teacher1 | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | teacher1 | C2 | editingteacher | + And "100" "users" exist with the following data: + | username | student[count] | + | firstname | Student | + | lastname | [count] | + | email | student[count]@example.com | + And "100" "course enrolments" exist with the following data: + | user | student[count] | + | course | C1 | + | role | student | + And the following "activity" exists: + | activity | assign | + | course | C1 | + | idnumber | a1 | + | name | Test assignment 1 | + | grade | 100 | + | intro | Submit your online text | + And "100" "grade grades" exist with the following data: + | gradeitem | Test assignment 1 | + | user | student[count] | + | grade | 80.00 | + And the following "course enrolment" exists: + | user | student1 | + | course | C2 | + | role | student | + And the following "activity" exists: + | activity | assign | + | course | C2 | + | idnumber | a2 | + | name | Test assignment 2 | + | grade | 100 | + | intro | Submit your online text | + And the following "grade grade" exists: + | gradeitem | Test assignment 2 | + | user | student1 | + | grade | 80.00 | + And I am on the "Test assignment 1" "assign activity editing" page logged in as teacher1 + And I expand all fieldsets + And I set the field "Rescale existing grades" to "Yes" + And I set the field "Maximum grade" to "50" + And I press "Save and return to course" + And I log out + + Scenario Outline: Task indicator displays on all grade reports when a calculation is pending + Given I am on the "Test course 2" "" page logged in as "" + Then I should not see "Grades are being recalculated due to recent changes." + And should exist + When I am on the "Test course 1" "" page logged in as "" + Then I should see "Grades are being recalculated due to recent changes." + And I should see "Task pending" + And I should see "0.0%" + And should not exist + + Examples: + | report | element | user | + | grades > Grader report > View | "user-grades" "table" | teacher1 | + | grades > Overview report > View | "overview-grade" "table" | teacher1 | + | grades > Single view > View | "Search for a user to view all their grades" "text" | teacher1 | + | grades > Grade summary > View | "Summary" "table" | teacher1 | + | grades > User report > View | "Search for a user to view their report" "text" | teacher1 | + | grades > User report > View | "table.user-grade" "css_element" | student1 | + + Scenario Outline: Gradebook settings can be accessed when a regrade is pending + Given I am on the "Test course 2" "" page logged in as "teacher1" + Then I should see "" + And I should not see "Grades are being recalculated due to recent changes." + Given I am on the "Test course 1" "" page logged in as "teacher1" + Then I should see "" + And I should not see "Grades are being recalculated due to recent changes." + + Examples: + | page | text | + | grades > Gradebook setup | Aggregation | + | grades > Course grade settings | General settings | + + Scenario: Task indicator displays on user profile grade reports when a grade calculation is pending + Given I log in as "student1" + When I follow "Grades" in the user menu + And I follow "Test course 2" + Then "table.user-grade" "css_element" should exist + Then I should not see "Grades are being recalculated due to recent changes." + When I follow "Grades" in the user menu + And I follow "Test course 1" + Then "table.user-grade" "css_element" should not exist + Then I should see "Grades are being recalculated due to recent changes." + + Scenario: Task indicator progresses and redirects when the task is run. + When I am on the "Test course 1" "grades > Grader report > View" page logged in as teacher1 + And I should see "Grades are being recalculated due to recent changes." + And I should see "Task pending" + And I should see "0.0%" + And "user-grades" "table" should not exist + When I run all adhoc tasks + # Progress bar should update. + # Manual wait is a bit of a fudge, but we need the progress bar to poll for an update. + And I wait "1" seconds + Then I should not see "Task pending" + And I should see "Recalculating grades" + And I should see "100%" + When I wait "2" seconds + # The page should reload after a short delay. + Then I should not see "Grades are being recalculated due to recent changes." + And I set the field "Search users" to "Student 1" + And "user-grades" "table" should exist + And "40.00" "text" should exist in the "student1@example.com" "table_row" + + Scenario: Admin should see a "Run now" button in the task indicator + When I am on the "Test course 1" "grades > Grader report > View" page logged in as admin + And I should see "Grades are being recalculated due to recent changes." + And I should see "Task pending" + And I should see "Run now" + + Scenario: Making changes on course with less than 100 grades performs the regrade synchronously, no indicator is shown. + Given I am on the "Test assignment 2" "assign activity editing" page logged in as teacher1 + And I expand all fieldsets + And I set the field "Rescale existing grades" to "Yes" + And I set the field "Maximum grade" to "50" + And I press "Save and return to course" + When I am on the "Test course 2" "grades > Grader report > View" page + Then I should not see "Grades are being recalculated due to recent changes." + And "user-grades" "table" should exist + + Scenario: Editing weights triggers a regrade, but further edits are possible + Given I run all adhoc tasks + And I am on the "Test course 1" "grades > Grader report > View" page logged in as "teacher1" + And I should not see "Grades are being recalculated due to recent changes." + And I am on the "Test course 1" "grades > Gradebook setup" page + When I set the field "Override weight of Test assignment 1" to "1" + And I press "Save changes" + And I am on the "Test course 1" "grades > Grader report > View" page + And I should see "Grades are being recalculated due to recent changes." + And I am on the "Test course 1" "grades > Gradebook setup" page + And I should not see "Grades are being recalculated due to recent changes." diff --git a/lang/en/grades.php b/lang/en/grades.php index 109cbfd8157..7c18c28c147 100644 --- a/lang/en/grades.php +++ b/lang/en/grades.php @@ -707,7 +707,7 @@ $string['real'] = 'Real'; $string['realletter'] = 'Real (letter)'; $string['realpercentage'] = 'Real (percentage)'; $string['recalculatinggrades'] = 'Recalculating grades'; -$string['recalculatinggradesadhoc'] = 'Grade recalculations are being performed in the background. Displayed grades may be incorrect until the process is complete.'; +$string['recalculatinggradesadhoc'] = 'The report will update automatically. You don\'t need to do anything.'; $string['recovergradesdefault'] = 'Recover grades default'; $string['recovergradesdefault_help'] = 'By default recover old grades when re-enrolling a user in a course.'; $string['refreshpreview'] = 'Refresh preview'; diff --git a/lib/amd/build/task_indicator.min.js b/lib/amd/build/task_indicator.min.js new file mode 100644 index 00000000000..da44e913379 --- /dev/null +++ b/lib/amd/build/task_indicator.min.js @@ -0,0 +1,14 @@ +define("core/task_indicator",["exports"],(function(_exports){Object.defineProperty(_exports,"__esModule",{value:!0}),_exports.default=void 0;return _exports.default= +/** + * Task indicator + * + * Watches the progress bar inside the task indicator for updates, and redirects when the progress is complete. + * + * @module core/task_indicator + * @copyright 2024 Catalyst IT Europe Ltd + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class{static init(id,redirectUrl){document.getElementById(id).addEventListener("update",(event=>{100===event.detail.percent&&window.setTimeout((()=>window.location.assign(redirectUrl)),2e3)}))}},_exports.default})); + +//# sourceMappingURL=task_indicator.min.js.map \ No newline at end of file diff --git a/lib/amd/build/task_indicator.min.js.map b/lib/amd/build/task_indicator.min.js.map new file mode 100644 index 00000000000..40edf051f79 --- /dev/null +++ b/lib/amd/build/task_indicator.min.js.map @@ -0,0 +1 @@ +{"version":3,"file":"task_indicator.min.js","sources":["../src/task_indicator.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * Task indicator\n *\n * Watches the progress bar inside the task indicator for updates, and redirects when the progress is complete.\n *\n * @module core/task_indicator\n * @copyright 2024 Catalyst IT Europe Ltd\n * @author Mark Johnson \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\nexport default class {\n /**\n * Watch the progress bar for updates.\n *\n * When the progress bar is updated to 100%, wait a couple of seconds so the user gets to see it if they are watching,\n * then redirect to the specified URL.\n *\n * @param {String} id\n * @param {String} redirectUrl\n */\n static init(id, redirectUrl) {\n document.getElementById(id).addEventListener('update', (event) => {\n if (event.detail.percent === 100) {\n window.setTimeout(() => window.location.assign(redirectUrl), 2000);\n }\n });\n }\n}\n"],"names":["id","redirectUrl","document","getElementById","addEventListener","event","detail","percent","window","setTimeout","location","assign"],"mappings":";;;;;;;;;;;kBAmCgBA,GAAIC,aACZC,SAASC,eAAeH,IAAII,iBAAiB,UAAWC,QACvB,MAAzBA,MAAMC,OAAOC,SACbC,OAAOC,YAAW,IAAMD,OAAOE,SAASC,OAAOV,cAAc"} \ No newline at end of file diff --git a/lib/amd/src/task_indicator.js b/lib/amd/src/task_indicator.js new file mode 100644 index 00000000000..ba963e18edc --- /dev/null +++ b/lib/amd/src/task_indicator.js @@ -0,0 +1,43 @@ +// This file is part of Moodle - http://moodle.org/ +// +// Moodle is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Moodle is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Moodle. If not, see . + +/** + * Task indicator + * + * Watches the progress bar inside the task indicator for updates, and redirects when the progress is complete. + * + * @module core/task_indicator + * @copyright 2024 Catalyst IT Europe Ltd + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +export default class { + /** + * Watch the progress bar for updates. + * + * When the progress bar is updated to 100%, wait a couple of seconds so the user gets to see it if they are watching, + * then redirect to the specified URL. + * + * @param {String} id + * @param {String} redirectUrl + */ + static init(id, redirectUrl) { + document.getElementById(id).addEventListener('update', (event) => { + if (event.detail.percent === 100) { + window.setTimeout(() => window.location.assign(redirectUrl), 2000); + } + }); + } +} diff --git a/lib/classes/output/task_indicator.php b/lib/classes/output/task_indicator.php new file mode 100644 index 00000000000..e5167a7879a --- /dev/null +++ b/lib/classes/output/task_indicator.php @@ -0,0 +1,112 @@ +. + +namespace core\output; + +use core\task\adhoc_task; +use core\task\stored_progress_task_trait; +use core\url; +use stdClass; + +/** + * Indicator for displaying status and progress of a background task + * + * @package core + * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class task_indicator implements renderable, templatable { + /** @var ?stdClass $taskrecord */ + protected ?stdClass $taskrecord; + + /** @var ?stored_progress_bar $progressbar */ + protected ?stored_progress_bar $progressbar; + + /** + * Find the task record, and get the progress bar object. + * + * @param adhoc_task $task The task whose progress is being indicated. The task class must use stored_progress_task_trait. + * @param string $heading The header text for the indicator. + * @param string $message A message to explain what is happening while the task is running. + * @param ?url $redirecturl An optional URL to redirect to when the task completes. + * @param ?pix_icon $icon An optional icon to display with the heading. + * @param array $extraclasses Extra class names to apply to the indicator's container. + * @throws \coding_exception + */ + public function __construct( + /** @var adhoc_task $task The task whose progress is being indicated. The task class must use stored_progress_task_trait. */ + protected adhoc_task $task, + /** @var string $heading The header text for the indicator. */ + protected string $heading, + /** @var string $message A message to explain what is happening while the task is running. */ + protected string $message, + /** @var ?url $redirecturl An optional URL to redirect to when the task completes. */ + protected ?url $redirecturl = null, + /** @var ?pix_icon $icon An optional icon to display with the heading. */ + protected ?pix_icon $icon = new pix_icon('i/timer', ''), + /** @var array $extraclasses Extra class names to apply to the indicator's container. */ + protected array $extraclasses = [], + ) { + if (!class_uses($task::class, stored_progress_task_trait::class)) { + throw new \coding_exception('task_indicator can only be used for tasks using stored_progress_task_trait.'); + } + $this->setup_task_data(); + } + + /** + * Fetch the task record matching the current task, if there is one. + * + * If one exists, also set up a progress bar, and set up the "run now" link if permitted. + * + * @return void + */ + protected function setup_task_data(): void { + $this->taskrecord = \core\task\manager::get_queued_adhoc_task_record($this->task) ?: null; + if ($this->taskrecord) { + $this->task->set_id($this->taskrecord->id); + $idnumber = stored_progress_bar::convert_to_idnumber($this->task::class, $this->task->get_id()); + $this->progressbar = stored_progress_bar::get_by_idnumber($idnumber); + } + } + + /** + * Check if there is a task record matching the defined task. + * + * If so, set the task ID and progress bar, then return true. If not, return false. + * + * @return bool + */ + public function has_task_record(): bool { + $this->setup_task_data(); + return !is_null($this->taskrecord); + } + + #[\Override] + public function export_for_template(renderer_base $output): array { + $export = []; + if ($this->taskrecord) { + $export['heading'] = $this->heading; + $export['message'] = $this->message; + $export['progress'] = $this->progressbar->export_for_template($output); + $export['icon'] = $this->icon ? $this->icon->export_for_template($output) : ''; + $export['redirecturl'] = $this->redirecturl->out(); + $export['extraclasses'] = implode(' ', $this->extraclasses); + $this->progressbar->init_js(); + } + return $export; + } +} diff --git a/lib/classes/task/manager.php b/lib/classes/task/manager.php index ac564611f17..e623fa3609a 100644 --- a/lib/classes/task/manager.php +++ b/lib/classes/task/manager.php @@ -194,7 +194,7 @@ class manager { * @param adhoc_task $task * @return \stdClass|false */ - protected static function get_queued_adhoc_task_record($task) { + public static function get_queued_adhoc_task_record($task) { global $DB; $record = self::record_from_adhoc_task($task); diff --git a/lib/gradelib.php b/lib/gradelib.php index b271f105864..48aba67bead 100644 --- a/lib/gradelib.php +++ b/lib/gradelib.php @@ -1153,6 +1153,11 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null, } else { if (!$course_item->needsupdate) { // nothing to do :-) + if ($progress instanceof \core\progress\stored) { + // The regrade was already run elsewhere without the stored progress, so just start and end it now. + $progress->start_progress(get_string('recalculatinggrades', 'grades')); + $progress->end_progress(); + } return true; } // Defer recalculation to an ad-hoc task. @@ -1219,7 +1224,7 @@ function grade_regrade_final_grades($courseid, $userid=null, $updated_item=null, } } - $progress->start_progress('regrade_course', $progresstotal); + $progress->start_progress(get_string('recalculatinggrades', 'grades'), $progresstotal); $errors = array(); $finalids = array(); diff --git a/lib/templates/task_indicator.mustache b/lib/templates/task_indicator.mustache new file mode 100644 index 00000000000..e6732e32c8c --- /dev/null +++ b/lib/templates/task_indicator.mustache @@ -0,0 +1,61 @@ +{{! + This file is part of Moodle - http://moodle.org/ + + Moodle is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Moodle is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with Moodle. If not, see . +}} +{{! + @template core/task_indicator + + Display the stored progress of a background task and optionally redirect when it is complete. + + Example context (json): + { + "heading": "Regrade in progress", + "icon": { + "attributes": [ + {"name": "src", "value": "/pix/i/timer.svg"}, + {"name": "alt", "value": ""} + ] + }, + "message": "Grades are being recalculated due to recent changes.", + "progress": { + "id": "progressbar_test", + "message": "Recalculating grades", + "idnumber": "progressbar_test", + "width": "500", + "value": "50" + } + } +}} +
+ {{#icon}} +
+ {{>core/pix_icon}} +
+ {{/icon}} +

{{heading}}

+

{{message}}

+ {{#progress}} + {{>core/progress_bar}} + {{/progress}} +
+ +{{#redirecturl}} + {{#js}} + require(['core/task_indicator'], function(TaskIndicator) { + TaskIndicator.init('{{progress.idnumber}}', '{{redirecturl}}'); + }); + {{/js}} +{{/redirecturl}} + diff --git a/pix/i/timer.svg b/pix/i/timer.svg new file mode 100644 index 00000000000..a76f6d3e3fd --- /dev/null +++ b/pix/i/timer.svg @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/theme/boost/scss/moodle.scss b/theme/boost/scss/moodle.scss index cb3cabb0d6c..07945e83a00 100644 --- a/theme/boost/scss/moodle.scss +++ b/theme/boost/scss/moodle.scss @@ -48,3 +48,4 @@ @import "moodle/moodlenet"; @import "moodle/dropdown"; @import "moodle/deprecated"; +@import "moodle/task-indicator"; diff --git a/theme/boost/scss/moodle/task-indicator.scss b/theme/boost/scss/moodle/task-indicator.scss new file mode 100644 index 00000000000..171951228e9 --- /dev/null +++ b/theme/boost/scss/moodle/task-indicator.scss @@ -0,0 +1,25 @@ +.task-indicator { + padding-top: 2rem; + + img.icon { + width: 145px; + height: 145px; + max-height: 145px; + max-width: 145px; + font-size: 145px; + padding-bottom: 2rem; + } + + .progressbar_container { + padding-top: 2rem; + } + + .stored-progress-bar { + opacity: 1; + transition: opacity 0.5s; + } + + .stored-progress-notstarted { + opacity: 0; + } +} \ No newline at end of file diff --git a/theme/boost/style/moodle.css b/theme/boost/style/moodle.css index 575f9885083..20e3f800507 100644 --- a/theme/boost/style/moodle.css +++ b/theme/boost/style/moodle.css @@ -47240,6 +47240,28 @@ input[type=button].btn-block { width: 100%; } +.task-indicator { + padding-top: 2rem; +} +.task-indicator img.icon { + width: 145px; + height: 145px; + max-height: 145px; + max-width: 145px; + font-size: 145px; + padding-bottom: 2rem; +} +.task-indicator .progressbar_container { + padding-top: 2rem; +} +.task-indicator .stored-progress-bar { + opacity: 1; + transition: opacity 0.5s; +} +.task-indicator .stored-progress-notstarted { + opacity: 0; +} + body { -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; diff --git a/theme/classic/style/moodle.css b/theme/classic/style/moodle.css index 49e141d1f82..28132b9e10d 100644 --- a/theme/classic/style/moodle.css +++ b/theme/classic/style/moodle.css @@ -47174,6 +47174,28 @@ input[type=button].btn-block { width: 100%; } +.task-indicator { + padding-top: 2rem; +} +.task-indicator img.icon { + width: 145px; + height: 145px; + max-height: 145px; + max-width: 145px; + font-size: 145px; + padding-bottom: 2rem; +} +.task-indicator .progressbar_container { + padding-top: 2rem; +} +.task-indicator .stored-progress-bar { + opacity: 1; + transition: opacity 0.5s; +} +.task-indicator .stored-progress-notstarted { + opacity: 0; +} + body { -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; From 69afa8a3ace3899d9b1fc487ee2303f6201adcb0 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 3 Sep 2024 14:37:39 +0100 Subject: [PATCH 6/7] MDL-81714 grades: Display "Run now" button for admins in task indicator --- .../moodle/components/task-indicator.md | 31 ++++++++++++- grade/tests/behat/grade_async_regrade.feature | 43 ++++++++----------- lib/amd/build/task_indicator.min.js | 2 +- lib/amd/build/task_indicator.min.js.map | 2 +- lib/amd/src/task_indicator.js | 19 ++++++-- lib/classes/output/task_indicator.php | 27 +++++++++++- lib/templates/task_indicator.mustache | 37 +++++++++------- 7 files changed, 114 insertions(+), 47 deletions(-) diff --git a/admin/tool/componentlibrary/content/moodle/components/task-indicator.md b/admin/tool/componentlibrary/content/moodle/components/task-indicator.md index 4216f49c3ef..6afc45f80ba 100644 --- a/admin/tool/componentlibrary/content/moodle/components/task-indicator.md +++ b/admin/tool/componentlibrary/content/moodle/components/task-indicator.md @@ -6,7 +6,7 @@ date: 2024-08-21T00:00:00+01:00 draft: false tags: - MDL-81714 -- 4.5 +- 5.0 --- {{< mustache template="core/task_indicator" >}} @@ -89,5 +89,34 @@ if ($taskindicator->has_task_record()) { {{< /php >}} +When the task begins running and the progress is updated, the progress bar will automatically be displayed. + If the optional `redirecturl` parameter is set when creating the indicator, the page will automatically reload or redirect to this URL when the progress bar completes. + +While the task is still queued, admins will see a "Run now" button below the indicator. This is designed for convenience if +a user is blocked on a job and needs the task run immediately. It will run the specific instance of the task tracked by the +indicator. + +{{< mustache template="core/task_indicator" >}} +{ + "heading": "Regrade in progress", + "icon": { + "attributes": [ + {"name": "src", "value": "/pix/i/timer.svg"}, + {"name": "alt", "value": ""} + ] + }, + "message": "Grades are being recalculated due to recent changes.", + "progress": { + "id": "progressbar_test", + "message": "Task pending", + "idnumber": "progressbar_test", + "class": "stored-progress-bar stored-progress-notstarted", + "width": "500", + "value": "0" + }, + "runurl": "http://example.com/runtask.php?id=1", + "runlabel": "Run now" +} +{{< /mustache >}} diff --git a/grade/tests/behat/grade_async_regrade.feature b/grade/tests/behat/grade_async_regrade.feature index a7a0f6efd8f..effe1f13cb7 100644 --- a/grade/tests/behat/grade_async_regrade.feature +++ b/grade/tests/behat/grade_async_regrade.feature @@ -54,15 +54,14 @@ Feature: Asynchronous regrade on a large course And I set the field "Maximum grade" to "50" And I press "Save and return to course" And I log out + And I change the viewport size to "medium" Scenario Outline: Task indicator displays on all grade reports when a calculation is pending Given I am on the "Test course 2" "" page logged in as "" - Then I should not see "Grades are being recalculated due to recent changes." + Then I should not see "The report will update automatically. You don't need to do anything." And should exist When I am on the "Test course 1" "" page logged in as "" - Then I should see "Grades are being recalculated due to recent changes." - And I should see "Task pending" - And I should see "0.0%" + Then I should see "The report will update automatically. You don't need to do anything." And should not exist Examples: @@ -77,10 +76,10 @@ Feature: Asynchronous regrade on a large course Scenario Outline: Gradebook settings can be accessed when a regrade is pending Given I am on the "Test course 2" "" page logged in as "teacher1" Then I should see "" - And I should not see "Grades are being recalculated due to recent changes." + And I should not see "The report will update automatically. You don't need to do anything." Given I am on the "Test course 1" "" page logged in as "teacher1" Then I should see "" - And I should not see "Grades are being recalculated due to recent changes." + And I should not see "The report will update automatically. You don't need to do anything." Examples: | page | text | @@ -92,36 +91,32 @@ Feature: Asynchronous regrade on a large course When I follow "Grades" in the user menu And I follow "Test course 2" Then "table.user-grade" "css_element" should exist - Then I should not see "Grades are being recalculated due to recent changes." + Then I should not see "The report will update automatically. You don't need to do anything." When I follow "Grades" in the user menu And I follow "Test course 1" Then "table.user-grade" "css_element" should not exist - Then I should see "Grades are being recalculated due to recent changes." + Then I should see "The report will update automatically. You don't need to do anything." Scenario: Task indicator progresses and redirects when the task is run. When I am on the "Test course 1" "grades > Grader report > View" page logged in as teacher1 - And I should see "Grades are being recalculated due to recent changes." - And I should see "Task pending" - And I should see "0.0%" + And I should see "The report will update automatically. You don't need to do anything." + And I should not see "Run now" + And I should not see "0.0%" And "user-grades" "table" should not exist - When I run all adhoc tasks + And I run all adhoc tasks # Progress bar should update. - # Manual wait is a bit of a fudge, but we need the progress bar to poll for an update. - And I wait "1" seconds - Then I should not see "Task pending" - And I should see "Recalculating grades" + And I wait until "Recalculating grades" "text" exists And I should see "100%" - When I wait "2" seconds # The page should reload after a short delay. - Then I should not see "Grades are being recalculated due to recent changes." + Then I wait until "Recalculating grades" "text" does not exist And I set the field "Search users" to "Student 1" And "user-grades" "table" should exist And "40.00" "text" should exist in the "student1@example.com" "table_row" Scenario: Admin should see a "Run now" button in the task indicator When I am on the "Test course 1" "grades > Grader report > View" page logged in as admin - And I should see "Grades are being recalculated due to recent changes." - And I should see "Task pending" + And I should see "The report will update automatically. You don't need to do anything." + And I should not see "0.0%" And I should see "Run now" Scenario: Making changes on course with less than 100 grades performs the regrade synchronously, no indicator is shown. @@ -131,17 +126,17 @@ Feature: Asynchronous regrade on a large course And I set the field "Maximum grade" to "50" And I press "Save and return to course" When I am on the "Test course 2" "grades > Grader report > View" page - Then I should not see "Grades are being recalculated due to recent changes." + Then I should not see "The report will update automatically. You don't need to do anything." And "user-grades" "table" should exist Scenario: Editing weights triggers a regrade, but further edits are possible Given I run all adhoc tasks And I am on the "Test course 1" "grades > Grader report > View" page logged in as "teacher1" - And I should not see "Grades are being recalculated due to recent changes." + And I should not see "The report will update automatically. You don't need to do anything." And I am on the "Test course 1" "grades > Gradebook setup" page When I set the field "Override weight of Test assignment 1" to "1" And I press "Save changes" And I am on the "Test course 1" "grades > Grader report > View" page - And I should see "Grades are being recalculated due to recent changes." + And I should see "The report will update automatically. You don't need to do anything." And I am on the "Test course 1" "grades > Gradebook setup" page - And I should not see "Grades are being recalculated due to recent changes." + And I should not see "The report will update automatically. You don't need to do anything." diff --git a/lib/amd/build/task_indicator.min.js b/lib/amd/build/task_indicator.min.js index da44e913379..858acf1becb 100644 --- a/lib/amd/build/task_indicator.min.js +++ b/lib/amd/build/task_indicator.min.js @@ -9,6 +9,6 @@ define("core/task_indicator",["exports"],(function(_exports){Object.defineProper * @author Mark Johnson * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -class{static init(id,redirectUrl){document.getElementById(id).addEventListener("update",(event=>{100===event.detail.percent&&window.setTimeout((()=>window.location.assign(redirectUrl)),2e3)}))}},_exports.default})); +class{static init(id,redirectUrl){const bar=document.getElementById(id);bar.addEventListener("update",(event=>{var _event$detail;const percent=null==event||null===(_event$detail=event.detail)||void 0===_event$detail?void 0:_event$detail.percent;if(percent>0){bar.classList.remove("stored-progress-notstarted");const runlink=document.querySelector(".runlink[data-idnumber=".concat(id,"]"));runlink&&runlink.remove()}""!==redirectUrl&&100===percent&&window.setTimeout((()=>window.location.assign(redirectUrl)),2e3)}))}},_exports.default})); //# sourceMappingURL=task_indicator.min.js.map \ No newline at end of file diff --git a/lib/amd/build/task_indicator.min.js.map b/lib/amd/build/task_indicator.min.js.map index 40edf051f79..35df3313865 100644 --- a/lib/amd/build/task_indicator.min.js.map +++ b/lib/amd/build/task_indicator.min.js.map @@ -1 +1 @@ -{"version":3,"file":"task_indicator.min.js","sources":["../src/task_indicator.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * Task indicator\n *\n * Watches the progress bar inside the task indicator for updates, and redirects when the progress is complete.\n *\n * @module core/task_indicator\n * @copyright 2024 Catalyst IT Europe Ltd\n * @author Mark Johnson \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\nexport default class {\n /**\n * Watch the progress bar for updates.\n *\n * When the progress bar is updated to 100%, wait a couple of seconds so the user gets to see it if they are watching,\n * then redirect to the specified URL.\n *\n * @param {String} id\n * @param {String} redirectUrl\n */\n static init(id, redirectUrl) {\n document.getElementById(id).addEventListener('update', (event) => {\n if (event.detail.percent === 100) {\n window.setTimeout(() => window.location.assign(redirectUrl), 2000);\n }\n });\n }\n}\n"],"names":["id","redirectUrl","document","getElementById","addEventListener","event","detail","percent","window","setTimeout","location","assign"],"mappings":";;;;;;;;;;;kBAmCgBA,GAAIC,aACZC,SAASC,eAAeH,IAAII,iBAAiB,UAAWC,QACvB,MAAzBA,MAAMC,OAAOC,SACbC,OAAOC,YAAW,IAAMD,OAAOE,SAASC,OAAOV,cAAc"} \ No newline at end of file +{"version":3,"file":"task_indicator.min.js","sources":["../src/task_indicator.js"],"sourcesContent":["// This file is part of Moodle - http://moodle.org/\n//\n// Moodle is free software: you can redistribute it and/or modify\n// it under the terms of the GNU General Public License as published by\n// the Free Software Foundation, either version 3 of the License, or\n// (at your option) any later version.\n//\n// Moodle is distributed in the hope that it will be useful,\n// but WITHOUT ANY WARRANTY; without even the implied warranty of\n// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the\n// GNU General Public License for more details.\n//\n// You should have received a copy of the GNU General Public License\n// along with Moodle. If not, see .\n\n/**\n * Task indicator\n *\n * Watches the progress bar inside the task indicator for updates, and redirects when the progress is complete.\n *\n * @module core/task_indicator\n * @copyright 2024 Catalyst IT Europe Ltd\n * @author Mark Johnson \n * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later\n */\nexport default class {\n /**\n * Watch the progress bar for updates.\n *\n * When the progress bar is updated to 100%, wait a couple of seconds so the user gets to see it if they are watching,\n * then redirect to the specified URL.\n *\n * @param {String} id The ID of the progress bar element.\n * @param {String} redirectUrl Optional URL to redirect to once the task is complete.\n */\n static init(id, redirectUrl) {\n const bar = document.getElementById(id);\n bar.addEventListener('update', (event) => {\n const percent = event?.detail?.percent;\n if (percent > 0) {\n // Once progress starts, display the progress bar and remove the run link.\n bar.classList.remove('stored-progress-notstarted');\n const runlink = document.querySelector(`.runlink[data-idnumber=${id}]`);\n if (runlink) {\n runlink.remove();\n }\n }\n // Once the progress bar completes, redirect the page.\n if (redirectUrl !== '' && percent === 100) {\n window.setTimeout(() => window.location.assign(redirectUrl), 2000);\n }\n });\n }\n}\n"],"names":["id","redirectUrl","bar","document","getElementById","addEventListener","event","percent","detail","_event$detail","classList","remove","runlink","querySelector","window","setTimeout","location","assign"],"mappings":";;;;;;;;;;;kBAmCgBA,GAAIC,mBACNC,IAAMC,SAASC,eAAeJ,IACpCE,IAAIG,iBAAiB,UAAWC,gCACtBC,QAAUD,MAAAA,6BAAAA,MAAOE,uCAAPC,cAAeF,WAC3BA,QAAU,EAAG,CAEbL,IAAIQ,UAAUC,OAAO,oCACfC,QAAUT,SAASU,+CAAwCb,SAC7DY,SACAA,QAAQD,SAII,KAAhBV,aAAkC,MAAZM,SACtBO,OAAOC,YAAW,IAAMD,OAAOE,SAASC,OAAOhB,cAAc"} \ No newline at end of file diff --git a/lib/amd/src/task_indicator.js b/lib/amd/src/task_indicator.js index ba963e18edc..79cb07aec8d 100644 --- a/lib/amd/src/task_indicator.js +++ b/lib/amd/src/task_indicator.js @@ -30,12 +30,23 @@ export default class { * When the progress bar is updated to 100%, wait a couple of seconds so the user gets to see it if they are watching, * then redirect to the specified URL. * - * @param {String} id - * @param {String} redirectUrl + * @param {String} id The ID of the progress bar element. + * @param {String} redirectUrl Optional URL to redirect to once the task is complete. */ static init(id, redirectUrl) { - document.getElementById(id).addEventListener('update', (event) => { - if (event.detail.percent === 100) { + const bar = document.getElementById(id); + bar.addEventListener('update', (event) => { + const percent = event?.detail?.percent; + if (percent > 0) { + // Once progress starts, display the progress bar and remove the run link. + bar.classList.remove('stored-progress-notstarted'); + const runlink = document.querySelector(`.runlink[data-idnumber=${id}]`); + if (runlink) { + runlink.remove(); + } + } + // Once the progress bar completes, redirect the page. + if (redirectUrl !== '' && percent === 100) { window.setTimeout(() => window.location.assign(redirectUrl), 2000); } }); diff --git a/lib/classes/output/task_indicator.php b/lib/classes/output/task_indicator.php index e5167a7879a..2bf4a1914bc 100644 --- a/lib/classes/output/task_indicator.php +++ b/lib/classes/output/task_indicator.php @@ -16,14 +16,21 @@ namespace core\output; +use core\plugin_manager; use core\task\adhoc_task; use core\task\stored_progress_task_trait; use core\url; +use core\context\system; use stdClass; /** * Indicator for displaying status and progress of a background task * + * This will display a section containing an icon, heading and message describing the background task being performed, + * as well as a progress bar that is updated as the task progresses. Optionally, it will redirect to a given URL (or reload + * the current one) when the task completes. If the task is still waiting in the queue, an admin viewing the indicator + * will also see a "Run now" button. + * * @package core * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} * @author Mark Johnson @@ -36,6 +43,12 @@ class task_indicator implements renderable, templatable { /** @var ?stored_progress_bar $progressbar */ protected ?stored_progress_bar $progressbar; + /** @var ?url $runurl The URL to manually run the task. */ + protected ?url $runurl = null; + + /** @var string $runlabel Label for the link to run the task. */ + protected string $runlabel = ''; + /** * Find the task record, and get the progress bar object. * @@ -80,6 +93,16 @@ class task_indicator implements renderable, templatable { $this->task->set_id($this->taskrecord->id); $idnumber = stored_progress_bar::convert_to_idnumber($this->task::class, $this->task->get_id()); $this->progressbar = stored_progress_bar::get_by_idnumber($idnumber); + // As long as the tool_task plugin hasn't been removed, + // allow admins to trigger the task manually if it's not running yet. + if ( + array_key_exists('task', plugin_manager::instance()->get_present_plugins('tool')) + && is_null($this->taskrecord->timestarted) + && has_capability('moodle/site:config', system::instance()) + ) { + $this->runurl = new url('/admin/tool/task/run_adhoctasks.php', ['id' => $this->taskrecord->id]); + $this->runlabel = get_string('runnow', 'tool_task'); + } } } @@ -103,8 +126,10 @@ class task_indicator implements renderable, templatable { $export['message'] = $this->message; $export['progress'] = $this->progressbar->export_for_template($output); $export['icon'] = $this->icon ? $this->icon->export_for_template($output) : ''; - $export['redirecturl'] = $this->redirecturl->out(); + $export['redirecturl'] = $this->redirecturl?->out(); $export['extraclasses'] = implode(' ', $this->extraclasses); + $export['runurl'] = $this->runurl?->out(); + $export['runlabel'] = $this->runlabel; $this->progressbar->init_js(); } return $export; diff --git a/lib/templates/task_indicator.mustache b/lib/templates/task_indicator.mustache index e6732e32c8c..c399300bffa 100644 --- a/lib/templates/task_indicator.mustache +++ b/lib/templates/task_indicator.mustache @@ -33,29 +33,36 @@ "id": "progressbar_test", "message": "Recalculating grades", "idnumber": "progressbar_test", + "class": "stored-progress-bar", "width": "500", "value": "50" } } }}
- {{#icon}} -
- {{>core/pix_icon}} -
- {{/icon}} -

{{heading}}

-

{{message}}

+
+ {{#icon}} +
+ {{>core/pix_icon}} +
+ {{/icon}} +

{{heading}}

+

{{message}}

+
+ {{#runurl}} +

+ {{runlabel}} +

+ {{/runurl}} {{#progress}} {{>core/progress_bar}} {{/progress}} + +
-{{#redirecturl}} - {{#js}} - require(['core/task_indicator'], function(TaskIndicator) { - TaskIndicator.init('{{progress.idnumber}}', '{{redirecturl}}'); - }); - {{/js}} -{{/redirecturl}} - +{{#js}} + require(['core/task_indicator'], function(TaskIndicator) { + TaskIndicator.init('{{progress.idnumber}}', '{{redirecturl}}'); + }); +{{/js}} From 0f60de575e5a6b12cc64cb0c1505fef9ae9fcb17 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Wed, 26 Feb 2025 13:30:35 +0000 Subject: [PATCH 7/7] MDL-81714 output/grades: Add upgrade notes --- .upgradenotes/MDL-81714-2025022613184221.yml | 46 ++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 .upgradenotes/MDL-81714-2025022613184221.yml diff --git a/.upgradenotes/MDL-81714-2025022613184221.yml b/.upgradenotes/MDL-81714-2025022613184221.yml new file mode 100644 index 00000000000..c00c1c22e16 --- /dev/null +++ b/.upgradenotes/MDL-81714-2025022613184221.yml @@ -0,0 +1,46 @@ +issueNumber: MDL-81714 +notes: + core: + - message: > + The stored progress API has been updated. The + `\core\output\stored_progress_bar` class has + + now has a `store_pending()` method, which will create a record for the + stored process, but + + without a start time or progress percentage. + + `\core\task\stored_progress_task_trait` has been updated with a new + `initialise_stored_progress()` method, + + which will call `store_pending()` for the task's progress bar. This + allows the progress bar to be displayed + + in a "pending" state, to show that a process has been queued but not + started. + type: improved + - message: > + A new `\core\output\task_indicator` component has been added to display + a progress bar and message + + for a background task using `\core\task\stored_progress_task_trait`. See + the "Task indicator" + + page in the component library for usage details. + type: improved + core_grades: + - message: > + `grade_regrade_final_grades()` now has an additional `async` parameter, + which allows full course + + regrades to be performed in the background. This avoids blocking the + user for long periods and + + while making changes to a large course. The actual regrade is performed + using the + + `\core_course\task\regrade_final_grades` adhoc task, which calls + `grade_regrade_final_grades()` + + with `async: false`. + type: improved