Merge pull request #10463 from JMAConsulting/CRM-20677

[ready for core team review] CRM-20677, used generalized function to retrieve financial account id
civicrm · Jul 14, 2017 · 100d147 · 100d147
2 parents 75b0867 + 02a9c0a
commit 100d147
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 61 deletions.
diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php
@@ -3667,7 +3667,10 @@ public static function updateFinancialAccounts(&$params, $context = NULL) {
             $itemParams['amount'] = self::getMultiplier($params['contribution']->contribution_status_id, $context) * $lineItemDetails['tax_amount'];
             $itemParams['description'] = $taxTerm;
             if ($lineItemDetails['financial_type_id']) {
-              $itemParams['financial_account_id'] = self::getFinancialAccountId($lineItemDetails['financial_type_id']);
+              $itemParams['financial_account_id'] = CRM_Contribute_PseudoConstant::getRelationalFinancialAccount(
+                $lineItemDetails['financial_type_id'],
+                'Sales Tax Account is'
+              );
             }
             CRM_Financial_BAO_FinancialItem::create($itemParams, NULL, $trxnIds);
           }
@@ -4177,27 +4180,6 @@ public static function getPaymentInfo($id, $component, $getTrxnInfo = FALSE, $us
     return $info;
   }
 
-  /**
-   * Get financial account id has 'Sales Tax Account is' account relationship with financial type.
-   *
-   * @param int $financialTypeId
-   *
-   * @return int
-   *   Financial Account Id
-   */
-  public static function getFinancialAccountId($financialTypeId) {
-    $accountRel = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE 'Sales Tax Account is' "));
-    $searchParams = array(
-      'entity_table' => 'civicrm_financial_type',
-      'entity_id' => $financialTypeId,
-      'account_relationship' => $accountRel,
-    );
-    $result = array();
-    CRM_Financial_BAO_FinancialTypeAccount::retrieve($searchParams, $result);
-
-    return CRM_Utils_Array::value('financial_account_id', $result);
-  }
-
   /**
    * Get the tax amount (misnamed function).
    *

diff --git a/CRM/Financial/BAO/FinancialItem.php b/CRM/Financial/BAO/FinancialItem.php
@@ -105,25 +105,19 @@ public static function add($lineItem, $contribution, $taxTrxnID = FALSE, $trxnId
       $taxTerm = CRM_Utils_Array::value('tax_term', $invoiceSettings);
       $params['amount'] = $lineItem->tax_amount;
       $params['description'] = $taxTerm;
-      $accountRel = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE 'Sales Tax Account is' "));
+      $accountRelName = 'Sales Tax Account is';
     }
     else {
       $accountRelName = 'Income Account is';
       if (property_exists($contribution, 'revenue_recognition_date') && !CRM_Utils_System::isNull($contribution->revenue_recognition_date)) {
         $accountRelName = 'Deferred Revenue Account is';
       }
-      $accountRel = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE '{$accountRelName}' "));
     }
     if ($lineItem->financial_type_id) {
-      $searchParams = array(
-        'entity_table' => 'civicrm_financial_type',
-        'entity_id' => $lineItem->financial_type_id,
-        'account_relationship' => $accountRel,
+      $params['financial_account_id'] = CRM_Contribute_PseudoConstant::getRelationalFinancialAccount(
+        $lineItem->financial_type_id,
+        $accountRelName
       );
-
-      $result = array();
-      CRM_Financial_BAO_FinancialTypeAccount::retrieve($searchParams, $result);
-      $params['financial_account_id'] = CRM_Utils_Array::value('financial_account_id', $result);
     }
     if (empty($trxnId)) {
       $trxnId['id'] = CRM_Contribute_BAO_Contribution::$_trxnIDs;

diff --git a/CRM/Financial/BAO/FinancialTypeAccount.php b/CRM/Financial/BAO/FinancialTypeAccount.php
@@ -39,12 +39,6 @@ public function __construct() {
     parent::__construct();
   }
 
-  /**
-   * Financial account.
-   * @var array
-   */
-  private static $financialAccount;
-
   /**
    * Fetch object based on array of properties.
    *
@@ -157,30 +151,20 @@ public static function del($financialTypeAccountId, $accountId = NULL) {
    * @param int $paymentInstrumentValue
    *   Payment instrument value.
    *
-   * @return array|null|string
+   * @return null|int
    */
-  public static function getInstrumentFinancialAccount($paymentInstrumentValue = NULL) {
-    if (!self::$financialAccount) {
-      $query = "SELECT ceft.financial_account_id, cov.value
-FROM civicrm_entity_financial_account ceft
-INNER JOIN civicrm_option_value cov ON cov.id = ceft.entity_id AND ceft.entity_table = 'civicrm_option_value'
-INNER JOIN civicrm_option_group cog ON cog.id = cov.option_group_id
-WHERE cog.name = 'payment_instrument' ";
-
-      if ($paymentInstrumentValue) {
-        $query .= " AND cov.value = '{$paymentInstrumentValue}' ";
-        return CRM_Core_DAO::singleValueQuery($query);
-      }
-      else {
-        $result = CRM_Core_DAO::executeQuery($query);
-        while ($result->fetch()) {
-          self::$financialAccount[$result->value] = $result->financial_account_id;
-        }
-        return self::$financialAccount;
-      }
-    }
-
-    return $paymentInstrumentValue ? self::$financialAccount[$paymentInstrumentValue] : self::$financialAccount;
+  public static function getInstrumentFinancialAccount($paymentInstrumentValue) {
+    $paymentInstrument = civicrm_api3('OptionValue', 'getsingle', array(
+      'return' => array("id"),
+      'value' => $paymentInstrumentValue,
+      'option_group_id' => "payment_instrument",
+    ));
+    $financialAccountId = CRM_Contribute_PseudoConstant::getRelationalFinancialAccount(
+      $paymentInstrument['id'],
+      NULL,
+      'civicrm_option_value'
+    );
+    return $financialAccountId;
   }
 
   /**

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
@@ -1148,6 +1148,9 @@ public function testCreateUpdateContributionPaymentInstrument() {
     $contribution = $this->callAPISuccess('contribution', 'create', $newParams);
     $this->assertAPISuccess($contribution);
     $this->_checkFinancialTrxn($contribution, 'paymentInstrument', $instrumentId);
+
+    // cleanup - delete created payment instrument
+    $this->_deletedAddedPaymentInstrument();
   }
 
   /**
@@ -1173,6 +1176,9 @@ public function testCreateUpdateNegativeContributionPaymentInstrument() {
     $contribution = $this->callAPISuccess('contribution', 'create', $newParams);
     $this->assertAPISuccess($contribution);
     $this->_checkFinancialTrxn($contribution, 'paymentInstrument', $instrumentId, array('total_amount' => '-100.00'));
+
+    // cleanup - delete created payment instrument
+    $this->_deletedAddedPaymentInstrument();
   }
 
   /**
@@ -3428,6 +3434,18 @@ public function _addPaymentInstrument() {
     return $optionValue['values'][$optionValue['id']]['value'];
   }
 
+  public function _deletedAddedPaymentInstrument() {
+    $result = $this->callAPISuccess('OptionValue', 'get', array(
+      'option_group_id' => 'payment_instrument',
+      'name' => 'Test Card',
+      'value' => '6',
+      'is_active' => 1,
+    ));
+    if ($id = CRM_Utils_Array::value('id', $result)) {
+      $this->callAPISuccess('OptionValue', 'delete', array('id' => $id));
+    }
+  }
+
   /**
    * Set up the basic recurring contribution for tests.
    *