Use Model.findById() rather than get()#7
Merged
daffl merged 1 commit intofeathersjs-ecosystem:masterfeathersjs-ecosystem/feathers-sequelize:masterfrom Jan 9, 2016
evanbarter:masterCopy head branch name to clipboard
Merged
Use Model.findById() rather than get()#7daffl merged 1 commit intofeathersjs-ecosystem:masterfeathersjs-ecosystem/feathers-sequelize:masterfrom evanbarter:masterCopy head branch name to clipboard
daffl merged 1 commit intofeathersjs-ecosystem:masterfeathersjs-ecosystem/feathers-sequelize:masterfrom
evanbarter:masterCopy head branch name to clipboard
Conversation
Member
|
Thank you. You are right, that behaviour is indeed inconsistent. I also noticed that passing on to |
daffl
added a commit
that referenced
this pull request
Jan 9, 2016
Use Model.findById() rather than get()
This was referenced Jan 9, 2016
Member
|
Thanks again. I made a 1.0.8 patch release with your fix and created a follow-up issue to review the consistency of that part of the API. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Calling this.get() from within remove() will not populate params.user object, and if you have a before get hook validating the presence of a user object (i.e. feathers-authentication.hooks.requireAuth) it will throw an error.
This is a bit of a weird situation because of some inconsistencies in the API. update() takes an id and Model.findById() is used so I guess the assumption is the author will validate access, permissions etc in a before update hook. patch() takes an id or params but ultimately does a find and passes the id in to the params.where object, if present.
This PR follows the update() method of retrieving a single instance for remove().