From bbdc55cb3a8e3e5e412235ed5d65f6158397a574 Mon Sep 17 00:00:00 2001 From: Peter Jaszkowiak Date: Sat, 4 Nov 2017 08:51:44 -0600 Subject: [PATCH] Better fix for #5993 (#6034) * Better fix for #5993 Also a nice newline so the line doesn't get overwritten when running `./nodebb upgrade` * Optimizations for custom homepage Move it into a separate module --- nodebb | 2 +- src/controllers/home.js | 62 +++++++++++++++++++++++++++ src/controllers/index.js | 48 +-------------------- src/meta/configs.js | 6 +-- src/routes/index.js | 5 ++- test/controllers.js | 93 ++++++++++++++++++++++++---------------- 6 files changed, 125 insertions(+), 91 deletions(-) create mode 100644 src/controllers/home.js diff --git a/nodebb b/nodebb index c632c72df9..adedd30826 100755 --- a/nodebb +++ b/nodebb @@ -466,7 +466,7 @@ var commands = { next(); }, function (next) { - process.stdout.write('1. '.bold + 'Bringing base dependencies up to date... '.yellow); + process.stdout.write('1. '.bold + 'Bringing base dependencies up to date... \n'.yellow); packageInstall.npmInstallProduction(); next(); }, diff --git a/src/controllers/home.js b/src/controllers/home.js new file mode 100644 index 0000000000..d5a8c30050 --- /dev/null +++ b/src/controllers/home.js @@ -0,0 +1,62 @@ +'use strict'; + +var plugins = require('../plugins'); +var meta = require('../meta'); +var user = require('../user'); +var pubsub = require('../pubsub'); + +var adminHomePageRoute; +var getRoute; + +function configUpdated() { + adminHomePageRoute = (meta.config.homePageRoute || meta.config.homePageCustom || '').replace(/^\/+/, '') || 'categories'; + getRoute = parseInt(meta.config.allowUserHomePage, 10) ? getRouteAllowUserHomePage : getRouteDisableUserHomePage; +} + +function getRouteDisableUserHomePage(uid, next) { + next(null, adminHomePageRoute); +} + +function getRouteAllowUserHomePage(uid, next) { + user.getSettings(uid, function (err, settings) { + if (err) { + return next(err); + } + + var route = adminHomePageRoute; + + if (settings.homePageRoute !== 'undefined' && settings.homePageRoute !== 'none') { + route = settings.homePageRoute || route; + } + + next(null, route); + }); +} + +pubsub.on('config:update', configUpdated); +configUpdated(); + +module.exports = function (req, res, next) { + if (req.path !== '/' && req.path !== '/api/') { + return next(); + } + + getRoute(req.uid, function (err, route) { + if (err) { + return next(err); + } + + var hook = 'action:homepage.get:' + route; + + if (plugins.hasListeners(hook)) { + return plugins.fireHook(hook, { + req: req, + res: res, + next: next, + }); + } + + req.url = req.path + route; + next(); + }); +}; diff --git a/src/controllers/index.js b/src/controllers/index.js index 847bbcedc6..7144f1a740 100644 --- a/src/controllers/index.js +++ b/src/controllers/index.js @@ -3,7 +3,6 @@ var async = require('async'); var nconf = require('nconf'); var validator = require('validator'); -var request = require('request'); var meta = require('../meta'); var user = require('../user'); @@ -13,6 +12,7 @@ var helpers = require('./helpers'); var Controllers = module.exports; +Controllers.home = require('./home'); Controllers.topics = require('./topics'); Controllers.posts = require('./posts'); Controllers.categories = require('./categories'); @@ -36,52 +36,6 @@ Controllers.osd = require('./osd'); Controllers['404'] = require('./404'); Controllers.errors = require('./errors'); -Controllers.home = function (req, res, next) { - var route = meta.config.homePageRoute || (meta.config.homePageCustom || '').replace(/^\/+/, '') || 'categories'; - - async.waterfall([ - function (next) { - user.getSettings(req.uid, next); - }, - function (settings, next) { - if (parseInt(meta.config.allowUserHomePage, 10) === 1 && settings.homePageRoute !== 'undefined' && settings.homePageRoute !== 'none') { - route = settings.homePageRoute || route; - } - - var hook = 'action:homepage.get:' + route; - - if (plugins.hasListeners(hook)) { - return plugins.fireHook(hook, { - req: req, - res: res, - next: next, - }); - } - - if (route === 'categories' || route === '/') { - Controllers.categories.list(req, res, next); - } else if (route === 'unread') { - Controllers.unread.get(req, res, next); - } else if (route === 'recent') { - Controllers.recent.get(req, res, next); - } else if (route === 'popular') { - Controllers.popular.get(req, res, next); - } else { - var match = /^category\/(\d+)\/(.*)$/.exec(route); - - if (match) { - req.params.topic_index = '1'; - req.params.category_id = match[1]; - req.params.slug = match[2]; - Controllers.category.get(req, res, next); - } else { - request.get(nconf.get('url') + '/' + route).pipe(res); - } - } - }, - ], next); -}; - Controllers.reset = function (req, res, next) { if (req.params.code) { async.waterfall([ diff --git a/src/meta/configs.js b/src/meta/configs.js index cd3a17c985..12553d5a50 100644 --- a/src/meta/configs.js +++ b/src/meta/configs.js @@ -121,11 +121,7 @@ function updateConfig(config) { } function updateLocalConfig(config) { - for (var field in config) { - if (config.hasOwnProperty(field)) { - Meta.config[field] = config[field]; - } - } + Object.assign(Meta.config, config); } pubsub.on('config:update', function onConfigReceived(config) { diff --git a/src/routes/index.js b/src/routes/index.js index 06121c60d6..e2afb836ab 100644 --- a/src/routes/index.js +++ b/src/routes/index.js @@ -22,8 +22,6 @@ var helpers = require('./helpers'); var setupPageRoute = helpers.setupPageRoute; function mainRoutes(app, middleware, controllers) { - setupPageRoute(app, '/', middleware, [], controllers.home); - var loginRegisterMiddleware = [middleware.redirectToAccountIfLoggedIn]; setupPageRoute(app, '/login', middleware, loginRegisterMiddleware, controllers.login); @@ -123,6 +121,9 @@ module.exports = function (app, middleware, hotswapIds, callback) { app.use(middleware.stripLeadingSlashes); + // handle custom homepage routes + app.use(relativePath, controllers.home); + adminRoutes(router, middleware, controllers); metaRoutes(router, middleware, controllers); apiRoutes(router, middleware, controllers); diff --git a/test/controllers.js b/test/controllers.js index 050c15aea4..fb3fda8830 100644 --- a/test/controllers.js +++ b/test/controllers.js @@ -69,62 +69,80 @@ describe('Controllers', function () { }); it('should load unread as home route', function (done) { - meta.config.homePageRoute = 'unread'; - request(nconf.get('url'), function (err, res, body) { + meta.configs.set('homePageRoute', 'unread', function (err) { assert.ifError(err); - assert.equal(res.statusCode, 200); - assert(body); - done(); + + request(nconf.get('url'), function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + assert(body); + done(); + }); }); }); it('should load recent as home route', function (done) { - meta.config.homePageRoute = 'recent'; - request(nconf.get('url'), function (err, res, body) { + meta.configs.set('homePageRoute', 'recent', function (err) { assert.ifError(err); - assert.equal(res.statusCode, 200); - assert(body); - done(); + + request(nconf.get('url'), function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + assert(body); + done(); + }); }); }); it('should load popular as home route', function (done) { - meta.config.homePageRoute = 'popular'; - request(nconf.get('url'), function (err, res, body) { + meta.configs.set('homePageRoute', 'popular', function (err) { assert.ifError(err); - assert.equal(res.statusCode, 200); - assert(body); - done(); + + request(nconf.get('url'), function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + assert(body); + done(); + }); }); }); it('should load category as home route', function (done) { - meta.config.homePageRoute = 'category/1/test-category'; - request(nconf.get('url'), function (err, res, body) { + meta.configs.set('homePageRoute', 'category/1/test-category', function (err) { assert.ifError(err); - assert.equal(res.statusCode, 200); - assert(body); - done(); + + request(nconf.get('url'), function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + assert(body); + done(); + }); }); }); it('should redirect to custom homepage', function (done) { - meta.config.homePageRoute = 'groups'; - request(nconf.get('url'), function (err, res, body) { + meta.configs.set('homePageRoute', 'groups', function (err) { assert.ifError(err); - assert.equal(res.statusCode, 200); - assert(body); - done(); + + request(nconf.get('url'), function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + assert(body); + done(); + }); }); }); it('should 404 if custom homepage does not exist', function (done) { - meta.config.homePageRoute = 'thisroutedoesnotexist'; - request(nconf.get('url'), function (err, res, body) { + meta.configs.set('homePageRoute', 'this-route-does-not-exist', function (err) { assert.ifError(err); - assert.equal(res.statusCode, 404); - assert(body); - done(); + + request(nconf.get('url'), function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 404); + assert(body); + done(); + }); }); }); @@ -139,13 +157,16 @@ describe('Controllers', function () { hook: 'action:homepage.get:custom', method: hookMethod, }); - meta.config.homePageRoute = 'custom'; - request(nconf.get('url'), function (err, res, body) { + meta.configs.set('homePageRoute', 'custom', function (err) { assert.ifError(err); - assert.equal(res.statusCode, 200); - assert.equal(body, '"works"'); - plugins.unregisterHook('myTestPlugin', 'action:homepage.get:custom', hookMethod); - done(); + + request(nconf.get('url'), function (err, res, body) { + assert.ifError(err); + assert.equal(res.statusCode, 200); + assert.equal(body, '"works"'); + plugins.unregisterHook('myTestPlugin', 'action:homepage.get:custom', hookMethod); + done(); + }); }); });