When using GridFieldEditableColumns with versioned objects, some values are saved incorrectly, triggering an erroneous new version

[ChrissiQ]

Version Info

SilverStripe version: 4.7.3
GridFieldExtensions version: 3.2.4
PHP version: 7.4.9
MySQL server version: 8.0.26

Issue

When using GridFieldEditableColumns with versioned objects, some values are saved incorrectly. Because the value comparison does not match when the DataObject checks for changes, the field is marked as changed, and thus a new version is created, though nothing has changed.

The screenshot below was taken after simply "saving" the page, with no modifications to the item shown as being "modified". It is reproducible every time. It is being caused by the empty varchar field which does not match the comparison when the field is checked for changes (before: NULL, after: empty string).

You can demonstrate this by saving a page containing a versioned inline gridfield, where the gridfield items contain a problematic field (such as a foreign ID for a has_one relation (Int type)), and that field is in the setDisplayFields array. Adding VersionedGridFieldState shows you that saving the page causes the record to be "modified", which creates a new version for the object.

The problematic field types which cause incorrectly saved values that I have noticed so far:

• ints are saved as strings
• empty string varchars are saved as strings rather than NULL

This does not seem like it would be a problem since the value is coerced when saved to the database, but it does trigger a change, causing a new version of the object to be created when the page is saved. This can be confusing for authors (the items are marked as "modified" when they have not changed) and bloats the database (adds a new version record for the object every time the page is saved regardless of whether the object has been modified). This could potentially be a lot of bloat if the page is saved often and/or has a lot of owned versioned objects with problematic fields in inline gridfields.

Steps to reproduce:

1. Create an inline gridfield of versioned objects which are owned by the page, using GridFieldEditableColumns and VersionedGridFieldState
2. Fill in the gridfield to populate it with some objects.
3. Publish the page to bring all changes to the live stage.
4. Do not make any changes, and Save the page.

Expected result:

Step 4 should not produce any changes and should not create any new versions of any objects, since nothing has changed.

Actual result:

The objects in the gridfield with problematic fields (eg. foreign ID for a has-one relation) now have a new version record in their _Versions table, and are marked as "modified".

Minimal Reproducible Example

https://gist.github.com/ChrissiQ/4fff5f47d79dd703f8c5e8737fd92990

The key part being:

    private static $has_many = [
        'TestObject1' => TestObject1::class,
    ];

    private static $owns = [
        'TestObject1',
    ];
    public function getCMSFields()
    {
        [...]

        $fields->addFieldsToTab('Root.Main', [
            GridField::create(
                'TestObject1',
                'TestObject1',
                $this->TestObject1(),
                $config = GridFieldConfig::create()
                    ->addComponent(new GridFieldButtonRow('after'))
                    ->addComponent(new GridFieldToolbarHeader())
                    ->addComponent(new GridFieldTitleHeader())
                    ->addComponent(new GridFieldEditableColumns())
                    ->addComponent(new GridFieldDeleteAction())
                    ->addComponent(new GridFieldAddNewInlineButton('buttons-after-right'))
                    ->addComponent(new VersionedGridFieldState())
            ),
        ], 'Metadata');

The TestObject1 model uses several fields which won't be saved correctly (TestObject2ID, both Title and Description if left empty), triggering a new version for each item on every page save even if no modification is made.

[elliot-sawyer]

I wonder if this might be related to https://github.com/symbiote/silverstripe-gridfieldextensions/blob/3/src/GridFieldEditableColumns.php#L172, there the 4th parameter of the write method is forced to true. There's an open pull request to give developers control over the parameter: #350

[NightJar]

The linked fix is relevant for relations of the inline edited object, where as this issue appears to describe inaccurate states on the DB section of the inline edited object itself.

This is more likely to do with indiscriminate saving of items held by the field, fixed by @satrun77 in June this year (9 months after this issue was opened).

But the fix doesn't address versioned items specifically, the only way to know if this issue is closed would be a re-test.