🐛 Fixed private blogging getting disabled after 2.17 migration (#10606)

no-issue

The 2.17 migration included a bug which set the `is_private`, `amp` and `force_i18n` setting values to `'false'` when they should have been `'true'`

We've reverted these changes by reading the most recent backup file, and setting the value to `'true'` if the backup has it set to `'true'` AND the current db has it set to false.

We've also amended the broken migration, so that it does not cause this issue for future installs

core/server/data/migrations/versions/2.17/1-normalize-settings.js

@@ -24,7 +24,9 @@ module.exports.up = (options) => {
                 if (['is_private', 'force_i18n', 'amp'].includes(entry.key)) {
                     // @NOTE: sending false to db for type TEXT will transform to "0"
                     if ((entry.value === '0' || entry.value === '1')) {
-                        common.logging.info(`Normalize setting ${entry.key}`);
+                        const value = (!!+entry.value).toString();
+
+                        common.logging.info(`Setting ${entry.key} to ${value} because it was ${entry.value}`);
 
                         /**
                          * @NOTE: we have update raw data, because otherwise the `Settings.edit` fn will re-fetch the data
@@ -34,19 +36,33 @@ module.exports.up = (options) => {
                             .transacting('settings')
                             .where('key', entry.key)
                             .update({
-                                value: (!!+entry.value).toString()
+                                value: value
                             });
                     }
 
-                    // @NOTE: Something else is stored (any other value, set to false), normalize boolean fields
-                    if (entry.value !== 'false' && entry.value !== 'value') {
-                        common.logging.info(`Normalize setting ${entry.key}`);
+                    // @NOTE: null or undefined were obviously intended to be false
+                    if (entry.value === 'null' || entry.value === 'undefined') {
+                        const value = 'false';
+                        common.logging.info(`Setting ${entry.key} to ${value} because it was ${entry.value}`);
 
                         return localOptions
                             .transacting('settings')
                             .where('key', entry.key)
                             .update({
-                                value: 'false'
+                                value
+                            });
+                    }
+
+                    // @NOTE: Something other than true/false is stored, set to true, because that's how it would have behaved
+                    if (entry.value !== 'false' && entry.value !== 'true') {
+                        const value = 'true';
+                        common.logging.info(`Setting ${entry.key} to ${value} because it was ${entry.value}`);
+
+                        return localOptions
+                            .transacting('settings')
+                            .where('key', entry.key)
+                            .update({
+                                value
                             });
                     }
                 }

core/server/data/migrations/versions/2.18/1-restore-settings-from-backup.js

@@ -0,0 +1,120 @@
+const _ = require('lodash');
+const Promise = require('bluebird');
+const common = require('../../../../lib/common');
+const settingsCache = require('../../../../services/settings/cache');
+const config = require('../../../../config');
+const moment = require('moment');
+const fs = require('fs-extra');
+const path = require('path');
+
+module.exports.config = {
+    transaction: true
+};
+
+const backupFileRegex = /ghost.([\d]{4}-[\d]{2}-[\d]{2}).json$/;
+
+module.exports.up = (options) => {
+    const contentPath = config.get('paths').contentPath;
+    const dataPath = path.join(contentPath, 'data');
+
+    const localOptions = _.merge({
+        context: {internal: true}
+    }, options);
+
+    return fs.readdir(dataPath).then(function (files) {
+        const backups = files.filter(function (filename) {
+            return backupFileRegex.test(filename);
+        }).sort(function (a, b) {
+            const dateA = new Date(a.match(backupFileRegex)[1]);
+            const dateB = new Date(b.match(backupFileRegex)[1]);
+
+            return dateB - dateA;
+        });
+
+        if (backups.length === 0) {
+            common.logging.warn('No backup files found, skipping...');
+            return;
+        }
+
+        const mostRecentBackup = backups[0];
+
+        common.logging.info(`Using backupfile ${path.join(dataPath, mostRecentBackup)}`);
+
+        const backup = require(path.join(dataPath, mostRecentBackup));
+        const settings = backup && backup.data && backup.data.settings;
+        const migrations = backup && backup.data && backup.data.migrations;
+
+        if (!settings) {
+            common.logging.warn('Could not read settings from backup file, skipping...');
+            return;
+        }
+
+        if (!migrations || !migrations.length) {
+            common.logging.warn('Skipping migration. Not affected.');
+            return;
+        }
+
+        // NOTE: If we you have a backup file which has 2.16, but not 2.17, you are affected
+        // NOTE: We have corrected 2.17. If you jump form 2.16 to 2.18, you are good
+        const isAffected = _.find(migrations, {version: '2.16'}) &&
+            !_.find(migrations, {version: '2.17'});
+
+        if (!isAffected) {
+            common.logging.warn('Skipping migration. Not affected.');
+            return;
+        }
+
+        common.logging.warn('...is affected.');
+
+        const relevantBackupSettings = settings.filter(function (entry) {
+            return ['is_private', 'force_i18n', 'amp'].includes(entry.key);
+        }).reduce(function (obj, entry) {
+            return Object.assign(obj, {
+                [entry.key]: entry
+            });
+        }, {});
+
+        return localOptions
+            .transacting('settings')
+            .then((response) => {
+                if (!response) {
+                    common.logging.warn('Cannot find settings.');
+                    return;
+                }
+
+                const relevantLiveSettings = response.filter(function (entry) {
+                    return ['is_private', 'force_i18n', 'amp'].includes(entry.key);
+                });
+
+                return Promise.each(relevantLiveSettings, (liveSetting) => {
+                    const backupSetting = relevantBackupSettings[liveSetting.key];
+
+                    if (liveSetting.value === 'false' && backupSetting.value === 'true') {
+                        common.logging.info(`Reverting setting ${liveSetting.key}`);
+
+                        return localOptions
+                            .transacting('settings')
+                            .where('key', liveSetting.key)
+                            .update({
+                                value: backupSetting.value
+                            })
+                            .then(() => {
+                                // CASE: we have to update settings cache, because Ghost is able to run migrations on the same process
+                                settingsCache.set(liveSetting.key, {
+                                    id: liveSetting.id,
+                                    key: liveSetting.key,
+                                    type: liveSetting.type,
+                                    created_at: moment(liveSetting.created_at).startOf('seconds').toDate(),
+                                    updated_at: moment().startOf('seconds').toDate(),
+                                    updated_by: liveSetting.updated_by,
+                                    created_by: liveSetting.created_by,
+                                    value: backupSetting.value === 'true'
+                                });
+                            });
+                    }
+
+                    return Promise.resolve();
+                });
+            });
+    });
+};