Make model optional

[fyockm]

model, while typically used, should be an optional parameter

[pgilad]

There are several other usages of model in this module, plus you (edit: might) need to reorganize params since options is the 3rd param...

[naugtur]

:+1: nice! Leaving it open for other opinions, will merge soon.

[fyockm]

A little more background...

I don't have a specific model that I want to update, nor do I want to create one. I know I could make a direct call to xhr, but for consistency sake, I'd prefer to keep the success / error callback format.

@pgilad yes, model is used elsewhere but can be sidestepped by passing correct options. I don't think the params should be reorganized, as passing in model is they typical use. However, this (small) change gives the ability to work around it.

Sample code for my use case:

function (userId, opts) {
        opts = _.defaults({
            url: this.url() + '/users',
            json: {
                _id: userId
            }
        }, opts);
        sync('create', null, opts);
}

[naugtur]

I looked through the code when reviewing this change and model is only really needed if there's no url in options. Every other case will behave reasonably.

Honestly, if someone calls sync without both url and model they probably don't expect a successful http request...

[pgilad]

Can you verify this code as well: (line 46)

if (options.data == null && model && (method === 'create' || method === 'update' || method === 'patch')) {
           params.json = options.attrs || model.toJSON(options);
}

the check for model should be optional too then...

[naugtur]

@pgilad My understanding of this is - if there's no options.data, and we're expected to POST something, get the data from model if it exists.

That means no model = no request body. Seems legit.

I don't know what attrs are used for though.

[fyockm]

@pgilad @naugtur params.json can be explicitly set in the options, and gets sent as the request body. If there's no model, then params.data probably wouldn't be used. Either way, code won't fail on this conditional if model is null.

[fyockm]

Any other questions on this one?

[naugtur]

Merged. Works for me. If anybody finds any problems with this after all, please feel free to revert ;)