Cleanup core pseudoconstant buildOptions

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.

diff --git a/CRM/ACL/BAO/ACL.php b/CRM/ACL/BAO/ACL.php
index 3f865314748620e5b13b02a4265eb1e525137b26..6b523272a351c283ecfe7023a588ce34d1d0957d 100644 (file)
--- a/CRM/ACL/BAO/ACL.php
+++ b/CRM/ACL/BAO/ACL.php
@@ -545,7 +545,7 @@ SELECT g.*
     }
 
     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);

diff --git a/CRM/Contact/BAO/GroupContact.php b/CRM/Contact/BAO/GroupContact.php
index 4f507b1e6ad7837f15b4bac4b088246ff91a1d13..3914a1c3667d1cca5c6ef2163ea4115b081d6285 100644 (file)
--- a/CRM/Contact/BAO/GroupContact.php
+++ b/CRM/Contact/BAO/GroupContact.php
@@ -779,8 +779,7 @@ AND    contact_id IN ( $contactStr )
    * @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?

diff --git a/CRM/Contribute/BAO/ContributionRecur.php b/CRM/Contribute/BAO/ContributionRecur.php
index 1bf45bdcdcb1bb0a0bb351481029f40d1099e201..804af1d112bad3df8df6b465ee57a9f939751de6 100644 (file)
--- a/CRM/Contribute/BAO/ContributionRecur.php
+++ b/CRM/Contribute/BAO/ContributionRecur.php
@@ -955,31 +955,20 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
   }
 
   /**
-   * 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.
@@ -995,7 +984,7 @@ INNER JOIN civicrm_contribution       con ON ( con.id = mp.contribution_id )
         \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);
   }
 
 }

diff --git a/CRM/Core/DAO.php b/CRM/Core/DAO.php
index d5d4a72048b5359514956cf145decbc2fb54ce00..e9e5732509f27d562b294ed8dbbf2a5b81e8bba3 100644 (file)
--- a/CRM/Core/DAO.php
+++ b/CRM/Core/DAO.php
@@ -2570,14 +2570,16 @@ SELECT contact_id
    * @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);
   }
 
   /**

diff --git a/CRM/Price/Form/Field.php b/CRM/Price/Form/Field.php
index c8e0f2a7b7da76c4da8cc487f045aa2c63bdd892..0b7cbf2f648e7a6cfba05d352719b0c51f7f239c 100644 (file)
--- a/CRM/Price/Form/Field.php
+++ b/CRM/Price/Form/Field.php
@@ -419,7 +419,7 @@ class CRM_Price_Form_Field extends CRM_Core_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++) {
 

diff --git a/CRM/Price/Form/Option.php b/CRM/Price/Form/Option.php
index 56c9aadcc736e8aa715d55402c40d70988aaf1ac..ff7a8758818b194b671f9bfb6c29a188ce8465ac 100644 (file)
--- a/CRM/Price/Form/Option.php
+++ b/CRM/Price/Form/Option.php
@@ -286,7 +286,7 @@ class CRM_Price_Form_Option extends CRM_Core_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);

diff --git a/tests/phpunit/CRM/Price/Form/FieldTest.php b/tests/phpunit/CRM/Price/Form/FieldTest.php
index 379265097600ce6d0f73c7f41152d7c8b6ace4fa..f7ba89f11d17f74fcbcb53c0ceb0c0d3252ab195 100644 (file)
--- a/tests/phpunit/CRM/Price/Form/FieldTest.php
+++ b/tests/phpunit/CRM/Price/Form/FieldTest.php
@@ -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,
     ]);

diff --git a/tests/phpunit/CRM/Price/Form/OptionTest.php b/tests/phpunit/CRM/Price/Form/OptionTest.php
index 64fbd4ffc6efd783c0519bf62c03f05e4a8e603f..f9ac7ed7b7f5303394466ff6588ec8c783bac0ce 100644 (file)
--- a/tests/phpunit/CRM/Price/Form/OptionTest.php
+++ b/tests/phpunit/CRM/Price/Form/OptionTest.php
@@ -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,
     ]);

diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php
index 6316cde92e82d091212b049553f3006fdc9e7f18..d2c2f95cdb3b53b9456a6c284c9036abb55dd1d5 100644 (file)
--- a/tests/phpunit/api/v3/ReportTemplateTest.php
+++ b/tests/phpunit/api/v3/ReportTemplateTest.php
@@ -1447,7 +1447,7 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     $pcp2 = CRM_PCP_BAO_PCP::create($pcpParams);
 
     // Get soft credit types, with the name column as the key.
-    $soft_credit_types = CRM_Contribute_BAO_ContributionSoft::buildOptions("soft_credit_type_id", NULL, ["flip" => TRUE, 'labelColumn' => 'name']);
+    $soft_credit_types = CRM_Core_PseudoConstant::get('CRM_Contribute_BAO_ContributionSoft', 'soft_credit_type_id', ['flip' => TRUE, 'labelColumn' => 'name']);
     $pcp_soft_credit_type_id = $soft_credit_types['pcp'];
 
     // Create two contributions assigned to this contribution page and