PDA

View Full Version : [CLOSED][3.??] Ext.data.Store priority of baseParams vs params



rraponi
25 Jun 2009, 1:55 PM
The params are not being set correctly in the Store

currently in svn there is this line for setting the params:

var params = Ext.apply({}, options.params, this.baseParams);

previously it was:

var params = Ext.apply(options.params || {}, this.baseParams);

Could you please review.

tryanDLS
25 Jun 2009, 2:17 PM
Do you have a test case for this? I just ran the following against RC2 and current SVN and I don't see any difference in what parms are sent



Ext.onReady(function() {
var store = new Ext.data.JsonStore({
// Load data at once
autoLoad: true,
// Override default http proxy settings
proxy: new Ext.data.HttpProxy({
method: 'POST',
url: 'service.asmx/getAgencyList',
// Ask for Json response
headers: { 'Content-Type': 'application/json;charset=utf-8' }
}),
// Root variable
root: 'd',
// Record identifier
id: 'agentId',
// Fields declaration
fields: ['agentId', 'name', 'description']
});

store.baseParams = {x:0};
store.load();
store.load({params:{foo:'bar'}});
});

mjlecomte
25 Jun 2009, 4:15 PM
Yes, I had noticed this and have also put a query forth to the developers. This is either a breaking change or fix depending on your perspective.

Previously baseParams would trump params, now the situation is reversed. I think someone complained that params should override baseParams. IIRC though, part of the reason to introduce the new setBaseParam method was to make altering the baseParams easier.

mjlecomte
25 Jun 2009, 4:17 PM
I just ran the following against RC2 and current SVN and I don't see any difference in what parms are sent



store.baseParams = {x:0};
store.load();
store.load({params:{foo:'bar'}});
});


Are you of same opinion if you use baseParams?

rraponi
25 Jun 2009, 4:37 PM
When I have:

options.params= {dir: "ASC", forceSelection: false, sort: "_id", start:0}

baseParams = {limit 100}


The new version for params returns:
{dir: "ASC", forceSelection: true, limit: 100, sort: "_id", start: 0}

The old version for params returns:
{dir: "ASC", forceSelection: false, limit: 100, sort: "_id", start: 0}

tryanDLS
25 Jun 2009, 4:43 PM
Are you of same opinion if you use baseParams?
Just went back and checked my test - the missing 's' was a typo when I added the line to the above post (fixed the above post).

Re-ran the above:
call x=0
call 2 x=0&foo=bar
call 3 x=0

Am I missing what the problem really is?

evant
25 Jun 2009, 4:46 PM
In your example, you've said that forceSelection changes, however I don't see that in either your params or baseParams. Can you please post a test case to demonstrate the issue?

mjlecomte
25 Jun 2009, 5:28 PM
Just went back and checked my test - the missing 's' was a typo when I added the line to the above post (fixed the above post).

Re-ran the above:
call x=0
call 2 x=0&foo=bar
call 3 x=0

Am I missing what the problem really is?

A little bit I think. The issue is priority. In your example you demonstrate you can set foo. The issue is if you have foo=bar as a param and foo=baz as a baseParam, which one takes priority.

Personally I like the new breaking change. I think the old way was counterintuitive.

rraponi
26 Jun 2009, 9:10 AM
I am updating some OLD code to use the latest.
In this code the params were a string.

ds.load({params:"start=0&forceSelection=true&forumId=4"});

I will update my code to use key/value pairs.
The couple problem areas I have with the Store interestingly have NO baseParams set.

mjlecomte
26 Jun 2009, 9:22 AM
Would you clarify if you have issues / still object to what is in svn or not? I'm not clear what your current position is.

My current thought is what is in svn is a breaking change, but an improvement, and there are a few spots the docs will need to be updated accordingly.

Animal
26 Jun 2009, 9:34 AM
IMHO, params should override any baseParams of the same name.

mystix
26 Jun 2009, 9:41 AM
IMHO, params should override any baseParams of the same name.

+1

rraponi
26 Jun 2009, 11:01 AM
considered it closed.

Just update the docs for others!

mschwartz
26 Jun 2009, 12:07 PM
IMHO, params should override any baseParams of the same name.

+1

I agree.

mjlecomte
26 Jun 2009, 1:28 PM
considered it closed.

Just update the docs for others!


Updated to CLOSED
Updated Docs
Updated Upgrade Guide

mjlecomte
28 Jun 2009, 8:23 PM
FYI, just linking the thread that caused the change
http://extjs.com/forum/showthread.php?p=344532#post344532