Skip to content

Commit

Permalink
Fixed resource id when multiple segments are present. Closes #36
Browse files Browse the repository at this point in the history
  • Loading branch information
tj committed Oct 15, 2011
1 parent 771af86 commit 60746f6
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 3 deletions.
7 changes: 4 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ Resource.prototype.load = function(fn){

Resource.prototype.__defineGetter__('defaultId', function(){
return this.name
? en.singularize(this.name)
? en.singularize(this.name.split('/').pop())

This comment has been minimized.

Copy link
@pmlopes

pmlopes Jan 3, 2012

This breaks setups like this:

/admin/items (rest resource where you can edit everything)
/user/items (rest where you can read only items from a user)

There will be 2 duplicate ids 'item' that will colide and always the last one defined will be used in the routes.

This comment has been minimized.

Copy link
@tj

tj Jan 3, 2012

Author Member

could you add a test case describing the issue? i'll take a look

: 'id';
});

Expand Down Expand Up @@ -171,8 +171,9 @@ Resource.prototype.add = function(resource){
, route;

// relative base
resource.base = this.base + (this.name ? this.name + '/': '') +
this.param + '/';
resource.base = this.base
+ (this.name ? this.name + '/': '')
+ this.param + '/';

// re-define previous actions
for (var method in resource.routes) {
Expand Down
8 changes: 8 additions & 0 deletions test/fixtures/cat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

exports.index = function(req, res){
res.send('list of cats');
};

exports.new = function(req, res){
res.send('new cat');
};
13 changes: 13 additions & 0 deletions test/resource.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,5 +315,18 @@ module.exports = {
assert.response(app,
{ url: '/users/logout' },
{ body: 'logout' });
},

'test several segments': function(){
var app = express.createServer();
var cat = app.resource('api/cat', require('./fixtures/cat'));

assert.response(app,
{ url: '/api/cat' },
{ body: 'list of cats' });

assert.response(app,
{ url: '/api/cat/new' },
{ body: 'new cat' });
}
};

1 comment on commit 60746f6

@pmlopes
Copy link

@pmlopes pmlopes commented on 60746f6 Jan 3, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix, actually breaks applications where you have several routes that finish with the same name such as /admin/items, /user/items since the functions that generates the ID is only considering the last keyword after the slash.

Please sign in to comment.