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

send action links on any page that extends CRM_Core_Page_basic thru hook_civicrm_links #13068

Merged
merged 1 commit into from
Mar 23, 2019

Conversation

alifrumin
Copy link
Contributor

@alifrumin alifrumin commented Nov 7, 2018

Overview

Makes it so extension developers can use hook_civicrm_links to add or remove links from the Relationship Types Administration Page (CiviCRM Navigation Menu->Administer->Customize Data and Screens->Relationship Types) and any other page that extends CRM_Core_Page_Basic

Before

Pages that extend CRM_Core_Page_Basic never added sent links thru hook_civicrm_links

After

Pages that extend CRM_Core_Page_Basic send links thru hook_civicrm_links

@civibot
Copy link

civibot bot commented Nov 7, 2018

(Standard links)

@civibot civibot bot added the master label Nov 7, 2018
@alifrumin alifrumin changed the title send links on any page that extends CRM_Core_Page_basic thru hook_civicrm_links send action links on any page that extends CRM_Core_Page_basic thru hook_civicrm_links Nov 8, 2018
@eileenmcnaughton
Copy link
Contributor

@twomice I feel like you owe @alifrumin some review :-)

@mattwire
Copy link
Contributor

mattwire commented Nov 8, 2018

@alifrumin Big +1 on the prinicpal here. This is a good, broad change that forces consistency at the same time as allowing extensions to modify more areas of CiviCRM. The code looks good, I haven't tested it!

@eileenmcnaughton
Copy link
Contributor

@mattwire do you think you could find to test this so we can hopefully merge it - on first blush this looks like a fairly safe change. Also @alifrumin has been a great reviewer on other PRs would be great to get some review back to her!

@eileenmcnaughton
Copy link
Contributor

I'm going to merge this - it makes sense as a change & I went through & looked at it & can't see any risks - @mattwire perhaps you have some extensions that run & will see this once merged?

@eileenmcnaughton eileenmcnaughton merged commit 30b6117 into civicrm:master Mar 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants