Skip to content

Commit

Permalink
πŸ™…πŸ½ Admin server split (#8142)
Browse files Browse the repository at this point in the history
refs #8140

✨ Support new default-prod.hbs template for admin
✨ Redirect ghost admin urls without a #
✨ Update admin urls to include #
🎨 Move the admin templates
πŸ”₯ Remove redirect to setup middleware
🚨 Tests for new middleware
  • Loading branch information
ErisDS authored and kirrg001 committed Mar 14, 2017
1 parent f4a68a2 commit 4a6f58d
Show file tree
Hide file tree
Showing 15 changed files with 102 additions and 175 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ config.*.json

# Built asset files
/core/built
/core/server/views/default.hbs
/core/server/admin/views/default*.hbs
/core/shared/ghost-url.min.js

# Coverage reports
Expand Down
6 changes: 3 additions & 3 deletions Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var overrides = require('./core/server/overrides'),
pkg: grunt.file.readJSON('package.json'),

clientFiles: [
'server/views/default.hbs',
'server/admin/views/default.hbs',
'built/assets/ghost.js',
'built/assets/ghost.css',
'built/assets/vendor.js',
Expand Down Expand Up @@ -465,7 +465,7 @@ var overrides = require('./core/server/overrides'),
return grunt.task.run(['lint']);
}

grunt.task.run(['stubClientFiles', 'test-all']);
grunt.task.run(['test-all']);
});

// ### Test-All
Expand All @@ -486,7 +486,7 @@ var overrides = require('./core/server/overrides'),
// ### test-setup *(utility)(
// `grunt test-setup` will run all the setup tasks required for running tests
grunt.registerTask('test-setup', 'Setup ready to run tests',
['knex-migrator', 'clean:test', 'setTestEnv']
['knex-migrator', 'clean:test', 'setTestEnv', 'stubClientFiles']
);

// ### Unit Tests *(sub task)*
Expand Down
4 changes: 2 additions & 2 deletions core/server/admin/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ var debug = require('debug')('ghost:admin'),
adminHbs = require('express-hbs').create(),

// Admin only middleware
redirectToSetup = require('../middleware/redirect-to-setup'),
adminMiddleware = require('./middleware'),

// Global/shared middleware?
cacheControl = require('../middleware/cache-control'),
Expand Down Expand Up @@ -61,7 +61,7 @@ module.exports = function setupAdminApp() {
// Admin is currently set to not be cached at all
adminApp.use(cacheControl('private'));
// Special redirects for the admin (these should have their own cache-control headers)
adminApp.use(redirectToSetup);
adminApp.use(adminMiddleware);

// Finally, routing
adminApp.get('*', require('./controller'));
Expand Down
5 changes: 4 additions & 1 deletion core/server/admin/controller.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
var debug = require('debug')('ghost:admin:controller'),
_ = require('lodash'),
config = require('../config'),
api = require('../api'),
updateCheck = require('../update-check'),
logging = require('../logging'),
Expand Down Expand Up @@ -33,7 +34,9 @@ module.exports = function adminController(req, res) {
}
});
}).finally(function noMatterWhat() {
res.render('default');
var defaultTemplate = config.get('env') === 'production' ? 'default-prod' : 'default';

res.render(defaultTemplate);
}).catch(function (err) {
logging.error(err);
});
Expand Down
14 changes: 14 additions & 0 deletions core/server/admin/middleware.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
var utils = require('../utils');

function redirectAdminUrls(req, res, next) {
var ghostPathMatch = req.originalUrl.match(/^\/ghost\/(.+)$/);
if (ghostPathMatch) {
return res.redirect(utils.url.urlJoin(utils.url.urlFor('admin'), '#', ghostPathMatch[1]));
}

next();
}

module.exports = [
redirectAdminUrls
];
Empty file.
4 changes: 2 additions & 2 deletions core/server/blog/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ frontendRoutes = function frontendRoutes() {

// ### Admin routes
router.get(/^\/(logout|signout)\/$/, function redirectToSignout(req, res) {
utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), 'signout/'));
utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), '#/signout/'));
});
router.get(/^\/signup\/$/, function redirectToSignup(req, res) {
utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), 'signup/'));
utils.redirect301(res, utils.url.urlJoin(utils.url.urlFor('admin'), '#/signup/'));
});

// redirect to /ghost and let that do the authentication to prevent redirects to /ghost//admin etc.
Expand Down
2 changes: 1 addition & 1 deletion core/server/config/overrides.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"corePath": "core/",
"clientAssets": "core/built/assets",
"helperTemplates": "core/server/helpers/tpl/",
"adminViews": "core/server/views/",
"adminViews": "core/server/admin/views/",
"defaultViews": "core/server/views/",
"internalAppPath": "core/server/apps/",
"internalStoragePath": "core/server/storage/",
Expand Down
4 changes: 2 additions & 2 deletions core/server/controllers/frontend/channel-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ channelConfig = function channelConfig() {
}
},
slugTemplate: true,
editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '/settings/tags/:slug/')
editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '#/settings/tags/:slug/')
},
author: {
name: 'author',
Expand All @@ -40,7 +40,7 @@ channelConfig = function channelConfig() {
}
},
slugTemplate: true,
editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '/team/:slug/')
editRedirect: utils.url.urlJoin(utils.url.urlFor('admin'), '#/team/:slug/')
}
};

Expand Down
19 changes: 0 additions & 19 deletions core/server/middleware/redirect-to-setup.js

This file was deleted.

40 changes: 7 additions & 33 deletions core/test/functional/routes/admin_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,25 +82,25 @@ describe('Admin Routing', function () {
});

describe('Legacy Redirects', function () {
it('should redirect /logout/ to /ghost/signout/', function (done) {
it('should redirect /logout/ to /ghost/#/signout/', function (done) {
request.get('/logout/')
.expect('Location', '/ghost/signout/')
.expect('Location', '/ghost/#/signout/')
.expect('Cache-Control', testUtils.cacheRules.year)
.expect(301)
.end(doEndNoAuth(done));
});

it('should redirect /signout/ to /ghost/signout/', function (done) {
it('should redirect /signout/ to /ghost/#/signout/', function (done) {
request.get('/signout/')
.expect('Location', '/ghost/signout/')
.expect('Location', '/ghost/#/signout/')
.expect('Cache-Control', testUtils.cacheRules.year)
.expect(301)
.end(doEndNoAuth(done));
});

it('should redirect /signup/ to /ghost/signup/', function (done) {
it('should redirect /signup/ to /ghost/#/signup/', function (done) {
request.get('/signup/')
.expect('Location', '/ghost/signup/')
.expect('Location', '/ghost/#/signup/')
.expect('Cache-Control', testUtils.cacheRules.year)
.expect(301)
.end(doEndNoAuth(done));
Expand Down Expand Up @@ -130,32 +130,6 @@ describe('Admin Routing', function () {
.end(doEndNoAuth(done));
});
});

describe('Ghost Admin Setup', function () {
it('should redirect from /ghost/ to /ghost/setup/ when no user/not installed yet', function (done) {
request.get('/ghost/')
.expect('Location', /ghost\/setup/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(302)
.end(doEnd(done));
});

it('should redirect from /ghost/signin/ to /ghost/setup/ when no user', function (done) {
request.get('/ghost/signin/')
.expect('Location', /ghost\/setup/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(302)
.end(doEnd(done));
});

it('should respond with html for /ghost/setup/', function (done) {
request.get('/ghost/setup/')
.expect('Content-Type', /html/)
.expect('Cache-Control', testUtils.cacheRules.private)
.expect(200)
.end(doEnd(done));
});
});
});

describe('FORK', function () {
Expand Down Expand Up @@ -194,7 +168,7 @@ describe('Admin Routing', function () {
});

it('should allow admin access over HTTPS', function (done) {
request.get('/ghost/setup/')
request.get('/ghost/')
.set('X-Forwarded-Proto', 'https')
.expect(200)
.end(doEnd(done));
Expand Down
4 changes: 2 additions & 2 deletions core/test/functional/routes/channel_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ describe('Channel Routes', function () {

it('should redirect to tag settings', function (done) {
request.get('/tag/getting-started/edit/')
.expect('Location', '/ghost/settings/tags/getting-started/')
.expect('Location', '/ghost/#/settings/tags/getting-started/')
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(302)
.end(doEnd(done));
Expand Down Expand Up @@ -549,7 +549,7 @@ describe('Channel Routes', function () {

it('should redirect to editor', function (done) {
request.get('/author/ghost-owner/edit/')
.expect('Location', '/ghost/team/ghost-owner/')
.expect('Location', '/ghost/#/team/ghost-owner/')
.expect('Cache-Control', testUtils.cacheRules.public)
.expect(302)
.end(doEnd(done));
Expand Down
63 changes: 63 additions & 0 deletions core/test/unit/admin_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
var should = require('should'), // jshint ignore:line
sinon = require('sinon'),

// Thing we are testing
redirectAdminUrls = require('../../server/admin/middleware')[0],

sandbox = sinon.sandbox.create();
describe('Admin App', function () {
afterEach(function () {
sandbox.restore();
});

describe('middleware', function () {
describe('redirectAdminUrls', function () {
var req, res, next;
// Input: req.originalUrl
// Output: either next or res.redirect are called
beforeEach(function () {
req = {};
res = {};
next = sandbox.stub();
res.redirect = sandbox.stub();
});

it('should redirect a url which starts with ghost', function () {
req.originalUrl = '/ghost/x';

redirectAdminUrls(req, res, next);

next.called.should.be.false();
res.redirect.called.should.be.true();
res.redirect.calledWith('/ghost/#/x').should.be.true();
});

it('should not redirect /ghost/ on its owh', function () {
req.originalUrl = '/ghost/';

redirectAdminUrls(req, res, next);

next.called.should.be.true();
res.redirect.called.should.be.false();
});

it('should not redirect url that has no slash', function () {
req.originalUrl = 'ghost/x';

redirectAdminUrls(req, res, next);

next.called.should.be.true();
res.redirect.called.should.be.false();
});

it('should not redirect url that starts with something other than /ghost/', function () {
req.originalUrl = 'x/ghost/x';

redirectAdminUrls(req, res, next);

next.called.should.be.true();
res.redirect.called.should.be.false();
});
});
});
});
2 changes: 1 addition & 1 deletion core/test/unit/controllers/frontend/channels_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ describe('Channels', function () {
describe('Edit', function () {
it('should redirect /edit/ to ghost admin', function (done) {
testChannelRedirect({url: '/tag/my-tag/edit/'}, function (path) {
path.should.eql('/ghost/settings/tags/my-tag/');
path.should.eql('/ghost/#/settings/tags/my-tag/');
postAPIStub.called.should.be.false();
}, done);
});
Expand Down
Loading

0 comments on commit 4a6f58d

Please sign in to comment.