From 3b90b1f3356204d46f41f70a99de480633e1ea75 Mon Sep 17 00:00:00 2001 From: Rishabh Garg Date: Thu, 4 Nov 2021 18:03:51 +0530 Subject: [PATCH] Moved `launchComplete` user setting as global editor setting (#13703) refs https://github.com/TryGhost/Team/issues/807 The launch wizard completed flag was previously stored at per user level in accessibility column of user table, so an administrator still got the option to complete the launch wizard even if the owner had completed it previously, which is not expected pattern. This change moves the launch complete flag for Admin to common settings from per user level so a site only needs to complete the launch wizard once irrespective of which user completes it - adds new `editor_is_launch_complete` setting to track if a site launch steps are completed in Admin - adds new migration util to easily allow adding new setting - adds migration to introduce new `editor_is_launch_complete` setting - adds migration to update launch complete flag for a site if any of the users have already completed the launch steps --- core/server/data/migrations/utils.js | 51 +++++++++++++++++++ .../4.22/01-add-is-launch-complete-setting.js | 8 +++ ...-launch-complete-setting-from-user-data.js | 39 ++++++++++++++ core/server/data/schema/default-settings.json | 8 +++ .../api/canary/admin/settings.test.js | 5 ++ test/regression/api/v2/admin/settings.test.js | 1 + test/regression/api/v3/admin/settings.test.js | 1 + test/regression/models/model_settings.test.js | 4 +- test/unit/server/data/exporter/index.test.js | 2 +- .../unit/server/data/schema/integrity.test.js | 2 +- 10 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 core/server/data/migrations/versions/4.22/01-add-is-launch-complete-setting.js create mode 100644 core/server/data/migrations/versions/4.22/02-update-launch-complete-setting-from-user-data.js diff --git a/core/server/data/migrations/utils.js b/core/server/data/migrations/utils.js index 9d390855bf..5517fa6dd0 100644 --- a/core/server/data/migrations/utils.js +++ b/core/server/data/migrations/utils.js @@ -409,12 +409,63 @@ function createDropColumnMigration(table, column, columnDefinition) { ); } +/** + * Creates a migration which will insert a new setting in settings table + * @param {object} settingSpec - setting key, value, group and type + * + * @returns {Object} migration object returning config/up/down properties + */ +function addSetting({key, value, type, group}) { + return createTransactionalMigration( + async function up(connection) { + const settingExists = await connection('settings') + .where('key', '=', key) + .first(); + if (settingExists) { + logging.warn(`Skipping adding setting: ${key} - setting already exists`); + return; + } + + logging.info(`Adding setting: ${key}`); + const now = connection.raw('CURRENT_TIMESTAMP'); + + return connection('settings') + .insert({ + id: ObjectId().toHexString(), + key, + value, + group, + type, + created_at: now, + created_by: MIGRATION_USER, + updated_at: now, + updated_by: MIGRATION_USER + }); + }, + async function down(connection) { + const settingExists = await connection('settings') + .where('key', '=', key) + .first(); + if (!settingExists) { + logging.warn(`Skipping dropping setting: ${key} - setting does not exist`); + return; + } + + logging.info(`Dropping setting: ${key}`); + return connection('settings') + .where('key', '=', key) + .del(); + } + ); +} + module.exports = { addTable, dropTables, addPermission, addPermissionToRole, addPermissionWithRoles, + addSetting, createTransactionalMigration, createNonTransactionalMigration, createIrreversibleMigration, diff --git a/core/server/data/migrations/versions/4.22/01-add-is-launch-complete-setting.js b/core/server/data/migrations/versions/4.22/01-add-is-launch-complete-setting.js new file mode 100644 index 0000000000..d9564f862c --- /dev/null +++ b/core/server/data/migrations/versions/4.22/01-add-is-launch-complete-setting.js @@ -0,0 +1,8 @@ +const {addSetting} = require('../../utils.js'); + +module.exports = addSetting({ + key: 'editor_is_launch_complete', + value: 'false', + type: 'boolean', + group: 'editor' +}); diff --git a/core/server/data/migrations/versions/4.22/02-update-launch-complete-setting-from-user-data.js b/core/server/data/migrations/versions/4.22/02-update-launch-complete-setting-from-user-data.js new file mode 100644 index 0000000000..7f8f4f529f --- /dev/null +++ b/core/server/data/migrations/versions/4.22/02-update-launch-complete-setting-from-user-data.js @@ -0,0 +1,39 @@ +const logging = require('@tryghost/logging'); +const {createTransactionalMigration} = require('../../utils.js'); + +module.exports = createTransactionalMigration( + async function up(knex) { + const userRows = await knex('users').select('accessibility').whereNotNull('accessibility'); + const hasLaunchComplete = userRows.find((user) => { + try { + const userAccessibility = JSON.parse(user.accessibility); + return userAccessibility.launchComplete; + } catch (e) { + return false; + } + }); + if (hasLaunchComplete) { + logging.info('Updating setting: "editor_is_launch_complete" to "true"'); + await knex('settings') + .where({ + key: 'editor_is_launch_complete' + }) + .update({ + value: 'true' + }); + } else { + logging.warn('Skipped setting update: "editor_is_launch_complete" setting - is already correct value!'); + } + }, + + async function down(knex) { + logging.info('Reverting setting: "editor_is_launch_complete" - to "false"'); + await knex('settings') + .where({ + key: 'editor_is_launch_complete' + }) + .update({ + value: 'false' + }); + } +); diff --git a/core/server/data/schema/default-settings.json b/core/server/data/schema/default-settings.json index 7c316ab059..066a8d2473 100644 --- a/core/server/data/schema/default-settings.json +++ b/core/server/data/schema/default-settings.json @@ -590,6 +590,14 @@ "editor_default_email_recipients_filter": { "defaultValue": "all", "type": "string" + }, + "editor_is_launch_complete": { + "defaultValue": "false", + "validations": { + "isEmpty": false, + "isIn": [["true", "false"]] + }, + "type": "boolean" } } } diff --git a/test/regression/api/canary/admin/settings.test.js b/test/regression/api/canary/admin/settings.test.js index 0d5dab7f74..bb227a0229 100644 --- a/test/regression/api/canary/admin/settings.test.js +++ b/test/regression/api/canary/admin/settings.test.js @@ -425,6 +425,11 @@ const defaultSettingsKeyTypes = [ type: 'string', group: 'editor' }, + { + key: 'editor_is_launch_complete', + type: 'boolean', + group: 'editor' + }, { key: 'labs', type: 'object', diff --git a/test/regression/api/v2/admin/settings.test.js b/test/regression/api/v2/admin/settings.test.js index 0be1e42a28..0a3c523f91 100644 --- a/test/regression/api/v2/admin/settings.test.js +++ b/test/regression/api/v2/admin/settings.test.js @@ -86,6 +86,7 @@ const defaultSettingsKeyTypes = [ {key: 'oauth_client_secret', type: 'oauth'}, {key: 'editor_default_email_recipients', type: 'editor'}, {key: 'editor_default_email_recipients_filter', type: 'editor'}, + {key: 'editor_is_launch_complete', type: 'editor'}, {key: 'labs', type: 'blog'}, {key: 'email_verification_required', type: 'bulk_email'} ]; diff --git a/test/regression/api/v3/admin/settings.test.js b/test/regression/api/v3/admin/settings.test.js index 26e5c22be2..1368521f3c 100644 --- a/test/regression/api/v3/admin/settings.test.js +++ b/test/regression/api/v3/admin/settings.test.js @@ -90,6 +90,7 @@ const defaultSettingsKeyTypes = [ {key: 'oauth_client_secret', type: 'oauth'}, {key: 'editor_default_email_recipients', type: 'editor'}, {key: 'editor_default_email_recipients_filter', type: 'editor'}, + {key: 'editor_is_launch_complete', type: 'editor'}, {key: 'labs', type: 'blog'}, {key: 'email_verification_required', type: 'bulk_email'} ]; diff --git a/test/regression/models/model_settings.test.js b/test/regression/models/model_settings.test.js index 0cf9b8d10a..9b128b712d 100644 --- a/test/regression/models/model_settings.test.js +++ b/test/regression/models/model_settings.test.js @@ -17,7 +17,7 @@ describe('Settings Model', function () { await models.Settings.populateDefaults(); const settingsPopulated = await models.Settings.findAll(); - settingsPopulated.length.should.equal(93); + settingsPopulated.length.should.equal(94); }); it('doesn\'t overwrite any existing settings', async function () { @@ -42,7 +42,7 @@ describe('Settings Model', function () { await models.Settings.populateDefaults(); const settingsPopulated = await models.Settings.findAll(); - settingsPopulated.length.should.equal(93); + settingsPopulated.length.should.equal(94); const titleSetting = settingsPopulated.models.find(s => s.get('key') === 'title'); titleSetting.get('value').should.equal('Testing Defaults'); diff --git a/test/unit/server/data/exporter/index.test.js b/test/unit/server/data/exporter/index.test.js index 2b37e9057d..c1e02d0643 100644 --- a/test/unit/server/data/exporter/index.test.js +++ b/test/unit/server/data/exporter/index.test.js @@ -199,7 +199,7 @@ describe('Exporter', function () { // NOTE: if default settings changed either modify the settings keys blocklist or increase allowedKeysLength // This is a reminder to think about the importer/exporter scenarios ;) - const allowedKeysLength = 83; + const allowedKeysLength = 84; totalKeysLength.should.eql(SETTING_KEYS_BLOCKLIST.length + allowedKeysLength); }); }); diff --git a/test/unit/server/data/schema/integrity.test.js b/test/unit/server/data/schema/integrity.test.js index 3e480ce4fa..cd6b9fcab2 100644 --- a/test/unit/server/data/schema/integrity.test.js +++ b/test/unit/server/data/schema/integrity.test.js @@ -36,7 +36,7 @@ describe('DB version integrity', function () { // Only these variables should need updating const currentSchemaHash = 'e649797a5de92d417744f6f2623c79cf'; const currentFixturesHash = '07d4b0c4cf159b34344a6b5e88c74e9f'; - const currentSettingsHash = 'aa3fcbc8ab119b630aeda7366ead5493'; + const currentSettingsHash = 'b06316ebbf62381158e1c46c97c0b77a'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01'; // If this test is failing, then it is likely a change has been made that requires a DB version bump,