Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow before and after filters to be created by chaining together declarations #131

Merged
merged 1 commit into from

2 participants

@redking

Controller before and after filters must be declared in the following way

controller.before('action1', filterA);
controller.before('action1', filterB);
controller.before('action1', filterC);

controller.before('action2', filterA);
controller.before('action2', filterB);

In cases where a controller has many actions and many filters, it would be nice if we could declare filters by chaining the declarations together

controller
 .before('action1', filterA)
 .before('action1', filterB)
 .before('action1', filterC);

controller
 .before('action2', filterA)
 .before('action2', filterB);

This makes the code a little easier to read, and encourages developers to arrange filters into distinct ordered blocks, rather than as code that could be inserted anywhere in the controller.

To support this, I've made two small changes to controller.js, and added a single test to each of the filter test suites, which should be sufficient to test the syntax.

@jaredhanson jaredhanson merged commit a677d66 into jaredhanson:master
@jaredhanson
Owner

Love it, thanks! Merged and published to npm as locomotive 0.4.1.

@redking redking deleted the unknown repository branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 17, 2014
This page is out of date. Refresh to see the latest.
View
4 lib/controller.js
@@ -417,6 +417,8 @@ Controller.prototype.before = function(action, fn) {
} else {
this.__beforeFilters.push({ action: action, fn: fn });
}
+
+ return this;
};
/**
@@ -447,6 +449,8 @@ Controller.prototype.after = function(action, fn) {
} else {
this.__afterFilters.push({ action: action, fn: fn });
}
+
+ return this;
};
/**
View
69 test/controller.filters.after.test.js
@@ -1274,5 +1274,72 @@ describe('Controller#after', function() {
expect(controller.order[2]).to.equal(2);
});
});
-
+
+ describe('filter declaration using chaining syntax', function() {
+ var app = new MockApplication();
+ var controller = new Controller();
+ controller.order = [];
+
+ controller.show = function() {
+ this.order.push('a');
+ this.song = 'Mr. Jones';
+ this._private = 'Untitled';
+ this.render();
+ };
+
+ // Chain together filters
+ controller
+ .after('show', function(next) {
+ this.order.push(1);
+ this.band = 'Counting Crows';
+ next();
+ })
+ .after('show', function(next) {
+ this.order.push(2);
+ this.album = 'August and Everything After';
+ next();
+ })
+ .after('index', function(next) {
+ this.order.push('x');
+ this.store = 'Amoeba Music';
+ next();
+ });
+
+ var req, res;
+
+ before(function(done) {
+ req = new MockRequest();
+ res = new MockResponse();
+
+ controller.after('show', function(next) {
+ return done();
+ });
+
+ controller._init(app, 'test');
+ controller._prepare(req, res, function(err) {
+ if (err) { return done(err); }
+ return done(new Error('should not call next'));
+ });
+ controller._invoke('show');
+ });
+
+ it('should apply filters in correct order', function() {
+ expect(controller.order).to.have.length(3);
+ expect(controller.order[0]).to.equal('a');
+ expect(controller.order[1]).to.equal(1);
+ expect(controller.order[2]).to.equal(2);
+ });
+
+ it('should render view without options', function() {
+ expect(res._view).to.equal('test/show.html.ejs');
+ expect(res._options).to.be.an('object');
+ expect(Object.keys(res._options)).to.have.length(0);
+ });
+
+ it('should assign locals', function() {
+ expect(res.locals).to.be.an('object');
+ expect(Object.keys(res.locals)).to.have.length(1);
+ expect(res.locals.song).to.equal('Mr. Jones');
+ });
+ });
});
View
73 test/controller.filters.before.data.test.js
@@ -868,5 +868,76 @@ describe('Controller#before', function() {
expect(Object.keys(res.locals)).to.have.length(0);
});
});
-
+
+ describe('filte declaration using chaining syntax', function() {
+ var app = new MockApplication();
+ var controller = new Controller();
+ controller.order = [];
+
+ // Chain together filters
+ controller
+ .before('index', function(next) {
+ var data = {
+ store: 'Amoeba Music'
+ };
+ this.order.push('x');
+ next(null, data);
+ })
+ .before('show', function(next) {
+ var data = {
+ band: 'Counting Crows'
+ };
+ this.order.push(1);
+ next(null, data);
+ })
+ .before('show', function(data, next) {
+ data.album = 'August and Everything After';
+ this.order.push(2);
+ next(null, data);
+ });
+
+ controller.show = function(data) {
+ this.order.push('a');
+ this.song = 'Mr. Jones';
+ this._private = 'Untitled';
+ this.band = data.band;
+ this.album = data.album;
+ this.render();
+ };
+
+ var req, res;
+
+ before(function(done) {
+ req = new MockRequest();
+ res = new MockResponse(done);
+
+ controller._init(app, 'test');
+ controller._prepare(req, res, function(err) {
+ if (err) { return done(err); }
+ return done(new Error('should not call next'));
+ });
+ controller._invoke('show');
+ });
+
+ it('should apply filters in correct order', function() {
+ expect(controller.order).to.have.length(3);
+ expect(controller.order[0]).to.equal(1);
+ expect(controller.order[1]).to.equal(2);
+ expect(controller.order[2]).to.equal('a');
+ });
+
+ it('should render view without options', function() {
+ expect(res._view).to.equal('test/show.html.ejs');
+ expect(res._options).to.be.an('object');
+ expect(Object.keys(res._options)).to.have.length(0);
+ });
+
+ it('should assign locals', function() {
+ expect(res.locals).to.be.an('object');
+ expect(Object.keys(res.locals)).to.have.length(3);
+ expect(res.locals.band).to.equal('Counting Crows');
+ expect(res.locals.album).to.equal('August and Everything After');
+ expect(res.locals.song).to.equal('Mr. Jones');
+ });
+ });
});
View
71 test/controller.filters.before.test.js
@@ -824,8 +824,7 @@ describe('Controller#before', function() {
expect(Object.keys(res.locals)).to.have.length(0);
});
});
-
-
+
describe('filters using middleware-style callback', function() {
var app = new MockApplication();
var controller = new Controller();
@@ -1060,5 +1059,71 @@ describe('Controller#before', function() {
expect(res.locals.song).to.equal('Mr. Jones');
});
});
-
+
+ describe('filter declaration using chaining syntax', function() {
+
+ var app = new MockApplication();
+ var controller = new Controller();
+ controller.order = [];
+
+ // Create filters by chaining them together
+ controller
+ .before('index', function(next) {
+ this.order.push('x');
+ this.store = 'Amoeba Music';
+ next();
+ })
+ .before('show', function(next) {
+ this.order.push(1);
+ this.band = 'Counting Crows';
+ next();
+ })
+ .before('show', function(next) {
+ this.order.push(2);
+ this.album = 'August and Everything After';
+ next();
+ });
+
+ controller.show = function() {
+ this.order.push('a');
+ this.song = 'Mr. Jones';
+ this._private = 'Untitled';
+ this.render();
+ };
+
+ var req, res;
+
+ before(function(done) {
+ req = new MockRequest();
+ res = new MockResponse(done);
+
+ controller._init(app, 'test');
+ controller._prepare(req, res, function(err) {
+ if (err) { return done(err); }
+ return done(new Error('should not call next'));
+ });
+ controller._invoke('show');
+ });
+
+ it('should apply filters in correct order', function() {
+ expect(controller.order).to.have.length(3);
+ expect(controller.order[0]).to.equal(1);
+ expect(controller.order[1]).to.equal(2);
+ expect(controller.order[2]).to.equal('a');
+ });
+
+ it('should render view without options', function() {
+ expect(res._view).to.equal('test/show.html.ejs');
+ expect(res._options).to.be.an('object');
+ expect(Object.keys(res._options)).to.have.length(0);
+ });
+
+ it('should assign locals', function() {
+ expect(res.locals).to.be.an('object');
+ expect(Object.keys(res.locals)).to.have.length(3);
+ expect(res.locals.band).to.equal('Counting Crows');
+ expect(res.locals.album).to.equal('August and Everything After');
+ expect(res.locals.song).to.equal('Mr. Jones');
+ });
+ });
});
Something went wrong with that request. Please try again.