fetch overwrites options.data with current state
Closed this issue · 1 comments
dan-f commented
The current implementation of fetch
allows the caller to pass in an options.data
object which maps querystring parameter names to values to be sent to the server. This options.data
is merged together with the collection's current state by examining the state
and queryParams
objects.
The problem is that state
/queryParams
currently take precedence. For example, if I run:
collection.fetch({data: {page: 2});
The collection will still fetch the first page. This seems like a bug to me; I expected options.data
to take precedence.
var i, kvp, k, v, kvps = _pairs(queryParams), thisCopy = _clone(this);
for (i = 0; i < kvps.length; i++) {
kvp = kvps[i], k = kvp[0], v = kvp[1];
v = _isFunction(v) ? v.call(thisCopy) : v;
if (state[k] != null && v != null) {
data[v] = state[k];
}
}
And a fix:
var i, kvp, k, v, kvps = _pairs(queryParams), thisCopy = _clone(this);
for (i = 0; i < kvps.length; i++) {
kvp = kvps[i], k = kvp[0], v = kvp[1];
v = _isFunction(v) ? v.call(thisCopy) : v;
if (state[k] != null && v != null && _.isUndefined(data[v])) { // Only set if this key/val pair isn't already present in `data`.
data[v] = state[k];
}
}
...And a more concise fix:
_.each(queryParams, function (v, k) {
v = _isFunction(v) ? v.call(thisCopy) : v;
if (state[k] != null && v != null && _.isUndefined(data[v])) {
data[v] = state[k];
}
}, this);
wyuenho commented
This looks good. Please submit a PR along with a test. Thanks so much.