MDL-66118 hub: Stop using the hub URL as the setting name suffix
authorDavid Mudrák <david@moodle.com>
Tue, 1 Oct 2019 17:52:09 +0000 (19:52 +0200)
committerDavid Mudrák <david@moodle.com>
Fri, 4 Oct 2019 19:23:11 +0000 (21:23 +0200)
Site indicators shared with the registration hub used to have the hub
URL in the relevant setting names - such as site_name_httpsmoodlenet.
This upgrade step converts all such settings and drops the URL suffix
because we do not support alternative hubs any more.

Additionally the upgrade step renames the official hub and updates its
URL so that existing registrations with https://moodle.net are still
valid.

lib/classes/hub/registration.php
lib/db/upgrade.php
lib/db/upgradelib.php
lib/tests/upgradelib_test.php
version.php

index 2586791..5991b32 100644 (file)
@@ -158,9 +158,8 @@ class registration {
         require_once($CFG->dirroot . "/course/lib.php");
 
         $siteinfo = array();
-        $cleanhuburl = clean_param(HUB_MOODLEORGHUBURL, PARAM_ALPHANUMEXT);
         foreach (self::FORM_FIELDS as $field) {
-            $siteinfo[$field] = get_config('hub', 'site_'.$field.'_' . $cleanhuburl);
+            $siteinfo[$field] = get_config('hub', 'site_'.$field);
             if ($siteinfo[$field] === false) {
                 $siteinfo[$field] = array_key_exists($field, $defaults) ? $defaults[$field] : null;
             }
@@ -264,13 +263,12 @@ class registration {
      * @param stdClass $formdata data from {@link site_registration_form}
      */
     public static function save_site_info($formdata) {
-        $cleanhuburl = clean_param(HUB_MOODLEORGHUBURL, PARAM_ALPHANUMEXT);
         foreach (self::FORM_FIELDS as $field) {
-            set_config('site_' . $field . '_' . $cleanhuburl, $formdata->$field, 'hub');
+            set_config('site_' . $field, $formdata->$field, 'hub');
         }
         // Even if the the connection with moodle.net fails, admin has manually submitted the form which means they don't need
         // to be redirected to the site registration page any more.
-        set_config('site_regupdateversion_' . $cleanhuburl, max(array_keys(self::CONFIRM_NEW_FIELDS)), 'hub');
+        set_config('site_regupdateversion', max(array_keys(self::CONFIRM_NEW_FIELDS)), 'hub');
     }
 
     /**
@@ -537,8 +535,7 @@ class registration {
             return $fieldsneedconfirm;
         }
 
-        $cleanhuburl = clean_param(HUB_MOODLEORGHUBURL, PARAM_ALPHANUMEXT);
-        $lastupdated = (int)get_config('hub', 'site_regupdateversion_' . $cleanhuburl);
+        $lastupdated = (int)get_config('hub', 'site_regupdateversion');
         foreach (self::CONFIRM_NEW_FIELDS as $version => $fields) {
             if ($version > $lastupdated) {
                 $fieldsneedconfirm = array_merge($fieldsneedconfirm, $fields);
index 5ab80e9..425deab 100644 (file)
@@ -3556,5 +3556,23 @@ function xmldb_main_upgrade($oldversion) {
         upgrade_main_savepoint(true, 2019092700.01);
     }
 
+    if ($oldversion < 2019100400.01) {
+        // Rename the official moodle sites directory the site is registered with.
+        $DB->execute("UPDATE {registration_hubs}
+                         SET hubname = ?, huburl = ?
+                       WHERE huburl = ?", ['moodle', 'https://stats.moodle.org', 'https://moodle.net']);
+
+        // Convert the hub site specific settings to the new naming format without the hub URL in the name.
+        $hubconfig = get_config('hub');
+
+        if (!empty($hubconfig)) {
+            foreach (upgrade_convert_hub_config_site_param_names($hubconfig, 'https://moodle.net') as $name => $value) {
+                set_config($name, $value, 'hub');
+            }
+        }
+
+        upgrade_main_savepoint(true, 2019100400.01);
+    }
+
     return true;
 }
index 4c47550..802417a 100644 (file)
@@ -350,10 +350,10 @@ function upgrade_course_letter_boundary($courseid = null) {
     }
     $lettercolumnsql = '';
     if ($usergradelettercolumnsetting) {
-        // the system default is to show a column with letters (and the course uses the defaults).
+        // The system default is to show a column with letters (and the course uses the defaults).
         $lettercolumnsql = '(gss.value is NULL OR ' . $DB->sql_compare_text('gss.value') .  ' <> \'0\')';
     } else {
-        // the course displays a column with letters.
+        // The course displays a column with letters.
         $lettercolumnsql = $DB->sql_compare_text('gss.value') .  ' = \'1\'';
     }
 
@@ -608,4 +608,48 @@ function upgrade_rename_prediction_actions_useful_incorrectly_flagged() {
 
         $DB->execute($updatesql, $params + ['modelid' => $model->id]);
     }
-}
\ No newline at end of file
+}
+
+/**
+ * Convert the site settings for the 'hub' component in the config_plugins table.
+ *
+ * @param stdClass $hubconfig Settings loaded for the 'hub' component.
+ * @param string $huburl The URL of the hub to use as the valid one in case of conflict.
+ * @return stdClass List of new settings to be applied (including null values to be unset).
+ */
+function upgrade_convert_hub_config_site_param_names(stdClass $hubconfig, string $huburl): stdClass {
+
+    $cleanhuburl = clean_param($huburl, PARAM_ALPHANUMEXT);
+    $converted = [];
+
+    foreach ($hubconfig as $oldname => $value) {
+        if (preg_match('/^site_([a-z]+)([A-Za-z0-9_-]*)/', $oldname, $matches)) {
+            $newname = 'site_'.$matches[1];
+
+            if ($oldname === $newname) {
+                // There is an existing value with the new naming convention already.
+                $converted[$newname] = $value;
+
+            } else if (!array_key_exists($newname, $converted)) {
+                // Add the value under a new name and mark the original to be unset.
+                $converted[$newname] = $value;
+                $converted[$oldname] = null;
+
+            } else if ($matches[2] === '_'.$cleanhuburl) {
+                // The new name already exists, overwrite only if coming from the valid hub.
+                $converted[$newname] = $value;
+                $converted[$oldname] = null;
+
+            } else {
+                // Just unset the old value.
+                $converted[$oldname] = null;
+            }
+
+        } else {
+            // Not a hub-specific site setting, just keep it.
+            $converted[$oldname] = $value;
+        }
+    }
+
+    return (object) $converted;
+}
index 108e185..54e4b4d 100644 (file)
@@ -1124,4 +1124,52 @@ class core_upgradelib_testcase extends advanced_testcase {
         $this->assertEquals(1, $DB->count_records('analytics_prediction_actions',
             ['actionname' => \core_analytics\prediction::ACTION_INCORRECTLY_FLAGGED]));
     }
+
+    /**
+     * Test the functionality of the {@link upgrade_convert_hub_config_site_param_names()} function.
+     */
+    public function test_upgrade_convert_hub_config_site_param_names() {
+
+        $config = (object) [
+            // This is how site settings related to registration at https://moodle.net are stored.
+            'site_name_httpsmoodlenet' => 'Foo Site',
+            'site_language_httpsmoodlenet' => 'en',
+            'site_emailalert_httpsmoodlenet' => 1,
+            // These are unexpected relics of a value as registered at the old http://hub.moodle.org site.
+            'site_name_httphubmoodleorg' => 'Bar Site',
+            'site_description_httphubmoodleorg' => 'Old description',
+            // This is the target value we are converting to - here it already somehow exists.
+            'site_emailalert' => 0,
+            // This is a setting not related to particular hub.
+            'custom' => 'Do not touch this',
+            // A setting defined for multiple alternative hubs.
+            'site_foo_httpfirsthuborg' => 'First',
+            'site_foo_httpanotherhubcom' => 'Another',
+            'site_foo_httpyetanotherhubcom' => 'Yet another',
+            // A setting defined for multiple alternative hubs and one referential one.
+            'site_bar_httpfirsthuborg' => 'First',
+            'site_bar_httpanotherhubcom' => 'Another',
+            'site_bar_httpsmoodlenet' => 'One hub to rule them all!',
+            'site_bar_httpyetanotherhubcom' => 'Yet another',
+        ];
+
+        $converted = upgrade_convert_hub_config_site_param_names($config, 'https://moodle.net');
+
+        // Values defined for the moodle.net take precedence over the ones defined for other hubs.
+        $this->assertSame($converted->site_name, 'Foo Site');
+        $this->assertSame($converted->site_bar, 'One hub to rule them all!');
+        $this->assertNull($converted->site_name_httpsmoodlenet);
+        $this->assertNull($converted->site_bar_httpfirsthuborg);
+        $this->assertNull($converted->site_bar_httpanotherhubcom);
+        $this->assertNull($converted->site_bar_httpyetanotherhubcom);
+        // Values defined for alternative hubs only do not have any guaranteed value. Just for convenience, we use the first one.
+        $this->assertSame($converted->site_foo, 'First');
+        $this->assertNull($converted->site_foo_httpfirsthuborg);
+        $this->assertNull($converted->site_foo_httpanotherhubcom);
+        $this->assertNull($converted->site_foo_httpyetanotherhubcom);
+        // Values that are already defined with the new name format are kept.
+        $this->assertSame($converted->site_emailalert, 0);
+        // Eventual custom values not following the expected hub-specific naming format, are kept.
+        $this->assertSame($converted->custom, 'Do not touch this');
+    }
 }
index 2ed4c39..5d86833 100644 (file)
@@ -29,7 +29,7 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$version  = 2019100400.00;              // YYYYMMDD      = weekly release date of this DEV branch.
+$version  = 2019100400.01;              // YYYYMMDD      = weekly release date of this DEV branch.
                                         //         RR    = release increments - 00 in DEV branches.
                                         //           .XX = incremental changes.