silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 94 forks source link

TreeDropdownField::render() passes invalid value to Select component #969

Open JoshuaCarter opened 5 years ago

JoshuaCarter commented 5 years ago

https://github.com/silverstripe/silverstripe-admin/blob/46ce1a49e3e2e430c5712f8e850c8629c068f6bd/client/src/components/TreeDropdownField/TreeDropdownField.js#L654-L695

This surfaces as the TreeDropdownField appearing to have no value on initial load in the CMS.

Looking at the code snippet above, this.props.value comes through as a string ID, and is then given to the Select component without further alterations (assuming multi is false). But as per labelKey and valueKey, the Select component expects value to be an array.

Currently working around this via:

private static $has_one = [
    'InternalLink' => SiteTree::class
];
//...
public function getCMSFields() {
    //...
    $fields->addFieldToTab('Root.Main',
        TreeDropdownField::create('InternalLinkID', 'Link To', SiteTree::class);
    );
    //...
}
SilverStripe\Core\Injector\Injector:
  SilverStripe\Forms\TreeDropdownField:
    class: Transport\Forms\TreeDropdownFieldCustom
    /**
     * Override of TreeDropdownField::getSchemaStateDefaults()
     *
     * Sets the initial state of 'value' for front-end props to an array
     * with the expected structure (instead of an ID integer).
     *
     * @return array
     */
    public function getSchemaStateDefaults()
    {
        $data = parent::getSchemaStateDefaults();
        if (isset($data['data']['valueObject'])) {
            $data['value'] = $data['data']['valueObject'];
        }
        return $data;
    }

    /**
     * Override of TreeDropdownField::saveInto()
     *
     * Sets the final state of 'value' for the DB to an ID integer
     * (instead of an array).
     */
    public function saveInto(DataObjectInterface $record)
    {
        $value = $this->dataValue();
        if ($this->name && is_array($value) && isset($value['id'])) {
            $record->setCastedField($this->name, $value['id']);
            return;
        }
        parent::saveInto($record);
    }

To summarise:

The frontend logic (TreeDropdownField.js) expects to get/give its value as an array like:

[
    id: int,
    title: string,
    treetitle: array(),
    titlePath: string
]

But the backend logic (TreeDropdownField.php) expects to get/give its value as a simple ID.

Thus my workaround is to convert between these formats.

Edit 1: Updated code to what's now being used on our end. Edit 2: Added field usage code and summary.

maxime-rainville commented 4 years ago

I had a got at replicating this and didn't manage to. The Treedropdown field seems to be working fine for me.

Could you provide some more replications steps?

JoshuaCarter commented 4 years ago

@maxime-rainville Thanks for taking a look. I've added some details (and updated my fix snippet) if you can give that a look. I'm very interested to know if/where your own observations have differed from my own. Perhaps there is something unique to our project, but it's not obvious to me at this point.

Cheers.