Skip to content

Commit

Permalink
MDL-68256 task admin: usability improvements
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@timhunt
timhunt committed Mar 28, 2020
1 parent d939d6e commit 6fdc0f8
Show file tree
Hide file tree
Showing 12 changed files with 158 additions and 79 deletions.
3 changes: 3 additions & 0 deletions admin/tasklogs.php
View file
Expand Up @@ -62,6 +62,8 @@
$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,
Expand All @@ -84,6 +86,7 @@
],
]);

// Output any matching logs.
$table = new \core_admin\task_log_table($filter, $result);
$table->baseurl = $pageurl;
$table->out(100, false);
Expand Down
2 changes: 1 addition & 1 deletion admin/templates/tasklogs.mustache
View file
Expand Up @@ -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>
Expand Down
10 changes: 6 additions & 4 deletions admin/tool/task/clear_fail_delay.php
View file
Expand Up @@ -39,13 +39,16 @@
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.
Expand All @@ -60,9 +63,8 @@
// 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 change: 1 addition & 0 deletions admin/tool/task/lang/en/tool_task.php
View file
Expand Up @@ -30,6 +30,7 @@
$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}';
Expand Down
123 changes: 88 additions & 35 deletions admin/tool/task/renderer.php
View file
Expand Up @@ -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();
Expand Down Expand Up @@ -68,7 +69,7 @@ public function scheduled_tasks_table($tasks) {
$table->colclasses['3'] = 'hidden';
}

$data = array();
$data = [];
$yes = get_string('yes');
$no = get_string('no');
$never = get_string('never');
Expand All @@ -77,35 +78,46 @@ public function scheduled_tasks_table($tasks) {
$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);
}
Expand All @@ -127,59 +139,100 @@ public function scheduled_tasks_table($tasks) {
}

$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]);
}
}
7 changes: 4 additions & 3 deletions admin/tool/task/schedule_task.php
View file
Expand Up @@ -71,9 +71,10 @@ function tool_task_mtrace_wrapper($message, $eol) {
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;
Expand All @@ -97,6 +98,6 @@ function tool_task_mtrace_wrapper($message, $eol) {
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();
25 changes: 9 additions & 16 deletions admin/tool/task/scheduledtasks.php
View file
Expand Up @@ -26,19 +26,12 @@
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;

Expand All @@ -55,15 +48,14 @@

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());
Expand All @@ -85,9 +77,9 @@

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();
Expand All @@ -97,8 +89,9 @@
}

} 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();
}
8 changes: 8 additions & 0 deletions admin/tool/task/tests/behat/clear_fail_delay.feature
View file
Expand Up @@ -9,17 +9,25 @@ 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"
And I press "Clear"

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"
And I press "Cancel"

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"

0 comments on commit 6fdc0f8

Please sign in to comment.