Announcement

**Isomorphic** · 3 Feb 2016, 15:46

It's not just reselectOnUpdate, it's how selection stores state in general. The same technique of using "_" property names is used by recordComponents, hilites, expansion components and other subsystems. The properties are easy to ignore since they always start with "_" and never collide with DataSource fields.

There's really no choice but to write such properties on the Records themselves, since with the absence of hashcode() in JS, there's no reasonable way to build a hashtable if you are not guaranteed a unique property on the Records, which we don't require for many grid use cases (it is, for instance, required for editing).

**antanas.arvasevicius** · 4 Feb 2016, 02:07

but DynamicForm.valuesHaveChanged() also reacts to these syntethic properties. shouldn't they be ignored by that logic too? we are getting false results that our DynamicForm have been changed. Should be implement our own valuesHaveChanged which ignores these random "_" properties on my own records?

PS. offtopic: I've seen that there is no such option which I could mark a FormItem as client only, I mean I want a FormItem which is read only (and programmatically set/calculated) and must not be sent to server (also completely skipped from valuesHaveChanged logic)

**Isomorphic** · 4 Feb 2016, 03:54

valuesHaveChanged() should not take such properties into consideration because there is no FormItem for editing the value, thus no way it would change while the user is editing. If you did something odd like manually call rememberValues() with another Record that had it's _selection property change, you might fool valuesHaveChanged(), but it would be hard to justify that as a "bug" (you have actually changed the comparison record mid-edit). Let us know if you can show a valid pattern of calls that results in an incorrect result from valuesHaveChanged().

To avoid a FormItem saving it's value to the form (and hence to the server or any further processing), set shouldSaveValue:false.

**antanas.arvasevicius** · 4 Feb 2016, 05:27

Hi, I'll try to reproduce it, because we have getting notices that form has been changed.

Why I cannot use shouldSaveValue, is this in DynamicForm.js: I'm getting a value from server, and this code will set my field to shouldSaveValue, but I don't want to save it.
Should I set again to shouldSaveValue on some callback in this situation? or what you suggest to do?

Code:

                this.logInfo("DynamicForm.setValues() passed a value for '" + item[this.fieldIdProperty] + "'." +
                             " The corresponding form item was declared with 'shouldSaveValue' set to " +
                             " false to exclude its value from the form's values object." +
                             " Setting 'shouldSaveValue' to true for this item." +
                             "\n[To avoid seeing this message in the future, set 'shouldSaveValue'" +
                             " to true for any form items whose values are to be managed via " +
                             " form.setValues() / form.getValues().]")

**antanas.arvasevicius** · 4 Feb 2016, 05:56

But are you sure that "_selection_29" property is perfectly valid for DynamicForm? because I'm not see any "_selection_29" property then I bound a record to form!

See this example:

1) click on any record in listgrid ( should not see any _selection )
2) click save (_selection appears from nothere)

Note: I've looked in code and see that DynamicForm.editSelectedData() will do full object clone and then set to form.values, so why "_selection" string appears on my data?

Code:

    var listGrid;

    var ds = isc.DataSource.create({
        ID: "ds",
        clientOnly: true,
        fields: [{name: 'id', primaryKey: true}, {name: 'title', type: 'text'}],
        testData: [
            {
                id: 1,
                title: 'Hello world'
            },
            {
                id: 2,
                title: 'Inside-Outside'
            },
            {
                id: 3,
                title: 'High-Low'
            }
        ]
    });

    var listgrid, form, output;

    var layout = isc.VLayout.create({
        autoDraw: true,
        width: "500",
        height: "300",
        members: [
            listgrid = isc.ListGrid.create({
                autoFetchData: true,
                width: "100%",
                height: "300",
                dataSource: "ds",
                recordClick: function () {
                    form.editSelectedData(listgrid);
                    output.setContents('<br/>getData(): ' + JSON.stringify(form.getData()) + '<br/>');
                }
            }),
            form = isc.DynamicForm.create({
                dataSource: "ds",
                fields: [
                    {
                        name: "title",
                        type: "text"
                    }
                ]
            }),
            isc.Button.create({
                title: 'Save',
                click: function () {
                    form.saveData(function () {
                        output.setContents('<br/>getData(): ' + JSON.stringify(form.getData()) + '<br/>');
                    });
                }
            }),
            output = isc.HTMLFlow.create({
                width: "100%",
                height: "30"
            })
        ]
    });

**antanas.arvasevicius** · 4 Feb 2016, 06:15

Te error is in here: Selection.js:

Code:

dataChanged : function (operationType,originalRecord,rowNum,updateData,filterChanged,dataFromCache) {
    if (this._ignoreNextDataChanged) {
        delete this._ignoreNextDataChanged;
        return;
    }

    if (this.reselectOnUpdate && operationType == "update" && originalRecord != null &&
        originalRecord[this.selectionProperty])
    {

        var modifiedRecord = this.data.findByKey(originalRecord);
        if (modifiedRecord) this.performReselectOnUpdate(modifiedRecord);

1) `modifiedRecord` is the same object as `updateData`
2) this.performReselectOnUpdate(modifiedRecord); will append "_selection" property.
3) that "updateData" object is used further by propagating response to DynamicForm obect thru "fireResponseCallbacks"
4) by that unintentional modification, it will call isc.EditorActionMethods.addInterfaceMethods._saveDataReply() which calls saveValues() on DynamicForm.
5) So form will think that the server has responded the response for saveData() command with an object which has "_selection_"

I'm guaranteed that it's not a conscious behavior which you have been implemented by intent.

**Isomorphic** · 4 Feb 2016, 12:47

You seem to be thinking that we're trying to avoid _selection and similar properties appearing on records. We're not.

Your test case doesn't seem to show a case where valuesHaveChanged() returns an unexpected result - do you have a test case that shows that?

About shouldSaveValue - that might not be the right property, but if so, we're not sure what is. It seems that this value comes from the server, then calculations are performed on it - so what don't you want to do? Just not save it? Do you care if it is transmitted to the server? Because your server code could just ignore it.

**Isomorphic** · 4 Feb 2016, 13:47

Just to clarify further: whether it happens in the code you showed or later, the record is going to end up with that _ selectionProperty and the property is going to end up in form.values via lots of different code paths.

This does not in general create a problem. If it's creating a problem for your app, trying to change this one particular case isn't going to fix the problem for you in general - we would expect that it will come up again some other way.

So the next step is still to see how you end up with valuesHaveChanged() returning a bad result.

**antanas.arvasevicius** · 5 Feb 2016, 03:43

Hi!
Yes, you are right, it was not about _selectionProperty. But I've found that in our data source schema was a mistake, our one field which returns array from server we've set "type: 'integer'" instead of type: "'valueMap"'.

But problem is in

Code:

DataBoundComponent.fieldValuesAreEqual : function (field, value1, value2)
isc.Canvas.getPrototype().fieldValuesAreEqual(..)

here:

Code:

    // no matter what the type, if we get this far, the field type had no custom comparison mechanism,
    // so, if the values are '==', always return true
    if (value1 == value2) return true;

So if we some how miss the type we'll get an unexpected behavior in future, and it's very hard to find it.

I think here needed to add some code to compare arrays before using fallback loose equallity (==).

but maybe it's for performance reasons left comparing values/objects only if type is explicitly set type:"valueMap" don't know.

But the real problem, is that your framework won't call rememberValues() if at least one field is not equal by checking with "this.fieldValuesAreEqual(field, currentValues[fieldName], compareVal)" function.
I strongly believe that you must call rememberValues for fields which is "equal" and skip those are "not equal", but not apply logic that "if one field is not equal" then do not remember all values after save.
Why they are not remembered completely all? because I've got response from server that these values are equal with what was posted, so they shouldn't be submitted after next "Save" action.

**Isomorphic** · 5 Feb 2016, 13:59

type:"valueMap" is an internal type not listed in the docs. It is used internally to describe dataSourceField.valueMap for editing in Visual Builder. What you probably want is type="integer" multiple="true". This is one of those cases where you can become confused by reading source code instead of docs - watch out for that.

The problem with having fieldValuesAreEqual automatically handling Arrays is - where do you stop? Why not compare each property of two Objects? Why not recursively compare everything up to a depth of 200? This is a case where the developer just needs to get the field type correct, and there's no way the framework can reasonably deal, at every point of processing, with having been told something should not be an Array and finding an Array. Partially handling this case is worse; it just further masks the problem.

The reason rememberValues() is not called is because, in the absence of a coding error like this case, differing values would indicate that the user had made changes while the save operation was in progress. For this more advanced scenario, it should be clear that rememberValues() cannot be called.

**antanas.arvasevicius** · 6 Feb 2016, 11:41

Thank you, yes, right, it must be type: "integer", multiple: true.

But I don't get that rememberValues() logic, why call rememberValues() only if ALL fields match, and what would go wrong if you call: rememberValuesExcept([list_of_notequal_field_names]); on save reply handler? As it looks more logically. Because current implementation will send again to server all values the same as in previous request, but not only ones which has became somehow "not equal"?

**Isomorphic** · 8 Feb 2016, 16:04

It's an ambiguous situation for the framework - the framework doesn't have enough information to know what the application wants to do, and any call to rememberValues might be the wrong thing. However, having detected this situation, application code could call rememberValues() with partial values in the way you suggest.

Announcement

[bug] ListGrid + editSelectedData + DynamicForm.saveData will corrupt record object.

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment