[email protected] compliance

[dhritzkiv]

Recently, xhr clarified its behaviour for the json option, suggesting that json is flipped on/off, and the body is instead set to… the body. This has the side effect of preventing true/false being sent as a value via the json option, but is otherwise backwards compatible.

This PR is mostly to bring ampersand-sync inline with how xhr prefers its data, but also solves one very specific, confusing use case that would fail (without this PR). I'll try to explain it:

//get an ampersand-model somehow…
// and then proceed to create a custom request using `sync` (with a custom `body`),
// which basically proxies the options to xhr

const opts = {
    body: {foo: "bar"},
    json: true
};

model.sync("create", model, opts);

//what currently happens is ampersand-sync will set the `json` option 
// to the model's data (`model.toJSON()`), which will
// prevent `opts.body` from being used in `xhr`, as `json` contain the model's data,
// and since it's not a boolean, it takes precedence over `body`

//with this PR, ampersand-sync will now set `params.body` to the model's data.
// However, when it comes time to invoke `xhr`, `ajaxSettings` is
// built up of `params` and `options` (with the values in `options` taking precedence via `assign`).
// so, in the end the `body` property of `ajaxSettings` –which is what `xhr`will use in `send()`–
// is equal to `opts.body` (which is arguably what we want) and not the value from `model.toJSON()`

In short, this should make .sync options behaviour more congruent with xhr's options behaviour. While using body is not an option specified within ampersand-sync's documentation, and data should probably be the preferred way to pass data, sometimes it's tricky to remember which property to use (body or data) when switching between using xhr and ampersand-sync.

Overall, this PR shouldn't introduce any breaking changes.

[wraithgar]

This looks great, thanks for staying on top of this change. +1 from me

[naugtur]

Great thinking, this could have been considered a bug before! Travis seems broken because of missing credentials. If they're not there, how did it work before?

[dhritzkiv]

@naugtur tests have been broken across all Ampersand modules for about a year now :(

I don't have the expertise or access to fix the CI tests.

[fyockm]

:+1:

[wraithgar]

Tests ran from PRs from external repos will always fail, only tests made from PRs from local branches will run.