Skip to content

Commit

Permalink
Cleanup core pseudoconstant buildOptions
Browse files Browse the repository at this point in the history
There has been some confusion about the $params vs $props variables, and some code was mistakenly using them interchangeably.
They are not the same, as CRM_Core_PseudoConstant::get expects sanitized input for $params, but CRM_*_DAO::buildOptions accepts raw user input as $props.
  • Loading branch information
colemanw committed Apr 23, 2020
1 parent 35f6155 commit 3f37471
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 28 deletions.
2 changes: 1 addition & 1 deletion CRM/ACL/BAO/ACL.php
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ public static function group(
}

if ($allGroups == NULL) {
$allGroups = CRM_Contact_BAO_Contact::buildOptions('group_id', NULL, ['onlyActive' => FALSE]);
$allGroups = CRM_Contact_BAO_Contact::buildOptions('group_id', 'get');
}

$acls = CRM_ACL_BAO_Cache::build($contactID);
Expand Down
3 changes: 1 addition & 2 deletions CRM/Contact/BAO/GroupContact.php
Original file line number Diff line number Diff line change
Expand Up @@ -779,8 +779,7 @@ public static function bulkAddContactsToGroup(
* @return array|bool
*/
public static function buildOptions($fieldName, $context = NULL, $props = []) {

$options = CRM_Core_PseudoConstant::get(__CLASS__, $fieldName, $props, $context);
$options = CRM_Core_PseudoConstant::get(__CLASS__, $fieldName, [], $context);

// Sort group list by hierarchy
// TODO: This will only work when api.entity is "group_contact". What about others?
Expand Down
25 changes: 7 additions & 18 deletions CRM/Contribute/BAO/ContributionRecur.php
Original file line number Diff line number Diff line change
Expand Up @@ -955,31 +955,20 @@ public static function getInactiveStatuses() {
}

/**
* Get options for the called BAO object's field.
*
* This function can be overridden by each BAO to add more logic related to context.
* The overriding function will generally call the lower-level CRM_Core_PseudoConstant::get
*
* @param string $fieldName
* @param string $context
* @see CRM_Core_DAO::buildOptionsContext
* @param array $props
* whatever is known about this bao object.
*
* @return array|bool
* @inheritDoc
*/
public static function buildOptions($fieldName, $context = NULL, $props = []) {

$params = [];
switch ($fieldName) {
case 'payment_processor_id':
if (isset(\Civi::$statics[__CLASS__]['buildoptions_payment_processor_id'])) {
return \Civi::$statics[__CLASS__]['buildoptions_payment_processor_id'];
}
$baoName = 'CRM_Contribute_BAO_ContributionRecur';
$props['condition']['test'] = "is_test = 0";
$liveProcessors = CRM_Core_PseudoConstant::get($baoName, $fieldName, $props, $context);
$props['condition']['test'] = "is_test != 0";
$testProcessors = CRM_Core_PseudoConstant::get($baoName, $fieldName, $props, $context);
$params['condition']['test'] = "is_test = 0";
$liveProcessors = CRM_Core_PseudoConstant::get($baoName, $fieldName, $params, $context);
$params['condition']['test'] = "is_test != 0";
$testProcessors = CRM_Core_PseudoConstant::get($baoName, $fieldName, $params, $context);
foreach ($testProcessors as $key => $value) {
if ($context === 'validate') {
// @fixme: Ideally the names would be different in the civicrm_payment_processor table but they are not.
Expand All @@ -995,7 +984,7 @@ public static function buildOptions($fieldName, $context = NULL, $props = []) {
\Civi::$statics[__CLASS__]['buildoptions_payment_processor_id'] = $allProcessors;
return $allProcessors;
}
return parent::buildOptions($fieldName, $context, $props);
return CRM_Core_PseudoConstant::get(__CLASS__, $fieldName, $params, $context);
}

}
6 changes: 4 additions & 2 deletions CRM/Core/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -2570,14 +2570,16 @@ public static function appendPseudoConstantsToFields(&$fields) {
* @param string $context
* @see CRM_Core_DAO::buildOptionsContext
* @param array $props
* whatever is known about this bao object.
* Raw field values; whatever is known about this bao object.
*
* Note: $props can contain unsanitized input and should not be passed directly to CRM_Core_PseudoConstant::get
*
* @return array|bool
*/
public static function buildOptions($fieldName, $context = NULL, $props = []) {
// If a given bao does not override this function
$baoName = get_called_class();
return CRM_Core_PseudoConstant::get($baoName, $fieldName, $props, $context);
return CRM_Core_PseudoConstant::get($baoName, $fieldName, [], $context);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion CRM/Price/Form/Field.php
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ public static function formRule($fields, $files, $form) {
$publicOptionCount = $_flagOption = $_rowError = 0;

$_showHide = new CRM_Core_ShowHideBlocks('', '');
$visibilityOptions = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', NULL, ['labelColumn' => 'name']);
$visibilityOptions = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', 'validate');

for ($index = 1; $index <= self::NUM_OPTION; $index++) {

Expand Down
2 changes: 1 addition & 1 deletion CRM/Price/Form/Option.php
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ public static function formRule($fields, $files, $form) {
}

$priceField = CRM_Price_BAO_PriceField::findById($fields['fieldId']);
$visibilityOptions = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', NULL, ['labelColumn' => 'name']);
$visibilityOptions = CRM_Core_PseudoConstant::get('CRM_Price_BAO_PriceFieldValue','visibility_id', ['labelColumn' => 'name']);

$publicCount = 0;
$options = CRM_Price_BAO_PriceField::getOptions($priceField->id);
Expand Down
2 changes: 1 addition & 1 deletion tests/phpunit/CRM/Price/Form/FieldTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class CRM_Price_Form_FieldTest extends CiviUnitTestCase {
public function setUp() {
parent::setUp();

$this->visibilityOptionsKeys = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', NULL, [
$this->visibilityOptionsKeys = CRM_Core_PseudoConstant::get('CRM_Price_BAO_PriceFieldValue', 'visibility_id', [
'labelColumn' => 'name',
'flip' => TRUE,
]);
Expand Down
4 changes: 2 additions & 2 deletions tests/phpunit/CRM/Price/Form/OptionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ class CRM_Price_Form_OptionTest extends CiviUnitTestCase {
public function setUp() {
parent::setUp();

$this->visibilityOptions = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', NULL, [
$this->visibilityOptions = CRM_Core_PseudoConstant::get('CRM_Price_BAO_PriceFieldValue', 'visibility_id', [
'labelColumn' => 'name',
]);
$this->visibilityOptionsKeys = CRM_Price_BAO_PriceFieldValue::buildOptions('visibility_id', NULL, [
$this->visibilityOptionsKeys = CRM_Core_PseudoConstant::get('CRM_Price_BAO_PriceFieldValue', 'visibility_id', [
'labelColumn' => 'name',
'flip' => TRUE,
]);
Expand Down

0 comments on commit 3f37471

Please sign in to comment.