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 using new CRM_Core_Component::isEnabled() #22687

Merged
merged 1 commit into from
Feb 6, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Feb 2, 2022

Overview

Cleans up code which checks if a component is enabled, using the new utility function created by @eileenmcnaughton in #22567

Technical Details

Switches more verbose array searches to the new utility function.

Switches more verbose array searches to the new utility function.
@civibot
Copy link

civibot bot commented Feb 2, 2022

(Standard links)

@civibot civibot bot added the master label Feb 2, 2022
if (in_array('CiviMember', $config->enableComponents)) {
$this->assign('CiviMember', TRUE);
}
$this->assign('CiviMember', CRM_Core_Component::isEnabled('CiviMember'));
Copy link
Member Author

Choose a reason for hiding this comment

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

Unconditional assign to align with our efforts to reduce undefined template vars.

$obj->creatNewShortcut($shortCuts, $newCredit);
}
foreach ($components as $obj) {
$obj->creatNewShortcut($shortCuts, $newCredit);
Copy link
Member Author

Choose a reason for hiding this comment

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

I had to read this code a couple times to make sense of it. Basically it was fetching a list of enabled components, then for each one, checking to see if it's enabled before taking an action. That's obviously redundant, so I removed the extra check.

if (in_array('CiviEvent', $config->enableComponents)) {
$this->assign('CiviEvent', TRUE);
}
$this->assign('CiviEvent', CRM_Core_Component::isEnabled('CiviEvent'));
Copy link
Member Author

Choose a reason for hiding this comment

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

Unconditional assign to align with our efforts to reduce undefined template vars.

Copy link
Contributor

Choose a reason for hiding this comment

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

yay

@@ -2067,8 +2067,7 @@ public static function getProfileFields() {
'is_current_revision',
'activity_is_deleted',
];
$config = CRM_Core_Config::singleton();
if (!in_array('CiviCampaign', $config->enableComponents)) {
if (!CRM_Core_Component::isEnabled('CiviCampaign')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -90,8 +90,7 @@ public static function tasks() {
],
];

$config = CRM_Core_Config::singleton();
if (in_array('CiviCase', $config->enableComponents)) {
if (CRM_Core_Component::isEnabled('CiviCase')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -269,8 +269,7 @@ public function buildQuickForm() {

// If CiviCase enabled AND "Add" mode OR "edit" mode for non-reserved activities, only allow user to pick Core or CiviCase component.
// FIXME: Each component should define whether adding new activity types is allowed.
$config = CRM_Core_Config::singleton();
if ($this->_gName == 'activity_type' && in_array("CiviCase", $config->enableComponents) &&
if ($this->_gName == 'activity_type' && CRM_Core_Component::isEnabled("CiviCase") &&
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

return in_array('CiviEvent', $config->enableComponents) &&
CRM_Core_Permission::check('view event participants');
return CRM_Core_Component::isEnabled('CiviEvent') &&
CRM_Core_Permission::check('view event participants');
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

return in_array('CiviMember', $config->enableComponents) &&
CRM_Core_Permission::check('access CiviMember');
return CRM_Core_Component::isEnabled('CiviMember') &&
CRM_Core_Permission::check('access CiviMember');
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

return in_array('CiviContribute', $config->enableComponents) &&
CRM_Core_Permission::check('access CiviContribute');
return CRM_Core_Component::isEnabled('CiviContribute') &&
CRM_Core_Permission::check('access CiviContribute');
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -29,8 +29,7 @@ public function __construct() {
* @return bool
*/
public function isActive() {
$config = CRM_Core_Config::singleton();
return in_array('CiviCase', $config->enableComponents);
return CRM_Core_Component::isEnabled('CiviCase');
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

if (!empty($DAOField['component']) &&
!in_array($DAOField['component'], \Civi::settings()->get('enable_components'), TRUE)
) {
if (!empty($DAOField['component']) && !\CRM_Core_Component::isEnabled($DAOField['component'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -53,7 +53,7 @@ public static function getInfo() {
'description' => ts('One or more related contacts'),
],
];
if (in_array('CiviCase', \Civi::settings()->get('enable_components'), TRUE)) {
if (\CRM_Core_Component::isEnabled('CiviCase')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

$config = CRM_Core_Config::singleton();

if (in_array('CiviMail', $config->enableComponents) &&
if (CRM_Core_Component::isEnabled('CiviMail') &&
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -39,8 +39,7 @@ public function setPreUpgradeMessage(&$preUpgradeMessage, $rev, $currentVer = NU
]);
}

$config = CRM_Core_Config::singleton();
if (in_array('CiviCase', $config->enableComponents)) {
if (CRM_Core_Component::isEnabled('CiviCase')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -62,8 +61,7 @@ public function upgrade_5_20_alpha1($rev) {
"tinyint(4) DEFAULT '0' COMMENT 'Shows this is a template for recurring contributions.'", FALSE, '5.20.alpha1');
$this->addTask('Add order_reference field to civicrm_financial_trxn', 'addColumn', 'civicrm_financial_trxn', 'order_reference',
"varchar(255) COMMENT 'Payment Processor external order reference'", FALSE, '5.20.alpha1');
$config = CRM_Core_Config::singleton();
if (in_array('CiviCase', $config->enableComponents)) {
if (CRM_Core_Component::isEnabled('CiviCase')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

$config = CRM_Core_Config::singleton();
$campaignEnabled = in_array("CiviCampaign", $config->enableComponents);
if ($rev == '5.13.alpha1' && $campaignEnabled) {
if ($rev == '5.13.alpha1' && CRM_Core_Component::isEnabled('CiviCampaign')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -294,7 +294,7 @@ public static function getPermissionedCampaigns(
* @return bool
*/
public static function isCampaignEnable(): bool {
return in_array('CiviCampaign', CRM_Core_Config::singleton()->enableComponents, TRUE);
return CRM_Core_Component::isEnabled('CiviCampaign');
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@@ -34,8 +34,7 @@ class CRM_Case_BAO_Case extends CRM_Case_DAO_Case {
* @return bool
*/
public static function enabled() {
$config = CRM_Core_Config::singleton();
return in_array('CiviCase', $config->enableComponents);
return CRM_Core_Component::isEnabled('CiviCase');
Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@eileenmcnaughton
Copy link
Contributor

I finally managed to eyeball all of these changes & they all look OK. Giving merge-ready so this can be merged as soon as the rc is cut (might as well give it that extra month since with so many files changed there is greater risk of some issue)

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Feb 3, 2022
@colemanw colemanw merged commit 654b491 into civicrm:master Feb 6, 2022
@colemanw colemanw deleted the cleanupComponentCheck branch February 6, 2022 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants