search

elliot-sawyer / silverstripe-linkfield

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

Add ability to configure link model fields per linkfield #24

Closed GuySartorelli closed 3 years ago

GuySartorelli commented 3 years ago

Addresses #1 Fixes #23 and #21 Also allows validating against a has_one Link using the RequiredFields validator.

The assumption at this stage is that for #23 you'll want to go with option 2 "Replace the unlink button on the has_one field with a delete button, and remove the option for linking to existing objects." but I can change it if you prefer another option.

GuySartorelli commented 3 years ago

@gorriecoe @elliot-sawyer Any update on the status of this?

elliot-sawyer commented 3 years ago

Sorry guys, overlooked this. I will have a look this weekend

elliot-sawyer commented 3 years ago

Apologies for the delays on this, I've had a number of personal issues come up since my last comment!

This looks pretty good, but I do have a concern about backwards-compatibility. Will anyone relying on the old global method find that all link types are allowed until specifically configured, or are the same defaults in place? Should this be considered a breaking change?

GuySartorelli commented 3 years ago

@elliot-sawyer No worries! I hope everything is okay now.

Will anyone relying on the old global method find that all link types are allowed until specifically configured, or are the same defaults in place?

By default, i.e. without passing in a configuration array, the $this->owner->link_requirements array will be empty in LinkExtension::updateTypes(), LinkExtension::updateCMSFields(), and LinkExtension::shouldDisplayTitleFields(), so no modifications to the allowed types or display of title will be made. In other words whatever way they're currently using to choose what types are allowed will still work if no configuration is passed in.

Should this be considered a breaking change?

I don't think so. These changes shouldn't take any affect until a configuration array is passed into the constructor of the LinkField or set via LinkField::setLinkConfig().

elliot-sawyer commented 3 years ago

Happy with this, I'll merge now! Sorry again for the delay!