From 473ac1285e5ad0d5e1c27b44cccbefc77fb6ae0b Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 1 Feb 2022 11:07:54 +0000 Subject: [PATCH 1/2] MDL-73726 dataformat: obey sort order when returning enabled plugins. --- lib/classes/plugininfo/dataformat.php | 37 ++++++---- lib/tests/plugininfo/dataformat_test.php | 94 ++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 13 deletions(-) create mode 100644 lib/tests/plugininfo/dataformat_test.php diff --git a/lib/classes/plugininfo/dataformat.php b/lib/classes/plugininfo/dataformat.php index 1a30daf141e..4ac86909b93 100644 --- a/lib/classes/plugininfo/dataformat.php +++ b/lib/classes/plugininfo/dataformat.php @@ -47,6 +47,25 @@ class dataformat extends base { } } + /** + * Given a list of dataformat types, return them sorted according to site configuration (if set) + * + * @param string[] $formats List of formats, ['csv', 'pdf', etc] + * @return string[] List of formats according to configured sort, ['csv', 'odf', etc] + */ + private static function get_plugins_sortorder(array $formats): array { + global $CFG; + + if (!empty($CFG->dataformat_plugins_sortorder)) { + $order = explode(',', $CFG->dataformat_plugins_sortorder); + $order = array_merge(array_intersect($order, $formats), array_diff($formats, $order)); + } else { + $order = $formats; + } + + return $order; + } + /** * Gathers and returns the information about all plugins of the given type * @@ -57,16 +76,9 @@ class dataformat extends base { * @return array of plugintype classes, indexed by the plugin name */ public static function get_plugins($type, $typerootdir, $typeclass, $pluginman) { - global $CFG; $formats = parent::get_plugins($type, $typerootdir, $typeclass, $pluginman); - if (!empty($CFG->dataformat_plugins_sortorder)) { - $order = explode(',', $CFG->dataformat_plugins_sortorder); - $order = array_merge(array_intersect($order, array_keys($formats)), - array_diff(array_keys($formats), $order)); - } else { - $order = array_keys($formats); - } + $order = static::get_plugins_sortorder(array_keys($formats)); $sortedformats = array(); foreach ($order as $formatname) { $sortedformats[$formatname] = $formats[$formatname]; @@ -79,18 +91,17 @@ class dataformat extends base { * @return array|null of enabled plugins $pluginname=>$pluginname, null means unknown */ public static function get_enabled_plugins() { - $enabled = array(); $plugins = core_plugin_manager::instance()->get_installed_plugins('dataformat'); - if (!$plugins) { return array(); } + $order = static::get_plugins_sortorder(array_keys($plugins)); $enabled = array(); - foreach ($plugins as $plugin => $version) { - $disabled = get_config('dataformat_' . $plugin, 'disabled'); + foreach ($order as $formatname) { + $disabled = get_config('dataformat_' . $formatname, 'disabled'); if (empty($disabled)) { - $enabled[$plugin] = $plugin; + $enabled[$formatname] = $formatname; } } return $enabled; diff --git a/lib/tests/plugininfo/dataformat_test.php b/lib/tests/plugininfo/dataformat_test.php new file mode 100644 index 00000000000..d8dd2965d60 --- /dev/null +++ b/lib/tests/plugininfo/dataformat_test.php @@ -0,0 +1,94 @@ +. + +declare(strict_types=1); + +namespace core\plugininfo; + +use advanced_testcase; + +/** + * Unit tests for the dataformat plugininfo class + * + * @package core + * @covers \core\plugininfo\dataformat + * @copyright 2022 Paul Holden + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class dataformat_test extends advanced_testcase { + + /** + * Helper method, to allow easy filtering of default formats in order to perform assertions without any third-party + * formats affecting expected results + * + * @param string $format + * @return bool + */ + private function filter_default_plugins(string $format): bool { + $defaultformats = ['csv', 'excel', 'html', 'json', 'ods', 'pdf']; + + return in_array($format, $defaultformats); + } + + /** + * Test getting enabled plugins + */ + public function test_get_enabled_plugins(): void { + $this->resetAfterTest(); + + // Check all default formats. + $plugins = array_filter(dataformat::get_enabled_plugins(), [$this, 'filter_default_plugins']); + $this->assertEquals([ + 'csv' => 'csv', + 'excel' => 'excel', + 'html' => 'html', + 'json' => 'json', + 'ods' => 'ods', + 'pdf' => 'pdf', + ], $plugins); + + // Disable excel & html. + dataformat::enable_plugin('excel', 0); + dataformat::enable_plugin('html', 0); + + $plugins = array_filter(dataformat::get_enabled_plugins(), [$this, 'filter_default_plugins']); + $this->assertEquals([ + 'csv' => 'csv', + 'json' => 'json', + 'ods' => 'ods', + 'pdf' => 'pdf', + ], $plugins); + } + + /** + * Test getting enabled plugins obeys configured sortorder + */ + public function test_get_enabled_plugins_sorted(): void { + $this->resetAfterTest(); + + set_config('dataformat_plugins_sortorder', 'csv,pdf,excel,json,html,ods'); + + $plugins = array_filter(dataformat::get_enabled_plugins(), [$this, 'filter_default_plugins']); + $this->assertEquals([ + 'csv' => 'csv', + 'pdf' => 'pdf', + 'excel' => 'excel', + 'json' => 'json', + 'html' => 'html', + 'ods' => 'ods', + ], $plugins); + } +} From 72286f9296294dc2e8d14bdcf6e489ab5e61dbdb Mon Sep 17 00:00:00 2001 From: Paul Holden Date: Tue, 1 Feb 2022 12:00:32 +0000 Subject: [PATCH 2/2] MDL-73726 reportbuilder: restrict schedule formats to enabled types. --- reportbuilder/classes/local/helpers/schedule.php | 6 +++--- .../classes/local/systemreports/report_schedules.php | 8 +++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/reportbuilder/classes/local/helpers/schedule.php b/reportbuilder/classes/local/helpers/schedule.php index 9dcfd65c853..c103f502ce1 100644 --- a/reportbuilder/classes/local/helpers/schedule.php +++ b/reportbuilder/classes/local/helpers/schedule.php @@ -339,10 +339,10 @@ class schedule { * @return string[] */ public static function get_format_options(): array { - $dataformats = core_plugin_manager::instance()->get_plugins_of_type('dataformat'); + $dataformats = dataformat::get_enabled_plugins(); - return array_map(static function(dataformat $dataformat): string { - return $dataformat->displayname; + return array_map(static function(string $pluginname): string { + return get_string('dataformat', 'dataformat_' . $pluginname); }, $dataformats); } diff --git a/reportbuilder/classes/local/systemreports/report_schedules.php b/reportbuilder/classes/local/systemreports/report_schedules.php index 071850c7f62..c42dce210cf 100644 --- a/reportbuilder/classes/local/systemreports/report_schedules.php +++ b/reportbuilder/classes/local/systemreports/report_schedules.php @@ -29,7 +29,6 @@ use core_reportbuilder\local\entities\user; use core_reportbuilder\local\filters\date; use core_reportbuilder\local\filters\text; use core_reportbuilder\local\helpers\format; -use core_reportbuilder\local\helpers\schedule as helper; use core_reportbuilder\local\models\report; use core_reportbuilder\local\models\schedule; use core_reportbuilder\local\report\action; @@ -198,8 +197,11 @@ class report_schedules extends system_report { ->add_fields("{$tablealias}.format") ->set_is_sortable(true) ->add_callback(static function(string $format): string { - $formats = helper::get_format_options(); - return $formats[$format] ?? ''; + if (get_string_manager()->string_exists('dataformat', 'dataformat_' . $format)) { + return get_string('dataformat', 'dataformat_' . $format); + } else { + return $format; + } }) );