Repository navigation
Conversation
This is a test to further address issue feathersjs-ecosystem#234. While PR feathersjs-ecosystem#252 did fix some of the issues, some attributes are still not being passed through to some of the service calls as these tests failures will indicate. Proposed fixes for these new test failures will be forthcoming.
This is a continuation of the work to fix issue feathersjs-ecosystem#234. Some service calls were still failing to pass through params.sequelize, which again could cause issues for users that are extending the Service class with their own getModel() method that depends upon certain parameters being passed through to it.
|
Just to respond to the comment from @DesignByOnyx over in #262:
I opted to focus on passing through params.sequelize to all service calls-made-by-service-calls, e.g. in 8a95fec. But if what @DesignByOnyx is saying here is that a user shouldn't even be using params.sequelize for this "extra parameters for the overridden getModel()" purpose, we can come up with some other parameter that gets whitelisted to be passed through to all subsequent-service-calls-made-by-service-calls. I propose |
The getModel parameter passing tests now also check that the custom getModel method has been called by the service methods. This will help protect against potential future regressions that somehow fail to call getModel().
| return Object.assign({ | ||
| sequelize: Object.assign({ | ||
| expectedAttribute: EXPECTED_ATTRIBUTE_VALUE, | ||
| getModelCalls: { count: 0 } |
There was a problem hiding this comment.
Note, keeping count in a separate object ensures that it is maintained as a reference rather than by value across any Object.assign() calls that service calls may do. For read-only attributes this isn't important, but count is something we write to and read back from.
|
Thank you, released as v4.0.5 |
This adds a set of service tests that ensure that all params.sequelize attributes are passed all the way down through to the getModel() of any service calls (and also to any subsequent service calls made by those service calls). This is important for reasons already outlined in #234 but I will paste the relevant section here with some slight mods:
It would be nice to have some tests in place to prevent regressions related to this functionality in the future, so that's why I've prepared these commits. Commit abcce1b adds the tests, and commit 8a95fec fixes a couple of test failures.
(Side note to @DesignByOnyx - I tried to integrate your feedback from the other PR and focused my attention on passing through only the .sequelize member of the params rather than the entire params object.)
Other note: to see the original PR that added getModel overriding with params, please see #166