diff --git a/admin/filters.php b/admin/filters.php index a79c2ecf617..3ee94bcec2e 100644 --- a/admin/filters.php +++ b/admin/filters.php @@ -31,14 +31,15 @@ $filterpath = optional_param('filterpath', '', PARAM_PLUGIN); admin_externalpage_setup('managefilters'); // Clean up bogus filter states first. +/** @var core\plugininfo\filter[] $plugininfos */ $plugininfos = core_plugin_manager::instance()->get_plugins_of_type('filter'); -$filters = array(); +$filters = []; $states = filter_get_global_states(); foreach ($states as $state) { if (!isset($plugininfos[$state->filter]) and !get_config('filter_'.$state->filter, 'version')) { - // Purge messy leftovers after incorrectly uninstalled plugins and unfinished installs. - $DB->delete_records('filter_active', array('filter' => $state->filter)); - $DB->delete_records('filter_config', array('filter' => $state->filter)); + // Purge messy leftovers after incorrectly uninstalled plugins and unfinished installations. + $DB->delete_records('filter_active', ['filter' => $state->filter]); + $DB->delete_records('filter_config', ['filter' => $state->filter]); error_log('Deleted bogus "filter_'.$state->filter.'" states and config data.'); } else { $filters[$state->filter] = $state; @@ -50,7 +51,6 @@ foreach ($plugininfos as $filter => $info) { if (isset($filters[$filter])) { continue; } - /** @var \core\plugininfo\base $info */ if ($info->is_installed_and_upgraded()) { filter_set_global_state($filter, TEXTFILTER_DISABLED); $states = filter_get_global_states(); @@ -72,7 +72,8 @@ switch ($action) { case 'setstate': if (isset($filters[$filterpath]) and $newstate = optional_param('newstate', '', PARAM_INT)) { - $class = \core_plugin_manager::resolve_plugininfo_class('filter'); + /** @var \core\plugininfo\filter $class */ + $class = core_plugin_manager::resolve_plugininfo_class('filter'); $class::enable_plugin($filterpath, $newstate); } break; @@ -117,12 +118,12 @@ $states = filter_get_global_states(); $stringfilters = filter_get_string_filters(); $table = new html_table(); -$table->head = array(get_string('filter'), get_string('isactive', 'filters'), - get_string('order'), get_string('applyto', 'filters'), get_string('settings'), get_string('uninstallplugin', 'core_admin')); +$table->head = [get_string('filter'), get_string('isactive', 'filters'), + get_string('order'), get_string('applyto', 'filters'), get_string('settings'), get_string('uninstallplugin', 'core_admin')]; $table->colclasses = array ('leftalign', 'leftalign', 'centeralign', 'leftalign', 'leftalign', 'leftalign'); $table->attributes['class'] = 'admintable generaltable'; $table->id = 'filterssetting'; -$table->data = array(); +$table->data = []; $lastactive = null; foreach ($states as $state) { @@ -159,15 +160,16 @@ die; /** * Return action URL. * - * @param string $filterpath - * @param string $action - * @return moodle_url + * @param string $filterpath which filter to get the URL for. + * @param string $action which action to get the URL for. + * @return moodle_url|null the requested URL. */ -function filters_action_url($filterpath, $action) { +function filters_action_url(string $filterpath, string $action): ?moodle_url { if ($action === 'delete') { return core_plugin_manager::instance()->get_uninstall_url('filter_'.$filterpath, 'manage'); } - return new moodle_url('/admin/filters.php', array('sesskey'=>sesskey(), 'filterpath'=>$filterpath, 'action'=>$action)); + return new moodle_url('/admin/filters.php', + ['sesskey' => sesskey(), 'filterpath' => $filterpath, 'action' => $action]); } /** @@ -180,24 +182,25 @@ function filters_action_url($filterpath, $action) { * @param bool $applytostrings * @return array data */ -function get_table_row(\core\plugininfo\filter $plugininfo, $state, $isfirstrow, $islastactive, $applytostrings) { +function get_table_row(\core\plugininfo\filter $plugininfo, stdClass $state, + bool $isfirstrow, bool $islastactive, bool $applytostrings): array { global $OUTPUT; - $row = array(); + $row = []; $filter = $state->filter; $active = $plugininfo->is_installed_and_upgraded(); static $activechoices; static $applytochoices; if (!isset($activechoices)) { - $activechoices = array( + $activechoices = [ TEXTFILTER_DISABLED => get_string('disabled', 'core_filters'), TEXTFILTER_OFF => get_string('offbutavailable', 'core_filters'), TEXTFILTER_ON => get_string('on', 'core_filters'), - ); - $applytochoices = array( + ]; + $applytochoices = [ 0 => get_string('content', 'core_filters'), 1 => get_string('contentandheadings', 'core_filters'), - ); + ]; } // Filter name. @@ -211,7 +214,7 @@ function get_table_row(\core\plugininfo\filter $plugininfo, $state, $isfirstrow, // Disable/off/on. $select = new single_select(filters_action_url($filter, 'setstate'), 'newstate', $activechoices, $state->active, null, 'active' . $filter); - $select->set_label(get_string('isactive', 'filters'), array('class' => 'accesshide')); + $select->set_label(get_string('isactive', 'filters'), ['class' => 'accesshide']); $row[] = $OUTPUT->render($select); // Re-order. @@ -219,12 +222,14 @@ function get_table_row(\core\plugininfo\filter $plugininfo, $state, $isfirstrow, $spacer = $OUTPUT->spacer(); if ($state->active != TEXTFILTER_DISABLED) { if (!$isfirstrow) { - $updown .= $OUTPUT->action_icon(filters_action_url($filter, 'up'), new pix_icon('t/up', get_string('up'), '', array('class' => 'iconsmall'))); + $updown .= $OUTPUT->action_icon(filters_action_url($filter, 'up'), + new pix_icon('t/up', get_string('up'), '', ['class' => 'iconsmall'])); } else { $updown .= $spacer; } if (!$islastactive) { - $updown .= $OUTPUT->action_icon(filters_action_url($filter, 'down'), new pix_icon('t/down', get_string('down'), '', array('class' => 'iconsmall'))); + $updown .= $OUTPUT->action_icon(filters_action_url($filter, 'down'), + new pix_icon('t/down', get_string('down'), '', ['class' => 'iconsmall'])); } else { $updown .= $spacer; } @@ -232,20 +237,23 @@ function get_table_row(\core\plugininfo\filter $plugininfo, $state, $isfirstrow, $row[] = $updown; // Apply to strings. - $select = new single_select(filters_action_url($filter, 'setapplyto'), 'stringstoo', $applytochoices, $applytostrings, null, 'applyto' . $filter); - $select->set_label(get_string('applyto', 'filters'), array('class' => 'accesshide')); + $select = new single_select(filters_action_url($filter, 'setapplyto'), + 'stringstoo', $applytochoices, $applytostrings, null, 'applyto' . $filter); + $select->set_label(get_string('applyto', 'filters'), ['class' => 'accesshide']); $select->disabled = ($state->active == TEXTFILTER_DISABLED); $row[] = $OUTPUT->render($select); // Settings link, if required. if ($active and filter_has_global_settings($filter)) { - $row[] = html_writer::link(new moodle_url('/admin/settings.php', array('section'=>'filtersetting'.$filter)), get_string('settings')); + $row[] = html_writer::link(new moodle_url('/admin/settings.php', + ['section' => 'filtersetting'.$filter]), get_string('settings')); } else { $row[] = ''; } // Uninstall. - $row[] = html_writer::link(filters_action_url($filter, 'delete'), get_string('uninstallplugin', 'core_admin')); + $row[] = html_writer::link(filters_action_url($filter, 'delete'), + get_string('uninstallplugin', 'core_admin')); return $row; } diff --git a/lib/filterlib.php b/lib/filterlib.php index b3f4162b8b7..ec80be2d720 100644 --- a/lib/filterlib.php +++ b/lib/filterlib.php @@ -664,6 +664,13 @@ function filter_set_global_state($filtername, $state, $move = 0) { // Move only active. if ($move != 0 and isset($on[$filter->filter])) { + // Capture the old order for logging. + $oldorder = implode(', ', array_map( + function($f) { + return $f->filter; + }, $on)); + + // Work out the new order. $i = 1; foreach ($on as $f) { $f->newsortorder = $i; @@ -686,6 +693,13 @@ function filter_set_global_state($filtername, $state, $move = 0) { } core_collator::asort_objects_by_property($on, 'newsortorder', core_collator::SORT_NUMERIC); + + // Log in config_log. + $neworder = implode(', ', array_map( + function($f) { + return $f->filter; + }, $on)); + add_to_config_log('order', $oldorder, $neworder, 'core_filter'); } // Inactive are sorted by filter name. diff --git a/lib/tests/filterlib_test.php b/lib/tests/filterlib_test.php index a896ca7b541..21df7ed847a 100644 --- a/lib/tests/filterlib_test.php +++ b/lib/tests/filterlib_test.php @@ -131,6 +131,8 @@ class core_filterlib_testcase extends advanced_testcase { } public function test_update_reorder_down() { + global $DB; + $this->resetAfterTest(); $this->remove_all_filters_from_config(); // Remove all filters. // Setup fixture. @@ -141,9 +143,19 @@ class core_filterlib_testcase extends advanced_testcase { filter_set_global_state('two', TEXTFILTER_ON, -1); // Validate. $this->assert_global_sort_order(array('two', 'one', 'three')); + + // Check this was logged in config log. + $logs = $DB->get_records('config_log', null, 'id DESC', '*', 0, 1); + $log = reset($logs); + $this->assertEquals('core_filter', $log->plugin); + $this->assertEquals('order', $log->name); + $this->assertEquals('two, one, three', $log->value); + $this->assertEquals('one, two, three', $log->oldvalue); } public function test_update_reorder_up() { + global $DB; + $this->resetAfterTest(); $this->remove_all_filters_from_config(); // Remove all filters. // Setup fixture. @@ -155,6 +167,14 @@ class core_filterlib_testcase extends advanced_testcase { filter_set_global_state('two', TEXTFILTER_ON, 1); // Validate. $this->assert_global_sort_order(array('one', 'three', 'two', 'four')); + + // Check this was logged in config log. + $logs = $DB->get_records('config_log', null, 'id DESC', '*', 0, 1); + $log = reset($logs); + $this->assertEquals('core_filter', $log->plugin); + $this->assertEquals('order', $log->name); + $this->assertEquals('one, three, two, four', $log->value); + $this->assertEquals('one, two, three, four', $log->oldvalue); } public function test_auto_sort_order_change_to_enabled() {