Issue with editing complex object defined as SimpleType in a list grid editor

[jaredm]

Hi,

I'm using 10.1-Pro 2016-03-27 and have identified a possible bug that causes my grid.data values for a cell to be clobbered when I enter row editing after making a change. I'm still tracing through my issue so please bear with me if I've identified something that's a non-issue.

I think the issue is in the getEditedRecord() method of ISC_DataBinding.js:

        var rtn = {},
            baseRecordCopy = {};
        // Combine a recursive copy of the base data, not the base data itself; if we don't do this,
        // the combined record includes pointers to any sub-objects in the base data, so values in
        // the base data will get overwritten by the combination process.  Note, this.getEditForm()
        // may well return null for some uses of this method, but it's OK because
        // DBC._duplicateValues() now copes with being passed a null component (it just performs a
        // straight schemaless dup)
        isc.Canvas._duplicateValues(this.getEditForm(), record, baseRecordCopy);
        this.combineRecords(rtn, baseRecordCopy);
        this.combineRecords(rtn, editValues);

The code comments above very clearly state that you're trying to avoid pointers to sub-objects and also that getEditForm() can be null. However, the issue is that if getEditForm() is null, there is no opportunity for the code in _duplicateValues to actually do a safe-copy (since the _duplicateValues method will have a null dataSource and therefore can't check if the SimpleType has a duplicate method defined, etc.).

What then happens is combineRecords gets called and that in turn calls isc.combineObjects, where the comment there explains that: "Note the goal here isn't to avoid the destination pointing to the same objects as the source"

The end result of all this is that the record instance does end up with pointers in the base data. I've verified this by step-debugging in the JavaScript console and calling getCellRecord() after each statement.

To test my hypothesis I extended the getEditedRecord() method of my ListGrid instance, copied the function definition verbatim from ISC_Grids and replaced the call to isc.Canvas._duplicateValues() with the following:

rtn = isc.clone(record);

This has solved my problem, although it is not a proper solution.

I then tried to use your original approach with a small tweak:

isc.Canvas._duplicateValues(this.getEditForm() [U][B]|| this[/B][/U], record, baseRecordCopy);

With this, if getEditForm() does return null at least you can pass in the ListGrid as an alternative - then isc.Canvas._duplicateValues can get the DataSource information from the ListGrid (providing its set). I've tested this and it works too.

If this change looks sensible to you then please feel free to incorporate it.

[Isomorphic]

Your explanation of how this bit of code goes wrong makes sense *except* that we wouldn't expect to enter this part of the code with a null editForm (at least in cases where duplication is meant to occur). So can you explain how to make that happen?

[jaredm]

Your explanation of how this bit of code goes wrong makes sense *except* that we wouldn't expect to enter this part of the code with a null editForm (at least in cases where duplication is meant to occur). So can you explain how to make that happen?

I should have some time tomorrow to make a slim-line test case and then we can examine from there. I'll update this thread with details when I have it. Thanks for the speedy response!