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

REF Switch to CRM_Core_Form::setTitle() instead of CRM_Utils_System::setTitle() part 1 #21193

Merged
merged 1 commit into from
Aug 20, 2021

Conversation

mattwire
Copy link
Contributor

Overview

Follows on from some work that @eileenmcnaughton started in 2019. Forms should use $this->setTitle() instead of CRM_Utils_System::setTitle() because it allows you to use $this->getTitle() on a form to get the title of the form that the user will see.

Before

Calls to CRM_Utils_System::setTitle()

After

Calls to $this->setTitle()

Technical Details

As can be observed CRM_Core_Form::setTitle() calls CRM_Utils_System::setTitle() internally after setting $this->_title showing that this function is a drop-in replacement. There are just over 200 instances that need converting and (for ease of review) I've only included the first 43 here.
I would suggest that r-run testing could be done with a few forms only instead of all of them - what needs to be verified is that it is a form that inherits from CRM_Core_Form (and not eg. a Page).

Comments

There are 4 commits in total that would complete this change and I'll follow up with the others - see https://github.com/civicrm/civicrm-core/compare/master...mattwire:settitle?expand=1

@civibot
Copy link

civibot bot commented Aug 20, 2021

(Standard links)

@civibot civibot bot added the master label Aug 20, 2021
@eileenmcnaughton
Copy link
Contributor

This seems safe, consistent & straight forward - merging

@eileenmcnaughton eileenmcnaughton merged commit 42dffbc into civicrm:master Aug 20, 2021
@mattwire mattwire deleted the settitle1 branch August 20, 2021 22:36
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.

2 participants