search

tommybananas / finale

Create flexible REST endpoints and controllers from Sequelize models in your Express app
187 stars 36 forks source link

associations: subResourceName is set by association.as if available #28

Closed wroberts closed 1 year ago

wroberts commented 6 years ago

I think it makes sense to be able to decide what the subResourceName of an association is. For instance, imagine that I have:

var Person = sequelize.define('person', { name: { type: DataTypes.STRING } }),
    Course = sequelize.define('course', { name: { type: DataTypes.STRING } });
Course.hasMany(Person, {as: 'students'});
Course.hasOne(Person, { as: 'teacher' });
rest.resource({model: Course, 
    endpoints: ['/courses', '/courses/:id'], 
    associations: true});

Under the current logic, the association Resources are named according to the model name, inflected for the number of the association. I think with my example here, we would get /courses/:id/person and /courses/:id/people/. Intuitively, though, it seems like we have gone to the trouble of naming the association in the model, and so we should get routes like /courses/:id/students/ and /courses/:id/teacher.

This patch checks the association.as field before setting the subResourceName.

tommybananas commented 6 years ago

Awesome I’ll check this out ASAP. It would help a lot if you added docs and tests as well.

On Fri, Aug 31, 2018 at 4:28 AM Will Roberts notifications@github.com wrote:

I think it makes sense to be able to decide what the subResourceName of an association is. For instance, imagine that I have:

var Person = sequelize.define('person', { name: { type: DataTypes.STRING } }), Course = sequelize.define('course', { name: { type: DataTypes.STRING } }); Course.hasMany(Person, {as: 'students'}); Course.hasOne(Person, { as: 'teacher' }); rest.resource({model: Course, endpoints: ['/courses', '/courses/:id'], associations: true});

Under the current logic, the association Resources are named according to the model name, inflected for the number of the association. I think with my example here, we would get /courses/:id/person and /courses/:id/people/. Intuitively, though, it seems like we have gone to the trouble of naming the association in the model, and so we should get routes like /courses/:id/students/ and /courses/:id/teacher.

This patch checks the association.as field before setting the subResourceName.

You can view, comment on, or merge this pull request online at:

https://github.com/tommybananas/finale/pull/28 Commit Summary

  • associations: subResourceName is set by association.as if available

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tommybananas/finale/pull/28, or mute the thread https://github.com/notifications/unsubscribe-auth/ACELEx1OGlO1G0pf7n16niDQn5nAb0YJks5uWQG4gaJpZM4WU5N6 .

wroberts commented 6 years ago

I added some really basic tests to show that sub-resources are named according to the association.as. This doesn't actually test the endpoints though.

tommybananas commented 6 years ago

restify seems to be failing