MDL-68256 task admin: usability improvements

In the table that lists the scheduled tasks:

1. There are badges to show which components are disabled.
2. The plugin name (e.g. auth_ldap) is shown as well as the
   human-readable name (e.g. LDAP server).
3. Where a time column has a non-default value, it is highlighted
   and the default is shown.
4. If the fail-delay is non-zero, the cell is highlighted.
4. If you just interacted with a task (looked at or edited the settings,
   did Run now, or cleared the fail delay) that row is highlighted,
   and scrolled into view when the page loads.

To support this, some of the methods for loading the default tasks
have been extended with an optional argument to leave 'R' as 'R'
rather than replacing with a random number.

Also, mixed into this commit are a bunch of coding style improvements.
Sorry I did not separate them out, but ultimately this makes the
Moodle code better.
This commit is contained in:
Tim Hunt
2020-03-28 18:40:51 +00:00
parent d939d6e769
commit 6fdc0f8796
12 changed files with 157 additions and 78 deletions
+3
View File
@@ -62,6 +62,8 @@ if (null !== $logid) {
$renderer = $PAGE->get_renderer('tool_task');
echo $OUTPUT->header();
// Output the search form.
echo $OUTPUT->render_from_template('core_admin/tasklogs', (object) [
'action' => $pageurl->out(),
'filter' => $filter,
@@ -84,6 +86,7 @@ echo $OUTPUT->render_from_template('core_admin/tasklogs', (object) [
],
]);
// Output any matching logs.
$table = new \core_admin\task_log_table($filter, $result);
$table->baseurl = $pageurl;
$table->out(100, false);
+1 -1
View File
@@ -17,7 +17,7 @@
{{!
@template core_admin/tasklogs
Task Logs template.
This is the template for the search form which appears above the task logs report.
}}
<form class="form-inline" method="GET" action="{{{action}}}">
<label class="sr-only" for="tasklog-filter">{{#str}}filter{{/str}}</label>
+6 -4
View File
@@ -39,13 +39,16 @@ if (!$task) {
print_error('cannotfindinfo', 'error', $taskname);
}
$returnurl = new moodle_url('/admin/tool/task/scheduledtasks.php',
['lastchanged' => get_class($task)]);
// If actually doing the clear, then carry out the task and redirect to the scheduled task page.
if (optional_param('confirm', 0, PARAM_INT)) {
require_sesskey();
\core\task\manager::clear_fail_delay($task);
redirect(new moodle_url('/admin/tool/task/scheduledtasks.php'));
redirect($returnurl);
}
// Start output.
@@ -60,9 +63,8 @@ echo $OUTPUT->header();
// they confirm.
echo $OUTPUT->confirm(get_string('clearfaildelay_confirm', 'tool_task', $task->get_name()),
new single_button(new moodle_url('/admin/tool/task/clear_fail_delay.php',
array('task' => $taskname, 'confirm' => 1, 'sesskey' => sesskey())),
['task' => $taskname, 'confirm' => 1, 'sesskey' => sesskey()]),
get_string('clear')),
new single_button(new moodle_url('/admin/tool/task/scheduledtasks.php'),
get_string('cancel'), false));
new single_button($returnurl, get_string('cancel'), false));
echo $OUTPUT->footer();
+1
View File
@@ -30,6 +30,7 @@ $string['clearfaildelay_confirm'] = 'Are you sure you want to clear the fail del
$string['component'] = 'Component';
$string['corecomponent'] = 'Core';
$string['default'] = 'Default';
$string['defaultx'] = 'Default: {$a}';
$string['disabled'] = 'Disabled';
$string['disabled_help'] = 'Disabled scheduled tasks are not executed from cron, however they can still be executed manually via the CLI tool.';
$string['edittaskschedule'] = 'Edit task schedule: {$a}';
+87 -34
View File
@@ -36,9 +36,10 @@ class tool_task_renderer extends plugin_renderer_base {
* This function will render one beautiful table with all the scheduled tasks.
*
* @param \core\task\scheduled_task[] $tasks - list of all scheduled tasks.
* @param string $lastchanged (optional) the last task edited. Gets highlighted in teh table.
* @return string HTML to output.
*/
public function scheduled_tasks_table($tasks) {
public function scheduled_tasks_table($tasks, $lastchanged = '') {
global $CFG;
$showloglink = \core\task\logmanager::has_log_report();
@@ -68,7 +69,7 @@ class tool_task_renderer extends plugin_renderer_base {
$table->colclasses['3'] = 'hidden';
}
$data = array();
$data = [];
$yes = get_string('yes');
$no = get_string('no');
$never = get_string('never');
@@ -77,35 +78,46 @@ class tool_task_renderer extends plugin_renderer_base {
$plugindisabledstr = get_string('plugindisabled', 'tool_task');
$runnabletasks = tool_task\run_from_cli::is_runnable();
foreach ($tasks as $task) {
$classname = get_class($task);
$defaulttask = \core\task\manager::get_default_scheduled_task($classname, false);
$customised = $task->is_customised() ? $no : $yes;
if (empty($CFG->preventscheduledtaskchanges)) {
$configureurl = new moodle_url('/admin/tool/task/scheduledtasks.php', array('action'=>'edit', 'task' => get_class($task)));
$editlink = $this->action_icon($configureurl, new pix_icon('t/edit', get_string('edittaskschedule', 'tool_task', $task->get_name())));
$configureurl = new moodle_url('/admin/tool/task/scheduledtasks.php',
['action' => 'edit', 'task' => $classname]);
$editlink = $this->output->action_icon($configureurl, new pix_icon('t/edit',
get_string('edittaskschedule', 'tool_task', $task->get_name())));
} else {
$editlink = $this->render(new pix_icon('t/locked', get_string('scheduledtaskchangesdisabled', 'tool_task')));
$editlink = $this->render(new pix_icon('t/locked',
get_string('scheduledtaskchangesdisabled', 'tool_task')));
}
$loglink = '';
if ($showloglink) {
$loglink = $this->action_icon(
\core\task\logmanager::get_url_for_task_class(get_class($task)),
$loglink = $this->output->action_icon(
\core\task\logmanager::get_url_for_task_class($classname),
new pix_icon('e/file-text', get_string('viewlogs', 'tool_task', $task->get_name())
));
}
$namecell = new html_table_cell($task->get_name() . "\n" . html_writer::tag('span', '\\'.get_class($task),
array('class' => 'task-class text-ltr')));
$namecell = new html_table_cell($task->get_name() . "\n" .
html_writer::span('\\' . $classname, 'task-class text-ltr'));
$namecell->header = true;
$component = $task->get_component();
$plugininfo = null;
list($type, $plugin) = core_component::normalize_component($component);
list($type) = core_component::normalize_component($component);
if ($type === 'core') {
$componentcell = new html_table_cell(get_string('corecomponent', 'tool_task'));
} else {
if ($plugininfo = core_plugin_manager::instance()->get_plugin_info($component)) {
$plugininfo->init_display_name();
$componentcell = new html_table_cell($plugininfo->displayname);
if (!$plugininfo->is_enabled()) {
$componentcell->text .= ' ' . html_writer::span(
get_string('disabled', 'tool_task'), 'badge badge-secondary');
}
$componentcell->text .= "\n" . html_writer::span($plugininfo->component, 'task-class text-ltr');
} else {
$componentcell = new html_table_cell($component);
}
@@ -127,59 +139,100 @@ class tool_task_renderer extends plugin_renderer_base {
}
$runnow = '';
if ( ! $disabled && get_config('tool_task', 'enablerunnow') && $runnabletasks ) {
if (!$disabled && get_config('tool_task', 'enablerunnow') && $runnabletasks ) {
$runnow = html_writer::div(html_writer::link(
new moodle_url('/admin/tool/task/schedule_task.php',
array('task' => get_class($task))),
['task' => $classname]),
get_string('runnow', 'tool_task')), 'task-runnow');
}
$clearfail = '';
$faildelaycell = new html_table_cell($task->get_fail_delay());
if ($task->get_fail_delay()) {
$clearfail = html_writer::div(html_writer::link(
$faildelaycell->text .= html_writer::div(html_writer::link(
new moodle_url('/admin/tool/task/clear_fail_delay.php',
array('task' => get_class($task), 'sesskey' => sesskey())),
['task' => $classname, 'sesskey' => sesskey()]),
get_string('clear')), 'task-clearfaildelay');
$faildelaycell->attributes['class'] = 'table-danger';
}
$row = new html_table_row(array(
$row = new html_table_row([
$namecell,
$componentcell,
new html_table_cell($editlink),
new html_table_cell($loglink),
new html_table_cell($lastrun . $runnow),
new html_table_cell($nextrun),
new html_table_cell($task->get_minute()),
new html_table_cell($task->get_hour()),
new html_table_cell($task->get_day()),
new html_table_cell($task->get_day_of_week()),
new html_table_cell($task->get_month()),
new html_table_cell($task->get_fail_delay() . $clearfail),
new html_table_cell($customised)));
// Cron-style values must always be LTR.
$row->cells[6]->attributes['class'] = 'text-ltr';
$row->cells[7]->attributes['class'] = 'text-ltr';
$row->cells[8]->attributes['class'] = 'text-ltr';
$row->cells[9]->attributes['class'] = 'text-ltr';
$row->cells[10]->attributes['class'] = 'text-ltr';
$this->time_cell($task->get_minute(), $defaulttask->get_minute()),
$this->time_cell($task->get_hour(), $defaulttask->get_hour()),
$this->time_cell($task->get_day(), $defaulttask->get_day()),
$this->time_cell($task->get_day_of_week(), $defaulttask->get_day_of_week()),
$this->time_cell($task->get_month(), $defaulttask->get_month()),
$faildelaycell,
new html_table_cell($customised)]);
$classes = [];
if ($disabled) {
$row->attributes['class'] = 'disabled';
$classes[] = 'disabled';
}
if (get_class($task) == $lastchanged) {
$classes[] = 'table-primary';
}
$row->attributes['class'] = implode(' ', $classes);
$data[] = $row;
}
$table->data = $data;
if ($lastchanged) {
$this->page->requires->js_init_code(
'document.querySelector("tr.table-primary").scrollIntoView({block: "center"});');
}
return html_writer::table($table);
}
/**
* Get a table cell to show one time, comparing it to the default.
*
* @param string $current the current setting.
* @param string $default the default setting from the db/tasks.php file.
* @return html_table_cell for use in the table.
*/
protected function time_cell(string $current, string $default): html_table_cell {
$cell = new html_table_cell($current);
// Cron-style values must always be LTR.
$cell->attributes['class'] = 'text-ltr';
// If the current value is default, that is all we want to do.
if ($default === '*') {
if ($current === '*') {
return $cell;
}
} else if ($default === 'R' ) {
if (is_numeric($current)) {
return $cell;
}
} else {
if ($default === $current) {
return $cell;
}
}
// Otherwise, highlight and show the default.
$cell->attributes['class'] .= ' table-warning';
$cell->text .= ' ' . html_writer::span(
get_string('defaultx', 'tool_task', $default), 'task-class');
return $cell;
}
/**
* Renders a link back to the scheduled tasks page (used from the 'run now' screen).
*
* @param string $taskclassname if specified, the list of tasks will scroll to show this task.
* @return string HTML code
*/
public function link_back() {
return $this->render_from_template('tool_task/link_back',
array('url' => new moodle_url('/admin/tool/task/scheduledtasks.php')));
public function link_back($taskclassname = '') {
$url = new moodle_url('/admin/tool/task/scheduledtasks.php');
if ($taskclassname) {
$url->param('lastchanged', $taskclassname);
}
return $this->render_from_template('tool_task/link_back', ['url' => $url]);
}
}
+4 -3
View File
@@ -71,9 +71,10 @@ echo $OUTPUT->heading($task->get_name());
if (!optional_param('confirm', 0, PARAM_INT)) {
echo $OUTPUT->confirm(get_string('runnow_confirm', 'tool_task', $task->get_name()),
new single_button(new moodle_url('/admin/tool/task/schedule_task.php',
array('task' => $taskname, 'confirm' => 1, 'sesskey' => sesskey())),
['task' => $taskname, 'confirm' => 1, 'sesskey' => sesskey()]),
get_string('runnow', 'tool_task')),
new single_button(new moodle_url('/admin/tool/task/scheduledtasks.php'),
new single_button(new moodle_url('/admin/tool/task/scheduledtasks.php',
['lastchanged' => get_class($task)]),
get_string('cancel'), false));
echo $OUTPUT->footer();
exit;
@@ -97,6 +98,6 @@ $output = $PAGE->get_renderer('tool_task');
echo $OUTPUT->single_button(new moodle_url('/admin/tool/task/schedule_task.php',
array('task' => $taskname, 'confirm' => 1, 'sesskey' => sesskey())),
get_string('runagain', 'tool_task'));
echo $output->link_back();
echo $output->link_back(get_class($task));
echo $OUTPUT->footer();
+9 -16
View File
@@ -26,19 +26,12 @@ require_once(__DIR__ . '/../../../config.php');
require_once($CFG->libdir.'/adminlib.php');
require_once($CFG->libdir.'/tablelib.php');
$PAGE->set_url('/admin/tool/task/scheduledtasks.php');
$PAGE->set_context(context_system::instance());
$PAGE->set_pagelayout('admin');
$strheading = get_string('scheduledtasks', 'tool_task');
$PAGE->set_title($strheading);
$PAGE->set_heading($strheading);
require_admin();
$renderer = $PAGE->get_renderer('tool_task');
admin_externalpage_setup('scheduledtasks');
$action = optional_param('action', '', PARAM_ALPHAEXT);
$taskname = optional_param('task', '', PARAM_RAW);
$lastchanged = optional_param('lastchanged', '', PARAM_RAW);
$task = null;
$mform = null;
@@ -55,15 +48,14 @@ if ($action == 'edit') {
if ($task) {
$mform = new tool_task_edit_scheduled_task_form(null, $task);
$nexturl = new moodle_url($PAGE->url, ['lastchanged' => $taskname]);
}
if ($mform && ($mform->is_cancelled() || !empty($CFG->preventscheduledtaskchanges))) {
redirect(new moodle_url('/admin/tool/task/scheduledtasks.php'));
redirect($nexturl);
} else if ($action == 'edit' && empty($CFG->preventscheduledtaskchanges)) {
if ($data = $mform->get_data()) {
if ($data->resettodefaults) {
$defaulttask = \core\task\manager::get_default_scheduled_task($taskname);
$task->set_minute($defaulttask->get_minute());
@@ -85,9 +77,9 @@ if ($mform && ($mform->is_cancelled() || !empty($CFG->preventscheduledtaskchange
try {
\core\task\manager::configure_scheduled_task($task);
redirect($PAGE->url, get_string('changessaved'), null, \core\output\notification::NOTIFY_SUCCESS);
redirect($nexturl, get_string('changessaved'), null, \core\output\notification::NOTIFY_SUCCESS);
} catch (Exception $e) {
redirect($PAGE->url, $e->getMessage(), null, \core\output\notification::NOTIFY_ERROR);
redirect($nexturl, $e->getMessage(), null, \core\output\notification::NOTIFY_ERROR);
}
} else {
echo $OUTPUT->header();
@@ -97,8 +89,9 @@ if ($mform && ($mform->is_cancelled() || !empty($CFG->preventscheduledtaskchange
}
} else {
$renderer = $PAGE->get_renderer('tool_task');
echo $OUTPUT->header();
$tasks = core\task\manager::get_all_scheduled_tasks();
echo $renderer->scheduled_tasks_table($tasks);
echo $renderer->scheduled_tasks_table($tasks, $lastchanged);
echo $OUTPUT->footer();
}
@@ -9,6 +9,11 @@ Feature: Clear scheduled task fail delay
And I log in as "admin"
And I navigate to "Server > Tasks > Scheduled tasks" in site administration
Scenario: Any fail delay is highlighted
Then I should see "60" in the "Send new user passwords" "table_row"
And I should see "Clear" in the "Send new user passwords" "table_row"
And I should see "60" in the "td.table-danger" "css_element"
Scenario: Clear fail delay
When I click on "Clear" "text" in the "Send new user passwords" "table_row"
And I should see "Are you sure you want to clear the fail delay"
@@ -16,6 +21,8 @@ Feature: Clear scheduled task fail delay
Then I should not see "60" in the "Send new user passwords" "table_row"
And I should not see "Clear" in the "Send new user passwords" "table_row"
And I should see "Send new user passwords" in the "tr.table-primary" "css_element"
Scenario: Cancel clearing the fail delay
When I click on "Clear" "text" in the "Send new user passwords" "table_row"
@@ -23,3 +30,4 @@ Feature: Clear scheduled task fail delay
Then I should see "60" in the "Send new user passwords" "table_row"
And I should see "Clear" in the "Send new user passwords" "table_row"
And I should see "Send new user passwords" in the "tr.table-primary" "css_element"
@@ -16,6 +16,7 @@ Feature: Manage scheduled tasks
And I press "Save changes"
Then I should see "Changes saved"
And I should see "Task disabled" in the "Log table cleanup" "table_row"
And I should see "Log table cleanup" in the "tr.table-primary" "css_element"
Scenario: Enable scheduled task
When I click on "Edit task schedule: Log table cleanup" "link" in the "Log table cleanup" "table_row"
@@ -25,6 +26,7 @@ Feature: Manage scheduled tasks
And I press "Save changes"
Then I should see "Changes saved"
And I should not see "Task disabled" in the "Log table cleanup" "table_row"
And I should see "Log table cleanup" in the "tr.table-primary" "css_element"
Scenario: Edit scheduled task
When I click on "Edit task schedule: Log table cleanup" "link" in the "Log table cleanup" "table_row"
@@ -38,8 +40,10 @@ Feature: Manage scheduled tasks
And I press "Save changes"
Then I should see "Changes saved"
And the following should exist in the "admintable" table:
| Component | Minute | Hour | Day | Day of week | Month |
| Standard log | */5 | 1 | 2 | 4 | 3 |
| Component | Minute | Hour | Day | Day of week | Month |
| Standard log logstore_standard | */5 Default: R | 1 Default: 4 | 2 Default: * | 4 Default: * | 3 Default: * |
And I should see "Log table cleanup" in the "tr.table-primary" "css_element"
And I should see "*/5 Default: R" in the "td.table-warning" "css_element"
Scenario: Reset scheduled task to default
When I click on "Edit task schedule: Log table cleanup" "link" in the "Log table cleanup" "table_row"
@@ -50,4 +54,5 @@ Feature: Manage scheduled tasks
Then I should see "Changes saved"
And the following should not exist in the "admintable" table:
| Name | Component | Minute | Hour | Day | Day of week | Month |
| Log table cleanup | Standard log | */5 | 1 | 2 | 4 | 3 |
| Log table cleanup | Standard log | */5 | 1 | 2 | 4 | 3 |
And I should see "Log table cleanup" in the "tr.table-primary" "css_element"
@@ -24,7 +24,7 @@ Feature: Add a bookmarks to an admin pages
And I navigate to "Notifications" in site administration
And I click on "Scheduled tasks" "link" in the "Admin bookmarks" "block"
# Verify that we are on the right page.
Then I should see "Scheduled tasks" in the "h1" "css_element"
Then I should see "Day of week" in the "admintable" "table"
Scenario: Admin page can be removed from bookmarks
Given I log in as "admin"
+17 -10
View File
@@ -40,9 +40,11 @@ class manager {
* Given a component name, will load the list of tasks in the db/tasks.php file for that component.
*
* @param string $componentname - The name of the component to fetch the tasks for.
* @param bool $expandr - if true (default) an 'R' value in a time is expanded to an appropriate int.
* If false, they are left as 'R'
* @return \core\task\scheduled_task[] - List of scheduled tasks for this component.
*/
public static function load_default_scheduled_tasks_for_component($componentname) {
public static function load_default_scheduled_tasks_for_component($componentname, $expandr = true) {
$dir = \core_component::get_component_directory($componentname);
if (!$dir) {
@@ -65,7 +67,7 @@ class manager {
foreach ($tasks as $task) {
$record = (object) $task;
$scheduledtask = self::scheduled_task_from_record($record);
$scheduledtask = self::scheduled_task_from_record($record, $expandr);
// Safety check in case the task in the DB does not match a real class (maybe something was uninstalled).
if ($scheduledtask) {
$scheduledtask->set_component($componentname);
@@ -318,9 +320,11 @@ class manager {
* Utility method to create a task from a DB record.
*
* @param \stdClass $record
* @return \core\task\scheduled_task
* @param bool $expandr - if true (default) an 'R' value in a time is expanded to an appropriate int.
* If false, they are left as 'R'
* @return \core\task\scheduled_task|false
*/
public static function scheduled_task_from_record($record) {
public static function scheduled_task_from_record($record, $expandr = true) {
$classname = self::get_canonical_class_name($record->classname);
if (!class_exists($classname)) {
debugging("Failed to load task: " . $classname, DEBUG_DEVELOPER);
@@ -342,10 +346,10 @@ class manager {
}
$task->set_blocking(!empty($record->blocking));
if (isset($record->minute)) {
$task->set_minute($record->minute);
$task->set_minute($record->minute, $expandr);
}
if (isset($record->hour)) {
$task->set_hour($record->hour);
$task->set_hour($record->hour, $expandr);
}
if (isset($record->day)) {
$task->set_day($record->day);
@@ -354,7 +358,7 @@ class manager {
$task->set_month($record->month);
}
if (isset($record->dayofweek)) {
$task->set_day_of_week($record->dayofweek);
$task->set_day_of_week($record->dayofweek, $expandr);
}
if (isset($record->faildelay)) {
$task->set_fail_delay($record->faildelay);
@@ -429,15 +433,18 @@ class manager {
* This function load the default scheduled task details for a given classname.
*
* @param string $classname
* @return \core\task\scheduled_task or false
* @param bool $expandr - if true (default) an 'R' value in a time is expanded to an appropriate int.
* If false, they are left as 'R'
* @return \core\task\scheduled_task|false
*/
public static function get_default_scheduled_task($classname) {
public static function get_default_scheduled_task($classname, $expandr = true) {
$task = self::get_scheduled_task($classname);
$componenttasks = array();
// Safety check in case no task was found for the given classname.
if ($task) {
$componenttasks = self::load_default_scheduled_tasks_for_component($task->get_component());
$componenttasks = self::load_default_scheduled_tasks_for_component(
$task->get_component(), $expandr);
}
foreach ($componenttasks as $componenttask) {
+12 -6
View File
@@ -106,9 +106,11 @@ abstract class scheduled_task extends task_base {
* Setter for $minute. Accepts a special 'R' value
* which will be translated to a random minute.
* @param string $minute
* @param bool $expandr - if true (default) an 'R' value in a time is expanded to an appropriate int.
* If false, they are left as 'R'
*/
public function set_minute($minute) {
if ($minute === 'R') {
public function set_minute($minute, $expandr = true) {
if ($minute === 'R' && $expandr) {
$minute = mt_rand(self::HOURMIN, self::HOURMAX);
}
$this->minute = $minute;
@@ -126,9 +128,11 @@ abstract class scheduled_task extends task_base {
* Setter for $hour. Accepts a special 'R' value
* which will be translated to a random hour.
* @param string $hour
* @param bool $expandr - if true (default) an 'R' value in a time is expanded to an appropriate int.
* If false, they are left as 'R'
*/
public function set_hour($hour) {
if ($hour === 'R') {
public function set_hour($hour, $expandr = true) {
if ($hour === 'R' && $expandr) {
$hour = mt_rand(self::HOURMIN, self::HOURMAX);
}
$this->hour = $hour;
@@ -177,9 +181,11 @@ abstract class scheduled_task extends task_base {
/**
* Setter for $dayofweek.
* @param string $dayofweek
* @param bool $expandr - if true (default) an 'R' value in a time is expanded to an appropriate int.
* If false, they are left as 'R'
*/
public function set_day_of_week($dayofweek) {
if ($dayofweek === 'R') {
public function set_day_of_week($dayofweek, $expandr = true) {
if ($dayofweek === 'R' && $expandr) {
$dayofweek = mt_rand(self::DAYOFWEEKMIN, self::DAYOFWEEKMAX);
}
$this->dayofweek = $dayofweek;