Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE ds-improved-ajax] Finer control over customizing a request #3099

Merged
merged 1 commit into from Mar 21, 2016
Merged

[FEATURE ds-improved-ajax] Finer control over customizing a request #3099

merged 1 commit into from Mar 21, 2016

Conversation

pangratz
Copy link
Member

Though ajax() (and ajaxOptions()) of DS.RESTAdapter are marked as
private, they have been overwritten in many applications, since there is
currently no other way to customize the request.

This feature adds new public methods to DS.RESTAdapter, which allow to
customize the properties of a request:

  • methodForRequest to get the HTTP verb
  • urlForRequest to get the URL
  • headersForRequest to get the headers
  • dataForRequest to get the data (query params or request body)

The params hash passed to those methods has all the properties with
which the corresponding find, createRecord, findQuery, ... call
have been invoked: store, type, snapshot(s), id(s) and query. The
requestType property indicates which method is requested; the possible
values are:

  • createRecord
  • updateRecord
  • deleteRecord
  • query
  • queryRecord
  • findRecord
  • findAll
  • findMany
  • findHasMany
  • findBelongsTo

Performing the actual AJAX request is handled by the makeRequest
method, which is similar to the existing ajax method: it makes the
request using jQuery.ajax and attaches success and failure handlers.


Say your API handles creation of resources via PUT, this can now be
customized as follows:

// adapters/application.js
import DS from 'ember-data';

export default DS.RESTAdapter.extend({
  methodForRequest: function(params) {
    if (params.requestType === 'createRecord') {
      return "PUT";
    }

    return this._super(...arguments);
  }
});

Credit for the initial idea goes to @igorT in #3047 (comment).

adapter.ajax = function(request) {
passedUrl = request.url;
passedVerb = request.method;
passedHash = request.body ? { data: request.body } : undefined;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to update all assertions below, hence this weird setup; I think this can be refactored/simplified more once this is accepted ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not relevant anymore, since everything is behind a feature flag...

urlForRequest(params) {
var { type, id, ids, snapshot, snapshots, requestType, query } = params;

// type and id is not passed from updateRecord and deleteRecord, hence they
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/is/are/

@SeyZ
Copy link

SeyZ commented Oct 21, 2015

Awesome thanks @pangratz

@pangratz
Copy link
Member Author

Just a heads up: I am rebasing this PR onto latest master and also try to put everything behind a feature flag...

@tchak
Copy link
Member

tchak commented Dec 22, 2015

💯 this is so awesome! I will be able to kill all the hacks I have in ajaxOptions :)

@workmanw
Copy link

Is the thought to make headersForRequest and bodyForRequest public? I'm currently using ajaxOptions and had no idea it was technically private. Several SO answers and blog posts mention using ajaxOptions.

Either way, there should definitely be a public way to do this that isn't declarative. Things like CSRFs and sessions can be rotating constantly.

@pangratz
Copy link
Member Author

@workmanw I haven't added documentation for the methods yet, but I think it would be favorable to make those methods public. Currently ajax and ajaxOptions are private but they are overwritten many times in different apps since there is currently no other way to customize the request...

@pangratz pangratz changed the title Split construction of AJAX options into separate methods [FEATURE ds-improved-ajax] Finer control over customizing a request Dec 23, 2015
@pangratz
Copy link
Member Author

I rebased onto latest master branch and put everything behind a feature flag ds-improved-ajax. Also, rudimentary documentation has been added for the new public methods.

For easier review of the commits which add the calls in the adapter methods, the whitespace should be ignored by appending ?w=1 to the GitHub commit URL:

@pangratz
Copy link
Member Author

I once again rebased onto latest master and squashed all commits into 1. This is ready to go @bmac.

* @param {Object} params
* @return {Object} request object
*/
requestFor(params) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets rename this to _requestFor.

Though `ajax()` (and `ajaxOptions()`) of `DS.RESTAdapter` are marked as
private, they have been overwritten in many applications, since there is
currently no other way to customize the request.

This feature adds new public methods to `DS.RESTAdapter`, which allow to
customize the properties of a request:

- `methodForRequest` to get the HTTP verb
- `urlForRequest` to get the URL
- `headersForRequest` to get the headers
- `dataForRequest` to get the data (query params or request body)

The `params` hash passed to those methods has all the properties with
which the corresponding `find`, `createRecord`, `findQuery`, ...  call
have been invoked: store, type, snapshot(s), id(s) and query. The
`requestType` property indicates which method is requested; the possible
values are:

- `createRecord`
- `updateRecord`
- `deleteRecord`
- `query`
- `queryRecord`
- `findRecord`
- `findAll`
- `findMany`
- `findHasMany`
- `findBelongsTo`

Performing the actual AJAX request is handled by the `_makeRequest`
method, which is similar to the existing `ajax` method: it makes the
request using `jQuery.ajax` and attaches success and failure handlers.

---

Say your API handles creation of resources via PUT, this can now be
customized as follows:

``` js
// adapters/application.js
import DS from 'ember-data';

export DS.RESTAdapter.extend({
  methodForRequest: function(params) {
    if (params.requestType === 'createRecord') {
      return "PUT";
    }

    return this._super(...arguments);
  }
});
```
@bmac
Copy link
Member

bmac commented Mar 21, 2016

Looks good.

bmac added a commit that referenced this pull request Mar 21, 2016
[FEATURE ds-improved-ajax] Finer control over customizing a request
@bmac bmac merged commit e57f67e into emberjs:master Mar 21, 2016
@pangratz pangratz deleted the request_options branch March 21, 2016 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants