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

NEW Add extension points for getURL methods #369

Conversation

Cambis
Copy link

@Cambis Cambis commented Mar 5, 2025

Description

  • Add extension points to update the link url.

Manual testing steps

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

@SimulatedPanda
Copy link

Given that this change is extremely low risk and provides a benefit to the CMS 5.4 release, I approve this change for inclusion in spite of the current change freeze for v5.4

Comment on lines 53 to 57
$url = $this->Email ? sprintf('mailto:%s', $this->Email) : '';

$this->extend('updateGetURL', $url);

return $url;
Copy link
Member

Choose a reason for hiding this comment

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

Please set this extension hook in Link::getURL() directly instead and name it updateURL. Then in these subclasses make this change:

Suggested change
$url = $this->Email ? sprintf('mailto:%s', $this->Email) : '';
$this->extend('updateGetURL', $url);
return $url;
$this->beforeExtending('updateURL', function (string &$url) {
$url = $this->Email ? sprintf('mailto:%s', $this->Email) : '';
});
return parent::getURL();

@Cambis Cambis force-pushed the pulls/4.2/add-get-url-extension-points branch from 9ced2db to 4c65632 Compare March 6, 2025 22:27
@Cambis Cambis requested a review from GuySartorelli March 6, 2025 22:29
Copy link
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.

LGTM, thanks for taking the time.

@GuySartorelli GuySartorelli merged commit 7024d2d into silverstripe:4.2 Mar 6, 2025
15 checks passed
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.

3 participants