elliot-sawyer / silverstripe-linkfield

Adds a Linkfield for gorriecoe/silverstripe-link
BSD 3-Clause "New" or "Revised" License
9 stars 25 forks source link

Saving with a LinkField can break the CMS in some cases #26

Open jakxnz opened 3 years ago

jakxnz commented 3 years ago

Version

gorriecoe/silverstripe-linkfield   1.0.1

Issue

In some cases, the instantiated LinkField can submit by its unaugmented name and override an object's component with the same name, which causes the CMS to break when saving the record.

Steps to reproduce

I'm not sure sadly. All I can observe concretely is that the LinkField needs to be used in such a way that Form::saveInto() considers it a $dataField(aka a saveableField)

Expected behaviour

The Field never writes data directly to the DataObject on save, because it is a superficial field.

Actual behaviour

The Field writes data to the object on save and overwrites the relation component that shares the same name, causing the CMS to be unable to process with writing the record because the component references are malaligned.

Uncaught TypeError: Argument 2 passed to SilverStripe\ORM\DataObject::skipWriteComponents() must be an instance of SilverStripe\ORM\DataObject, null given, called in /var/www/mysite/www/vendor/silverstripe/framework/src/ORM/DataObject.php on line 1625
GuySartorelli commented 2 years ago

Steps to reproduce: I'm not sure sadly.

Is this intermittent, or can you reliably reproduce it on some specific DataObject? If the latter, it is likely to do with a combination of the $db and $has_one (or $has_many or $many_many) config arrays and the getCMSFields method, so if you can reproduce it, providing those in this ticket might help us gets some steps to reproduce the behaviour.

If it's intermittent, then it's hard to tell whether your PR is actually resolving an issue or not, because there's no way for us to test it. If this is still an issue for you I think you'll need to dig down to see if you can find some steps to reliably reproduce the problem.

jakxnz commented 2 years ago

It depends how you look at it @GuySartorelli

gorriecoe\LinkField\LinkField is clearly a superficial field, and has no data to save. So the changes in my PR align the decoration of the class to match its role in a form.

I created the PR in a project which had this issue, but due to time constraints had to work directly towards a solution and couldn't take the time to investigate the root cause. The changes in my PR are working and resolve the issue in production.

I would say my PR isn't strictly a fix for this issue, it is an enhancement to the integrity of gorriecoe\LinkField\LinkField that just so happens to resolve this issue.