Namespace query #3459
Namespace query #3459
Conversation
This makes sense to me.
I believe
Go for it! |
|
|
|
Alright I think this PR is ready for a check ! |
| switch (requestType) { | ||
| case 'findRecord': | ||
| return this.urlForFindRecord(id, modelName, snapshot); | ||
| case 'findAll': | ||
| return this.urlForFindAll(modelName); | ||
| case 'query': | ||
| return this.urlForQuery(query, modelName); | ||
| return this.urlForQuery(options, modelName); |
There was a problem hiding this comment.
This and line 52 seems like it could be a breaking change. Any thoughts on how to make the transition easier @igorT?
|
@bmac just pushed the changes we talked about, are we waiting for @igorT about the issue you mentioned in We could do a simple deprecation stuff : What do you think ? |
| @param {String or subclass of DS.Model} type | ||
| @param {any} query an opaque query to be used by the adapter | ||
| @param {String} modelName | ||
| @param {Hash} options, query an opaque query of the form {query: { any }} to be used by the adapter |
There was a problem hiding this comment.
The API docs prefer the term Object instead of Hash for the type annotations. Same for line 1045 above.
|
Btw can I use the ES6 default parameter feature ( |
|
@bmac should be good to go ! |
|
ping @igorT r? |
|
@bmac I think this PR has been descoped |
|
ping @igorT. This pr still has an open question on backwards compatibility. |
|
Hi @thaume. Thanks so much for this work. Unfortunately, we weren't able to get this pr merged before the Ember Data 2.0 release. I'm going to close this pr for now because I believe we missed the opportunity to make this change without breaking semver. Please feel free to reopen if you disagree. |
|
Hi @bmac. I knew it already ;) Thanks for your work on ember data ! |
@igorT a few questions :
1/ Tests directory
Should I move the "integration/adapter/queries - Queries” test module to "integration/store/query” (alongside "integration/store/queryRecord”)
2/ Function param
(modelName, query)=>(modelName, options)Does that make sense ? Writing
options.queryseems better thanquery.query, what do you think ?3/ In "integration/adapter/rest_adapter" Rename
findQuerytoqueryin tests ?It is still a WIP, I need to read myself again to catch any typo/non-up-to-date comments