compare against name not label

diff --git a/CRM/Core/PseudoConstant.php b/CRM/Core/PseudoConstant.php
index eb77d5ff3342fd9cfdaa7f890873c6cb630b5f95..2fdc708682357c399b1e6ca87c1831a7150512ee 100644 (file)
--- a/CRM/Core/PseudoConstant.php
+++ b/CRM/Core/PseudoConstant.php
@@ -1376,19 +1376,21 @@ WHERE  id = %1
    * The static array option values is returned
    *
    *
-   * @param bool $optionGroupName
-   *   Get All Option Group values- default is to get only active ones.
+   * @param string $optionGroupName
+   *   Name of option group
    *
    * @param int $id
-   * @param null $condition
+   * @param string $condition
+   * @param string $column
+   *   Whether to return 'name' or 'label'
    *
    * @return array
-   *   array reference of all Option Group Name
+   *   array reference of all Option Values
    */
-  public static function accountOptionValues($optionGroupName, $id = NULL, $condition = NULL) {
-    $cacheKey = $optionGroupName . '_' . $condition;
+  public static function accountOptionValues($optionGroupName, $id = NULL, $condition = NULL, $column = 'label') {
+    $cacheKey = $optionGroupName . '_' . $condition . '_' . $column;
     if (empty(self::$accountOptionValues[$cacheKey])) {
-      self::$accountOptionValues[$cacheKey] = CRM_Core_OptionGroup::values($optionGroupName, FALSE, FALSE, FALSE, $condition);
+      self::$accountOptionValues[$cacheKey] = CRM_Core_OptionGroup::values($optionGroupName, FALSE, FALSE, FALSE, $condition, $column);
     }
     if ($id) {
       return self::$accountOptionValues[$cacheKey][$id] ?? NULL;

diff --git a/CRM/Financial/BAO/FinancialAccount.php b/CRM/Financial/BAO/FinancialAccount.php
index 72b9b7a13a99b9e232dd1c6addfe2180f7113923..32fe022f824e83f57586d373b948234064c2e8b8 100644 (file)
--- a/CRM/Financial/BAO/FinancialAccount.php
+++ b/CRM/Financial/BAO/FinancialAccount.php
@@ -216,6 +216,8 @@ WHERE cft.id = %1
    *
    * Note that we avoid the CRM_Core_PseudoConstant function as it stores one
    * account per financial type and is unreliable.
+   * @todo Not sure what the above comment means, and the function uses the
+   * PseudoConstant twice. Three times if you count the for loop.
    *
    * @param int $financialTypeID
    *
@@ -224,7 +226,12 @@ WHERE cft.id = %1
    * @return int
    */
   public static function getFinancialAccountForFinancialTypeByRelationship($financialTypeID, $relationshipType) {
-    $relationTypeId = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE '{$relationshipType}' "));
+    // This is keyed on the `value` column from civicrm_option_value
+    $accountRelationshipsByValue = CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, NULL, 'name');
+    // We look up by the name a couple times below, so flip it.
+    $accountRelationships = array_flip($accountRelationshipsByValue);
+
+    $relationTypeId = $accountRelationships[$relationshipType] ?? NULL;
 
     if (!isset(Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$relationTypeId])) {
       $accounts = civicrm_api3('EntityFinancialAccount', 'get', [
@@ -236,14 +243,12 @@ WHERE cft.id = %1
         Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$account['account_relationship']] = $account['financial_account_id'];
       }
 
-      $accountRelationships = CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL);
-
-      $incomeAccountRelationshipID = array_search('Income Account is', $accountRelationships);
+      $incomeAccountRelationshipID = $accountRelationships['Income Account is'] ?? FALSE;
       $incomeAccountFinancialAccountID = Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$incomeAccountRelationshipID];
 
       foreach (['Chargeback Account is', 'Credit/Contra Revenue Account is'] as $optionalAccountRelationship) {
 
-        $accountRelationshipID = array_search($optionalAccountRelationship, $accountRelationships);
+        $accountRelationshipID = $accountRelationships[$optionalAccountRelationship] ?? FALSE;
         if (empty(Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$accountRelationshipID])) {
           Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$accountRelationshipID] = $incomeAccountFinancialAccountID;
         }

diff --git a/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php b/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php
index 0b34e9a503adb3b8bb521a2584bbaf4cfa7c8886..c2f7195b5e9b21377b75a5102de6c2562453683f 100644 (file)
--- a/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php
+++ b/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php
@@ -162,6 +162,28 @@ class CRM_Financial_BAO_FinancialAccountTest extends CiviUnitTestCase {
     $this->assertEquals(2, CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(2, 'Income Account is'));
   }
 
+  /**
+   * Test getting financial account for a given financial Type with a particular relationship with label changed.
+   */
+  public function testGetFinancialAccountByFinancialTypeAndRelationshipBuiltInLabel() {
+    // change the label
+    $optionValue = $this->callAPISuccess('OptionValue', 'get', [
+      'option_group_id' => 'account_relationship',
+      'name' => 'Income Account is',
+    ]);
+    $this->callAPISuccess('OptionValue', 'create', [
+      'id' => $optionValue['id'],
+      'label' => 'Changed label',
+    ]);
+    // run test
+    $this->assertEquals(2, CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(2, 'Income Account is'));
+    // restore label
+    $this->callAPISuccess('OptionValue', 'create', [
+      'id' => $optionValue['id'],
+      'label' => 'Income Account is',
+    ]);
+  }
+
   /**
    * Test getting financial account for a given financial Type with a particular relationship.
    */
@@ -169,6 +191,28 @@ class CRM_Financial_BAO_FinancialAccountTest extends CiviUnitTestCase {
     $this->assertEquals(2, CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(2, 'Credit/Contra Revenue Account is'));
   }
 
+  /**
+   * Test getting financial account for a given financial Type with a particular relationship with label changed.
+   */
+  public function testGetFinancialAccountByFinancialTypeAndRelationshipBuiltInRefundedLabel() {
+    // change the label
+    $optionValue = $this->callAPISuccess('OptionValue', 'get', [
+      'option_group_id' => 'account_relationship',
+      'name' => 'Credit/Contra Revenue Account is',
+    ]);
+    $this->callAPISuccess('OptionValue', 'create', [
+      'id' => $optionValue['id'],
+      'label' => 'Changed label',
+    ]);
+    // run test
+    $this->assertEquals(2, CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(2, 'Credit/Contra Revenue Account is'));
+    // restore label
+    $this->callAPISuccess('OptionValue', 'create', [
+      'id' => $optionValue['id'],
+      'label' => 'Credit/Contra Revenue Account is',
+    ]);
+  }
+
   /**
    * Test getting financial account for a given financial Type with a particular relationship.
    */
@@ -176,6 +220,28 @@ class CRM_Financial_BAO_FinancialAccountTest extends CiviUnitTestCase {
     $this->assertEquals(2, CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(2, 'Chargeback Account is'));
   }
 
+  /**
+   * Test getting financial account for a given financial Type with a particular relationship with label changed.
+   */
+  public function testGetFinancialAccountByFinancialTypeAndRelationshipBuiltInChargeBackLabel() {
+    // change the label
+    $optionValue = $this->callAPISuccess('OptionValue', 'get', [
+      'option_group_id' => 'account_relationship',
+      'name' => 'Chargeback Account is',
+    ]);
+    $this->callAPISuccess('OptionValue', 'create', [
+      'id' => $optionValue['id'],
+      'label' => 'Changed label',
+    ]);
+    // run test
+    $this->assertEquals(2, CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(2, 'Chargeback Account is'));
+    // restore label
+    $this->callAPISuccess('OptionValue', 'create', [
+      'id' => $optionValue['id'],
+      'label' => 'Chargeback Account is',
+    ]);
+  }
+
   /**
    * Test getting financial account for a given financial Type with a particular relationship.
    */