PDA

View Full Version : [CLOSED][3.0.1] Ext.data.Store.execute() method 'baseParams' issue



DATABASICS
28 Aug 2009, 1:20 PM
At our application, we use the store 'beforeload' event to set some common parameters on the property 'baseParams' before load the store. But the store does not submit the parameters set by the 'beforeload' event handler as it did at Ext2.2. We found that at Ext3.0.1, the Ext.data.Store.execute() method ignores the parameters at the 'baseParams' when submits the request.

The source code:
execute : function(action, rs, options) {
....

if (this.writer && this.proxy.url && !this.proxy.restful && !Ext.data.Api.hasUniqueUrl(this.proxy, action)) {
options.params.xaction = action; // <-- really old, probaby unecessary.
}
//when executes the code bellow, the 'params' does not include the parameters from 'baseParams' property.
this.proxy.request(Ext.data.Api.actions[action], rs, options.params, this.reader, this.createCallback(action, rs), this, options);
}
return doRequest;
}


When modified according to the following, it works correctly.

options.params = Ext.apply({}, options.params, this.baseParams);
if (this.writer && this.proxy.url && !this.proxy.restful && !Ext.data.Api.hasUniqueUrl(this.proxy, action)) {
options.params.xaction = action; // <-- really old, probaby unecessary.
}
this.proxy.request(Ext.data.Api.actions[action], rs, options.params, this.reader, this.createCallback(action, rs), this, options);
}
return doRequest;

analpaper
28 Aug 2009, 6:49 PM
in revision 5237, baseParams's modifications inside beforeload event handler are still ignored.

it dont seems to work correctly with the patch 'options.params = Ext.apply({}, options.params, this.baseParams);';
First call looks nice, but if second call modifies again the param, it will not be reflected until call third. (crazy)

better than:


beforeload:function(s) {
s.baseParams.modMe = Ext.id();
}

is:


beforeload:function(s,o) {
o.params.modMe = Ext.id();
}


no?

evant
30 Aug 2009, 7:47 PM
Please post a short test case to demonstrate the issue. Thanks.

analpaper
30 Aug 2009, 8:50 PM
sorry cos this is not my thread, but here it is a the test case:

please, run this in firebug console (for example i tested with http://localhost/lab/ext-3.0+/examples/grid/row-editor.html):

var modMe = 0;
var s = new Ext.data.Store({
url:location.pathname,
baseParams:{modMe:modMe},
listeners:{
beforeload:function(s){
s.baseParams.modMe = modMe++;
}
}
});
s.load();
s.load();
s.load();
s.load();

if you see the value of the POST params of each xhr, you will see values:
0
0
1
2
but the expected was:
0
1
2
3

note that the code below works as expected:

var modMe = 0;
var s = new Ext.data.Store({
url:location.pathname,
baseParams:{modMe:modMe},
listeners:{
beforeload:function(s,o){
o.params.modMe = modMe++;
}
}
});
s.load();
s.load();
s.load();
s.load();

P.D tested with revision 5238

Condor
30 Aug 2009, 10:35 PM
This is expected behaviour.

The baseParams have already been applied to the options params when beforeload is called.

DATABASICS
2 Sep 2009, 5:14 AM
The setting options.params approach works perfectly for 'beforeload' event handler.

Thanks, Condor.

Condor
2 Sep 2009, 6:00 AM
You don't even need to use baseParams if you use the beforeload event to add parameters, e.g.

var modMe = 0;
var s = new Ext.data.Store({
url: location.pathname,
//baseParams: {modMe: modMe},
listeners:{
beforeload: function(s, o){
o.params.modMe = modMe++;
}
}
});
s.load();
s.load();
s.load();
s.load();

MaxT
6 Oct 2009, 8:37 AM
You can't use baseParams in Ext 3.0.2 because this is broken.

store execute() is buggy, took us a while to fix this one!

MaxT
9 Oct 2009, 6:17 AM
Ext 3.0.2 code below.

This is still faulty, as in the report above where the baseParams of the previous request get sent on the current request.

I believe that there should not be a change to the basic capability of being able to modify baseParams in the beforeload event. The baseParams are static, and often people will only modify them in a beforeload event. This is the way things worked in Ext 2.x and Ext 3.0.0.


Reason:

1. Ext.applyIf(options.params, this.baseParams);

This code gets executed and copies baseParams into options.params if they don't already exist in options.params. However, on typical store loads such as with a paged grid this will be the baseParams from the previous request.


2. doRequest = this.fireEvent('beforeload', this, options);
The beforeload event is executed. Here, existing baseParams are modified.


3. var params = Ext.apply({}, options.params, this.baseParams);
All the params are now combined. This part is wrong because the modified baseParams from 2. are not picked up. This is because they already exist in options.params and Ext.apply will only overwrite them if they don't already exist. The third option in Ext.apply specifies defaults to be applied if they don't already exist.





execute : function(action, rs, options) {
// blow up if action not Ext.data.CREATE, READ, UPDATE, DESTROY
if (!Ext.data.Api.isAction(action)) {
throw new Ext.data.Api.Error('execute', action);
}
// make sure options has a params key
options = Ext.applyIf(options||{}, {
params: {}
});

// have to separate before-events since load has a different signature than create,destroy and save events since load does not
// include the rs (record resultset) parameter. Capture return values from the beforeaction into doRequest flag.
var doRequest = true;

if (action === 'read') {
Ext.applyIf(options.params, this.baseParams);
doRequest = this.fireEvent('beforeload', this, options);
}
else {
// if Writer is configured as listful, force single-record rs to be [{}] instead of {}
// TODO Move listful rendering into DataWriter where the @cfg is defined. Should be easy now.
if (this.writer.listful === true && this.restful !== true) {
rs = (Ext.isArray(rs)) ? rs : [rs];
}
// if rs has just a single record, shift it off so that Writer writes data as '{}' rather than '[{}]'
else if (Ext.isArray(rs) && rs.length == 1) {
rs = rs.shift();
}
// Write the action to options.params
if ((doRequest = this.fireEvent('beforewrite', this, action, rs, options)) !== false) {
this.writer.write(action, options.params, rs);
}
}
if (doRequest !== false) {
// Send request to proxy.
var params = Ext.apply({}, options.params, this.baseParams);
if (this.writer && this.proxy.url && !this.proxy.restful && !Ext.data.Api.hasUniqueUrl(this.proxy, action)) {
params.xaction = action; // <-- really old, probaby unecessary.
}
// Note: Up until this point we've been dealing with 'action' as a key from Ext.data.Api.actions.
// We'll flip it now and send the value into DataProxy#request, since it's the value which maps to
// the user's configured DataProxy#api
this.proxy.request(Ext.data.Api.actions[action], rs, params, this.reader, this.createCallback(action, rs), this, options);
}
return doRequest;
},

Condor
9 Oct 2009, 6:23 AM
baseParams aren't broken! You just can't modify them anymore in the beforeload event or you need to apply them to both the params and the baseParams, e.g.

beforeload: function(store, options){
var myParams = {...};
Ext.apply(store.baseParams, myParams);
Ext.apply(options.params, myParams);
}

MaxT
11 Oct 2009, 11:50 PM
You just can't modify them anymore in the beforeload event

Why? How is this an improvement in any way? There must be a good reason for this change.

I don't see it mentioned in the docs anywhere:

http://www.extjs.com/products/extjs/CHANGES_ext-3.0.2.html
http://www.extjs.com/products/extjs/CHANGES_ext-3.0.1.html
http://www.extjs.com/products/extjs/CHANGES_ext-3.0.0.html

All I can find in 3.0.0 is:


Ext.data.Store
* baseParams may be overwritten by params of same name passed to load()

analpaper
12 Oct 2009, 1:57 AM
Why? How is this an improvement in any way? There must be a good reason for this change.

Later than update my deprecated code, the size of the whole app results as -981 bytes.

okok, seriously, i dont care too much about docs or changelogs, im just happy cos the later term is shorter.. I updated the code only to make-it-work, but without a reasonable reason.

The updatability of the deprecated methology related to modify baseParams property inside beforeload event is unfeasible in your app? or the issue is that you dont want to do a not-reasonable update?

Im just curious about why you are fighting for this closed case.
If i can help you in any way just share your pain with us, but im sorry cos you looks like searching for a reason, not for a solution.. so no way for me to help you. but you need to close this thread also xD so please, anybody can give a polite reason?!

MaxT
12 Oct 2009, 2:34 AM
It's simple. This change breaks just about every page in my application, and I have a large application. I don't see why I should change and retest my code for what I consider to be a buggy and bad change. Personally, I have overriden the execute method to keep it compatible, although I don't want to do this in the long run.

I can vaguely see from an object oriented perspective why you could define baseParams as not to be modifiable in a beforeload listener, but since it has never been that way, why change it now in a point release that is supposed to be bug fixes only? (In fact, if this is the case then the stategy of applying baseParams, executing beforeload, then re-applying baseParams as defaults looks very odd.) If this change was required then 3.0.0 would have been the appropriate point.

I don't see why restoring the previous behaviour of baseParams would do any harm at all.

Zolcsi
16 Nov 2009, 7:29 AM
Hi, I had the same problem, took me a lot of search and testing to figure out what's causing the problem, and even more to rewrite everything to this version.
I could have really used some warning about this in the changelog, but what's done is done, so everything works again :)