Skip to content

Commit

Permalink
Refactor views hook to bind view targets more directly to res.view
Browse files Browse the repository at this point in the history
Also uses `_.get` to allow for deeply nested views in view syntax, e.g. `/foo: {view: 'foo/bar/baz'}`
refs #3604
  • Loading branch information
sgress454 committed Feb 29, 2016
1 parent ef91974 commit b516c89
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 104 deletions.
59 changes: 1 addition & 58 deletions lib/hooks/views/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,68 +9,11 @@ var _ = require('lodash');
*/
module.exports = function detectAndPrepareViews (sails, hook) {
sails.modules.statViews(function (err, detectedViews) {
if (err) throw err;
if (err) {throw err;}

// Save existence tree in `sails.views` for consumption later
sails.views = detectedViews || {};

// Generate view-serving middleware and stow it in `hook.middleware`
createMiddleware(detectedViews);
});

/**
* Generates view-serving middleware for each view
*/
function createMiddleware (detectedViews) {
// If there are any matching views which don't have an action
// create middleware to serve them
_.each(detectedViews, function (view, id) {

// Create middleware for a top-level view
if (view === true) {
sails.log.silly('Building action for view: ', id);
hook.middleware[id] = serveView(id);
}

// Create middleware for each subview
else {
hook.middleware[id] = {};
for (var subViewId in detectedViews[id]) {
sails.log.silly('Building action for view: ', id, '/', subViewId);
hook.middleware[id][subViewId] = serveView(id, subViewId);
}
}
});
}

/**
* Returns a middleware chain that remembers a view id and
* runs simple middleware to template and serve the view file.
* Used to serve views w/o controllers
*
* (This concatenation approach is crucial to allow policies to be bound)
*
* @param {[type]} viewId [description]
* @param {[type]} subViewId [description]
* @return {Array}
*/
function serveView (viewId, subViewId) {
// Save for use in closure
// (handle top-level and subview cases)
var viewExpression = viewId + (subViewId ? '/' + subViewId : '');

return [
function rememberViewId(req, res, next) {
// Save reference for view in res.view() middleware
// (only needs to happen if subViewId is not set [top-level view])
if (viewId) {
req.options.view = viewExpression;
}
next();
},
function serveView(req, res) {
res.view();
}
];
}
};
63 changes: 19 additions & 44 deletions lib/hooks/views/onRoute.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,56 +52,31 @@ module.exports = function onRoute (sails, route) {
*/
function bindView ( path, target, verb, options ) { //:TODO 'options' is defined but never used.

// Get view names
var view = target.view.split('/')[0];
var subview = target.view.split('/')[1] || 'index';
// Transform the view path into something Lodash _.get will understand (i.e. dots not slashes)
var viewPath = target.view.replace(/\//g, '.');

// Look up appropriate view and make sure it exists
var viewMiddleware = sails.middleware.views[view];
// Dereference subview if the top-level view middleware is actually an object
if (_.isPlainObject(viewMiddleware)) {
viewMiddleware = viewMiddleware[subview];
}

// If a view was specified but it doesn't match,
// ignore the attempt and inform the user
if ( !viewMiddleware ) {
sails.log.error(
'Ignoring attempt to bind route (' +
path + ') to unknown view: ' + target.view
);
return;
}

// Make sure the view function (+/- policies, etc.) is usable
// If it's an array, bind each action to the destination route in order
else if (_.isArray(viewMiddleware)) {
_.each(viewMiddleware, function (fn) {
sails.router.bind(path, viewHandler(fn), verb, _.extend(target, {log: 'VIEW'}));
// Look up the view in our views hash
if (_.get(sails.views, viewPath) === true) {
return sails.router.bind(path, function serveView(req, res) {
res.view(target.view);
});
return;
}

// Bind an action which renders this view to the destination route
else {
sails.router.bind(path, viewHandler(viewMiddleware), verb, _.extend(target, {log: 'VIEW'}));
return;
// For backwards compatibility, look for an index view if the specified view was
// top-level and could not be found
if (viewPath.indexOf('.') === -1) {
if (_.get(sails.views, viewPath + '.index') === true) {
return sails.router.bind(path, function serveView(req, res) {
res.view(viewPath + '/index');
});
}
}

sails.log.error(
'Ignoring attempt to bind route (' +
path + ') to unknown view: ' + target.view
);
return;

// Wrap up the view middleware to supply access to
// the original target when requests comes in
function viewHandler (originalFn) {

if ( !_.isFunction(originalFn) ) {
sails.log.error(
'Error binding view to route :: View middleware is not a function!',
originalFn, 'for path: ', path, verb ? ('and verb: ' + verb) : '');
return;
}

// Bind intercepted middleware function to route
return originalFn;
}
}
};
1 change: 1 addition & 0 deletions test/integration/fixtures/sampleapp/views/app/index.ejs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
App index file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
I'm deeply nested!
53 changes: 51 additions & 2 deletions test/integration/router.viewRendering.test.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,27 @@
var assert = require('assert');
var httpHelper = require('./helpers/httpHelper.js');
var appHelper = require('./helpers/appHelper');

var _ = require('lodash');
var fs = require('fs');
describe('router :: ', function() {
describe('View routes', function() {
var appName = 'testApp';

before(function(done) {
appHelper.build(done);
appHelper.build(function() {
fs.writeFileSync('config/extraroutes.js', 'module.exports.routes = ' + JSON.stringify({
'/testView': {
view: 'viewtest/index'
},
'/app': {
view: 'app'
},
'/user': {
view: 'app/user/homepage'
}
}));
return done();
});
});

beforeEach(function(done) {
Expand Down Expand Up @@ -51,6 +65,41 @@ describe('router :: ', function() {

});

describe('with specified routing using the "view:" syntax', function() {

it('route with config {view: "app"} should respond to a get request with the "app/index.ejs" view if "app.ejs" does not exist', function(done) {

httpHelper.testRoute('get', 'app', function(err, response) {
if (err) {return done(new Error(err));}
assert(response.body.indexOf('not found') < 0);
assert(response.body.indexOf('App index file') > -1);
done();
});
});

it('route with config {view: "viewtest/index"} should respond to a get request with "viewtest/index.ejs"', function(done) {

httpHelper.testRoute('get', 'testView', function(err, response) {
if (err) {return done(new Error(err));}
assert(response.body.indexOf('not found') < 0);
assert(response.body.indexOf('indexView') > -1);
done();
});
});

it('route with config {view: "app/user/homepage"} should respond to a get request with "app/user/homepage.ejs"', function(done) {

httpHelper.testRoute('get', 'user', function(err, response) {
if (err) {return done(new Error(err));}
assert(response.body.indexOf('not found') < 0);
assert(response.body.indexOf('I\'m deeply nested!') > -1);
done();
});
});


});

describe('with no specified routing', function() {

before(function() {
Expand Down

0 comments on commit b516c89

Please sign in to comment.