PDA

View Full Version : [FIXED][3.0.0] Record holding Store instance when removed



bt_bruno
21 Jul 2009, 9:33 AM
When adding records to a store, record will hold store's instance :



//Ext.data.Store.add
add:function(){
....
for(var i = 0, len = records.length; i < len; i++){
records[i].join(this);
}
...
}

//Ext.data.Record
join : function(store){
this.store = store;
}


But if I remove record from store, the property isn't cleaned. Soo if I remove, and update a record, store's listeners will fire, and some errors can occour.

How to reproduce

1.take array grid example from ext examples
2.attach update listener to store

,listeners:{
update: function(){ alert('store listener: data has been updated!'); }
}
3.Create this button in grid's bbar

,bbar:[{
text:'Remove record 0'
,handler:function()
{
//take record 0
var record = store.getAt(0);

//remove it from store, as from grid
store.remove(record);

alert('record has been removed!')

//update data... but fires store's listener?
record.set('company','changed');
}
}]

Sorry if the error has already been found. I've try to search in forum...

mystix
21 Jul 2009, 9:43 AM
same issue exists in the 2.x branch.

the Store should delete the reference to itself from the Record by calling Record#join(null) on Record removal:


Ext.override(Ext.data.Store, {
remove : function(record){
var index = this.data.indexOf(record);
if(index > -1){
record.join(null);
this.data.removeAt(index);
if(this.pruneModifiedRecords){
this.modified.remove(record);
}
if(this.snapshot){
this.snapshot.remove(record);
}
this.fireEvent('remove', this, record, index);
}
}
});

mystix
21 Jul 2009, 9:52 AM
Store#removeAll() needs similar treatment:


Ext.override(Ext.data.Store, {
removeAll : function(){
this.data.each(function(rec) {
rec.join(null);
});
this.data.clear();
if(this.snapshot){
this.snapshot.clear();
}
if(this.pruneModifiedRecords){
this.modified = [];
}
this.fireEvent('clear', this);
}
});

Condor
21 Jul 2009, 9:57 AM
Also, all 'update' listeners should be able to handle an update that is not in the store (because it is currently filtered out).

GridPanel (in older Ext versions) and DataView (still present) have this problem.

evant
21 Jul 2009, 7:30 PM
Condor, can you expand on that?

Condor
21 Jul 2009, 11:22 PM
The problem described by the OP would cause an 'update' event to be triggered for a record that is not in the store (which obviously should be fixed).

But it is also possible to generate an 'update' event for a record that is in the store, but which doesn't have an index because it is filtered out (it's part of the snapshot, but not of data).

GridView.onUpdate (refreshRow actually) was fixed, so it ignores updates for records that don't have an index.

However, DataView.onUpdate still fails if it receives an update of a record for which indexOf returns -1.

(I'm not saying anything new here, there is an Ext 2.x bugreport for this)

evant
22 Jul 2009, 5:17 PM
Both of these issues are fixed in SVN (both branches).