From 8bf55f7b2887a3147e55ea445360b21b2af51974 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Thu, 2 Mar 2017 20:08:27 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=E2=9C=A8=20Move=20activation=20to=20themes?= =?UTF-8?q?=20endpoint?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - browse will now include the correct activated theme again - PUT /theme/:name/activate will activate a theme TODO: tests! --- core/server/api/app.js | 5 +++++ core/server/api/themes.js | 19 +++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/core/server/api/app.js b/core/server/api/app.js index 9b7815adf46..1108323c6b6 100644 --- a/core/server/api/app.js +++ b/core/server/api/app.js @@ -149,6 +149,11 @@ function apiRoutes() { api.http(api.themes.upload) ); + apiRouter.put('/themes/:name/activate', + authenticatePrivate, + api.http(api.themes.activate) + ); + apiRouter.del('/themes/:name', authenticatePrivate, api.http(api.themes.destroy) diff --git a/core/server/api/themes.js b/core/server/api/themes.js index afe109643e2..d7b5ed54575 100644 --- a/core/server/api/themes.js +++ b/core/server/api/themes.js @@ -13,6 +13,8 @@ var debug = require('debug')('ghost:api:themes'), apiUtils = require('./utils'), utils = require('./../utils'), i18n = require('../i18n'), + settings = require('./settings'), + settingsCache = require('../settings/cache'), themeUtils = require('../themes'), themeList = themeUtils.list, packageUtils = require('../utils/packages'), @@ -26,11 +28,24 @@ var debug = require('debug')('ghost:api:themes'), themes = { browse: function browse() { debug('browsing'); - var result = packageUtils.filterPackages(themeList.getAll()); + var result = packageUtils.filterPackages(themeList.getAll(), settingsCache.get('activeTheme')); debug('got result'); return Promise.resolve({themes: result}); }, + activate: function activate(options) { + var themeName = options.name, + newSettings = [{ + key: 'activeTheme', + value: themeName + }]; + + return settings.edit({settings: newSettings}, options).then(function () { + var result = packageUtils.filterPackages(themeList.getAll(), themeName); + return Promise.resolve({themes: result}); + }); + }, + upload: function upload(options) { options = options || {}; @@ -90,7 +105,7 @@ themes = { // @TODO fix this craziness var toFilter = {}; toFilter[zip.shortName] = themeObject; - themeObject = packageUtils.filterPackages(toFilter); + themeObject = packageUtils.filterPackages(toFilter, zip.shortName); // gscan theme structure !== ghost theme structure if (theme.results.warning.length > 0) { themeObject.warnings = _.cloneDeep(theme.results.warning); From d3f2f1cd8a3a2281f606a9f535e346374a835ecb Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 6 Mar 2017 18:49:08 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=8E=A8=20Cleanup=20themes=20API=20a?= =?UTF-8?q?=20bit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/server/api/themes.js | 12 +++++------- core/server/themes/list.js | 13 +++++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/core/server/api/themes.js b/core/server/api/themes.js index d7b5ed54575..a5571f03b9d 100644 --- a/core/server/api/themes.js +++ b/core/server/api/themes.js @@ -17,7 +17,6 @@ var debug = require('debug')('ghost:api:themes'), settingsCache = require('../settings/cache'), themeUtils = require('../themes'), themeList = themeUtils.list, - packageUtils = require('../utils/packages'), themes; /** @@ -28,7 +27,7 @@ var debug = require('debug')('ghost:api:themes'), themes = { browse: function browse() { debug('browsing'); - var result = packageUtils.filterPackages(themeList.getAll(), settingsCache.get('activeTheme')); + var result = themeList.toAPI(themeList.getAll(), settingsCache.get('activeTheme')); debug('got result'); return Promise.resolve({themes: result}); }, @@ -41,7 +40,7 @@ themes = { }]; return settings.edit({settings: newSettings}, options).then(function () { - var result = packageUtils.filterPackages(themeList.getAll(), themeName); + var result = themeList.toAPI(themeList.getAll(), themeName); return Promise.resolve({themes: result}); }); }, @@ -102,14 +101,12 @@ themes = { return themeUtils.loadOne(zip.shortName); }) .then(function (themeObject) { - // @TODO fix this craziness - var toFilter = {}; - toFilter[zip.shortName] = themeObject; - themeObject = packageUtils.filterPackages(toFilter, zip.shortName); + themeObject = themeList.toAPI(themeObject, zip.shortName); // gscan theme structure !== ghost theme structure if (theme.results.warning.length > 0) { themeObject.warnings = _.cloneDeep(theme.results.warning); } + return {themes: themeObject}; }) .finally(function () { @@ -173,6 +170,7 @@ themes = { .then(function () { themeList.del(name); events.emit('theme.deleted', name); + // Delete returns an empty 204 response }); } }; diff --git a/core/server/themes/list.js b/core/server/themes/list.js index 287cdea27ce..e1df9db17bd 100644 --- a/core/server/themes/list.js +++ b/core/server/themes/list.js @@ -2,6 +2,7 @@ * Store themes after loading them from the file system */ var _ = require('lodash'), + packages = require('../utils/packages'), themeListCache = {}; module.exports = { @@ -32,5 +33,17 @@ module.exports = { }); return themeListCache; + }, + toAPI: function toAPI(themes, active) { + var toFilter; + + if (themes.hasOwnProperty('name')) { + toFilter = {}; + toFilter[themes.name] = themes; + } else { + toFilter = themes; + } + + return packages.filterPackages(toFilter, active); } }; From 8170d135c0c67c1b6f48c05f9d2774a6fef6c7a6 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Mon, 6 Mar 2017 18:49:39 +0000 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=9A=A8=20Imprv=20tests:=20use=20tmpdi?= =?UTF-8?q?r=20and=20check=20errors?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tests now read from a temp directory not content/themes - all tests check errors and responses --- .../test/functional/routes/api/themes_spec.js | 290 ++++++++++++------ core/test/utils/api.js | 2 +- 2 files changed, 197 insertions(+), 95 deletions(-) diff --git a/core/test/functional/routes/api/themes_spec.js b/core/test/functional/routes/api/themes_spec.js index c362e71724b..e1eed7cb6e0 100644 --- a/core/test/functional/routes/api/themes_spec.js +++ b/core/test/functional/routes/api/themes_spec.js @@ -2,13 +2,12 @@ var testUtils = require('../../../utils'), should = require('should'), supertest = require('supertest'), fs = require('fs-extra'), - path = require('path'), + join = require('path').join, + tmp = require('tmp'), _ = require('lodash'), - ghost = testUtils.startGhost, - config = require('../../../../../core/server/config'), request; -describe('Themes API', function () { +describe('Themes API (Forked)', function () { var scope = { ownerAccessToken: '', editorAccessToken: '', @@ -22,46 +21,68 @@ describe('Themes API', function () { .attach(fieldName, themePath); }, editor: null - }, ghostServer; + }, forkedGhost, tmpContentPath; - before(function (done) { - ghost().then(function (_ghostServer) { - ghostServer = _ghostServer; - return ghostServer.start(); - }).then(function () { - request = supertest.agent(config.get('url')); - }).then(function () { - return testUtils.doAuth(request); - }).then(function (token) { - scope.ownerAccessToken = token; - - return testUtils.createUser({ - user: testUtils.DataGenerator.forKnex.createUser({email: 'test+1@ghost.org'}), - role: testUtils.DataGenerator.Content.roles[1] - }); - }).then(function (user) { - scope.editor = user; - - request.user = scope.editor; - return testUtils.doAuth(request); - }).then(function (token) { - scope.editorAccessToken = token; - done(); - }).catch(done); - }); + function setupThemesFolder() { + tmpContentPath = tmp.dirSync({unsafeCleanup: true}); - after(function () { - // clean successful uploaded themes - fs.removeSync(config.getContentPath('themes') + '/valid'); - fs.removeSync(config.getContentPath('themes') + '/casper.zip'); + fs.mkdirSync(join(tmpContentPath.name, 'themes')); + fs.mkdirSync(join(tmpContentPath.name, 'themes', 'casper')); + fs.writeFileSync( + join(tmpContentPath.name, 'themes', 'casper', 'package.json'), + JSON.stringify({name: 'casper', version: '0.1.2'}) + ); + } - // gscan creates /test/tmp in test mode - fs.removeSync(config.get('paths').appRoot + '/test'); + function teardownThemesFolder() { + return tmpContentPath.removeCallback(); + } - return testUtils.clearData() + before(function (done) { + // Setup a temporary themes directory + setupThemesFolder(); + // Fork Ghost to read from the temp directory, not the developer's themes + testUtils.fork.ghost({ + paths: { + contentPath: tmpContentPath.name + } + }, 'themetests') + .then(function (child) { + forkedGhost = child; + request = supertest('http://127.0.0.1:' + child.port); + }) .then(function () { - return ghostServer.stop(); - }); + return testUtils.doAuth(request); + }) + .then(function (token) { + scope.ownerAccessToken = token; + + return testUtils.createUser({ + user: testUtils.DataGenerator.forKnex.createUser({email: 'test+1@ghost.org'}), + role: testUtils.DataGenerator.Content.roles[1] + }); + }) + .then(function (user) { + scope.editor = user; + + request.user = scope.editor; + return testUtils.doAuth(request); + }) + .then(function (token) { + scope.editorAccessToken = token; + done(); + }) + .catch(done); + }); + + after(function (done) { + teardownThemesFolder(); + + if (forkedGhost) { + forkedGhost.kill(done); + } else { + done(new Error('No forked ghost process exists, test setup must have failed.')); + } }); describe('success cases', function () { @@ -76,92 +97,161 @@ describe('Themes API', function () { var jsonResponse = res.body; should.exist(jsonResponse.themes); testUtils.API.checkResponse(jsonResponse, 'themes'); - jsonResponse.themes.length.should.be.above(0); + jsonResponse.themes.length.should.eql(1); + + testUtils.API.checkResponse(jsonResponse.themes[0], 'theme'); + jsonResponse.themes[0].name.should.eql('casper'); + jsonResponse.themes[0].package.should.be.an.Object().with.properties('name', 'version'); + jsonResponse.themes[0].active.should.be.true(); + + done(); + }); + }); + + it('download theme', function (done) { + request.get(testUtils.API.getApiQuery('themes/casper/download/')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect('Content-Type', /application\/zip/) + .expect('Content-Disposition', 'attachment; filename=casper.zip') + .expect(200) + .end(function (err) { + if (err) { + return done(err); + } + done(); }); }); it('upload theme', function (done) { - scope.uploadTheme({themePath: path.join(__dirname, '/../../../utils/fixtures/themes/valid.zip')}) + var jsonResponse; + + scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/themes/valid.zip')}) .end(function (err, res) { if (err) { return done(err); } - res.statusCode.should.eql(200); - should.exist(res.body.themes); - res.body.themes.length.should.eql(1); + jsonResponse = res.body; - should.exist(res.body.themes[0].name); - should.exist(res.body.themes[0].package); + should.exist(jsonResponse.themes); + testUtils.API.checkResponse(jsonResponse, 'themes'); + jsonResponse.themes.length.should.eql(1); + testUtils.API.checkResponse(jsonResponse.themes[0], 'theme'); + jsonResponse.themes[0].name.should.eql('valid'); // upload same theme again to force override - scope.uploadTheme({themePath: path.join(__dirname, '/../../../utils/fixtures/themes/valid.zip')}) - .end(function (err) { + scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/themes/valid.zip')}) + .end(function (err, res) { if (err) { return done(err); } - // ensure contains two files (zip and extracted theme) - fs.readdirSync(config.getContentPath('themes')).join().match(/valid/gi).length.should.eql(1); - done(); + jsonResponse = res.body; + + should.exist(jsonResponse.themes); + testUtils.API.checkResponse(jsonResponse, 'themes'); + jsonResponse.themes.length.should.eql(1); + testUtils.API.checkResponse(jsonResponse.themes[0], 'theme'); + jsonResponse.themes[0].name.should.eql('valid'); + + // ensure tmp theme folder contains two themes now + var tmpFolderContents = fs.readdirSync(join(tmpContentPath.name, 'themes')); + tmpFolderContents.should.be.an.Array().with.lengthOf(2); + tmpFolderContents[0].should.eql('casper'); + tmpFolderContents[1].should.eql('valid'); + + // Check the Themes API returns the correct result + request.get(testUtils.API.getApiQuery('themes/')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } + + var addedTheme, casperTheme; + jsonResponse = res.body; + + should.exist(jsonResponse.themes); + testUtils.API.checkResponse(jsonResponse, 'themes'); + jsonResponse.themes.length.should.eql(2); + + // Casper should be present and still active + casperTheme = _.find(jsonResponse.themes, {name: 'casper'}); + should.exist(casperTheme); + testUtils.API.checkResponse(casperTheme, 'theme'); + casperTheme.active.should.be.true(); + + // The added theme should be here + addedTheme = _.find(jsonResponse.themes, {name: 'valid'}); + should.exist(addedTheme); + testUtils.API.checkResponse(addedTheme, 'theme'); + addedTheme.active.should.be.false(); + + done(); + }); }); }); }); - it('get all themes + new theme', function (done) { - request.get(testUtils.API.getApiQuery('themes/')) + // NOTE: This test requires the previous upload test + // @TODO make this test independent + it('delete theme', function (done) { + var jsonResponse; + + request.del(testUtils.API.getApiQuery('themes/valid')) .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(204) .end(function (err, res) { if (err) { return done(err); } - var jsonResponse = res.body; - should.exist(jsonResponse.themes); - testUtils.API.checkResponse(jsonResponse, 'themes'); - jsonResponse.themes.length.should.be.above(0); + jsonResponse = res.body; + // Delete requests have empty bodies + jsonResponse.should.eql({}); - // ensure the new 'valid' theme is available - should.exist(_.find(jsonResponse.themes, {name: 'valid'})); - done(); - }); - }); + // ensure tmp theme folder contains one theme again now + var tmpFolderContents = fs.readdirSync(join(tmpContentPath.name, 'themes')); + tmpFolderContents.should.be.an.Array().with.lengthOf(1); + tmpFolderContents[0].should.eql('casper'); - it('download theme uuid', function (done) { - request.get(testUtils.API.getApiQuery('themes/casper/download/')) - .set('Authorization', 'Bearer ' + scope.ownerAccessToken) - .expect('Content-Type', /application\/zip/) - .expect('Content-Disposition', 'attachment; filename=casper.zip') - .expect(200) - .end(function (err) { - if (err) { - return done(err); - } + // Check the settings API returns the correct result + request.get(testUtils.API.getApiQuery('themes/')) + .set('Authorization', 'Bearer ' + scope.ownerAccessToken) + .expect(200) + .end(function (err, res) { + if (err) { + return done(err); + } - done(); - }); - }); + var deletedTheme, casperTheme; + jsonResponse = res.body; - it('delete theme uuid', function (done) { - request.del(testUtils.API.getApiQuery('themes/valid')) - .set('Authorization', 'Bearer ' + scope.ownerAccessToken) - .expect(204) - .end(function (err) { - if (err) { - return done(err); - } + should.exist(jsonResponse.themes); + testUtils.API.checkResponse(jsonResponse, 'themes'); + jsonResponse.themes.length.should.eql(1); - fs.existsSync(config.getContentPath('themes') + '/valid').should.eql(false); - fs.existsSync(config.getContentPath('themes') + '/valid.zip').should.eql(false); - done(); + // Casper should be present and still active + casperTheme = _.find(jsonResponse.themes, {name: 'casper'}); + should.exist(casperTheme); + testUtils.API.checkResponse(casperTheme, 'theme'); + casperTheme.active.should.be.true(); + + // The deleted theme should not be here + deletedTheme = _.find(jsonResponse.themes, {name: 'valid'}); + should.not.exist(deletedTheme); + + done(); + }); }); }); }); describe('error cases', function () { it('upload invalid theme', function (done) { - scope.uploadTheme({themePath: path.join(__dirname, '/../../../utils/fixtures/themes/invalid.zip')}) + scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/themes/invalid.zip')}) .end(function (err, res) { if (err) { return done(err); @@ -176,7 +266,7 @@ describe('Themes API', function () { }); it('upload casper.zip', function (done) { - scope.uploadTheme({themePath: path.join(__dirname, '/../../../utils/fixtures/themes/casper.zip')}) + scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/themes/casper.zip')}) .end(function (err, res) { if (err) { return done(err); @@ -194,11 +284,15 @@ describe('Themes API', function () { request.del(testUtils.API.getApiQuery('themes/casper')) .set('Authorization', 'Bearer ' + scope.ownerAccessToken) .expect(422) - .end(function (err) { + .end(function (err, res) { if (err) { return done(err); } + res.body.errors.length.should.eql(1); + res.body.errors[0].errorType.should.eql('ValidationError'); + res.body.errors[0].message.should.eql('Deleting the default casper theme is not allowed.'); + done(); }); }); @@ -207,31 +301,39 @@ describe('Themes API', function () { request.del(testUtils.API.getApiQuery('themes/not-existent')) .set('Authorization', 'Bearer ' + scope.ownerAccessToken) .expect(404) - .end(function (err) { + .end(function (err, res) { if (err) { return done(err); } + res.body.errors.length.should.eql(1); + res.body.errors[0].errorType.should.eql('NotFoundError'); + res.body.errors[0].message.should.eql('Theme does not exist.'); + done(); }); }); it('upload non application/zip', function (done) { - scope.uploadTheme({themePath: path.join(__dirname, '/../../../utils/fixtures/csv/single-column-with-header.csv')}) + scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/csv/single-column-with-header.csv')}) .end(function (err, res) { if (err) { return done(err); } res.statusCode.should.eql(415); + res.body.errors.length.should.eql(1); + res.body.errors[0].errorType.should.eql('UnsupportedMediaTypeError'); + res.body.errors[0].message.should.eql('Please select a valid zip file.'); + done(); }); }); - // @TODO: does not pass in travis with 0.10.x, but local it works + // @TODO: make this a nicer error! it.skip('upload different field name', function (done) { scope.uploadTheme({ - themePath: path.join(__dirname, '/../../../utils/fixtures/csv/single-column-with-header.csv'), + themePath: join(__dirname, '/../../../utils/fixtures/csv/single-column-with-header.csv'), fieldName: 'wrong' }).end(function (err, res) { if (err) { @@ -247,7 +349,7 @@ describe('Themes API', function () { describe('As Editor', function () { it('no permissions to upload theme', function (done) { scope.uploadTheme({ - themePath: path.join(__dirname, '/../../../utils/fixtures/themes/valid.zip'), + themePath: join(__dirname, '/../../../utils/fixtures/themes/valid.zip'), accessToken: scope.editorAccessToken }).end(function (err, res) { if (err) { diff --git a/core/test/utils/api.js b/core/test/utils/api.js index 0401cee5202..fbd333244c6 100644 --- a/core/test/utils/api.js +++ b/core/test/utils/api.js @@ -31,7 +31,7 @@ var _ = require('lodash'), role: _.keys(schema.roles), permission: _.keys(schema.permissions), notification: ['type', 'message', 'status', 'id', 'dismissible', 'location'], - theme: ['uuid', 'name', 'version', 'active'], + theme: ['name', 'package', 'active'], themes: ['themes'], invites: _(schema.invites).keys().without('token').value() }; From 77f57dc3dd368927532cca6cd5cbe1da0afe1859 Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 7 Mar 2017 13:17:52 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=90=9B=20Ensure=20upload=20reflects?= =?UTF-8?q?=20active=20theme?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- core/server/api/themes.js | 2 +- core/test/functional/routes/api/themes_spec.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/core/server/api/themes.js b/core/server/api/themes.js index a5571f03b9d..4de0db2096d 100644 --- a/core/server/api/themes.js +++ b/core/server/api/themes.js @@ -101,7 +101,7 @@ themes = { return themeUtils.loadOne(zip.shortName); }) .then(function (themeObject) { - themeObject = themeList.toAPI(themeObject, zip.shortName); + themeObject = themeList.toAPI(themeObject, settingsCache.get('activeTheme')); // gscan theme structure !== ghost theme structure if (theme.results.warning.length > 0) { themeObject.warnings = _.cloneDeep(theme.results.warning); diff --git a/core/test/functional/routes/api/themes_spec.js b/core/test/functional/routes/api/themes_spec.js index e1eed7cb6e0..183c1b66a64 100644 --- a/core/test/functional/routes/api/themes_spec.js +++ b/core/test/functional/routes/api/themes_spec.js @@ -139,6 +139,7 @@ describe('Themes API (Forked)', function () { jsonResponse.themes.length.should.eql(1); testUtils.API.checkResponse(jsonResponse.themes[0], 'theme'); jsonResponse.themes[0].name.should.eql('valid'); + jsonResponse.themes[0].active.should.be.false(); // upload same theme again to force override scope.uploadTheme({themePath: join(__dirname, '/../../../utils/fixtures/themes/valid.zip')}) @@ -154,6 +155,7 @@ describe('Themes API (Forked)', function () { jsonResponse.themes.length.should.eql(1); testUtils.API.checkResponse(jsonResponse.themes[0], 'theme'); jsonResponse.themes[0].name.should.eql('valid'); + jsonResponse.themes[0].active.should.be.false(); // ensure tmp theme folder contains two themes now var tmpFolderContents = fs.readdirSync(join(tmpContentPath.name, 'themes'));