From bf2cf926f1d3900dbc64c33bc28b1471b523cf7e Mon Sep 17 00:00:00 2001 From: eileen Date: Thu, 4 Feb 2016 23:22:00 +1300 Subject: [PATCH] CRM-17951 add support for having an account relationship for refunds. The support covers the relationship 'falling back' to current behaviour if a Contra/Credit Revenue Account is entry is not configured for the financial type. --- CRM/Contribute/BAO/Contribution.php | 45 ++++++++++++++---- CRM/Financial/BAO/FinancialAccount.php | 46 ++++++++++++++++++ .../Financial/BAO/FinancialAccountTest.php | 47 +++++++++++++++++-- tests/phpunit/api/v3/ContributionTest.php | 39 +++++++++++++++ 4 files changed, 163 insertions(+), 14 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index be18cf3971..748099942f 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -3086,8 +3086,10 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac 'In Progress', ); if (in_array($contributionStatus, $pendingStatus)) { - $relationTypeId = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE 'Accounts Receivable Account is' ")); - $params['to_financial_account_id'] = CRM_Contribute_PseudoConstant::financialAccountType($params['financial_type_id'], $relationTypeId); + $params['to_financial_account_id'] = CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship( + $params['financial_type_id'], + 'Accounts Receivable Account is' + ); } elseif (!empty($params['payment_processor'])) { $params['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getFinancialAccount($params['payment_processor'], 'civicrm_payment_processor', 'financial_account_id'); @@ -3464,17 +3466,14 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac foreach ($params['line_item'] as $fieldId => $fields) { foreach ($fields as $fieldValueId => $fieldValues) { $prevParams['entity_id'] = $fieldValues['id']; - $prevfinancialItem = CRM_Financial_BAO_FinancialItem::retrieve($prevParams, CRM_Core_DAO::$_nullArray); + $prevFinancialItem = CRM_Financial_BAO_FinancialItem::retrieve($prevParams, CRM_Core_DAO::$_nullArray); $receiveDate = CRM_Utils_Date::isoToMysql($params['prevContribution']->receive_date); if ($params['contribution']->receive_date) { $receiveDate = CRM_Utils_Date::isoToMysql($params['contribution']->receive_date); } - $financialAccount = $prevfinancialItem->financial_account_id; - if (!empty($params['financial_account_id'])) { - $financialAccount = $params['financial_account_id']; - } + $financialAccount = self::getFinancialAccountForStatusChangeTrxn($params, $prevFinancialItem); $currency = $params['prevContribution']->currency; if ($params['contribution']->currency) { @@ -3501,8 +3500,8 @@ INNER JOIN civicrm_activity ON civicrm_activity_contact.activity_id = civicrm_ac 'contact_id' => $params['prevContribution']->contact_id, 'currency' => $currency, 'amount' => $amount, - 'description' => $prevfinancialItem->description, - 'status_id' => $prevfinancialItem->status_id, + 'description' => $prevFinancialItem->description, + 'status_id' => $prevFinancialItem->status_id, 'financial_account_id' => $financialAccount, 'entity_table' => 'civicrm_line_item', 'entity_id' => $fieldValues['id'], @@ -4856,4 +4855,32 @@ LIMIT 1;"; } } + /** + * Get the financial account for the item associated with the new transaction. + * + * @param array $params + * @param CRM_Financial_BAO_FinancialItem $prevFinancialItem + * + * @return int + */ + public static function getFinancialAccountForStatusChangeTrxn($params, $prevFinancialItem) { + + if (!empty($params['financial_account_id'])) { + return $params['financial_account_id']; + } + $contributionStatus = CRM_Contribute_PseudoConstant::contributionStatus($params['contribution_status_id'], 'name'); + $preferredAccountsRelationships = array( + 'Refunded' => 'Credit/Contra Revenue Account is', + 'Chargeback' => 'Chargeback Account is', + ); + if (in_array($contributionStatus, array_keys($preferredAccountsRelationships))) { + $financialTypeID = !empty($params['financial_type_id']) ? $params['financial_type_id'] : $params['prevContribution']->financial_type_id; + return CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship( + $financialTypeID, + $preferredAccountsRelationships[$contributionStatus] + ); + } + return $prevFinancialItem->financial_account_id; + } + } diff --git a/CRM/Financial/BAO/FinancialAccount.php b/CRM/Financial/BAO/FinancialAccount.php index 2a5dd5a73d..79657577d8 100644 --- a/CRM/Financial/BAO/FinancialAccount.php +++ b/CRM/Financial/BAO/FinancialAccount.php @@ -200,4 +200,50 @@ WHERE cft.id = %1 return CRM_Core_DAO::singleValueQuery($query, $params); } + /** + * Get the Financial Account for a Financial Type Relationship Combo. + * + * Note that some relationships are optionally configured - so far + * Chargeback and Credit / Contra. Since these are the only 2 currently Income + * is an appropriate fallback. In future it might make sense to extend the logic. + * + * Note that we avoid the CRM_Core_PseudoConstant function as it stores one + * account per financial type and is unreliable. + * + * @param int $financialTypeID + * + * @param string $relationshipType + * + * @return int + */ + public static function getFinancialAccountForFinancialTypeByRelationship($financialTypeID, $relationshipType) { + $relationTypeId = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE '{$relationshipType}' ")); + + if (!isset(Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$relationTypeId])) { + $accounts = civicrm_api3('EntityFinancialAccount', 'get', array( + 'entity_id' => $financialTypeID, + 'entity_table' => 'civicrm_financial_type', + )); + + foreach ($accounts['values'] as $account) { + 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); + $incomeAccountFinancialAccountID = Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$incomeAccountRelationshipID]; + + foreach (array('Chargeback Account is', 'Credit/Contra Revenue Account is') as $optionalAccountRelationship) { + + $accountRelationshipID = array_search($optionalAccountRelationship, $accountRelationships); + if (empty(Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$accountRelationshipID])) { + Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$accountRelationshipID] = $incomeAccountFinancialAccountID; + } + } + + } + return Civi::$statics[__CLASS__]['entity_financial_account'][$financialTypeID][$relationTypeId]; + } + } diff --git a/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php b/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php index f42e92752f..f60aed5860 100644 --- a/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php +++ b/tests/phpunit/CRM/Financial/BAO/FinancialAccountTest.php @@ -31,14 +31,11 @@ class CRM_Financial_BAO_FinancialAccountTest extends CiviUnitTestCase { public function setUp() { + $this->useTransaction(TRUE); parent::setUp(); $this->organizationCreate(); } - public function teardown() { - $this->financialAccountDelete('Donations'); - } - /** * Check method add() */ @@ -72,7 +69,7 @@ class CRM_Financial_BAO_FinancialAccountTest extends CiviUnitTestCase { 'is_active' => 1, ); $ids = $defaults = array(); - $contributionType = CRM_Financial_BAO_FinancialAccount::add($params, $ids); + CRM_Financial_BAO_FinancialAccount::add($params); $result = CRM_Financial_BAO_FinancialAccount::retrieve($params, $defaults); @@ -139,4 +136,44 @@ class CRM_Financial_BAO_FinancialAccountTest extends CiviUnitTestCase { $this->assertEquals($accountingCode, 4800, 'Verify accounting code.'); } + /** + * Test getting financial account for a given financial Type with a particular relationship. + */ + public function testGetFinancialAccountByFinancialTypeAndRelationshipBuiltIn() { + $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. + */ + public function testGetFinancialAccountByFinancialTypeAndRelationshipBuiltInRefunded() { + $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. + */ + public function testGetFinancialAccountByFinancialTypeAndRelationshipBuiltInChargeBack() { + $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. + */ + public function testGetFinancialAccountByFinancialTypeAndRelationshipCustomAddedRefunded() { + $financialAccount = $this->callAPISuccess('FinancialAccount', 'create', array( + 'name' => 'Refund Account', + 'is_active' => TRUE, + )); + + $this->callAPISuccess('EntityFinancialAccount', 'create', array( + 'entity_id' => 2, + 'entity_table' => 'civicrm_financial_type', + 'account_relationship' => 'Credit/Contra Revenue Account is', + 'financial_account_id' => 'Refund Account', + )); + $this->assertEquals($financialAccount['id'], + CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(2, 'Credit/Contra Revenue Account is')); + } + } diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 597b15cf98..63b1a016df 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -1067,6 +1067,45 @@ class api_v3_ContributionTest extends CiviUnitTestCase { ))); } + /** + * Refund a contribution for a financial type with a contra account. + * + * CRM-17951 the contra account is a financial account with a relationship to a + * financial type. It is not always configured but should be reflected + * in the financial_trxn & financial_item table if it is. + */ + public function testCreateUpdateRefundContributionConfiguredContraAccount() { + $financialAccount = $this->callAPISuccess('FinancialAccount', 'create', array( + 'name' => 'Refund Account', + 'is_active' => TRUE, + )); + + $entityFinancialAccount = $this->callAPISuccess('EntityFinancialAccount', 'create', array( + 'entity_id' => $this->_financialTypeId, + 'entity_table' => 'civicrm_financial_type', + 'account_relationship' => 'Credit/Contra Revenue Account is', + 'financial_account_id' => 'Refund Account', + )); + + $contribution = $this->callAPISuccess('Contribution', 'create', $this->_params); + $this->callAPISuccess('Contribution', 'create', array( + 'id' => $contribution['id'], + 'contribution_status_id' => 'Refunded', + )); + + $lineItems = $this->callAPISuccessGetSingle( + 'LineItem', array( + 'contribution_id' => $contribution['id'], + 'api.FinancialItem.getsingle' => array('amount' => array('<' => 0)), + )); + $this->assertEquals($financialAccount['id'], $lineItems['api.FinancialItem.getsingle']['financial_account_id']); + + $this->callAPISuccess('Contribution', 'delete', array('id' => $contribution['id'])); + $this->callAPISuccess('EntityFinancialAccount', 'delete', array('id' => $entityFinancialAccount['id'])); + $this->callAPISuccess('FinancialAccount', 'delete', array('id' => $financialAccount['id'])); + + } + /** * Function tests that trxn_id is set when passed in. * -- 2.25.1