PDA

View Full Version : [CLOSED] Client generated ID is written for phantom records



Greendrake
15 Aug 2015, 5:39 PM
Consider this example (https://fiddle.sencha.com/#fiddle/s84):
Ext.define('Foo', {
extend: 'Ext.data.Model',
fields: ['name'],
proxy: {
type: 'ajax',
url: location.href
}
});
var foo = Ext.create('Foo', {name : 'McDuck'});
foo.save();

In Ext JS 4, the JSON sent to the server side will read:

{"name":"McDuck"}
That was really good.

Now, since version 5 the JSON is:
{"name":"McDuck","id":"Foo-1"}
That is bad and sad falls a bit short of expectations ;).

I guess that is probably a side effect of introducing new features such as allDataOptions (https://docs.sencha.com/extjs/6.0/6.0.0-classic/#!/api/Ext.data.writer.Writer-cfg-allDataOptions) and partialDataOptions (https://docs.sencha.com/extjs/6.0/6.0.0-classic/#!/api/Ext.data.writer.Writer-cfg-partialDataOptions). Now not only the client-generated record ID is unnecessarily sent for phantom records, but also nothing seems to work to prevent it. Well, writeRecordId: false works, but it prevents sending the ID for non phantom records also.

I think, by default, sending client-generated record ID for phantom records is only needed when creating multiple records in a single call, in which case clientIdProperty (https://docs.sencha.com/extjs/6.0/6.0.0-classic/#!/api/Ext.data.Model-cfg-clientIdProperty) would be in use. So, unless clientIdProperty is specified on the model, client-generated ID should not be sent.

Update:

Just figured out a quick fix:
Ext.define('Ext.overrides.data.writer.Writer', {
override: 'Ext.data.writer.Writer',
getWriteRecordId: function() {
return !this.isPhantom && this.callParent();
},
getRecordData: function(record) {
this.isPhantom = record.phantom;
var r = this.callParent(arguments);
delete this.isPhantom;
return r;
}
});

joel.watson
16 Aug 2015, 4:54 AM
Yes, this behavior was added in Ext JS 5, as outlined here:

https://docs.sencha.com/extjs/5.1/whats_new/5.0/extjs_upgrade_guide.html#Properties___Configs

Instead of using an override, there are other methods you could use to achieve the same functionality. You might check out the options outlined here, under "Using Model Ids in Ext JS 5":

https://www.sencha.com/blog/top-support-tips-4/

Thanks!
Joel

Greendrake
16 Aug 2015, 6:16 AM
Many thanks Joel! I missed those bits, but now things are clear.

I have a feature request now though. Please consider moving this thread to Feature Requests.

The feature I am asking is just the ability to specify clientIdProperty: false on Ext.data.writer.Writer in order to prevent writing the "id" property for phantom records. This would be implemented by replacing the following lines in the getRecordData method:
if (clientIdProperty) { // if (phantom and have clientIdProperty)
ret[clientIdProperty] = value; // must read data and write ret
delete data[key]; // in case ret === data (must not send "id")
}
else if (!me.getWriteRecordId()) {
delete data[key];
}

by
if (clientIdProperty) { // if (phantom and have clientIdProperty)
ret[clientIdProperty] = value; // must read data and write ret
}
if (
// null is the default value for clientIdProperty.
// If it is not null here, it would be either a string or false.
// In either of those, we do not write "id"
(phantom && clientIdProperty !== null)
||
!me.getWriteRecordId() // writing "id" is explicitly prohibited
) {
delete data[key];
}

See example: https://fiddle.sencha.com/#fiddle/s8d

Reasoning:

In the article (https://www.sencha.com/blog/top-support-tips-4/) you pointed me to, you are suggesting three (!) ways of dealing with the original issue this thread was about. Two of them are trade-offs / workarounds (the ability to have unusual e.g. negative values for the "id" property with id generators, and renaming the "id" property something else using clientIdProperty). The third one (transform()) is a solution, though heavy and risky. The bottom line is that the easiness of not having the "id" property written at all is just not there any more. Version 5 has taken it out!

But making it easy again is very easy, as shown by the code I suggested above.

joel.watson
18 Aug 2015, 1:25 PM
Hi--

There is currently a feature request for this (EXTJS-18207).

Thanks
Joel