MDL-62777 Administration: CLI upgrade new setting notification
authorMatt Porritt <mattp@catalyst-au.net>
Mon, 22 Oct 2018 02:09:58 +0000 (13:09 +1100)
committerMatt Porritt <mattp@catalyst-au.net>
Mon, 22 Oct 2018 03:28:06 +0000 (14:28 +1100)
During a CLI upgrade when there are new settings in core or in
a plugin, the settings are set to the defined defaults
automatically. There is no ouput shown on the CLI about which
new settings have been introduced or what default values the
setting are set to.

This patch outputs the name of the new setting and what the
default value being is set is to the CLI during an upgrade.
Objects and arrays are expanded into a human readable format.
This plugin also makes the function that sets the defaults to
be more robust so it isno longer required to be called multiple
times to ensure all settings are set.

admin/cli/upgrade.php
lang/en/admin.php
lib/adminlib.php
lib/installlib.php
lib/upgrade.txt
lib/upgradelib.php

index 5915e24..8e439c5 100644 (file)
@@ -51,7 +51,8 @@ list($options, $unrecognized) = cli_get_params(
         'non-interactive'   => false,
         'allow-unstable'    => false,
         'help'              => false,
-        'lang'              => $lang
+        'lang'              => $lang,
+        'verbose-settings'  => false
     ),
     array(
         'h' => 'help'
@@ -84,6 +85,9 @@ Options:
                       site language if not set. Defaults to 'en' if the lang
                       parameter is invalid or if the language pack is not
                       installed.
+--verbose-settings    Show new settings values. By default only the name of
+                      new core or plugin settings are displayed. This option
+                      outputs the new values as well as the setting name.
 -h, --help            Print out this help
 
 Example:
@@ -184,9 +188,24 @@ upgrade_noncore(true);
 // log in as admin - we need doanything permission when applying defaults
 \core\session\manager::set_user(get_admin());
 
-// apply all default settings, just in case do it twice to fill all defaults
-admin_apply_default_settings(NULL, false);
-admin_apply_default_settings(NULL, false);
+// Apply default settings and output those that have changed.
+cli_heading(get_string('cliupgradedefaultheading', 'admin'));
+$settingsoutput = admin_apply_default_settings(null, false);
+
+foreach ($settingsoutput as $setting => $value) {
+
+    if ($options['verbose-settings']) {
+        $stringvlaues = array(
+                'name' => $setting,
+                'defaultsetting' => var_export($value, true) // Expand objects.
+        );
+        echo get_string('cliupgradedefaultverbose', 'admin', $stringvlaues) . PHP_EOL;
+
+    } else {
+        echo get_string('cliupgradedefault', 'admin', $setting) . PHP_EOL;
+
+    }
+}
 
 // This needs to happen at the end to ensure it occurs after all caches
 // have been purged for the last time.
index dd31c0c..5992386 100644 (file)
@@ -128,6 +128,9 @@ $string['clitypevaluedefault'] = 'type value, press Enter to use default value (
 $string['cliunknowoption'] = 'Unrecognised options:
   {$a}
 Please use --help option.';
+$string['cliupgradedefault'] = 'New setting: {$a}';
+$string['cliupgradedefaultheading'] = 'Setting new default values';
+$string['cliupgradedefaultverbose'] = 'New setting: {$a->name}, Default value: {$a->defaultsetting}';
 $string['cliupgradefinished'] = 'Command line upgrade from {$a->oldversion} to {$a->newversion} completed successfully.';
 $string['cliupgradenoneed'] = 'No upgrade needed for the installed version {$a}. Thanks for coming anyway!';
 $string['cliyesnoprompt'] = 'type y (means yes) or n (means no)';
index d6cd9ac..9c53c36 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+
 // This file is part of Moodle - http://moodle.org/
 //
 // Moodle is free software: you can redistribute it and/or modify
@@ -8006,12 +8007,16 @@ function admin_get_root($reload=false, $requirefulltree=true) {
 
 /**
  * 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.
  *
  * @param object $node, NULL means complete tree, null by default
- * @param bool $unconditional if true overrides all values with defaults, null buy 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
  */
-function admin_apply_default_settings($node=NULL, $unconditional=true) {
-    global $CFG;
+function admin_apply_default_settings($node=null, $unconditional=true, $admindefaultsettings=array(), $settingsoutput=array()) {
 
     if (is_null($node)) {
         core_plugin_manager::reset_caches();
@@ -8021,26 +8026,46 @@ function admin_apply_default_settings($node=NULL, $unconditional=true) {
     if ($node instanceof admin_category) {
         $entries = array_keys($node->children);
         foreach ($entries as $entry) {
-            admin_apply_default_settings($node->children[$entry], $unconditional);
+            $settingsoutput = admin_apply_default_settings(
+                    $node->children[$entry], $unconditional, $admindefaultsettings, $settingsoutput
+                    );
         }
 
     } else if ($node instanceof admin_settingpage) {
-            foreach ($node->settings as $setting) {
-                if (!$unconditional and !is_null($setting->get_setting())) {
-                //do not override existing defaults
-                    continue;
-                }
-                $defaultsetting = $setting->get_defaultsetting();
-                if (is_null($defaultsetting)) {
-                // no value yet - default maybe applied after admin user creation or in upgradesettings
-                    continue;
-                }
+        foreach ($node->settings as $setting) {
+            if (!$unconditional and !is_null($setting->get_setting())) {
+                // Do not override existing defaults.
+                continue;
+            }
+            $defaultsetting = $setting->get_defaultsetting();
+            if (is_null($defaultsetting)) {
+                // 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);
+            } else {
+                unset($admindefaultsettings[$settingname]); // Remove processed settings.
             }
         }
+    }
+
+    // Call this function recursively until all settings are processed.
+    if (($node instanceof admin_root) && (!empty($admindefaultsettings))) {
+        $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;
 }
 
 /**
index 88bfaaf..3de940a 100644 (file)
@@ -510,8 +510,7 @@ function install_cli_database(array $options, $interactive) {
     // log in as admin - we need do anything when applying defaults
     \core\session\manager::set_user(get_admin());
 
-    // apply all default settings, do it twice to fill all defaults - some settings depend on other setting
-    admin_apply_default_settings(NULL, true);
+    // Apply all default settings.
     admin_apply_default_settings(NULL, true);
     set_config('registerauth', '');
 
index bdb18ab..6bea9f1 100644 (file)
@@ -159,6 +159,12 @@ the groupid field.
     When $CFG->keepmessagingallusersenabled is set to true, $CFG->messagingallusers will be also set to true to enable messaging site users.
     However, when it is empty, $CFG->messagingallusers will be disabled during the upgrading process, so the users will only be able to
     message contacts and users sharing a course with them.
+* There has been interface and functional changes to admin_apply_default_settings() (/lib/adminlib.php).  The function now takes two
+  additional optional parameters, $admindefaultsettings and $settingsoutput.  It also has a return value $settingsoutput.
+  The function now does not need to be called twice to ensure all default settings are set.  Instead the function calls itself recursively
+  until all settings have been set. The additional parameters are used recursively and shouldn't be need to be explicitly passed in when calling
+  the function from other parts of Moodle.
+  The return value: $settingsoutput is an array of setting names and the values that were set by the function.
 
 === 3.5 ===
 
index ffd7f86..e11e2e3 100644 (file)
@@ -2642,9 +2642,6 @@ function upgrade_fix_config_auth_plugin_defaults($plugin) {
         include($settingspath);
 
         if ($settings) {
-            // Consistently with what admin/cli/upgrade.php does, apply the default settings twice.
-            // I assume this is done for theoretical cases when a default value depends on an other.
-            admin_apply_default_settings($settings, false);
             admin_apply_default_settings($settings, false);
         }
     }