commitRecords not working with multiple server records

[ivanleblanc]

Ext.data.Operations.commitRecords does not seem to be syncing the server data to the client record when the server returns multiple records.

This is for a multiple create operation, I have not tried a multiple update operation.

4.0.7 worked perfectly handling multiple records but as soon as I swap the API out with 4.1.0 rc3 it does not work. I noticed that the commitRecords function was changed quite a bit from 4.0.7 but I have not been able to determine why is has stopped working.

Again this only happens when multiple records are returned from the server, when one is returned it works fine in both 4.0.7 and 4.1.0 because of these lines (taken from 4.1.0 commitRecords):

clientRec = clientRecords[0];
serverRec = serverRecords[0];

The code doesn't seem to be able to match the server records back to the client records when there are multiple records returned form the server.

[nan21eu]

Do you have a clientId field in your model ? This may be a reason as the model uses a clientIdProperty:"clientId" to do this matching . I had a similar problem, as i use clientId as a tenant identificator and is present in most of my models, so my fix was:

Ext.override(Ext.data.Model, {
    clientIdProperty : "__clientRecordId__"
});

[ivanleblanc]

No I do not.
I just don't see how the commitRecords function works I guess.
If anyone can shed some light on it that would be great. So this is what i understand of it.
commitRecords code from 4.1.0 rc3

var me = this,
    mc, index, clientRecords, serverRec, clientRec;


if (!me.actionSkipSyncRe.test(me.action)) {
    clientRecords = me.records;
    if (clientRecords && clientRecords.length) {
        if(clientRecords.length > 1) {
            // if this operation has multiple records, client records need to be matched up with server records
            // so that any data returned from the server can be updated in the client records.
            mc = new Ext.util.MixedCollection();
            mc.addAll(serverRecords);


            for (index = clientRecords.length; index--; ) {
                clientRec = clientRecords[index];
                serverRec = mc.findBy(function(record) {
                    var clientRecordId = clientRec.getId();
                    if(clientRecordId && record.getId() === clientRecordId) {
                        return true;
                    }
This is where I get confused. So at this point the function knows that I have multiple records to commit and it
 has already determined that this is a new record not an update record.
But how can record.internalId === clientRec.internalId ever be true??
The clientRec.internalId is usually "ModelName"-{some random number Ext assigns if its a phantom record}
The serverRec.internalId is "ModelName"-{the unique id my server gives the new record}
                    // if the server record cannot be found by id, find by internalId.
                    // this allows client records that did not previously exist on the server
                    // to be updated with the correct server id and data.
                    return record.internalId === clientRec.internalId;
                });
                
                // replace client record data with server record data
                me.updateClientRecord(clientRec, serverRec);
            }
        } else {
            // operation only has one record, so just match the first client record up with the first server record
            clientRec = clientRecords[0];
            serverRec = serverRecords[0];
            // if the client record is not a phantom, make sure the ids match before replacing the client data with server data.
            if(serverRec && (clientRec.phantom || clientRec.getId() === serverRec.getId())) {
                me.updateClientRecord(clientRec, serverRec);
            }
        }


        if (me.actionCommitRecordsRe.test(me.action)) {
            for (index = clientRecords.length; index--; ) {
                clientRecords[index].commit();
            }
        }
    }
}

[dnorman]

Just smacked right into this problem myself.

Here's what I think is going on.
In 4.0.7, Ext.data.Store formerly did the following. (Note the very relevant inline comment)

 
    onCreateRecords: function(records, operation, success) {
        if (success) {
            var i = 0,
                data = this.data,
                snapshot = this.snapshot,
                length = records.length,
                originalRecords = operation.records,
                record,
                original,
                index;


            /*
             * Loop over each record returned from the server. Assume they are
             * returned in order of how they were sent. If we find a matching
             * record, replace it with the newly created one.
             */
            for (; i < length; ++i) {
                record = records[i];
                original = originalRecords[i];
                if (original) {
                    index = data.indexOf(original);
                    if (index > -1) {
                        data.removeAt(index);
                        data.insert(index, record);
                    }
/*  SNIP  */

This code is completely gone now, thank goodness!
The whole remove + insert thing royally sucked. Good riddance.

This is now being handled through commitRecords from Ext.data.Operation.
However, it would appear that this is crazy FiFo business ( seriously, who does that? gahh ) has fallen out of fashion, and all stores are expected to pass my clientId through to the other side, which would then be used to set internalId and match up the records based on that, yada yada yada. Seeing as how I'd rather not hack the living bujeesuz out of my server app, I thought it better to patch it.

Just a single line added below.
Bear in mind that I'm still using 4.1rc1 for the time being, so your version may differ slightly.

Ext.define('MyApp.patch.Operation.Omg.Wtf', {
    override: 'Ext.data.Operation',
    commitRecords: function (serverRecords) {
        var me = this,
            mc, index, clientRecords, serverRec, clientRec;


        if (!me.actionSkipSyncRe.test(me.action)) {
            clientRecords = me.records;


            if (clientRecords && clientRecords.length) {
                if(clientRecords.length > 1) {
                    // if this operation has multiple records, client records need to be matched up with server records
                    // so that any data returned from the server can be updated in the client records.
                    mc = new Ext.util.MixedCollection();
                    mc.addAll(serverRecords);


                    for (index = clientRecords.length; index--; ) {
                        clientRec = clientRecords[index];
                        serverRec = mc.findBy(function(record) {
                            var clientRecordId = clientRec.getId();
                            if(clientRecordId && record.getId() === clientRecordId) {
                                return true;
                            }
                            // if the server record cannot be found by id, find by internalId.
                            // this allows client records that did not previously exist on the server
                            // to be updated with the correct server id and data.
                            return record.internalId === clientRec.internalId;
                        });
                        
                        if(!serverRec && me.action == 'create') serverRec = serverRecords[index]; // MODIFIED - Oh the horror!
                        
                        // replace client record data with server record data
                        me.updateClientRecord(clientRec, serverRec);
                    }
                } else {
                    // operation only has one record, so just match the first client record up with the first server record
                    clientRec = clientRecords[0];
                    serverRec = serverRecords[0];
                    // if the client record is not a phantom, make sure the ids match before replacing the client data with server data.
                    if(serverRec && (clientRec.phantom || clientRec.getId() === serverRec.getId())) {
                        me.updateClientRecord(clientRec, serverRec);
                    }
                }


                if (me.actionCommitRecordsRe.test(me.action)) {
                    for (index = clientRecords.length; index--; ) {
                        clientRecords[index].commit();
                    }
                }
            }
        }
    }
});

[ivanleblanc]

Hey great fix!
As far as I can tell that one liner makes it work for now. (in 4.1.0 rc3)

[dnorman]

I'm glad it worked for you.

Sencha: Any way we can slip this one-liner into the GA release?
Otherwise it's going to break a buttload of applications when people switch to 4.1.

[ivanleblanc]

So the 4.1 GA release still seemed to not work on its own, but it does work with your fix as far as I can tell. The commitRecord code does need to be copied and pasted though from the 4.1 GA release because it has changed form the beta releases.

[dnorman]

Does sencha have a position on this issue?