Switch to CRM_Core_Form::setTitle() instead of CRM_Utils_System::setTitle() part 4 #21368

mattwire · 2021-09-04T08:58:22Z

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) this is the final 43.
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

This is the final one

civibot · 2021-09-04T08:58:24Z

(Standard links)

If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

eileenmcnaughton · 2021-09-04T21:56:39Z

ext/legacycustomsearches/CRM/Contact/Form/Search/Custom/DateAdded.php

@@ -366,10 +366,10 @@ public function templateFile() {
   */
  public function setTitle($title) {
    if ($title) {
-      CRM_Utils_System::setTitle($title);
+      $this->setTitle($title);


This will cause a loop

I've moved title changes for legacycustomsearches to #21375 and taken them out of this PR

eileenmcnaughton · 2021-09-04T21:56:53Z

CRM/Member/Form/MembershipRenewal.php

@@ -162,7 +162,7 @@ public function preProcess() {
      CRM_Custom_Form_CustomData::setDefaultValues($this);
    }

-    CRM_Utils_System::setTitle(ts('Renew Membership'));
+    $this->setTitle(ts('Renew Membership'));


OK - checked

eileenmcnaughton · 2021-09-04T21:58:02Z

CRM/Member/Form/MembershipView.php

@@ -341,7 +341,7 @@ public function preProcess() {
      }

      // omitting contactImage from title for now since the summary overlay css doesn't work outside crm-container
-      CRM_Utils_System::setTitle(ts('View Membership for') . ' ' . $displayName);
+      $this->setTitle(ts('View Membership for') . ' ' . $displayName);


eileenmcnaughton · 2021-09-04T21:58:14Z

CRM/Member/Form/Task/Batch.php

@@ -77,7 +77,7 @@ public function buildQuickForm() {
      CRM_Core_Error::statusBounce(ts('ufGroupId is missing'));
    }
    $this->_title = ts('Update multiple memberships') . ' - ' . CRM_Core_BAO_UFGroup::getTitle($ufGroupId);
-    CRM_Utils_System::setTitle($this->_title);
+    $this->setTitle($this->_title);


eileenmcnaughton · 2021-09-04T21:58:23Z

CRM/Member/Form/Task/PickProfile.php

@@ -50,7 +50,7 @@ public function preProcess() {
    $session = CRM_Core_Session::singleton();
    $this->_userContext = $session->readUserContext();

-    CRM_Utils_System::setTitle(ts('Update multiple memberships'));
+    $this->setTitle(ts('Update multiple memberships'));


eileenmcnaughton · 2021-09-04T22:01:36Z

CRM/PCP/Form/Campaign.php

@@ -44,7 +44,7 @@ public function preProcess() {
      $title = ts('Edit Your Personal Campaign Page');
    }

-    CRM_Utils_System::setTitle($title);
+    $this->setTitle($title);


ok - checked

eileenmcnaughton · 2021-09-04T22:01:46Z

CRM/PCP/Form/Event.php

@@ -42,7 +42,7 @@ public function setDefaultValues() {
    $defaults = [];
    if (isset($this->_id)) {
      $title = CRM_Core_DAO::getFieldValue('CRM_Event_DAO_Event', $this->_id, 'title');
-      CRM_Utils_System::setTitle(ts('Personal Campaign Page Settings (%1)', [1 => $title]));
+      $this->setTitle(ts('Personal Campaign Page Settings (%1)', [1 => $title]));


eileenmcnaughton · 2021-09-04T22:01:59Z

CRM/PCP/Form/PCPAccount.php

@@ -100,7 +100,7 @@ public function preProcess() {
    $this->assign('pcpComponent', $this->_component);

    if ($this->_single) {
-      CRM_Utils_System::setTitle(ts('Update Contact Information'));
+      $this->setTitle(ts('Update Contact Information'));


eileenmcnaughton · 2021-09-04T22:02:10Z

CRM/Pledge/Form/Payment.php

@@ -54,7 +54,7 @@ public function preProcess() {

    $this->_id = CRM_Utils_Request::retrieve('ppId', 'Positive', $this);

-    CRM_Utils_System::setTitle(ts('Edit Scheduled Pledge Payment'));
+    $this->setTitle(ts('Edit Scheduled Pledge Payment'));


eileenmcnaughton · 2021-09-04T22:02:42Z

I've noted the ones I've checked so they dont need to be rechecked. Test fails relate

…itle() part 4

mattwire · 2021-09-05T19:46:12Z

Thanks @eileenmcnaughton I've moved title changes for legacycustomsearches to #21375 and taken them out of this PR as they need a slightly different approach.

eileenmcnaughton · 2021-09-05T22:38:51Z

CRM/Pledge/Form/PledgeView.php

@@ -99,7 +99,7 @@ public function preProcess() {
      $displayName .= ' (' . ts('default organization') . ')';
    }
    // omitting contactImage from title for now since the summary overlay css doesn't work outside of our crm-container
-    CRM_Utils_System::setTitle(ts('View Pledge by') . ' ' . $displayName);
+    $this->setTitle(ts('View Pledge by') . ' ' . $displayName);


eileenmcnaughton · 2021-09-05T22:38:59Z

CRM/Price/Form/DeleteField.php

@@ -50,7 +50,7 @@ public function preProcess() {

    $this->assign('title', $this->_title);

-    CRM_Utils_System::setTitle(ts('Confirm Price Field Delete'));
+    $this->setTitle(ts('Confirm Price Field Delete'));


eileenmcnaughton · 2021-09-05T22:39:06Z

CRM/Price/Form/Set.php

@@ -108,7 +108,7 @@ public function preProcess() {
    elseif ($this->_action & CRM_Core_Action::VIEW) {
      $title = ts('Preview %1', [1 => $title]);
    }
-    CRM_Utils_System::setTitle($title);
+    $this->setTitle($title);


eileenmcnaughton · 2021-09-05T22:39:14Z

CRM/Report/Form/Register.php

@@ -22,7 +22,7 @@ public function preProcess() {
    $this->_action = CRM_Utils_Request::retrieve('action', 'String', $this, FALSE);
    $this->_id = CRM_Utils_Request::retrieve('id', 'String', $this, FALSE);

-    CRM_Utils_System::setTitle(ts('Report Template'));
+    $this->setTitle(ts('Report Template'));


eileenmcnaughton · 2021-09-05T22:39:20Z

CRM/UF/Form/Group.php

@@ -124,7 +124,7 @@ public function preProcess() {

    // setting title for html page
    if ($this->_action & CRM_Core_Action::UPDATE) {
-      CRM_Utils_System::setTitle(ts('Profile Settings') . " - $title");
+      $this->setTitle(ts('Profile Settings') . " - $title");


eileenmcnaughton · 2021-09-05T22:39:29Z

CRM/UF/Form/Group.php

@@ -149,7 +149,7 @@ public function preProcess() {
      $this->assign('message', $message);
    }
    else {
-      CRM_Utils_System::setTitle(ts('New CiviCRM Profile'));
+      $this->setTitle(ts('New CiviCRM Profile'));


eileenmcnaughton · 2021-09-05T22:39:41Z

CRM/Upgrade/Form.php

@@ -187,7 +187,7 @@ public function source($fileName, $isQueryString = FALSE) {
  }

  public function preProcess() {
-    CRM_Utils_System::setTitle($this->getTitle());
+    $this->setTitle($this->getTitle());


eileenmcnaughton · 2021-09-05T22:40:05Z

tests/extensions/test.extension.manager.searchtest/main.php

@@ -22,7 +22,7 @@ public function __construct(&$formValues) {
   * @return void
   */
  public function buildForm(&$form) {
-    CRM_Utils_System::setTitle(ts('My Search Title'));
+    $form->setTitle(ts('My Search Title'));


ok checkedish & it's not live code anyway

eileenmcnaughton · 2021-09-05T22:40:20Z

these all look good

civibot bot added the master label Sep 4, 2021

eileenmcnaughton reviewed Sep 4, 2021

View reviewed changes

mattwire force-pushed the settitle branch 2 times, most recently from 4b6b11d to 91e8134 Compare September 5, 2021 19:40

Switch to CRM_Core_Form::setTitle() instead of CRM_Utils_System::setT…

203b473

…itle() part 4

mattwire force-pushed the settitle branch from 6a3e69f to 203b473 Compare September 5, 2021 19:43

eileenmcnaughton reviewed Sep 5, 2021

View reviewed changes

eileenmcnaughton merged commit 15f57ff into civicrm:master Sep 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to CRM_Core_Form::setTitle() instead of CRM_Utils_System::setTitle() part 4 #21368

Switch to CRM_Core_Form::setTitle() instead of CRM_Utils_System::setTitle() part 4 #21368

mattwire commented Sep 4, 2021 •

edited

Loading

civibot bot commented Sep 4, 2021

eileenmcnaughton Sep 4, 2021

mattwire Sep 5, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton Sep 4, 2021

eileenmcnaughton commented Sep 4, 2021

mattwire commented Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton Sep 5, 2021

eileenmcnaughton commented Sep 5, 2021

Switch to CRM_Core_Form::setTitle() instead of CRM_Utils_System::setTitle() part 4 #21368

Switch to CRM_Core_Form::setTitle() instead of CRM_Utils_System::setTitle() part 4 #21368

Conversation

mattwire commented Sep 4, 2021 • edited Loading

Overview

Before

After

Technical Details

Comments

civibot bot commented Sep 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Sep 4, 2021

mattwire commented Sep 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Sep 5, 2021

mattwire commented Sep 4, 2021 •

edited

Loading