From fa8555bda2f8d3d1a0910e01cb8406997ec504eb Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 22 Mar 2016 18:02:00 +0000 Subject: [PATCH 1/8] Initial commit of 005 version refs #6301 - bump the default version & update corresponding test - add empty task folders for 005 data & fixture migrations - update tests to cover the new 005 upgrades --- core/server/data/migration/005/index.js | 3 + .../data/migration/fixtures/005/index.js | 3 + core/server/data/schema/default-settings.json | 2 +- core/test/unit/migration_fixture_spec.js | 47 ++++++++++++++ core/test/unit/migration_spec.js | 64 +++++++++++++++++-- 5 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 core/server/data/migration/005/index.js create mode 100644 core/server/data/migration/fixtures/005/index.js diff --git a/core/server/data/migration/005/index.js b/core/server/data/migration/005/index.js new file mode 100644 index 0000000000..129bc998bc --- /dev/null +++ b/core/server/data/migration/005/index.js @@ -0,0 +1,3 @@ +module.exports = [ + // Nothing to do yet +]; diff --git a/core/server/data/migration/fixtures/005/index.js b/core/server/data/migration/fixtures/005/index.js new file mode 100644 index 0000000000..129bc998bc --- /dev/null +++ b/core/server/data/migration/fixtures/005/index.js @@ -0,0 +1,3 @@ +module.exports = [ + // Nothing to do yet +]; diff --git a/core/server/data/schema/default-settings.json b/core/server/data/schema/default-settings.json index dc7e991e13..20b73713a9 100644 --- a/core/server/data/schema/default-settings.json +++ b/core/server/data/schema/default-settings.json @@ -1,7 +1,7 @@ { "core": { "databaseVersion": { - "defaultValue": "004" + "defaultValue": "005" }, "dbHash": { "defaultValue": null diff --git a/core/test/unit/migration_fixture_spec.js b/core/test/unit/migration_fixture_spec.js index a87f77f847..b9a81f5406 100644 --- a/core/test/unit/migration_fixture_spec.js +++ b/core/test/unit/migration_fixture_spec.js @@ -12,6 +12,7 @@ var should = require('should'), update = rewire('../../server/data/migration/fixtures/update'), populate = rewire('../../server/data/migration/fixtures/populate'), fixtures004 = require('../../server/data/migration/fixtures/004'), + fixtures005 = require('../../server/data/migration/fixtures/005'), ensureDefaultSettings = require('../../server/data/migration/fixtures/settings'), sandbox = sinon.sandbox.create(); @@ -124,6 +125,11 @@ describe('Fixtures', function () { clientEditStub = sandbox.stub(models.Client, 'edit').returns(Promise.resolve()); }); + it('should have tasks for 004', function () { + should.exist(fixtures004); + fixtures004.should.be.an.Array().with.lengthOf(8); + }); + describe('01-move-jquery-with-alert', function () { it('tries to move jQuery to ghost_foot', function (done) { getObjStub.get.returns(''); @@ -685,6 +691,47 @@ describe('Fixtures', function () { }); }); }); + + describe('Update to 005', function () { + it('should call all the 005 fixture upgrades', function (done) { + // Setup + // Create a new stub, this will replace sequence, so that db calls don't actually get run + var sequenceStub = sandbox.stub(), + sequenceReset = update.__set__('sequence', sequenceStub); + + // The first time we call sequence, it should be to execute a top level version, e.g 005 + // yieldsTo('0') means this stub will execute the function at index 0 of the array passed as the + // first argument. In short the `runVersionTasks` function gets executed, and sequence gets called + // again with the array of tasks to execute for 005, which is what we want to check + + // Can't yield until we have at least one task + // sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); + sequenceStub.returns(Promise.resolve([])); + + update(['005'], loggerStub).then(function (result) { + should.exist(result); + + loggerStub.info.calledOnce.should.be.true(); + loggerStub.warn.called.should.be.false(); + + sequenceStub.calledOnce.should.be.true(); + + sequenceStub.firstCall.calledWith(sinon.match.array, sinon.match.object, loggerStub).should.be.true(); + sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(0); + + // Reset + sequenceReset(); + done(); + }).catch(done); + }); + + describe('Tasks:', function () { + it('should have tasks for 005', function () { + should.exist(fixtures005); + fixtures005.should.be.an.Array().with.lengthOf(0); + }); + }); + }); }); describe('Populate fixtures', function () { diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 287b3dcf6c..987c3cfa40 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -21,6 +21,7 @@ var should = require('should'), populate = require('../../server/data/migration/populate'), update = rewire('../../server/data/migration/update'), updates004 = require('../../server/data/migration/004'), + updates005 = require('../../server/data/migration/005'), defaultSettings = schema.defaultSettings, schemaTables = Object.keys(schema.tables), @@ -33,7 +34,7 @@ var should = require('should'), // both of which are required for migrations to work properly. describe('DB version integrity', function () { // Only these variables should need updating - var currentDbVersion = '004', + var currentDbVersion = '005', currentSchemaHash = 'a195562bf4915e3f3f610f6d178aba01', currentFixturesHash = '77ebb081539f9e0c49f487faf7fd929e'; @@ -343,7 +344,7 @@ describe('Migrations', function () { // yieldsTo('0') means this stub will execute the function at index 0 of the array passed as the // first argument. In short the `runVersionTasks` function gets executed, and sequence gets called // again with the array of tasks to execute for 004, which is what we want to check - sequenceStub.onFirstCall().yieldsTo('0'); + sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); // Execute update('003', '004', loggerStub).then(function () { @@ -359,14 +360,13 @@ describe('Migrations', function () { tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(5); sequenceStub.calledTwice.should.be.true(); + sequenceStub.firstCall.calledWith(sinon.match.array, loggerStub).should.be.true(); - sequenceStub.secondCall.calledWith(sinon.match.array, loggerStub).should.be.true(); - sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(1); - sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(5); - sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); + sequenceStub.secondCall.calledWith(sinon.match.array, loggerStub).should.be.true(); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(5); sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'addTourColumnToUsers'); sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'addSortOrderColumnToPostsTags'); sequenceStub.secondCall.args[0][2].should.be.a.Function().with.property('name', 'addManyColumnsToClients'); @@ -402,6 +402,11 @@ describe('Migrations', function () { knexStub.restore(); }); + it('should have tasks for 004', function () { + should.exist(updates004); + updates004.should.be.an.Array().with.lengthOf(5); + }); + describe('01-add-tour-column-to-users', function () { it('does not try to add a new column if the table does not exist', function (done) { // Setup @@ -771,6 +776,53 @@ describe('Migrations', function () { }); }); }); + + describe('Update to 005', function () { + it('should call all the 005 database upgrade tasks', function (done) { + // Setup + // Create a new stub, this will replace sequence, so that db calls don't actually get run + var sequenceStub = sandbox.stub(), + sequenceReset = update.__set__('sequence', sequenceStub); + + // The first time we call sequence, it should be to execute a top level version, e.g 005 + // yieldsTo('0') means this stub will execute the function at index 0 of the array passed as the + // first argument. In short the `runVersionTasks` function gets executed, and sequence gets called + // again with the array of tasks to execute for 005, which is what we want to check + + // Can't yield until we have at least one task + // sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); + + // Execute + update('004', '005', loggerStub).then(function () { + errorStub.called.should.be.false(); + loggerStub.info.called.should.be.false(); + + versionsSpy.calledOnce.should.be.true(); + versionsSpy.calledWith('004', '005').should.be.true(); + versionsSpy.returned(['004', '005']).should.be.true(); + + tasksSpy.calledOnce.should.be.true(); + tasksSpy.calledWith('005', loggerStub).should.be.true(); + tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(0); + + sequenceStub.calledOnce.should.be.true(); + + sequenceStub.firstCall.calledWith(sinon.match.array, loggerStub).should.be.true(); + sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(0); + + // Reset sequence + sequenceReset(); + done(); + }).catch(done); + }); + + describe('Tasks:', function () { + it('should have tasks for 005', function () { + should.exist(updates005); + updates005.should.be.an.Array().with.lengthOf(0); + }); + }); + }); }); describe('Update Database Schema', function () { From f08fe28834799899a7f76a7decb45d0e22ab8236 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 22 Mar 2016 18:27:48 +0000 Subject: [PATCH 2/8] Move fixClientSecret to 005 fixture migration refs #6301 - move the temporary `fixClientSecret` function from migration.init into being a proper fixture migration task - update the tests accordingly --- .../005/01-update-ghost-client-secrets.js | 26 ++++++ .../data/migration/fixtures/005/index.js | 3 +- core/server/data/migration/index.js | 24 +---- core/test/unit/migration_fixture_spec.js | 60 +++++++++++-- core/test/unit/migration_spec.js | 87 +------------------ 5 files changed, 86 insertions(+), 114 deletions(-) create mode 100644 core/server/data/migration/fixtures/005/01-update-ghost-client-secrets.js diff --git a/core/server/data/migration/fixtures/005/01-update-ghost-client-secrets.js b/core/server/data/migration/fixtures/005/01-update-ghost-client-secrets.js new file mode 100644 index 0000000000..abc2dffbfa --- /dev/null +++ b/core/server/data/migration/fixtures/005/01-update-ghost-client-secrets.js @@ -0,0 +1,26 @@ +// Update the `ghost-*` clients so that they definitely have a proper secret +var models = require('../../../../models'), + _ = require('lodash'), + Promise = require('bluebird'), + crypto = require('crypto'), + + message = 'Updating client secret'; + +module.exports = function updateGhostClientsSecrets(options, logger) { + return models.Clients.forge().query('where', 'secret', '=', 'not_available').fetch().then(function (results) { + if (results.models.length === 0) { + logger.warn(message); + return; + } + + return Promise.map(results.models, function mapper(client) { + logger.info(message + ' (' + client.slug + ')'); + client.secret = crypto.randomBytes(6).toString('hex'); + + return models.Client.edit( + _.extend({}, client, {secret: crypto.randomBytes(6).toString('hex')}, + _.extend({}, options, {id: client.id})) + ); + }); + }); +}; diff --git a/core/server/data/migration/fixtures/005/index.js b/core/server/data/migration/fixtures/005/index.js index 129bc998bc..291f252e04 100644 --- a/core/server/data/migration/fixtures/005/index.js +++ b/core/server/data/migration/fixtures/005/index.js @@ -1,3 +1,4 @@ module.exports = [ - // Nothing to do yet + // add jquery setting and privacy info + require('./01-update-ghost-client-secrets') ]; diff --git a/core/server/data/migration/index.js b/core/server/data/migration/index.js index 6cc63dfe91..96b2e4359a 100644 --- a/core/server/data/migration/index.js +++ b/core/server/data/migration/index.js @@ -1,12 +1,8 @@ -var Promise = require('bluebird'), - crypto = require('crypto'), - versioning = require('../schema').versioning, +var versioning = require('../schema').versioning, errors = require('../../errors'), - models = require('../../models'), // private logger, - fixClientSecret, populate = require('./populate'), update = require('./update'), @@ -28,21 +24,8 @@ logger = { } }; -// TODO: move to migration.to005() for next DB version -fixClientSecret = function () { - return models.Clients.forge().query('where', 'secret', '=', 'not_available').fetch().then(function updateClients(results) { - return Promise.map(results.models, function mapper(client) { - if (process.env.NODE_ENV.indexOf('testing') !== 0) { - logger.info('Updating client secret'); - client.secret = crypto.randomBytes(6).toString('hex'); - } - return models.Client.edit(client, {context: {internal: true}, id: client.id}); - }); - }); -}; - // Check for whether data is needed to be bootstrapped or not -init = function (tablesOnly) { +init = function init(tablesOnly) { tablesOnly = tablesOnly || false; // There are 4 possibilities: @@ -63,8 +46,7 @@ init = function (tablesOnly) { // 1. The database exists and is up-to-date } else if (databaseVersion === defaultVersion) { logger.info('Up-to-date at version ' + databaseVersion); - // TODO: temporary fix for missing client.secret - return fixClientSecret(); + return; // 3. The database exists but the currentVersion setting does not or cannot be understood } else { diff --git a/core/test/unit/migration_fixture_spec.js b/core/test/unit/migration_fixture_spec.js index b9a81f5406..a75c9e9fff 100644 --- a/core/test/unit/migration_fixture_spec.js +++ b/core/test/unit/migration_fixture_spec.js @@ -703,21 +703,23 @@ describe('Fixtures', function () { // yieldsTo('0') means this stub will execute the function at index 0 of the array passed as the // first argument. In short the `runVersionTasks` function gets executed, and sequence gets called // again with the array of tasks to execute for 005, which is what we want to check - - // Can't yield until we have at least one task - // sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); - sequenceStub.returns(Promise.resolve([])); + sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); update(['005'], loggerStub).then(function (result) { should.exist(result); - loggerStub.info.calledOnce.should.be.true(); + loggerStub.info.calledTwice.should.be.true(); loggerStub.warn.called.should.be.false(); - sequenceStub.calledOnce.should.be.true(); + sequenceStub.calledTwice.should.be.true(); sequenceStub.firstCall.calledWith(sinon.match.array, sinon.match.object, loggerStub).should.be.true(); - sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(0); + sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); + + sequenceStub.secondCall.calledWith(sinon.match.array, sinon.match.object, loggerStub).should.be.true(); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'updateGhostClientsSecrets'); // Reset sequenceReset(); @@ -728,7 +730,49 @@ describe('Fixtures', function () { describe('Tasks:', function () { it('should have tasks for 005', function () { should.exist(fixtures005); - fixtures005.should.be.an.Array().with.lengthOf(0); + fixtures005.should.be.an.Array().with.lengthOf(1); + }); + + describe('01-update-ghost-client-secrets', function () { + var queryStub, clientForgeStub, clientEditStub; + + beforeEach(function () { + queryStub = { + query: sandbox.stub().returnsThis(), + fetch: sandbox.stub() + }; + + clientForgeStub = sandbox.stub(models.Clients, 'forge').returns(queryStub); + clientEditStub = sandbox.stub(models.Client, 'edit'); + }); + + it('should do nothing if there are no incorrect secrets', function (done) { + // Setup + queryStub.fetch.returns(new Promise.resolve({models: []})); + + // Execute + fixtures005[0]({}, loggerStub).then(function () { + clientForgeStub.calledOnce.should.be.true(); + clientEditStub.called.should.be.false(); + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + done(); + }).catch(done); + }); + + it('should try to fix any incorrect secrets', function (done) { + // Setup + queryStub.fetch.returns(new Promise.resolve({models: [{id: 1}]})); + + // Execute + fixtures005[0]({}, loggerStub).then(function () { + clientForgeStub.calledOnce.should.be.true(); + clientEditStub.called.should.be.true(); + loggerStub.info.calledOnce.should.be.true(); + loggerStub.warn.called.should.be.false(); + done(); + }).catch(done); + }); }); }); }); diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 987c3cfa40..91bd2aa42b 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -13,9 +13,6 @@ var should = require('should'), exporter = require('../../server/data/export'), schema = require('../../server/data/schema'), - // TODO: can go when fixClientSecret is moved - models = require('../../server/models'), - migration = rewire('../../server/data/migration'), fixtures = require('../../server/data/migration/fixtures'), populate = require('../../server/data/migration/populate'), @@ -865,8 +862,8 @@ describe('Migrations', function () { }); describe('Init', function () { - var defaultVersionStub, databaseVersionStub, errorStub, updateStub, populateStub, fixSecretStub, - resetLog, resetUpdate, resetPopulate, resetFixSecret; + var defaultVersionStub, databaseVersionStub, errorStub, updateStub, populateStub, + resetLog, resetUpdate, resetPopulate; beforeEach(function () { defaultVersionStub = sandbox.stub(schema.versioning, 'getDefaultDatabaseVersion'); @@ -874,19 +871,16 @@ describe('Migrations', function () { errorStub = sandbox.stub(errors, 'logErrorAndExit'); updateStub = sandbox.stub(); populateStub = sandbox.stub(); - fixSecretStub = sandbox.stub(); resetLog = migration.__set__('logger', loggerStub); resetUpdate = migration.__set__('update', updateStub); resetPopulate = migration.__set__('populate', populateStub); - resetFixSecret = migration.__set__('fixClientSecret', fixSecretStub); }); afterEach(function () { resetLog(); resetUpdate(); resetPopulate(); - resetFixSecret(); }); it('should do an UPDATE if default version is higher', function (done) { @@ -906,7 +900,6 @@ describe('Migrations', function () { errorStub.called.should.be.false(); populateStub.called.should.be.false(); - fixSecretStub.called.should.be.false(); done(); }).catch(done); @@ -929,13 +922,12 @@ describe('Migrations', function () { errorStub.called.should.be.false(); populateStub.called.should.be.false(); - fixSecretStub.called.should.be.false(); done(); }).catch(done); }); - it('should do FIX SECRET if versions are the same', function (done) { + it('should just return if versions are the same', function (done) { // Setup defaultVersionStub.returns('004'); databaseVersionStub.returns(new Promise.resolve('004')); @@ -947,8 +939,6 @@ describe('Migrations', function () { loggerStub.info.calledOnce.should.be.true(); loggerStub.warn.called.should.be.false(); - fixSecretStub.called.should.be.true(); - errorStub.called.should.be.false(); updateStub.called.should.be.false(); populateStub.called.should.be.false(); @@ -975,7 +965,6 @@ describe('Migrations', function () { errorStub.called.should.be.false(); populateStub.called.should.be.false(); - fixSecretStub.called.should.be.false(); delete process.env.FORCE_MIGRATION; done(); @@ -999,7 +988,6 @@ describe('Migrations', function () { errorStub.called.should.be.false(); updateStub.called.should.be.false(); - fixSecretStub.called.should.be.false(); done(); }).catch(done); @@ -1022,7 +1010,6 @@ describe('Migrations', function () { errorStub.called.should.be.false(); updateStub.called.should.be.false(); - fixSecretStub.called.should.be.false(); done(); }).catch(done); @@ -1043,7 +1030,6 @@ describe('Migrations', function () { populateStub.called.should.be.false(); updateStub.called.should.be.false(); - fixSecretStub.called.should.be.false(); done(); }).catch(done); @@ -1064,7 +1050,6 @@ describe('Migrations', function () { defaultVersionStub.calledOnce.should.be.false(); populateStub.called.should.be.false(); updateStub.called.should.be.false(); - fixSecretStub.called.should.be.false(); done(); }).catch(done); @@ -1096,70 +1081,4 @@ describe('Migrations', function () { errorsInfoStub.called.should.be.false(); }); }); - - // TODO: move this to 005!! - describe('FixClientSecret', function () { - var fixClientSecret, queryStub, clientForgeStub, clientEditStub, toStringStub, cryptoStub; - - beforeEach(function () { - fixClientSecret = migration.__get__('fixClientSecret'); - queryStub = { - query: sandbox.stub().returnsThis(), - fetch: sandbox.stub() - }; - - models.init(); - toStringStub = {toString: sandbox.stub().returns('TEST')}; - cryptoStub = sandbox.stub(crypto, 'randomBytes').returns(toStringStub); - clientForgeStub = sandbox.stub(models.Clients, 'forge').returns(queryStub); - clientEditStub = sandbox.stub(models.Client, 'edit'); - }); - - it('should do nothing if there are no incorrect secrets', function (done) { - // Setup - queryStub.fetch.returns(new Promise.resolve({models: []})); - - // Execute - fixClientSecret().then(function () { - clientForgeStub.calledOnce.should.be.true(); - clientEditStub.called.should.be.false(); - toStringStub.toString.called.should.be.false(); - cryptoStub.called.should.be.false(); - done(); - }).catch(done); - }); - - it('should try to fix any incorrect secrets', function (done) { - // Setup - queryStub.fetch.returns(new Promise.resolve({models: [{id: 1}]})); - - // Execute - fixClientSecret().then(function () { - clientForgeStub.calledOnce.should.be.true(); - clientEditStub.called.should.be.true(); - toStringStub.toString.called.should.be.false(); - cryptoStub.called.should.be.false(); - done(); - }).catch(done); - }); - - it('should try to create a new secret, if the mode is not testing', function (done) { - // Setup - var envTemp = process.env.NODE_ENV; - process.env.NODE_ENV = 'development'; - queryStub.fetch.returns(new Promise.resolve({models: [{id: 1}]})); - - // Execute - fixClientSecret().then(function () { - clientForgeStub.calledOnce.should.be.true(); - clientEditStub.called.should.be.true(); - toStringStub.toString.called.should.be.true(); - cryptoStub.calledOnce.should.be.true(); - - // reset - process.env.NODE_ENV = envTemp; - done(); - }).catch(done); - }); - }); }); From b4ae469c1260fa1676580e3288c34b8b47e47cdf Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Fri, 25 Mar 2016 10:57:50 +0000 Subject: [PATCH 3/8] Drop hidden column from tags table refs #6301 - column is not used and we'll be adding a visibility column to serve the intended purpose --- .../005/01-drop-hidden-column-from-tags.js | 24 ++++ core/server/data/migration/005/index.js | 3 +- core/server/data/schema/schema.js | 1 - core/test/unit/migration_spec.js | 107 ++++++++++++++++-- 4 files changed, 124 insertions(+), 11 deletions(-) create mode 100644 core/server/data/migration/005/01-drop-hidden-column-from-tags.js diff --git a/core/server/data/migration/005/01-drop-hidden-column-from-tags.js b/core/server/data/migration/005/01-drop-hidden-column-from-tags.js new file mode 100644 index 0000000000..688929ea0e --- /dev/null +++ b/core/server/data/migration/005/01-drop-hidden-column-from-tags.js @@ -0,0 +1,24 @@ +var commands = require('../../schema').commands, + db = require('../../db'), + + table = 'tags', + column = 'hidden', + message = 'Removing column: ' + table + '.' + column; + +module.exports = function dropHiddenColumnFromTags(logger) { + return db.knex.schema.hasTable(table).then(function (exists) { + if (exists) { + return db.knex.schema.hasColumn(table, column).then(function (exists) { + if (exists) { + logger.info(message); + return commands.dropColumn(table, column); + } else { + logger.warn(message); + } + }); + } else { + // @TODO: this should probably be an error + logger.warn(message); + } + }); +}; diff --git a/core/server/data/migration/005/index.js b/core/server/data/migration/005/index.js index 129bc998bc..6856607482 100644 --- a/core/server/data/migration/005/index.js +++ b/core/server/data/migration/005/index.js @@ -1,3 +1,4 @@ module.exports = [ - // Nothing to do yet + // Drop hidden column from tags table + require('./01-drop-hidden-column-from-tags') ]; diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index 312601c1cc..e801bffda2 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -105,7 +105,6 @@ module.exports = { slug: {type: 'string', maxlength: 150, nullable: false, unique: true}, description: {type: 'string', maxlength: 200, nullable: true}, image: {type: 'text', maxlength: 2000, nullable: true}, - hidden: {type: 'bool', nullable: false, defaultTo: false, validations: {isIn: [[0, 1, false, true]]}}, parent_id: {type: 'integer', nullable: true}, meta_title: {type: 'string', maxlength: 150, nullable: true}, meta_description: {type: 'string', maxlength: 200, nullable: true}, diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 91bd2aa42b..5c3f56d519 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -32,7 +32,7 @@ var should = require('should'), describe('DB version integrity', function () { // Only these variables should need updating var currentDbVersion = '005', - currentSchemaHash = 'a195562bf4915e3f3f610f6d178aba01', + currentSchemaHash = 'cc249220eb57b2249fc82f8494ad3912', currentFixturesHash = '77ebb081539f9e0c49f487faf7fd929e'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, @@ -785,14 +785,12 @@ describe('Migrations', function () { // yieldsTo('0') means this stub will execute the function at index 0 of the array passed as the // first argument. In short the `runVersionTasks` function gets executed, and sequence gets called // again with the array of tasks to execute for 005, which is what we want to check - - // Can't yield until we have at least one task - // sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); + sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); // Execute update('004', '005', loggerStub).then(function () { errorStub.called.should.be.false(); - loggerStub.info.called.should.be.false(); + loggerStub.info.calledTwice.should.be.true(); versionsSpy.calledOnce.should.be.true(); versionsSpy.calledWith('004', '005').should.be.true(); @@ -800,12 +798,17 @@ describe('Migrations', function () { tasksSpy.calledOnce.should.be.true(); tasksSpy.calledWith('005', loggerStub).should.be.true(); - tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(0); + tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(1); - sequenceStub.calledOnce.should.be.true(); + sequenceStub.calledTwice.should.be.true(); sequenceStub.firstCall.calledWith(sinon.match.array, loggerStub).should.be.true(); - sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(0); + sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); + + sequenceStub.secondCall.calledWith(sinon.match.array, loggerStub).should.be.true(); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'dropHiddenColumnFromTags'); // Reset sequence sequenceReset(); @@ -814,9 +817,95 @@ describe('Migrations', function () { }); describe('Tasks:', function () { + var dropColumnStub, + knexStub, knexMock; + + beforeEach(function () { + knexMock = sandbox.stub().returns({}); + knexMock.schema = { + hasTable: sandbox.stub(), + hasColumn: sandbox.stub() + }; + // this MUST use sinon, not sandbox, see sinonjs/sinon#781 + knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); + + dropColumnStub = sandbox.stub(schema.commands, 'dropColumn'); + }); + + afterEach(function () { + knexStub.restore(); + }); + it('should have tasks for 005', function () { should.exist(updates005); - updates005.should.be.an.Array().with.lengthOf(0); + updates005.should.be.an.Array().with.lengthOf(1); + }); + + describe('01-drop-hidden-column-from-tags', function () { + it('does not try to drop column if the table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('tags').returns(Promise.resolve(false)); + + // Execute + updates005[0](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('tags').should.be.true(); + + knexMock.schema.hasColumn.called.should.be.false(); + + dropColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + + it('does not try to drop column if the column does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('tags').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('tags', 'hidden').returns(Promise.resolve(false)); + + // Execute + updates005[0](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('tags').should.be.true(); + + knexMock.schema.hasColumn.calledOnce.should.be.true(); + knexMock.schema.hasColumn.calledWith('tags', 'hidden').should.be.true(); + + dropColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + + it('tries to add drop column if table and column are both present', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('tags').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('tags', 'hidden').returns(Promise.resolve(true)); + + // Execute + updates005[0](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('tags').should.be.true(); + + knexMock.schema.hasColumn.calledOnce.should.be.true(); + knexMock.schema.hasColumn.calledWith('tags', 'hidden').should.be.true(); + + dropColumnStub.calledOnce.should.be.true(); + dropColumnStub.calledWith('tags', 'hidden').should.be.true(); + + loggerStub.info.calledOnce.should.be.true(); + loggerStub.warn.called.should.be.false(); + + done(); + }).catch(done); + }); }); }); }); From e7cc18d5fb049624d11f811d30f495c4a83b41a1 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Fri, 25 Mar 2016 20:45:40 +0000 Subject: [PATCH 4/8] Add visibility column to posts, tags and users refs #6301, #6165 - visibility is added as a new column on posts, tags and users. - has a relevant default value for each table --- .../02-add-visibility-column-to-key-tables.js | 27 ++++ core/server/data/migration/005/index.js | 4 +- core/server/data/schema/schema.js | 3 + core/test/unit/migration_spec.js | 136 +++++++++++++++++- 4 files changed, 164 insertions(+), 6 deletions(-) create mode 100644 core/server/data/migration/005/02-add-visibility-column-to-key-tables.js diff --git a/core/server/data/migration/005/02-add-visibility-column-to-key-tables.js b/core/server/data/migration/005/02-add-visibility-column-to-key-tables.js new file mode 100644 index 0000000000..6aa736bb95 --- /dev/null +++ b/core/server/data/migration/005/02-add-visibility-column-to-key-tables.js @@ -0,0 +1,27 @@ +var Promise = require('bluebird'), + commands = require('../../schema').commands, + db = require('../../db'), + + tables = ['posts', 'tags', 'users'], + column = 'visibility'; + +module.exports = function addVisibilityColumnToKeyTables(logger) { + return Promise.mapSeries(tables, function (table) { + var message = 'Adding column: ' + table + '.' + column; + return db.knex.schema.hasTable(table).then(function (exists) { + if (exists) { + return db.knex.schema.hasColumn(table, column).then(function (exists) { + if (!exists) { + logger.info(message); + return commands.addColumn(table, column); + } else { + logger.warn(message); + } + }); + } else { + // @TODO: this should probably be an error + logger.warn(message); + } + }); + }); +}; diff --git a/core/server/data/migration/005/index.js b/core/server/data/migration/005/index.js index 6856607482..d948cf72d5 100644 --- a/core/server/data/migration/005/index.js +++ b/core/server/data/migration/005/index.js @@ -1,4 +1,6 @@ module.exports = [ // Drop hidden column from tags table - require('./01-drop-hidden-column-from-tags') + require('./01-drop-hidden-column-from-tags'), + // Add visibility column to posts, tags, and users tables + require('./02-add-visibility-column-to-key-tables') ]; diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index e801bffda2..91cbad2ec4 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -11,6 +11,7 @@ module.exports = { page: {type: 'bool', nullable: false, defaultTo: false, validations: {isIn: [[0, 1, false, true]]}}, status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'draft'}, language: {type: 'string', maxlength: 6, nullable: false, defaultTo: 'en_US'}, + visibility: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'public', validations: {isIn: [['public']]}}, meta_title: {type: 'string', maxlength: 150, nullable: true}, meta_description: {type: 'string', maxlength: 200, nullable: true}, author_id: {type: 'integer', nullable: false}, @@ -36,6 +37,7 @@ module.exports = { accessibility: {type: 'text', maxlength: 65535, nullable: true}, status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'active'}, language: {type: 'string', maxlength: 6, nullable: false, defaultTo: 'en_US'}, + visibility: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'public', validations: {isIn: [['public']]}}, meta_title: {type: 'string', maxlength: 150, nullable: true}, meta_description: {type: 'string', maxlength: 200, nullable: true}, tour: {type: 'text', maxlength: 65535, nullable: true}, @@ -106,6 +108,7 @@ module.exports = { description: {type: 'string', maxlength: 200, nullable: true}, image: {type: 'text', maxlength: 2000, nullable: true}, parent_id: {type: 'integer', nullable: true}, + visibility: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'public', validations: {isIn: [['public', 'internal']]}}, meta_title: {type: 'string', maxlength: 150, nullable: true}, meta_description: {type: 'string', maxlength: 200, nullable: true}, created_at: {type: 'dateTime', nullable: false}, diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 5c3f56d519..7cddab6c65 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -32,7 +32,7 @@ var should = require('should'), describe('DB version integrity', function () { // Only these variables should need updating var currentDbVersion = '005', - currentSchemaHash = 'cc249220eb57b2249fc82f8494ad3912', + currentSchemaHash = '2b823f290d2ffa02ad5a10e31b77dab4', currentFixturesHash = '77ebb081539f9e0c49f487faf7fd929e'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, @@ -798,7 +798,7 @@ describe('Migrations', function () { tasksSpy.calledOnce.should.be.true(); tasksSpy.calledWith('005', loggerStub).should.be.true(); - tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(1); + tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(2); sequenceStub.calledTwice.should.be.true(); @@ -807,8 +807,9 @@ describe('Migrations', function () { sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); sequenceStub.secondCall.calledWith(sinon.match.array, loggerStub).should.be.true(); - sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(2); sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'dropHiddenColumnFromTags'); + sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'addVisibilityColumnToKeyTables'); // Reset sequence sequenceReset(); @@ -817,7 +818,7 @@ describe('Migrations', function () { }); describe('Tasks:', function () { - var dropColumnStub, + var dropColumnStub, addColumnStub, knexStub, knexMock; beforeEach(function () { @@ -830,6 +831,7 @@ describe('Migrations', function () { knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); dropColumnStub = sandbox.stub(schema.commands, 'dropColumn'); + addColumnStub = sandbox.stub(schema.commands, 'addColumn'); }); afterEach(function () { @@ -838,7 +840,7 @@ describe('Migrations', function () { it('should have tasks for 005', function () { should.exist(updates005); - updates005.should.be.an.Array().with.lengthOf(1); + updates005.should.be.an.Array().with.lengthOf(2); }); describe('01-drop-hidden-column-from-tags', function () { @@ -907,6 +909,130 @@ describe('Migrations', function () { }).catch(done); }); }); + + describe('02-add-visibility-column-to-key-tables', function () { + it('does not try to add new column if the table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts').returns(Promise.resolve(false)); + knexMock.schema.hasTable.withArgs('tags').returns(Promise.resolve(false)); + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(false)); + + // Execute + updates005[1](loggerStub).then(function () { + knexMock.schema.hasTable.calledThrice.should.be.true(); + knexMock.schema.hasTable.calledWith('posts').should.be.true(); + knexMock.schema.hasTable.calledWith('tags').should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.called.should.be.false(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledThrice.should.be.true(); + + done(); + }).catch(done); + }); + + it('does not try to add new columns if the columns already exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts').returns(Promise.resolve(true)); + knexMock.schema.hasTable.withArgs('tags').returns(Promise.resolve(true)); + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(true)); + + knexMock.schema.hasColumn.withArgs('posts', 'visibility').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('tags', 'visibility').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('users', 'visibility').returns(Promise.resolve(true)); + + // Execute + updates005[1](loggerStub).then(function () { + knexMock.schema.hasTable.calledThrice.should.be.true(); + knexMock.schema.hasTable.calledWith('posts').should.be.true(); + knexMock.schema.hasTable.calledWith('tags').should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.calledThrice.should.be.true(); + knexMock.schema.hasColumn.calledWith('posts', 'visibility').should.be.true(); + knexMock.schema.hasColumn.calledWith('tags', 'visibility').should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'visibility').should.be.true(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledThrice.should.be.true(); + + done(); + }).catch(done); + }); + + it('tries to add new columns if table is present but columns are not', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts').returns(Promise.resolve(true)); + knexMock.schema.hasTable.withArgs('tags').returns(Promise.resolve(true)); + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(true)); + + knexMock.schema.hasColumn.withArgs('posts', 'visibility').returns(Promise.resolve(false)); + knexMock.schema.hasColumn.withArgs('tags', 'visibility').returns(Promise.resolve(false)); + knexMock.schema.hasColumn.withArgs('users', 'visibility').returns(Promise.resolve(false)); + + // Execute + updates005[1](loggerStub).then(function () { + knexMock.schema.hasTable.calledThrice.should.be.true(); + knexMock.schema.hasTable.calledWith('posts').should.be.true(); + knexMock.schema.hasTable.calledWith('tags').should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.calledThrice.should.be.true(); + knexMock.schema.hasColumn.calledWith('posts', 'visibility').should.be.true(); + knexMock.schema.hasColumn.calledWith('tags', 'visibility').should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'visibility').should.be.true(); + + addColumnStub.calledThrice.should.be.true(); + addColumnStub.calledWith('posts', 'visibility').should.be.true(); + addColumnStub.calledWith('tags', 'visibility').should.be.true(); + addColumnStub.calledWith('users', 'visibility').should.be.true(); + + loggerStub.info.calledThrice.should.be.true(); + loggerStub.warn.called.should.be.false(); + + done(); + }).catch(done); + }); + + it('will only try to add columns that do not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts').returns(Promise.resolve(true)); + knexMock.schema.hasTable.withArgs('tags').returns(Promise.resolve(true)); + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(true)); + + knexMock.schema.hasColumn.withArgs('posts', 'visibility').returns(Promise.resolve(false)); + knexMock.schema.hasColumn.withArgs('tags', 'visibility').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('users', 'visibility').returns(Promise.resolve(false)); + // Execute + updates005[1](loggerStub).then(function () { + knexMock.schema.hasTable.calledThrice.should.be.true(); + knexMock.schema.hasTable.calledWith('posts').should.be.true(); + knexMock.schema.hasTable.calledWith('tags').should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.calledThrice.should.be.true(); + knexMock.schema.hasColumn.calledWith('posts', 'visibility').should.be.true(); + knexMock.schema.hasColumn.calledWith('tags', 'visibility').should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'visibility').should.be.true(); + + addColumnStub.calledTwice.should.be.true(); + addColumnStub.calledWith('posts', 'visibility').should.be.true(); + addColumnStub.calledWith('tags', 'visibility').should.be.false(); + addColumnStub.calledWith('users', 'visibility').should.be.true(); + + loggerStub.info.calledTwice.should.be.true(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + }); }); }); }); From 2685970d96c7676f3bcb9ba45eeae7c153012910 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Sat, 26 Mar 2016 14:07:59 +0000 Subject: [PATCH 5/8] Add mobiledoc column to posts refs #6301, #6255 - new, extra-long, column for storing mobiledoc content format --- .../005/03-add-mobiledoc-column-to-posts.js | 24 ++++++ core/server/data/migration/005/index.js | 4 +- core/server/data/schema/schema.js | 1 + core/test/unit/migration_spec.js | 76 ++++++++++++++++++- 4 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 core/server/data/migration/005/03-add-mobiledoc-column-to-posts.js diff --git a/core/server/data/migration/005/03-add-mobiledoc-column-to-posts.js b/core/server/data/migration/005/03-add-mobiledoc-column-to-posts.js new file mode 100644 index 0000000000..a771b131cc --- /dev/null +++ b/core/server/data/migration/005/03-add-mobiledoc-column-to-posts.js @@ -0,0 +1,24 @@ +var commands = require('../../schema').commands, + db = require('../../db'), + + table = 'posts', + column = 'mobiledoc', + message = 'Adding column: ' + table + '.' + column; + +module.exports = function addMobiledocColumnToPosts(logger) { + return db.knex.schema.hasTable(table).then(function (exists) { + if (exists) { + return db.knex.schema.hasColumn(table, column).then(function (exists) { + if (!exists) { + logger.info(message); + return commands.addColumn(table, column); + } else { + logger.warn(message); + } + }); + } else { + // @TODO: this should probably be an error + logger.warn(message); + } + }); +}; diff --git a/core/server/data/migration/005/index.js b/core/server/data/migration/005/index.js index d948cf72d5..56818fb59a 100644 --- a/core/server/data/migration/005/index.js +++ b/core/server/data/migration/005/index.js @@ -2,5 +2,7 @@ module.exports = [ // Drop hidden column from tags table require('./01-drop-hidden-column-from-tags'), // Add visibility column to posts, tags, and users tables - require('./02-add-visibility-column-to-key-tables') + require('./02-add-visibility-column-to-key-tables'), + // Add mobiledoc column to posts + require('./03-add-mobiledoc-column-to-posts') ]; diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index 91cbad2ec4..fa35cedcb0 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -5,6 +5,7 @@ module.exports = { title: {type: 'string', maxlength: 150, nullable: false}, slug: {type: 'string', maxlength: 150, nullable: false, unique: true}, markdown: {type: 'text', maxlength: 16777215, fieldtype: 'medium', nullable: true}, + mobiledoc: {type: 'text', maxlength: 1000000000, fieldtype: 'long', nullable: true}, html: {type: 'text', maxlength: 16777215, fieldtype: 'medium', nullable: true}, image: {type: 'text', maxlength: 2000, nullable: true}, featured: {type: 'bool', nullable: false, defaultTo: false, validations: {isIn: [[0, 1, false, true]]}}, diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 7cddab6c65..68c931d3b5 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -32,7 +32,7 @@ var should = require('should'), describe('DB version integrity', function () { // Only these variables should need updating var currentDbVersion = '005', - currentSchemaHash = '2b823f290d2ffa02ad5a10e31b77dab4', + currentSchemaHash = '4ae166ee14946fd617fcbe51b40daa7a', currentFixturesHash = '77ebb081539f9e0c49f487faf7fd929e'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, @@ -798,7 +798,7 @@ describe('Migrations', function () { tasksSpy.calledOnce.should.be.true(); tasksSpy.calledWith('005', loggerStub).should.be.true(); - tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(2); + tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(3); sequenceStub.calledTwice.should.be.true(); @@ -807,9 +807,10 @@ describe('Migrations', function () { sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); sequenceStub.secondCall.calledWith(sinon.match.array, loggerStub).should.be.true(); - sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(2); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(3); sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'dropHiddenColumnFromTags'); sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'addVisibilityColumnToKeyTables'); + sequenceStub.secondCall.args[0][2].should.be.a.Function().with.property('name', 'addMobiledocColumnToPosts'); // Reset sequence sequenceReset(); @@ -840,7 +841,7 @@ describe('Migrations', function () { it('should have tasks for 005', function () { should.exist(updates005); - updates005.should.be.an.Array().with.lengthOf(2); + updates005.should.be.an.Array().with.lengthOf(3); }); describe('01-drop-hidden-column-from-tags', function () { @@ -1033,6 +1034,73 @@ describe('Migrations', function () { }).catch(done); }); }); + + describe('03-add-mobiledoc-column-to-posts', function () { + it('does not try to add a new column if the table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts').returns(Promise.resolve(false)); + + // Execute + updates005[2](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('posts').should.be.true(); + + knexMock.schema.hasColumn.called.should.be.false(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + + it('does not try to add a new column if the column already exists', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts').returns(new Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('posts', 'mobiledoc').returns(Promise.resolve(true)); + + // Execute + updates005[2](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('posts').should.be.true(); + + knexMock.schema.hasColumn.calledOnce.should.be.true(); + knexMock.schema.hasColumn.calledWith('posts', 'mobiledoc').should.be.true(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + + it('tries to add a new column if table is present but column is not', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('posts', 'mobiledoc').returns(Promise.resolve(false)); + + // Execute + updates005[2](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('posts').should.be.true(); + + knexMock.schema.hasColumn.calledOnce.should.be.true(); + knexMock.schema.hasColumn.calledWith('posts', 'mobiledoc').should.be.true(); + + addColumnStub.calledOnce.should.be.true(); + addColumnStub.calledWith('posts', 'mobiledoc').should.be.true(); + + loggerStub.info.calledOnce.should.be.true(); + loggerStub.warn.called.should.be.false(); + + done(); + }).catch(done); + }); + }); }); }); }); From 739977a368c8580e617569b830895f06d959c0e2 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Sat, 26 Mar 2016 14:28:52 +0000 Subject: [PATCH 6/8] Add social media columns to users refs #6301, #6534 - adds facebook and twitter columns, which should contain urls --- .../04-add-social-media-columns-to-users.js | 27 +++++ core/server/data/migration/005/index.js | 5 +- core/server/data/schema/schema.js | 2 + core/test/unit/migration_spec.js | 107 +++++++++++++++++- 4 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 core/server/data/migration/005/04-add-social-media-columns-to-users.js diff --git a/core/server/data/migration/005/04-add-social-media-columns-to-users.js b/core/server/data/migration/005/04-add-social-media-columns-to-users.js new file mode 100644 index 0000000000..674e2d92c3 --- /dev/null +++ b/core/server/data/migration/005/04-add-social-media-columns-to-users.js @@ -0,0 +1,27 @@ +var Promise = require('bluebird'), + commands = require('../../schema').commands, + db = require('../../db'), + + table = 'users', + columns = ['facebook', 'twitter']; + +module.exports = function addSocialMediaColumnsToUsers(logger) { + return db.knex.schema.hasTable(table).then(function (exists) { + if (exists) { + return Promise.mapSeries(columns, function (column) { + var message = 'Adding column: ' + table + '.' + column; + return db.knex.schema.hasColumn(table, column).then(function (exists) { + if (!exists) { + logger.info(message); + return commands.addColumn(table, column); + } else { + logger.warn(message); + } + }); + }); + } else { + // @TODO: this should probably be an error + logger.warn('Adding columns to table: ' + table); + } + }); +}; diff --git a/core/server/data/migration/005/index.js b/core/server/data/migration/005/index.js index 56818fb59a..c15a9763af 100644 --- a/core/server/data/migration/005/index.js +++ b/core/server/data/migration/005/index.js @@ -4,5 +4,8 @@ module.exports = [ // Add visibility column to posts, tags, and users tables require('./02-add-visibility-column-to-key-tables'), // Add mobiledoc column to posts - require('./03-add-mobiledoc-column-to-posts') + require('./03-add-mobiledoc-column-to-posts'), + // Add social media columns to isers + require('./04-add-social-media-columns-to-users') + ]; diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index fa35cedcb0..488e4e95d4 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -35,6 +35,8 @@ module.exports = { bio: {type: 'string', maxlength: 200, nullable: true}, website: {type: 'text', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}}, location: {type: 'text', maxlength: 65535, nullable: true}, + facebook: {type: 'text', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}}, + twitter: {type: 'text', maxlength: 2000, nullable: true, validations: {isEmptyOrURL: true}}, accessibility: {type: 'text', maxlength: 65535, nullable: true}, status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'active'}, language: {type: 'string', maxlength: 6, nullable: false, defaultTo: 'en_US'}, diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 68c931d3b5..4d0d663d9d 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -32,7 +32,7 @@ var should = require('should'), describe('DB version integrity', function () { // Only these variables should need updating var currentDbVersion = '005', - currentSchemaHash = '4ae166ee14946fd617fcbe51b40daa7a', + currentSchemaHash = 'be706cdbeb06103d90703ee733efc556', currentFixturesHash = '77ebb081539f9e0c49f487faf7fd929e'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, @@ -798,7 +798,7 @@ describe('Migrations', function () { tasksSpy.calledOnce.should.be.true(); tasksSpy.calledWith('005', loggerStub).should.be.true(); - tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(3); + tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(4); sequenceStub.calledTwice.should.be.true(); @@ -807,10 +807,11 @@ describe('Migrations', function () { sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); sequenceStub.secondCall.calledWith(sinon.match.array, loggerStub).should.be.true(); - sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(3); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(4); sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'dropHiddenColumnFromTags'); sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'addVisibilityColumnToKeyTables'); sequenceStub.secondCall.args[0][2].should.be.a.Function().with.property('name', 'addMobiledocColumnToPosts'); + sequenceStub.secondCall.args[0][3].should.be.a.Function().with.property('name', 'addSocialMediaColumnsToUsers'); // Reset sequence sequenceReset(); @@ -841,7 +842,7 @@ describe('Migrations', function () { it('should have tasks for 005', function () { should.exist(updates005); - updates005.should.be.an.Array().with.lengthOf(3); + updates005.should.be.an.Array().with.lengthOf(4); }); describe('01-drop-hidden-column-from-tags', function () { @@ -1101,6 +1102,104 @@ describe('Migrations', function () { }).catch(done); }); }); + + describe('04-add-social-media-columns-to-users', function () { + it('does not try to add new columns if the table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(false)); + + // Execute + updates005[3](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.called.should.be.false(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + + it('does not try to add new columns if the columns already exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('users', 'facebook').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('users', 'twitter').returns(Promise.resolve(true)); + + // Execute + updates005[3](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.calledTwice.should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'facebook').should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'twitter').should.be.true(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledTwice.should.be.true(); + + done(); + }).catch(done); + }); + + it('tries to add new columns if table is present but columns are not', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('users', 'facebook').returns(Promise.resolve(false)); + knexMock.schema.hasColumn.withArgs('users', 'twitter').returns(Promise.resolve(false)); + + // Execute + updates005[3](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.calledTwice.should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'facebook').should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'twitter').should.be.true(); + + addColumnStub.calledTwice.should.be.true(); + addColumnStub.calledWith('users', 'facebook').should.be.true(); + addColumnStub.calledWith('users', 'twitter').should.be.true(); + + loggerStub.info.calledTwice.should.be.true(); + loggerStub.warn.called.should.be.false(); + + done(); + }).catch(done); + }); + + it('will only try to add columns that do not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('users').returns(Promise.resolve(true)); + knexMock.schema.hasColumn.withArgs('users', 'facebook').returns(Promise.resolve(false)); + knexMock.schema.hasColumn.withArgs('users', 'twitter').returns(Promise.resolve(true)); + + // Execute + updates005[3](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.calledTwice.should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'facebook').should.be.true(); + knexMock.schema.hasColumn.calledWith('users', 'twitter').should.be.true(); + + addColumnStub.callCount.should.eql(1); + addColumnStub.calledWith('users', 'facebook').should.be.true(); + addColumnStub.calledWith('users', 'twitter').should.be.false(); + + loggerStub.info.calledOnce.should.be.true(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + }); }); }); }); From 8b9734ea315caa0b2a5383c3d889c1165359b83d Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Sun, 10 Apr 2016 19:22:00 +0100 Subject: [PATCH 7/8] Add new ghost-scheduler client refs #6301, #6399 - new scheduler client will be used by any web app that handles time and calls back to the scheduling API at the right time - new scheduler client will need to be confidential, rather than public, hence the 'web' type instead of 'ua' - adds validation to client type that it must have a type of 'ua', 'web', or 'native' --- .../005/02-add-ghost-scheduler-client.js | 16 +++++++ .../data/migration/fixtures/005/index.js | 4 +- .../data/migration/fixtures/fixtures.json | 6 +++ core/server/data/schema/schema.js | 2 +- core/test/integration/migration_spec.js | 3 +- core/test/unit/migration_fixture_spec.js | 47 +++++++++++++++++-- core/test/unit/migration_spec.js | 2 +- 7 files changed, 72 insertions(+), 8 deletions(-) create mode 100644 core/server/data/migration/fixtures/005/02-add-ghost-scheduler-client.js diff --git a/core/server/data/migration/fixtures/005/02-add-ghost-scheduler-client.js b/core/server/data/migration/fixtures/005/02-add-ghost-scheduler-client.js new file mode 100644 index 0000000000..6e1d57be04 --- /dev/null +++ b/core/server/data/migration/fixtures/005/02-add-ghost-scheduler-client.js @@ -0,0 +1,16 @@ +// Create a new `ghost-scheduler` client for use in themes +var models = require('../../../../models'), + + schedulerClient = require('../utils').findModelFixtureEntry('Client', {slug: 'ghost-scheduler'}), + message = 'Add ghost-scheduler client fixture'; + +module.exports = function addGhostFrontendClient(options, logger) { + return models.Client.findOne({slug: schedulerClient.slug}).then(function (client) { + if (!client) { + logger.info(message); + return models.Client.add(schedulerClient, options); + } else { + logger.warn(message); + } + }); +}; diff --git a/core/server/data/migration/fixtures/005/index.js b/core/server/data/migration/fixtures/005/index.js index 291f252e04..8fccf79b8c 100644 --- a/core/server/data/migration/fixtures/005/index.js +++ b/core/server/data/migration/fixtures/005/index.js @@ -1,4 +1,6 @@ module.exports = [ // add jquery setting and privacy info - require('./01-update-ghost-client-secrets') + require('./01-update-ghost-client-secrets'), + // add ghost-scheduler client + require('./02-add-ghost-scheduler-client') ]; diff --git a/core/server/data/migration/fixtures/fixtures.json b/core/server/data/migration/fixtures/fixtures.json index bf99d57ee2..675a5920de 100644 --- a/core/server/data/migration/fixtures/fixtures.json +++ b/core/server/data/migration/fixtures/fixtures.json @@ -42,6 +42,12 @@ "name": "Ghost Frontend", "slug": "ghost-frontend", "status": "enabled" + }, + { + "name": "Ghost Scheduler", + "slug": "ghost-scheduler", + "status": "enabled", + "type": "web" } ] }, diff --git a/core/server/data/schema/schema.js b/core/server/data/schema/schema.js index 488e4e95d4..6bb36fc952 100644 --- a/core/server/data/schema/schema.js +++ b/core/server/data/schema/schema.js @@ -172,7 +172,7 @@ module.exports = { redirection_uri: {type: 'string', maxlength: 2000, nullable: true}, logo: {type: 'string', maxlength: 2000, nullable: true}, status: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'development'}, - type: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'ua'}, + type: {type: 'string', maxlength: 150, nullable: false, defaultTo: 'ua', validations: {isIn: [['ua', 'web', 'native']]}}, description: {type: 'string', maxlength: 200, nullable: true}, created_at: {type: 'dateTime', nullable: false}, created_by: {type: 'integer', nullable: false}, diff --git a/core/test/integration/migration_spec.js b/core/test/integration/migration_spec.js index 9923c0537e..22ce1c0805 100644 --- a/core/test/integration/migration_spec.js +++ b/core/test/integration/migration_spec.js @@ -159,9 +159,10 @@ describe('Database Migration (special functions)', function () { // Clients should.exist(result.clients); - result.clients.length.should.eql(2); + result.clients.length.should.eql(3); result.clients.at(0).get('name').should.eql('Ghost Admin'); result.clients.at(1).get('name').should.eql('Ghost Frontend'); + result.clients.at(2).get('name').should.eql('Ghost Scheduler'); // User (Owner) should.exist(result.users); diff --git a/core/test/unit/migration_fixture_spec.js b/core/test/unit/migration_fixture_spec.js index a75c9e9fff..07b6d6961f 100644 --- a/core/test/unit/migration_fixture_spec.js +++ b/core/test/unit/migration_fixture_spec.js @@ -718,8 +718,9 @@ describe('Fixtures', function () { sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); sequenceStub.secondCall.calledWith(sinon.match.array, sinon.match.object, loggerStub).should.be.true(); - sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(2); sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'updateGhostClientsSecrets'); + sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'addGhostFrontendClient'); // Reset sequenceReset(); @@ -730,7 +731,7 @@ describe('Fixtures', function () { describe('Tasks:', function () { it('should have tasks for 005', function () { should.exist(fixtures005); - fixtures005.should.be.an.Array().with.lengthOf(1); + fixtures005.should.be.an.Array().with.lengthOf(2); }); describe('01-update-ghost-client-secrets', function () { @@ -774,6 +775,44 @@ describe('Fixtures', function () { }).catch(done); }); }); + + describe('02-add-ghost-scheduler-client', function () { + var clientOneStub; + + beforeEach(function () { + clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve({})); + }); + + it('tries to add client correctly', function (done) { + var clientAddStub = sandbox.stub(models.Client, 'add').returns(Promise.resolve()); + clientOneStub.returns(Promise.resolve()); + + fixtures005[1]({}, loggerStub).then(function () { + clientOneStub.calledOnce.should.be.true(); + clientOneStub.calledWith({slug: 'ghost-scheduler'}).should.be.true(); + clientAddStub.calledOnce.should.be.true(); + loggerStub.info.calledOnce.should.be.true(); + loggerStub.warn.called.should.be.false(); + sinon.assert.callOrder(clientOneStub, loggerStub.info, clientAddStub); + + done(); + }).catch(done); + }); + + it('does not try to add client if it already exists', function (done) { + var clientAddStub = sandbox.stub(models.Client, 'add').returns(Promise.resolve()); + + fixtures005[1]({}, loggerStub).then(function () { + clientOneStub.calledOnce.should.be.true(); + clientOneStub.calledWith({slug: 'ghost-scheduler'}).should.be.true(); + clientAddStub.called.should.be.false(); + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + }); }); }); }); @@ -821,8 +860,8 @@ describe('Fixtures', function () { tagAddStub.calledOnce.should.be.true(); roleOneStub.callCount.should.be.aboveOrEqual(4); roleAddStub.callCount.should.eql(4); - clientOneStub.calledTwice.should.be.true(); - clientAddStub.calledTwice.should.be.true(); + clientOneStub.calledThrice.should.be.true(); + clientAddStub.calledThrice.should.be.true(); permOneStub.callCount.should.eql(30); permsAddStub.called.should.be.true(); diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 4d0d663d9d..0b9e328bbf 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -33,7 +33,7 @@ describe('DB version integrity', function () { // Only these variables should need updating var currentDbVersion = '005', currentSchemaHash = 'be706cdbeb06103d90703ee733efc556', - currentFixturesHash = '77ebb081539f9e0c49f487faf7fd929e'; + currentFixturesHash = '21dd859601c8e1c12eaff9eccfbe966a'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, // and the values above will need updating as confirmation From 5884fe0323d5254b9ee14702b003c8f42cb4bf76 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Fri, 8 Apr 2016 10:09:26 +0100 Subject: [PATCH 8/8] Add permissions models & relations for clients refs #6301, #4176 Add migration for: - 5 new client permissions - 15 relations between the admin, editor & author role and the 5 new permissions - updates to tests to show that permissions get updated properly --- .../fixtures/005/03-add-client-permissions.js | 30 ++++++++ .../data/migration/fixtures/005/index.js | 4 +- .../data/migration/fixtures/fixtures.json | 34 ++++++++- core/test/integration/migration_spec.js | 69 +++++++++++++++++- core/test/unit/migration_fixture_spec.js | 70 +++++++++++++++++-- .../test/unit/migration_fixture_utils_spec.js | 16 ++--- core/test/unit/migration_spec.js | 2 +- 7 files changed, 205 insertions(+), 20 deletions(-) create mode 100644 core/server/data/migration/fixtures/005/03-add-client-permissions.js diff --git a/core/server/data/migration/fixtures/005/03-add-client-permissions.js b/core/server/data/migration/fixtures/005/03-add-client-permissions.js new file mode 100644 index 0000000000..bd56051dac --- /dev/null +++ b/core/server/data/migration/fixtures/005/03-add-client-permissions.js @@ -0,0 +1,30 @@ +// Update the permissions & permissions_roles tables to get the new entries +var utils = require('../utils'); + +function getClientPermissions() { + return utils.findModelFixtures('Permission', {object_type: 'client'}); +} + +function getClientRelations() { + return utils.findPermissionRelationsForObject('client'); +} + +function printResult(logger, result, message) { + if (result.done === result.expected) { + logger.info(message); + } else { + logger.warn('(' + result.done + '/' + result.expected + ') ' + message); + } +} + +module.exports = function addClientPermissions(options, logger) { + var modelToAdd = getClientPermissions(), + relationToAdd = getClientRelations(); + + return utils.addFixturesForModel(modelToAdd).then(function (result) { + printResult(logger, result, 'Adding permissions fixtures for clients'); + return utils.addFixturesForRelation(relationToAdd); + }).then(function (result) { + printResult(logger, result, 'Adding permissions_roles fixtures for clients'); + }); +}; diff --git a/core/server/data/migration/fixtures/005/index.js b/core/server/data/migration/fixtures/005/index.js index 8fccf79b8c..ee2a4d994a 100644 --- a/core/server/data/migration/fixtures/005/index.js +++ b/core/server/data/migration/fixtures/005/index.js @@ -2,5 +2,7 @@ module.exports = [ // add jquery setting and privacy info require('./01-update-ghost-client-secrets'), // add ghost-scheduler client - require('./02-add-ghost-scheduler-client') + require('./02-add-ghost-scheduler-client'), + // add client permissions and permission_role relations + require('./03-add-client-permissions') ]; diff --git a/core/server/data/migration/fixtures/fixtures.json b/core/server/data/migration/fixtures/fixtures.json index 675a5920de..ad2835b499 100644 --- a/core/server/data/migration/fixtures/fixtures.json +++ b/core/server/data/migration/fixtures/fixtures.json @@ -224,6 +224,31 @@ "name": "Browse roles", "action_type": "browse", "object_type": "role" + }, + { + "name": "Browse clients", + "action_type": "browse", + "object_type": "client" + }, + { + "name": "Read clients", + "action_type": "read", + "object_type": "client" + }, + { + "name": "Edit clients", + "action_type": "edit", + "object_type": "client" + }, + { + "name": "Add clients", + "action_type": "add", + "object_type": "client" + }, + { + "name": "Delete clients", + "action_type": "destroy", + "object_type": "client" } ] } @@ -251,7 +276,8 @@ "tag": "all", "theme": "all", "user": "all", - "role": "all" + "role": "all", + "client": "all" }, "Editor": { "post": "all", @@ -259,7 +285,8 @@ "slug": "all", "tag": "all", "user": "all", - "role": "all" + "role": "all", + "client": "all" }, "Author": { "post": ["browse", "read", "add"], @@ -267,7 +294,8 @@ "slug": "all", "tag": ["browse", "read", "add"], "user": ["browse", "read"], - "role": ["browse"] + "role": ["browse"], + "client": "all" } } }, diff --git a/core/test/integration/migration_spec.js b/core/test/integration/migration_spec.js index 22ce1c0805..08b49347df 100644 --- a/core/test/integration/migration_spec.js +++ b/core/test/integration/migration_spec.js @@ -6,6 +6,7 @@ var testUtils = require('../utils'), Promise = require('bluebird'), fixtures = require('../../server/data/migration/fixtures'), + fixtures005 = require('../../server/data/migration/fixtures/005'), Models = require('../../server/models'), sandbox = sinon.sandbox.create(); @@ -122,6 +123,18 @@ describe('Database Migration (special functions)', function () { permissions[28].should.be.AssignedToRoles(['Administrator', 'Editor']); permissions[29].name.should.eql('Browse roles'); permissions[29].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + + // Clients + permissions[30].name.should.eql('Browse clients'); + permissions[30].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[31].name.should.eql('Read clients'); + permissions[31].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[32].name.should.eql('Edit clients'); + permissions[32].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[33].name.should.eql('Add clients'); + permissions[33].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[34].name.should.eql('Delete clients'); + permissions[34].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); }); describe('Populate', function () { @@ -180,7 +193,7 @@ describe('Database Migration (special functions)', function () { result.roles.at(3).get('name').should.eql('Owner'); // Permissions - result.permissions.length.should.eql(30); + result.permissions.length.should.eql(35); result.permissions.toJSON().should.be.CompletePermissions(); done(); @@ -188,6 +201,60 @@ describe('Database Migration (special functions)', function () { }).catch(done); }); }); + + describe('Update', function () { + // We need the roles, and lets add some other perms to simulate the "Update" environment + beforeEach(testUtils.setup('users:roles', 'perms:db', 'perms:init')); + + it('should update client permissions correctly', function (done) { + fixtures005[2]({}, loggerStub).then(function () { + var props = { + roles: Models.Role.findAll(), + permissions: Models.Permission.findAll({include: ['roles']}) + }, permissions; + + loggerStub.info.called.should.be.true(); + loggerStub.warn.called.should.be.false(); + + return Promise.props(props).then(function (result) { + should.exist(result); + + should.exist(result.roles); + result.roles.length.should.eql(4); + result.roles.at(0).get('name').should.eql('Administrator'); + result.roles.at(1).get('name').should.eql('Editor'); + result.roles.at(2).get('name').should.eql('Author'); + result.roles.at(3).get('name').should.eql('Owner'); + + // Permissions + result.permissions.length.should.eql(8); + permissions = result.permissions.toJSON(); + + // DB Perms + permissions[0].name.should.eql('Export database'); + permissions[0].should.be.AssignedToRoles(['Administrator']); + permissions[1].name.should.eql('Import database'); + permissions[1].should.be.AssignedToRoles(['Administrator']); + permissions[2].name.should.eql('Delete all content'); + permissions[2].should.be.AssignedToRoles(['Administrator']); + + // Client Perms + permissions[3].name.should.eql('Browse clients'); + permissions[3].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[4].name.should.eql('Read clients'); + permissions[4].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[5].name.should.eql('Edit clients'); + permissions[5].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[6].name.should.eql('Add clients'); + permissions[6].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + permissions[7].name.should.eql('Delete clients'); + permissions[7].should.be.AssignedToRoles(['Administrator', 'Editor', 'Author']); + + done(); + }).catch(done); + }); + }); + }); }); }); diff --git a/core/test/unit/migration_fixture_spec.js b/core/test/unit/migration_fixture_spec.js index 07b6d6961f..c4af88ecdf 100644 --- a/core/test/unit/migration_fixture_spec.js +++ b/core/test/unit/migration_fixture_spec.js @@ -11,6 +11,7 @@ var should = require('should'), versioning = require('../../server/data/schema/versioning'), update = rewire('../../server/data/migration/fixtures/update'), populate = rewire('../../server/data/migration/fixtures/populate'), + fixtureUtils = require('../../server/data/migration/fixtures/utils'), fixtures004 = require('../../server/data/migration/fixtures/004'), fixtures005 = require('../../server/data/migration/fixtures/005'), ensureDefaultSettings = require('../../server/data/migration/fixtures/settings'), @@ -718,9 +719,10 @@ describe('Fixtures', function () { sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); sequenceStub.secondCall.calledWith(sinon.match.array, sinon.match.object, loggerStub).should.be.true(); - sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(2); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(3); sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'updateGhostClientsSecrets'); sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'addGhostFrontendClient'); + sequenceStub.secondCall.args[0][2].should.be.a.Function().with.property('name', 'addClientPermissions'); // Reset sequenceReset(); @@ -731,7 +733,7 @@ describe('Fixtures', function () { describe('Tasks:', function () { it('should have tasks for 005', function () { should.exist(fixtures005); - fixtures005.should.be.an.Array().with.lengthOf(2); + fixtures005.should.be.an.Array().with.lengthOf(3); }); describe('01-update-ghost-client-secrets', function () { @@ -813,6 +815,62 @@ describe('Fixtures', function () { }).catch(done); }); }); + + describe('03-add-client-permissions', function () { + var modelResult, addModelStub, relationResult, addRelationStub; + + beforeEach(function () { + modelResult = {expected: 1, done: 1}; + addModelStub = sandbox.stub(fixtureUtils, 'addFixturesForModel') + .returns(Promise.resolve(modelResult)); + + relationResult = {expected: 1, done: 1}; + addRelationStub = sandbox.stub(fixtureUtils, 'addFixturesForRelation') + .returns(Promise.resolve(relationResult)); + }); + + it('should find the correct model & relation to add', function (done) { + // Execute + fixtures005[2]({}, loggerStub).then(function () { + addModelStub.calledOnce.should.be.true(); + addModelStub.calledWith( + fixtureUtils.findModelFixtures('Permission', {object_type: 'client'}) + ).should.be.true(); + + addRelationStub.calledOnce.should.be.true(); + addRelationStub.calledWith( + fixtureUtils.findPermissionRelationsForObject('client') + ).should.be.true(); + + loggerStub.info.calledTwice.should.be.true(); + loggerStub.warn.called.should.be.false(); + + done(); + }); + }); + + it('should warn the result shows less work was done than expected', function (done) { + // Setup + modelResult.expected = 3; + // Execute + fixtures005[2]({}, loggerStub).then(function () { + addModelStub.calledOnce.should.be.true(); + addModelStub.calledWith( + fixtureUtils.findModelFixtures('Permission', {object_type: 'client'}) + ).should.be.true(); + + addRelationStub.calledOnce.should.be.true(); + addRelationStub.calledWith( + fixtureUtils.findPermissionRelationsForObject('client') + ).should.be.true(); + + loggerStub.info.calledOnce.should.be.true(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }); + }); + }); }); }); }); @@ -863,9 +921,9 @@ describe('Fixtures', function () { clientOneStub.calledThrice.should.be.true(); clientAddStub.calledThrice.should.be.true(); - permOneStub.callCount.should.eql(30); + permOneStub.callCount.should.eql(35); permsAddStub.called.should.be.true(); - permsAddStub.callCount.should.eql(30); + permsAddStub.callCount.should.eql(35); permsAllStub.calledOnce.should.be.true(); rolesAllStub.calledOnce.should.be.true(); @@ -874,8 +932,8 @@ describe('Fixtures', function () { // Relations modelMethodStub.filter.called.should.be.true(); - // 22 permissions, 1 tag - modelMethodStub.filter.callCount.should.eql(22 + 1); + // 25 permissions, 1 tag + modelMethodStub.filter.callCount.should.eql(25 + 1); modelMethodStub.find.called.should.be.true(); // 3 roles, 1 post modelMethodStub.find.callCount.should.eql(3 + 1); diff --git a/core/test/unit/migration_fixture_utils_spec.js b/core/test/unit/migration_fixture_utils_spec.js index 1bb8a54cfc..c619945aae 100644 --- a/core/test/unit/migration_fixture_utils_spec.js +++ b/core/test/unit/migration_fixture_utils_spec.js @@ -152,21 +152,21 @@ describe('Utils', function () { fixtureUtils.addFixturesForRelation(fixtures.relations[0]).then(function (result) { should.exist(result); result.should.be.an.Object(); - result.should.have.property('expected', 22); - result.should.have.property('done', 22); + result.should.have.property('expected', 25); + result.should.have.property('done', 25); // Permissions & Roles permsAllStub.calledOnce.should.be.true(); rolesAllStub.calledOnce.should.be.true(); - dataMethodStub.filter.callCount.should.eql(22); + dataMethodStub.filter.callCount.should.eql(25); dataMethodStub.find.callCount.should.eql(3); - fromItem.related.callCount.should.eql(22); - fromItem.findWhere.callCount.should.eql(22); - toItem[0].get.callCount.should.eql(44); + fromItem.related.callCount.should.eql(25); + fromItem.findWhere.callCount.should.eql(25); + toItem[0].get.callCount.should.eql(50); - fromItem.permissions.callCount.should.eql(22); - fromItem.attach.callCount.should.eql(22); + fromItem.permissions.callCount.should.eql(25); + fromItem.attach.callCount.should.eql(25); fromItem.attach.calledWith(toItem).should.be.true(); done(); diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 0b9e328bbf..94b2b46a00 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -33,7 +33,7 @@ describe('DB version integrity', function () { // Only these variables should need updating var currentDbVersion = '005', currentSchemaHash = 'be706cdbeb06103d90703ee733efc556', - currentFixturesHash = '21dd859601c8e1c12eaff9eccfbe966a'; + currentFixturesHash = 'ba195b645386b019a69c4b79e6854138'; // If this test is failing, then it is likely a change has been made that requires a DB version bump, // and the values above will need updating as confirmation