PDA

View Full Version : [OPEN] [FIXED-EXTJSIV-393] writeAllFields does not work when saving just created model



Greendrake
27 Mar 2011, 1:24 AM
Trying to create and save a model with omitted fields causes them all being sent to server as empty values (converted to their types), even if the writer has writeAllFields set to false.

var user = Ext.ModelMgr.create({
}, 'User');
user.save();
The code above (as from the example in Model documentation) will save the model as having:

{"id":0,"name":"","age":0}
I guess when writeAllFields is set to false, the writer should not assume default empty values for non-provided fields (particularly when the defaults are not explicitly defined). It should either assume them to be null or, better, omit them at all.

evant
27 Mar 2011, 1:46 AM
This was done intentionally. If you have a new record, there's no concept of the fields being dirty or not, as it hasn't been created yet.

I'm open for suggestion but I don't really see a decent default other than what exists already.

Greendrake
27 Mar 2011, 2:26 AM
I don't really see a decent default other than what exists already.
Zero is a value. Empty string is a value too. (I wonder what would be saved in case of boolean field) Why would these values be saved if they have not been specified? Isn't it obvious that not saving them at all (or, alternatively, saving NULLs) is much more decent behaviour than saving empties? I guess there is no need to connect this behaviour with the concept of being dirty. It is just about doing exactly what was ordered to do and not assuming unnecessary (and potentially harmful) values.

Greendrake
27 Mar 2011, 11:30 AM
Ok I can see now this is actually an issue with Model itself, there is no connection to writers.

Ext.regModel('User', {
fields: [
{name: 'node_id', type: 'int'},
{name: 'name', type: 'string'},
{name: 'age', type: 'int'},
{name: 'foo', type: 'boolean'}
]
});
var user = Ext.ModelMgr.create({age : 24}, 'User');
console.log(user.get('foo'));
The output of the code above is false. IMHO this is ridiculous and not acceptable. It should be either null or undefined.

evant
27 Mar 2011, 2:09 PM
That is what the useNull option is for on those number types. In the next release the same option is applied to strings/bools.

Greendrake
28 Mar 2011, 3:38 AM
That is what the useNull option is for on those number types
Thanks for the hint.

I've been back to the writer issue trying to make it sending only those fields that have been set either explicitly or through defaultValue (currently ALL fields are sent if the record is new/phantom, regardless writeAllFields). The point is simple: if a field value has not been defined, have an option of not sending it at all. Quite often we need server side code to receive exactly the same data that have been created on the client side, without any additional assumptions / alterations. I have reached the solution by the following amendments. Would you consider them?

Ext.data.Field defaultValue: undefined rather than empty string
Ext.data.Writer getRecordData: ignoring record.phantom, writing all fields only if writeAllFields is set
Ext.data.Model constructor: checking if field.defaultValue is defined before applying it to data to be set:

for (i = 0; i < length; i++) {
field = fields[i];
name = field.name;

if (data[name] === undefined && Ext.isDefined(field.defaultValue)) {
data[name] = field.defaultValue;
}
}
and commenting out the dirty/modified reset:

//~ this.dirty = false;
//~ this.modified = {};