Skip to content

ENH: Allow adding link(s) to unsaved DataObjects (closes #387)#388

Closed
lozcalver wants to merge 1 commit intosilverstripe:5from
bigfork:link-before-save
Closed

ENH: Allow adding link(s) to unsaved DataObjects (closes #387)#388
lozcalver wants to merge 1 commit intosilverstripe:5from
bigfork:link-before-save

Conversation

@lozcalver
Copy link
Copy Markdown
Member

@lozcalver lozcalver commented Jun 11, 2025

Description

This seems surprisingly trivial in terms of code changes! There’s some minor refactoring in that the jsonError() calls are moved out of most of the getters, but these are all private methods anyway so there aren’t any BC concerns.

Manual testing steps

Add a DataObject class which has_one and/or has_many links. Create a new, unsaved DataObject (basically anything that isn’t a page or elemental block) and you should now be able to add links instead of seeing the old “Cannot create links” message.

Issues

Pull request checklist

  • The target branch is correct
  • All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
    • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
  • The commit messages follow our commit message guidelines
  • The PR follows our contribution guidelines
  • Code changes follow our coding conventions
  • This change is covered with tests (or tests aren't necessary for this change)
  • Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
  • CI is green

@lozcalver
Copy link
Copy Markdown
Member Author

lozcalver commented Jun 11, 2025

Marked as draft because I still need to look at PHP/Behat coverage

Edit: actually it’s not working properly for MultiLinkField, but is working for a single has_one link. I’ll investigate, but even if that can’t be fixed it’s probably still a worthwhile change for just has_one links.

@lozcalver
Copy link
Copy Markdown
Member Author

I’ve pushed up a working solution and test coverage, but I’m struggling to get Behat running at the moment

@lozcalver lozcalver marked this pull request as ready for review June 17, 2025 15:03
Copy link
Copy Markdown
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We aren't releasing any new minor releases for the CMS 5 compatible branches. Please retarget this to the 5 branch (you may need to reset your commits).

This also needs a behat test - if you're using DDEV my solution lately has been to install chromedriver and chromium into the web container since the various DDEV selenium addons stopped working for me a few months ago.
here is my docker file and here is my behat command

Alternatively if you're still having trouble let me know and I can make a behat feature for this separately.

Please also update docs and make this new behaviour configurable per #387 (comment)

Comment thread src/Form/AbstractLinkField.php Outdated
@lozcalver lozcalver changed the base branch from 4 to 5 December 15, 2025 14:27
@lozcalver
Copy link
Copy Markdown
Member Author

I’ve rebased onto the 5 branch and pushed up a Behat scenario, I don’t know what’s going on with CI though – seems like it’s trying to install the CMS5 installer. I guess it’s a side-effect of switching the base branch?

@lozcalver
Copy link
Copy Markdown
Member Author

Even after re-running all the CI actions, the base ref is still wrong (it says 4 when it should be 5): https://github.com/silverstripe/silverstripe-linkfield/actions/runs/20235733741/job/58090381662?pr=388#step:2:317. I’ll try closing this and opening a new PR

@lozcalver lozcalver closed this Dec 15, 2025
@GuySartorelli
Copy link
Copy Markdown
Member

Replaced by #425

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants