Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Grid field detail form can't write relations #9162

Open
sminnee opened this issue Aug 2, 2019 · 2 comments · Fixed by #9164
Open

Grid field detail form can't write relations #9162

sminnee opened this issue Aug 2, 2019 · 2 comments · Fixed by #9164

Comments

@sminnee
Copy link
Member

sminnee commented Aug 2, 2019

When a grid field is editing a many-many-through relation, the "join object" will be available on each record via DataObject::getJoin()..

Although it's currently a bit awkward and requires boilerplate such as this in your DataObject, you can then open these records up for editing in your edit form:

    public function getNewImpactID() {
        if ($this->joinRecord instanceof RiskReduction) {
            return $this->joinRecord->NewImpactID;
        }
    }
    public function setNewImpactID($value) {
        if ($this->joinRecord instanceof RiskReduction) {
            return $this->joinRecord->NewImpactID = $value;
        }
    }

The problem is that write() by default doesn't write components. Triggering this requires one of the flags passed to write being set here: https://github.com/silverstripe/silverstripe-framework/blob/4/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php#L615

I'd like to set the flag. We could either set it in all cases (which would let people modify any has_one relationship on a grid field edit form), or just for objects that have a join record (which would limit the bugfix to the place I tripped over it, possibly prevent current code that relies on this bug from being broken, but risking that we need to change this code a second time.

@sminnee
Copy link
Member Author

sminnee commented Aug 2, 2019

My instinct is to set writeComponent = true in all cases, as it's safe and will make the framework more flexible, especially if #9163 is also implemented.

@sminnee
Copy link
Member Author

sminnee commented May 20, 2020

I'm going to need to re-introduce this as a config option rather than a change to the default, assuming #9519 is merged into 4.5, which seems to be the consensus.

Should I target 4.5 or 4.6+?

@sminnee sminnee self-assigned this May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants