Pass requestType to buildURL #2898
Conversation
|
Can you rebase? Seems like it's not merging cleanly |
amiel
force-pushed
the
requestType-to-buildURL
branch
2 times, most recently
from
d863109 to
e3dfab8
Compare
|
Rebased, sorry about that. |
| var data = {}; | ||
| var serializer = store.serializerFor(type.typeKey); | ||
| var snapshot = record._createSnapshot(); | ||
| var url = this.buildURL(type.typeKey, null, snapshot, 'createRecord'); |
There was a problem hiding this comment.
The code for createRecord pass 'create' to buildURL, but in this example it's 'createRecord', typo?
There was a problem hiding this comment.
Good catch. Which do you think it should be?
There was a problem hiding this comment.
I think it makes sense to keep it consistent with the method names?
There was a problem hiding this comment.
Ok, I'll go with that.
I think I was thinking that the method names were an implementation detail, but I think the consistency would be worth it.
Thanks for your input :)
There was a problem hiding this comment.
Ok, I've changed these all to match the original function name (createRecord, updateRecord, etc)
|
|
||
| if (sinceToken) { | ||
| query = { since: sinceToken }; | ||
| } | ||
|
|
||
| return this.ajax(this.buildURL(type.typeKey), 'GET', { data: query }); | ||
| url = this.buildURL(type.typeKey, undefined, undefined, 'findAll'); |
There was a problem hiding this comment.
Could these missing arguments be null instead of undefined? This will make it more consistent with https://github.com/emberjs/data/pull/2898/files#diff-24089ab4caa677e410cbc5c13588c012L533
|
👍 |
|
I've made changes suggested by @bmac, but I need to rebase again. |
amiel
force-pushed
the
requestType-to-buildURL
branch
2 times, most recently
from
a9f8331 to
11b06f0
Compare
This is another step toward emberjs/rfcs#4. Once url-templates lands, most users extending BuildURLMixin will not need to handle requestType explicitly, as it will be possible to define a template for each request type. Also move buildURL specific RESTAdapter tests to a new file
amiel
force-pushed
the
requestType-to-buildURL
branch
from
11b06f0 to
ff35ee7
Compare
|
@igorT this should be ready now. |
|
Thanks @amiel! |
|
Thank you! |
|
👍 |
This is another step toward emberjs/rfcs#4.
Others have requested ways to handle different urls per request type (see #1078).
Once url-templates lands, most users extending
BuildURLMixinwill not need to handlerequestTypeexplicitly, as it will be possible to define a template for each request type.