diff --git a/admin/settings/server.php b/admin/settings/server.php index a77d0343951..fb1f11f593b 100644 --- a/admin/settings/server.php +++ b/admin/settings/server.php @@ -516,8 +516,12 @@ if ($hassiteconfig) { $temp->add(new admin_setting_heading('noreplydomainheading', new lang_string('noreplydomain', 'admin'), new lang_string('noreplydomaindetail', 'admin'))); + $default = clean_param('noreply@' . get_host_from_url($CFG->wwwroot), PARAM_EMAIL); + if (!$default) { + $default = null; + } $temp->add(new admin_setting_configtext('noreplyaddress', new lang_string('noreplyaddress', 'admin'), - new lang_string('confignoreplyaddress', 'admin'), 'noreply@' . get_host_from_url($CFG->wwwroot), PARAM_EMAIL)); + new lang_string('confignoreplyaddress', 'admin'), $default, PARAM_EMAIL)); $temp->add(new admin_setting_configtextarea('allowedemaildomains', new lang_string('allowedemaildomains', 'admin'), diff --git a/lib/adminlib.php b/lib/adminlib.php index adad417fe77..ad81ea2664a 100644 --- a/lib/adminlib.php +++ b/lib/adminlib.php @@ -8791,39 +8791,65 @@ function admin_get_root($reload=false, $requirefulltree=true) { /// settings utility functions /** - * This function applies default settings. - * Because setting the defaults of some settings can enable other settings, - * this function is called recursively until no more new settings are found. + * This function applies default settings recursively. * - * @param object $node, NULL means complete tree, null by default - * @param bool $unconditional if true overrides all values with defaults, true by default - * @param array $admindefaultsettings default admin settings to apply. Used recursively - * @param array $settingsoutput The names and values of the changed settings. Used recursively - * @return array $settingsoutput The names and values of the changed settings + * Because setting the defaults of some settings can enable other settings, + * this function calls itself repeatedly (max 4 times) until no more new settings are saved. + * + * NOTE: previous "internal" parameters $admindefaultsettings, $settingsoutput were removed in Moodle 4.3. + * + * @param part_of_admin_tree|null $node NULL means apply all settings with repeated recursion + * @param bool $unconditional if true overrides all values with defaults (true for installation, false for CLI upgrade) + * @return array The names and values of the applied setting defaults */ -function admin_apply_default_settings($node=null, $unconditional=true, $admindefaultsettings=array(), $settingsoutput=array()) { - $counter = 0; - - // This function relies heavily on config cache, so we need to enable in-memory caches if it - // is used during install when normal caching is disabled. - $token = new \core_cache\allow_temporary_caches(); - +function admin_apply_default_settings(?part_of_admin_tree $node = null, bool $unconditional = true): array { if (is_null($node)) { + // This function relies heavily on config cache, so we need to enable in-memory caches if it + // is used during install when normal caching is disabled. + $token = new \core_cache\allow_temporary_caches(); // Value not used intentionally, see its destructor. + core_plugin_manager::reset_caches(); - $node = admin_get_root(true, true); - $counter = count($settingsoutput); + $root = admin_get_root(true, true); + $saved = admin_apply_default_settings($root, $unconditional); + if (!$saved) { + return []; + } + + for ($i = 1; $i <= 3; $i++) { + core_plugin_manager::reset_caches(); + $root = admin_get_root(true, true); + // No need to force defaults in repeated runs. + $moresaved = admin_apply_default_settings($root, false); + if (!$moresaved) { + // No more setting defaults to save. + return $saved; + } + $saved += $moresaved; + } + + // We should not get here unless there are some problematic settings.php files. + core_plugin_manager::reset_caches(); + return $saved; } + // Recursive applying of defaults in admin tree. + $saved = []; if ($node instanceof admin_category) { - $entries = array_keys($node->children); - foreach ($entries as $entry) { - $settingsoutput = admin_apply_default_settings( - $node->children[$entry], $unconditional, $admindefaultsettings, $settingsoutput - ); + foreach ($node->children as $child) { + if ($child === null) { + // This should not happen, + // this is to prevent theoretical infinite loops. + continue; + } + if ($child instanceof admin_externalpage) { + continue; + } + $saved += admin_apply_default_settings($child, $unconditional); } } else if ($node instanceof admin_settingpage) { - foreach ($node->settings as $setting) { + /** @var admin_setting $setting */ + foreach ((array)$node->settings as $setting) { if ($setting->nosave) { // Not a real setting, must be a heading or description. continue; @@ -8837,30 +8863,28 @@ function admin_apply_default_settings($node=null, $unconditional=true, $admindef // No value yet - default maybe applied after admin user creation or in upgradesettings. continue; } - - $settingname = $node->name . '_' . $setting->name; // Get a unique name for the setting. - - if (!array_key_exists($settingname, $admindefaultsettings)) { // Only update a setting if not already processed. - $admindefaultsettings[$settingname] = $settingname; - $settingsoutput[$settingname] = $defaultsetting; - - // Set the default for this setting. - $setting->write_setting($defaultsetting); - $setting->write_setting_flags(null); + // This should be unique-enough setting name that matches administration UI. + if ($setting->plugin === null) { + $settingname = $setting->name; } else { - unset($admindefaultsettings[$settingname]); // Remove processed settings. + $settingname = $setting->plugin . '/' . $setting->name; + } + // Set the default for this setting. + $error = $setting->write_setting($defaultsetting); + if ($error === '') { + $setting->write_setting_flags(null); + if (is_int($defaultsetting) || $defaultsetting instanceof lang_string + || $defaultsetting instanceof moodle_url) { + $defaultsetting = (string)$defaultsetting; + } + $saved[$settingname] = $defaultsetting; + } else { + debugging("Error applying default setting '$settingname': " . $error, DEBUG_DEVELOPER); } } } - // Call this function recursively until all settings are processed. - if (($node instanceof admin_root) && ($counter != count($settingsoutput))) { - $settingsoutput = admin_apply_default_settings(null, $unconditional, $admindefaultsettings, $settingsoutput); - } - // Just in case somebody modifies the list of active plugins directly. - core_plugin_manager::reset_caches(); - - return $settingsoutput; + return $saved; } /** @@ -10557,7 +10581,7 @@ class admin_setting_configstoredfile extends admin_setting { // Let's not deal with validation here, this is for admins only. $current = $this->get_setting(); - if (empty($data) && $current === null) { + if (empty($data) && ($current === null || $current === '')) { // This will be the case when applying default settings (installation). return ($this->config_write($this->name, '') ? '' : get_string('errorsetting', 'admin')); } else if (!is_number($data)) { diff --git a/lib/tests/adminlib_test.php b/lib/tests/adminlib_test.php index cd60aced38c..5800dd35161 100644 --- a/lib/tests/adminlib_test.php +++ b/lib/tests/adminlib_test.php @@ -126,4 +126,83 @@ class adminlib_test extends \advanced_testcase { $actual = db_should_replace($table, $column, $additionalskiptables); $this->assertSame($actual, $expected); } + + /** + * Test method used by upgradesettings.php to make sure + * there are no missing settings in PHPUnit and Behat tests. + * + * @covers ::admin_output_new_settings_by_page + */ + public function test_admin_output_new_settings_by_page() { + $this->resetAfterTest(); + $this->setAdminUser(); + + // Add settings not set during PHPUnit init. + set_config('supportemail', 'support@example.com'); + $frontpage = new \admin_setting_special_frontpagedesc(); + $frontpage->write_setting('test test'); + + // NOTE: if this test fails then it is most likely extra setting in + // some additional plugin without default - developer needs to add + // a workaround into their db/install.php for PHPUnit and Behat. + + $root = admin_get_root(true, true); + $new = admin_output_new_settings_by_page($root); + $this->assertSame([], $new); + + unset_config('numbering', 'book'); + unset_config('supportemail'); + $root = admin_get_root(true, true); + $new = admin_output_new_settings_by_page($root); + $this->assertCount(2, $new); + } + + /** + * Test repeated recursive application of default settings. + * + * @covers ::admin_apply_default_settings + */ + public function test_admin_apply_default_settings() { + global $DB; + + $this->resetAfterTest(); + $this->setAdminUser(); + + // There should not be any pending new defaults. + $saved = admin_apply_default_settings(null, false); + $this->assertSame([], $saved); + + // Emulation of upgrades from CLI. + unset_config('logocompact', 'core_admin'); + unset_config('grade_aggregationposition'); + unset_config('numbering', 'book'); + unset_config('enabled', 'core_competency'); + unset_config('pushcourseratingstouserplans', 'core_competency'); + $saved = admin_apply_default_settings(null, false); + $expected = [ + 'core_competency/enabled' => '1', + 'grade_aggregationposition' => '1', + 'book/numbering' => '1', + 'core_admin/logocompact' => '', + 'core_competency/pushcourseratingstouserplans' => '1', + ]; + $this->assertEquals($expected, $saved); + + // Repeated application of defaults - not done usually. + $saved = admin_apply_default_settings(null, true); + $this->assertGreaterThan(500, count($saved)); + $saved = admin_apply_default_settings(); + $this->assertGreaterThan(500, count($saved)); + + // Emulate initial application of defaults. + $DB->delete_records('config', []); + $DB->delete_records('config_plugins', []); + purge_all_caches(); + $saved = admin_apply_default_settings(null, true); + $this->assertGreaterThan(500, count($saved)); + + // Make sure there were enough repetitions. + $saved = admin_apply_default_settings(null, false); + $this->assertSame([], $saved); + } } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 2366ca5cb4b..a6bc93bea9c 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -3,6 +3,9 @@ information provided here is intended especially for developers. === 4.3 === +* Unnecessary parameters of admin_apply_default_settings() function were removed; upgrade script lists + setting names in the same format as admin UI; default setting writing errors now trigger debugging messages; + duplicate setting names (with different plugin part) in one setting page do not cause problems any more. * Added a new render of caption for the table in render_caption. It can be used by set_caption($caption, $captionattributes). e.g. $caption = 'Caption for table'