From 2883a678a42803e89122a96be993b0f8cd807ea7 Mon Sep 17 00:00:00 2001 From: Daniel Thee Roperto Date: Thu, 29 Sep 2016 12:15:59 +1000 Subject: [PATCH] Issue #59 - Fixed problem related to config existing in DB but empty. --- classes/form/baseform.php | 2 +- classes/local/outagelib.php | 20 ++++++++++--- docs/screencast.md | 0 tests/phpunit/local/outagelib_test.php | 39 ++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) delete mode 100644 docs/screencast.md diff --git a/classes/form/baseform.php b/classes/form/baseform.php index 68f0ee5..3021d78 100644 --- a/classes/form/baseform.php +++ b/classes/form/baseform.php @@ -23,7 +23,7 @@ defined('MOODLE_INTERNAL') || die(); require_once($CFG->libdir.'/formslib.php'); /** - * Outage base for forms, extends Moodle form to fix a but in the validation method. + * Outage base for forms, extends Moodle form to fix an issue in the validation method. * * @package auth_outage * @author Daniel Thee Roperto diff --git a/classes/local/outagelib.php b/classes/local/outagelib.php index e34903e..e372a8b 100644 --- a/classes/local/outagelib.php +++ b/classes/local/outagelib.php @@ -19,6 +19,7 @@ namespace auth_outage\local; use auth_outage\dml\outagedb; use auth_outage\local\controllers\infopage; use auth_outage\output\renderer; +use coding_exception; use Exception; use stdClass; @@ -96,9 +97,20 @@ class outagelib { * Creates a configuration object ensuring all parameters are set, * loading defaults even if the plugin is not configured. * @return stdClass Configuration object with all parameters set. + * @throws coding_exception */ public static function get_config() { - return (object)array_merge(self::get_config_defaults(), (array)get_config('auth_outage')); + $config = (array)get_config('auth_outage'); + foreach ($config as $k => $v) { + if (!is_string($v)) { + throw new coding_exception('Config is expected to give string.'); + } + if (trim($v) == '') { + unset($config[$k]); + } + } + + return (object)array_merge(self::get_config_defaults(), $config); } /** @@ -109,9 +121,9 @@ class outagelib { global $CFG; return [ - 'default_autostart' => false, - 'default_duration' => (60 * 60), - 'default_warning_duration' => (60 * 60), + 'default_autostart' => '0', + 'default_duration' => (string)(60 * 60), + 'default_warning_duration' => (string)(60 * 60), 'default_title' => get_string('defaulttitlevalue', 'auth_outage'), 'default_description' => get_string('defaultdescriptionvalue', 'auth_outage'), 'css' => file_get_contents($CFG->dirroot.'/auth/outage/views/warningbar/warningbar.css'), diff --git a/docs/screencast.md b/docs/screencast.md deleted file mode 100644 index e69de29..0000000 diff --git a/tests/phpunit/local/outagelib_test.php b/tests/phpunit/local/outagelib_test.php index f6dd57b..ce18f32 100644 --- a/tests/phpunit/local/outagelib_test.php +++ b/tests/phpunit/local/outagelib_test.php @@ -147,5 +147,44 @@ class outagelib_test extends advanced_testcase { public function test_inject_noactive() { outagelib::reinject(); } + + public function test_get_config() { + $this->resetAfterTest(true); + $keys = [ + 'css', + 'default_autostart', + 'default_description', + 'default_duration', + 'default_title', + 'default_warning_duration', + ]; + // Set config with invalid values. + foreach ($keys as $k) { + set_config($k, $k.'_value', 'auth_outage'); + } + // Ensure it is not using any defaults. + $config = outagelib::get_config(); + foreach ($keys as $k) { + self::assertSame($config->$k, $k.'_value', 'auth_outage'); + } + } + + public function test_get_config_invalid() { + $this->resetAfterTest(true); + // Set config with invalid values. + set_config('css', " \n", 'auth_outage'); + set_config('default_autostart', " \n", 'auth_outage'); + set_config('default_description', " \n", 'auth_outage'); + set_config('default_duration', " \n", 'auth_outage'); + set_config('default_title', " \n", 'auth_outage'); + set_config('default_warning_duration', " \n", 'auth_outage'); + // Get defaults. + $defaults = outagelib::get_config_defaults(); + $config = outagelib::get_config(); + // Ensure it is using all defailts. + foreach ($defaults as $k => $v) { + self::assertSame($v, $config->$k); + } + } }