From a386fd96f244d3cb7cb53d4902825f0ce978fbed Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 23 Nov 2022 11:58:34 -0500 Subject: [PATCH 1/2] Alias CRM_Financial_BAO_FinancialTypeAccount -> CRM_Financial_BAO_EntityFinancialAccount This BAO class did not follow the standard naming convention which effectively made it invisible to the API. Why was named this way in the first place was never explained, I suspect it was simply a mistake. A BAO class actually did get added before this PR, in 37c608f6e which further added to the confusion. This consolidates the two classes. Because it was an "invisible" BAO class, the create and del functions were never used by the API. For consistency, I've therefore tagged them as @deprecated which keeps them hidden from the API. We can add business logic back in as needed using hook listeners. --- CRM/Contribute/BAO/Contribution.php | 6 +- CRM/Core/BAO/OptionValue.php | 2 +- CRM/Core/CodeGen/GenerateData.php | 2 +- CRM/Financial/BAO/EntityFinancialAccount.php | 251 ++++++++++++++++ CRM/Financial/BAO/FinancialType.php | 2 +- CRM/Financial/BAO/FinancialTypeAccount.php | 269 +----------------- CRM/Financial/BAO/PaymentProcessor.php | 2 +- CRM/Financial/Form/FinancialAccount.php | 2 +- CRM/Financial/Form/FinancialTypeAccount.php | 12 +- CRM/Financial/Form/PaymentEdit.php | 2 +- CRM/Financial/Page/FinancialType.php | 2 +- CRM/Financial/Page/FinancialTypeAccount.php | 2 +- .../CRM/Contribute/PseudoConstantTest.php | 2 +- tests/phpunit/CRM/Core/PseudoConstantTest.php | 2 +- .../BAO/FinancialTypeAccountTest.php | 12 +- .../phpunit/CRMTraits/Financial/TaxTrait.php | 2 +- tests/phpunit/CiviTest/CiviUnitTestCase.php | 4 +- tests/phpunit/api/v3/ContributionTest.php | 10 +- .../api/v3/TaxContributionPageTest.php | 6 +- 19 files changed, 290 insertions(+), 302 deletions(-) diff --git a/CRM/Contribute/BAO/Contribution.php b/CRM/Contribute/BAO/Contribution.php index 714c8811ad72..9e433bc12c0d 100644 --- a/CRM/Contribute/BAO/Contribution.php +++ b/CRM/Contribute/BAO/Contribution.php @@ -914,7 +914,7 @@ public static function getToFinancialAccount($contribution, $params) { return CRM_Contribute_PseudoConstant::getRelationalFinancialAccount($params['payment_processor_id'], NULL, 'civicrm_payment_processor'); } if (!empty($params['payment_instrument_id'])) { - return CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($contribution['payment_instrument_id']); + return CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($contribution['payment_instrument_id']); } else { $relationTypeId = key(CRM_Core_PseudoConstant::accountOptionValues('financial_account_type', NULL, " AND v.name LIKE 'Asset' ")); @@ -3104,11 +3104,11 @@ public static function recordFinancialAccounts(&$params, CRM_Contribute_DAO_Cont ]); } elseif (!empty($params['payment_instrument_id'])) { - $params['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($params['payment_instrument_id']); + $params['to_financial_account_id'] = CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($params['payment_instrument_id']); } // dev/financial#160 - If this is a contribution update, also check for an existing payment_instrument_id. elseif ($isUpdate && $params['prevContribution']->payment_instrument_id) { - $params['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount((int) $params['prevContribution']->payment_instrument_id); + $params['to_financial_account_id'] = CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount((int) $params['prevContribution']->payment_instrument_id); } else { $relationTypeId = key(CRM_Core_PseudoConstant::accountOptionValues('financial_account_type', NULL, " AND v.name LIKE 'Asset' ")); diff --git a/CRM/Core/BAO/OptionValue.php b/CRM/Core/BAO/OptionValue.php index 8893639abee6..5b82142359dc 100644 --- a/CRM/Core/BAO/OptionValue.php +++ b/CRM/Core/BAO/OptionValue.php @@ -220,7 +220,7 @@ public static function add(&$params, $ids = []) { 'account_relationship' => $relationTypeId, 'financial_account_id' => $params['financial_account_id'], ]; - CRM_Financial_BAO_FinancialTypeAccount::add($params); + CRM_Financial_BAO_EntityFinancialAccount::add($params); } } return $optionValue; diff --git a/CRM/Core/CodeGen/GenerateData.php b/CRM/Core/CodeGen/GenerateData.php index d0ba51a8c31e..94eb6aa00840 100644 --- a/CRM/Core/CodeGen/GenerateData.php +++ b/CRM/Core/CodeGen/GenerateData.php @@ -2181,7 +2181,7 @@ private function addAccountingEntries() { $select = 'SELECT contribution.id contribution_id, cli.id as line_item_id, contribution.contact_id, contribution.receive_date, contribution.total_amount, contribution.currency, cli.label, cli.financial_type_id, cefa.financial_account_id, contribution.payment_instrument_id, contribution.check_number, contribution.trxn_id'; $where = 'WHERE cefa.account_relationship = 1'; - $financialAccountId = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount(4); + $financialAccountId = CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount(4); foreach ($components as $component) { if ($component == 'contribution') { $from = 'FROM `civicrm_contribution` contribution'; diff --git a/CRM/Financial/BAO/EntityFinancialAccount.php b/CRM/Financial/BAO/EntityFinancialAccount.php index 1449091204cd..df7c195749b1 100644 --- a/CRM/Financial/BAO/EntityFinancialAccount.php +++ b/CRM/Financial/BAO/EntityFinancialAccount.php @@ -16,6 +16,257 @@ */ class CRM_Financial_BAO_EntityFinancialAccount extends CRM_Financial_DAO_EntityFinancialAccount { + /** + * Fetch object based on array of properties. + * + * @param array $params + * (reference ) an assoc array of name/value pairs. + * @param array $defaults + * (reference ) an assoc array to hold the flattened values. + * + * @param array $allValues + * @deprecated + * @return array + */ + public static function retrieve(&$params, &$defaults = [], &$allValues = []) { + $financialTypeAccount = new CRM_Financial_DAO_EntityFinancialAccount(); + $financialTypeAccount->copyValues($params); + $financialTypeAccount->find(); + while ($financialTypeAccount->fetch()) { + CRM_Core_DAO::storeValues($financialTypeAccount, $defaults); + $allValues[] = $defaults; + } + return $defaults; + } + + /** + * Add the financial types. + * + * @param array $params + * Reference array contains the values submitted by the form. + * @param array $ids + * Reference array contains one possible value + * - entityFinancialAccount. + * + * @return CRM_Financial_DAO_EntityFinancialAccount + * @deprecated + * @throws \CRM_Core_Exception + */ + public static function add(&$params, $ids = NULL) { + // action is taken depending upon the mode + $financialTypeAccount = new CRM_Financial_DAO_EntityFinancialAccount(); + if ($params['entity_table'] !== 'civicrm_financial_type') { + $financialTypeAccount->entity_id = $params['entity_id']; + $financialTypeAccount->entity_table = $params['entity_table']; + $financialTypeAccount->find(TRUE); + } + if (!empty($ids['entityFinancialAccount'])) { + $financialTypeAccount->id = $ids['entityFinancialAccount']; + $financialTypeAccount->find(TRUE); + } + $financialTypeAccount->copyValues($params); + self::validateRelationship($financialTypeAccount); + $financialTypeAccount->save(); + unset(Civi::$statics['CRM_Core_PseudoConstant']['taxRates']); + return $financialTypeAccount; + } + + /** + * Delete financial Types. + * + * @param int $financialTypeAccountId + * @param int $accountId + * @deprecated + * @throws \CRM_Core_Exception + */ + public static function del($financialTypeAccountId, $accountId = NULL) { + // check if financial type is present + $check = FALSE; + $relationValues = CRM_Core_PseudoConstant::get('CRM_Financial_DAO_EntityFinancialAccount', 'account_relationship'); + + $financialTypeId = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_EntityFinancialAccount', $financialTypeAccountId, 'entity_id'); + // check dependencies + // FIXME hardcoded list = bad + $dependency = [ + ['Contribute', 'Contribution'], + ['Contribute', 'ContributionPage'], + ['Member', 'MembershipType'], + ['Price', 'PriceFieldValue'], + ['Grant', 'Grant'], + ['Contribute', 'PremiumsProduct'], + ['Contribute', 'Product'], + ['Price', 'LineItem'], + ]; + + foreach ($dependency as $name) { + $daoString = 'CRM_' . $name[0] . '_DAO_' . $name[1]; + if (class_exists($daoString)) { + /** @var \CRM_Core_DAO $dao */ + $dao = new $daoString(); + $dao->financial_type_id = $financialTypeId; + if ($dao->find(TRUE)) { + $check = TRUE; + break; + } + } + } + + if ($check) { + if ($name[1] === 'PremiumsProduct' || $name[1] === 'Product') { + CRM_Core_Session::setStatus(ts('You cannot remove an account with a %1 relationship while the Financial Type is used for a Premium.', [1 => $relationValues[$financialTypeAccountId]])); + } + else { + $accountRelationShipId = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_EntityFinancialAccount', $financialTypeAccountId, 'account_relationship'); + CRM_Core_Session::setStatus(ts('You cannot remove an account with a %1 relationship because it is being referenced by one or more of the following types of records: Contributions, Contribution Pages, or Membership Types. Consider disabling this type instead if you no longer want it used.', [1 => $relationValues[$accountRelationShipId]]), NULL, 'error'); + } + CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/admin/financial/financialType/accounts', "reset=1&action=browse&aid={$accountId}")); + } + + // delete from financial Type table + $financialType = new CRM_Financial_DAO_EntityFinancialAccount(); + $financialType->id = $financialTypeAccountId; + $financialType->find(TRUE); + $financialType->delete(); + CRM_Core_Session::setStatus(ts('Unbalanced transactions may be created if you delete the account of type: %1.', [1 => $relationValues[$financialType->account_relationship]])); + } + + /** + * Financial Account for payment instrument. + * + * @param int $paymentInstrumentValue + * Payment instrument value. + * + * @return null|int + * @throws \CRM_Core_Exception + */ + public static function getInstrumentFinancialAccount($paymentInstrumentValue) { + if (!isset(\Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue])) { + $paymentInstrumentID = civicrm_api3('OptionValue', 'getvalue', [ + 'return' => 'id', + 'value' => $paymentInstrumentValue, + 'option_group_id' => "payment_instrument", + ]); + $accounts = civicrm_api3('EntityFinancialAccount', 'get', [ + 'return' => 'financial_account_id', + 'entity_table' => 'civicrm_option_value', + 'entity_id' => $paymentInstrumentID, + 'options' => ['limit' => 1], + 'sequential' => 1, + ])['values']; + if (empty($accounts)) { + \Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue] = NULL; + } + else { + \Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue] = $accounts[0]['financial_account_id']; + } + } + return \Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue]; + } + + /** + * Create default entity financial accounts + * for financial type + * @see https://issues.civicrm.org/jira/browse/CRM-12470 + * + * @param $financialType + * + * @return array + */ + public static function createDefaultFinancialAccounts($financialType) { + $titles = []; + $financialAccountTypeID = CRM_Core_OptionGroup::values('financial_account_type', FALSE, FALSE, FALSE, NULL, 'name'); + $accountRelationship = CRM_Core_OptionGroup::values('account_relationship', FALSE, FALSE, FALSE, NULL, 'name'); + + $relationships = [ + array_search('Accounts Receivable Account is', $accountRelationship) => array_search('Asset', $financialAccountTypeID), + array_search('Expense Account is', $accountRelationship) => array_search('Expenses', $financialAccountTypeID), + array_search('Cost of Sales Account is', $accountRelationship) => array_search('Cost of Sales', $financialAccountTypeID), + array_search('Income Account is', $accountRelationship) => array_search('Revenue', $financialAccountTypeID), + ]; + + $dao = CRM_Core_DAO::executeQuery('SELECT id, financial_account_type_id FROM civicrm_financial_account WHERE name LIKE %1', + [1 => [$financialType->name, 'String']] + ); + $dao->fetch(); + $existingFinancialAccount = []; + if (!$dao->N) { + $params = [ + 'name' => $financialType->name, + 'contact_id' => CRM_Core_BAO_Domain::getDomain()->contact_id, + 'financial_account_type_id' => array_search('Revenue', $financialAccountTypeID), + 'description' => $financialType->description, + 'account_type_code' => 'INC', + 'is_active' => 1, + ]; + $financialAccount = CRM_Financial_BAO_FinancialAccount::add($params); + } + else { + $existingFinancialAccount[$dao->financial_account_type_id] = $dao->id; + } + $params = [ + 'entity_table' => 'civicrm_financial_type', + 'entity_id' => $financialType->id, + ]; + foreach ($relationships as $key => $value) { + if (!array_key_exists($value, $existingFinancialAccount)) { + if ($accountRelationship[$key] == 'Accounts Receivable Account is') { + $params['financial_account_id'] = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialAccount', 'Accounts Receivable', 'id', 'name'); + if (!empty($params['financial_account_id'])) { + $titles[] = 'Accounts Receivable'; + } + else { + $query = "SELECT financial_account_id, name FROM civicrm_entity_financial_account + LEFT JOIN civicrm_financial_account ON civicrm_financial_account.id = civicrm_entity_financial_account.financial_account_id + WHERE account_relationship = {$key} AND entity_table = 'civicrm_financial_type' LIMIT 1"; + $dao = CRM_Core_DAO::executeQuery($query); + $dao->fetch(); + $params['financial_account_id'] = $dao->financial_account_id; + $titles[] = $dao->name; + } + } + elseif ($accountRelationship[$key] == 'Income Account is' && empty($existingFinancialAccount)) { + $params['financial_account_id'] = $financialAccount->id; + } + else { + $query = "SELECT id, name FROM civicrm_financial_account WHERE is_default = 1 AND financial_account_type_id = {$value}"; + $dao = CRM_Core_DAO::executeQuery($query); + $dao->fetch(); + $params['financial_account_id'] = $dao->id; + $titles[] = $dao->name; + } + } + else { + $params['financial_account_id'] = $existingFinancialAccount[$value]; + $titles[] = $financialType->name; + } + $params['account_relationship'] = $key; + self::add($params); + } + if (!empty($existingFinancialAccount)) { + $titles = []; + } + return $titles; + } + + /** + * Validate account relationship with financial account type + * + * @param CRM_Financial_DAO_EntityFinancialAccount $financialTypeAccount of CRM_Financial_DAO_EntityFinancialAccount + * + * @throws CRM_Core_Exception + */ + public static function validateRelationship($financialTypeAccount) { + $financialAccountLinks = CRM_Financial_BAO_FinancialAccount::getfinancialAccountRelations(); + $financialAccountType = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialAccount', $financialTypeAccount->financial_account_id, 'financial_account_type_id'); + if (CRM_Utils_Array::value($financialTypeAccount->account_relationship, $financialAccountLinks) != $financialAccountType) { + $accountRelationships = CRM_Core_PseudoConstant::get('CRM_Financial_DAO_EntityFinancialAccount', 'account_relationship'); + $params = [ + 1 => $accountRelationships[$financialTypeAccount->account_relationship], + ]; + throw new CRM_Core_Exception(ts("This financial account cannot have '%1' relationship.", $params)); + } + } + /** * Whitelist of possible values for the entity_table field * diff --git a/CRM/Financial/BAO/FinancialType.php b/CRM/Financial/BAO/FinancialType.php index 21b8b10447a1..e832df68ae05 100644 --- a/CRM/Financial/BAO/FinancialType.php +++ b/CRM/Financial/BAO/FinancialType.php @@ -144,7 +144,7 @@ public static function self_hook_civicrm_pre(\Civi\Core\Event\PreEvent $event) { */ public static function self_hook_civicrm_post(\Civi\Core\Event\PostEvent $event) { if ($event->action === 'create') { - $titles = CRM_Financial_BAO_FinancialTypeAccount::createDefaultFinancialAccounts($event->object); + $titles = CRM_Financial_BAO_EntityFinancialAccount::createDefaultFinancialAccounts($event->object); $event->object->titles = $titles; } if ($event->action === 'delete') { diff --git a/CRM/Financial/BAO/FinancialTypeAccount.php b/CRM/Financial/BAO/FinancialTypeAccount.php index a3b8bbfecbf6..4e0443aa64ad 100644 --- a/CRM/Financial/BAO/FinancialTypeAccount.php +++ b/CRM/Financial/BAO/FinancialTypeAccount.php @@ -1,270 +1,7 @@ copyValues($params); - $financialTypeAccount->find(); - while ($financialTypeAccount->fetch()) { - CRM_Core_DAO::storeValues($financialTypeAccount, $defaults); - $allValues[] = $defaults; - } - return $defaults; - } - - /** - * Add the financial types. - * - * @param array $params - * Reference array contains the values submitted by the form. - * @param array $ids - * Reference array contains one possible value - * - entityFinancialAccount. - * - * @return CRM_Financial_DAO_EntityFinancialAccount - * - * @throws \CRM_Core_Exception - */ - public static function add(&$params, $ids = NULL) { - // action is taken depending upon the mode - $financialTypeAccount = new CRM_Financial_DAO_EntityFinancialAccount(); - if ($params['entity_table'] !== 'civicrm_financial_type') { - $financialTypeAccount->entity_id = $params['entity_id']; - $financialTypeAccount->entity_table = $params['entity_table']; - $financialTypeAccount->find(TRUE); - } - if (!empty($ids['entityFinancialAccount'])) { - $financialTypeAccount->id = $ids['entityFinancialAccount']; - $financialTypeAccount->find(TRUE); - } - $financialTypeAccount->copyValues($params); - self::validateRelationship($financialTypeAccount); - $financialTypeAccount->save(); - unset(Civi::$statics['CRM_Core_PseudoConstant']['taxRates']); - return $financialTypeAccount; - } - - /** - * Delete financial Types. - * - * @param int $financialTypeAccountId - * @param int $accountId - * - * @throws \CRM_Core_Exception - */ - public static function del($financialTypeAccountId, $accountId = NULL) { - // check if financial type is present - $check = FALSE; - $relationValues = CRM_Core_PseudoConstant::get('CRM_Financial_DAO_EntityFinancialAccount', 'account_relationship'); - - $financialTypeId = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_EntityFinancialAccount', $financialTypeAccountId, 'entity_id'); - // check dependencies - // FIXME hardcoded list = bad - $dependency = [ - ['Contribute', 'Contribution'], - ['Contribute', 'ContributionPage'], - ['Member', 'MembershipType'], - ['Price', 'PriceFieldValue'], - ['Grant', 'Grant'], - ['Contribute', 'PremiumsProduct'], - ['Contribute', 'Product'], - ['Price', 'LineItem'], - ]; - - foreach ($dependency as $name) { - $daoString = 'CRM_' . $name[0] . '_DAO_' . $name[1]; - if (class_exists($daoString)) { - /** @var \CRM_Core_DAO $dao */ - $dao = new $daoString(); - $dao->financial_type_id = $financialTypeId; - if ($dao->find(TRUE)) { - $check = TRUE; - break; - } - } - } - - if ($check) { - if ($name[1] === 'PremiumsProduct' || $name[1] === 'Product') { - CRM_Core_Session::setStatus(ts('You cannot remove an account with a %1 relationship while the Financial Type is used for a Premium.', [1 => $relationValues[$financialTypeAccountId]])); - } - else { - $accountRelationShipId = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_EntityFinancialAccount', $financialTypeAccountId, 'account_relationship'); - CRM_Core_Session::setStatus(ts('You cannot remove an account with a %1 relationship because it is being referenced by one or more of the following types of records: Contributions, Contribution Pages, or Membership Types. Consider disabling this type instead if you no longer want it used.', [1 => $relationValues[$accountRelationShipId]]), NULL, 'error'); - } - return CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/admin/financial/financialType/accounts', "reset=1&action=browse&aid={$accountId}")); - } - - // delete from financial Type table - $financialType = new CRM_Financial_DAO_EntityFinancialAccount(); - $financialType->id = $financialTypeAccountId; - $financialType->find(TRUE); - $financialType->delete(); - CRM_Core_Session::setStatus(ts('Unbalanced transactions may be created if you delete the account of type: %1.', [1 => $relationValues[$financialType->account_relationship]])); - } - - /** - * Financial Account for payment instrument. - * - * @param int $paymentInstrumentValue - * Payment instrument value. - * - * @return null|int - * @throws \CRM_Core_Exception - */ - public static function getInstrumentFinancialAccount($paymentInstrumentValue) { - if (!isset(\Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue])) { - $paymentInstrumentID = civicrm_api3('OptionValue', 'getvalue', [ - 'return' => 'id', - 'value' => $paymentInstrumentValue, - 'option_group_id' => "payment_instrument", - ]); - $accounts = civicrm_api3('EntityFinancialAccount', 'get', [ - 'return' => 'financial_account_id', - 'entity_table' => 'civicrm_option_value', - 'entity_id' => $paymentInstrumentID, - 'options' => ['limit' => 1], - 'sequential' => 1, - ])['values']; - if (empty($accounts)) { - \Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue] = NULL; - } - else { - \Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue] = $accounts[0]['financial_account_id']; - } - } - return \Civi::$statics[__CLASS__]['instrument_financial_accounts'][$paymentInstrumentValue]; - } - - /** - * Create default entity financial accounts - * for financial type - * @see https://issues.civicrm.org/jira/browse/CRM-12470 - * - * @param $financialType - * - * @return array - */ - public static function createDefaultFinancialAccounts($financialType) { - $titles = []; - $financialAccountTypeID = CRM_Core_OptionGroup::values('financial_account_type', FALSE, FALSE, FALSE, NULL, 'name'); - $accountRelationship = CRM_Core_OptionGroup::values('account_relationship', FALSE, FALSE, FALSE, NULL, 'name'); - - $relationships = [ - array_search('Accounts Receivable Account is', $accountRelationship) => array_search('Asset', $financialAccountTypeID), - array_search('Expense Account is', $accountRelationship) => array_search('Expenses', $financialAccountTypeID), - array_search('Cost of Sales Account is', $accountRelationship) => array_search('Cost of Sales', $financialAccountTypeID), - array_search('Income Account is', $accountRelationship) => array_search('Revenue', $financialAccountTypeID), - ]; - - $dao = CRM_Core_DAO::executeQuery('SELECT id, financial_account_type_id FROM civicrm_financial_account WHERE name LIKE %1', - [1 => [$financialType->name, 'String']] - ); - $dao->fetch(); - $existingFinancialAccount = []; - if (!$dao->N) { - $params = [ - 'name' => $financialType->name, - 'contact_id' => CRM_Core_BAO_Domain::getDomain()->contact_id, - 'financial_account_type_id' => array_search('Revenue', $financialAccountTypeID), - 'description' => $financialType->description, - 'account_type_code' => 'INC', - 'is_active' => 1, - ]; - $financialAccount = CRM_Financial_BAO_FinancialAccount::add($params); - } - else { - $existingFinancialAccount[$dao->financial_account_type_id] = $dao->id; - } - $params = [ - 'entity_table' => 'civicrm_financial_type', - 'entity_id' => $financialType->id, - ]; - foreach ($relationships as $key => $value) { - if (!array_key_exists($value, $existingFinancialAccount)) { - if ($accountRelationship[$key] == 'Accounts Receivable Account is') { - $params['financial_account_id'] = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialAccount', 'Accounts Receivable', 'id', 'name'); - if (!empty($params['financial_account_id'])) { - $titles[] = 'Accounts Receivable'; - } - else { - $query = "SELECT financial_account_id, name FROM civicrm_entity_financial_account - LEFT JOIN civicrm_financial_account ON civicrm_financial_account.id = civicrm_entity_financial_account.financial_account_id - WHERE account_relationship = {$key} AND entity_table = 'civicrm_financial_type' LIMIT 1"; - $dao = CRM_Core_DAO::executeQuery($query); - $dao->fetch(); - $params['financial_account_id'] = $dao->financial_account_id; - $titles[] = $dao->name; - } - } - elseif ($accountRelationship[$key] == 'Income Account is' && empty($existingFinancialAccount)) { - $params['financial_account_id'] = $financialAccount->id; - } - else { - $query = "SELECT id, name FROM civicrm_financial_account WHERE is_default = 1 AND financial_account_type_id = {$value}"; - $dao = CRM_Core_DAO::executeQuery($query); - $dao->fetch(); - $params['financial_account_id'] = $dao->id; - $titles[] = $dao->name; - } - } - else { - $params['financial_account_id'] = $existingFinancialAccount[$value]; - $titles[] = $financialType->name; - } - $params['account_relationship'] = $key; - self::add($params); - } - if (!empty($existingFinancialAccount)) { - $titles = []; - } - return $titles; - } - - /** - * Validate account relationship with financial account type - * - * @param CRM_Financial_DAO_EntityFinancialAccount $financialTypeAccount of CRM_Financial_DAO_EntityFinancialAccount - * - * @throws CRM_Core_Exception - */ - public static function validateRelationship($financialTypeAccount) { - $financialAccountLinks = CRM_Financial_BAO_FinancialAccount::getfinancialAccountRelations(); - $financialAccountType = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialAccount', $financialTypeAccount->financial_account_id, 'financial_account_type_id'); - if (CRM_Utils_Array::value($financialTypeAccount->account_relationship, $financialAccountLinks) != $financialAccountType) { - $accountRelationships = CRM_Core_PseudoConstant::get('CRM_Financial_DAO_EntityFinancialAccount', 'account_relationship'); - $params = [ - 1 => $accountRelationships[$financialTypeAccount->account_relationship], - ]; - throw new CRM_Core_Exception(ts("This financial account cannot have '%1' relationship.", $params)); - } - } - -} +class_alias('CRM_Financial_BAO_EntityFinancialAccount', 'CRM_Financial_BAO_FinancialTypeAccount'); diff --git a/CRM/Financial/BAO/PaymentProcessor.php b/CRM/Financial/BAO/PaymentProcessor.php index 332002ce24ed..99ad17f8cffb 100644 --- a/CRM/Financial/BAO/PaymentProcessor.php +++ b/CRM/Financial/BAO/PaymentProcessor.php @@ -71,7 +71,7 @@ public static function create(array $params): CRM_Financial_DAO_PaymentProcessor 'account_relationship' => $relationTypeId, 'financial_account_id' => $params['financial_account_id'], ]; - CRM_Financial_BAO_FinancialTypeAccount::add($values); + CRM_Financial_BAO_EntityFinancialAccount::add($values); } if (isset($params['id']) && isset($params['is_active']) && !isset($params['is_test'])) { diff --git a/CRM/Financial/Form/FinancialAccount.php b/CRM/Financial/Form/FinancialAccount.php index 93f4aa668b47..8c52ce6fc2e0 100644 --- a/CRM/Financial/Form/FinancialAccount.php +++ b/CRM/Financial/Form/FinancialAccount.php @@ -159,7 +159,7 @@ public static function formRule($values, $files, $self) { 'financial_account_id' => $self->_id, 'account_relationship' => $relationshipId, ]; - $result = CRM_Financial_BAO_FinancialTypeAccount::retrieve($params, $defaults); + $result = CRM_Financial_BAO_EntityFinancialAccount::retrieve($params, $defaults); if ($result) { $errorMsg['is_tax'] = ts('Is Tax? must be set for this financial account'); } diff --git a/CRM/Financial/Form/FinancialTypeAccount.php b/CRM/Financial/Form/FinancialTypeAccount.php index fae3624ab93d..561853ad384d 100644 --- a/CRM/Financial/Form/FinancialTypeAccount.php +++ b/CRM/Financial/Form/FinancialTypeAccount.php @@ -61,7 +61,7 @@ public function preProcess() { $url = CRM_Utils_System::url('civicrm/admin/financial/financialType/accounts', "reset=1&action=browse&aid={$this->_aid}"); - $this->_BAOName = 'CRM_Financial_BAO_FinancialTypeAccount'; + $this->_BAOName = 'CRM_Financial_BAO_EntityFinancialAccount'; if ($this->_aid && ($this->_action & CRM_Core_Action::ADD)) { $this->_title = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialType', $this->_aid, 'name'); $this->setTitle($this->_title . ' - ' . ts('Financial Accounts')); @@ -132,7 +132,7 @@ public function buildQuickForm() { if (isset($this->_id)) { $params = ['id' => $this->_id]; - CRM_Financial_BAO_FinancialTypeAccount::retrieve($params, $defaults); + CRM_Financial_BAO_EntityFinancialAccount::retrieve($params, $defaults); $this->setDefaults($defaults); $financialAccountTitle = CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialAccount', $defaults['financial_account_id'], 'name'); } @@ -242,7 +242,7 @@ public static function formRule($values, $files, $self) { $errorMsg['financial_account_id'] = ts('Is Tax? must be set for respective financial account'); } } - $result = CRM_Financial_BAO_FinancialTypeAccount::retrieve($params, $defaults); + $result = CRM_Financial_BAO_EntityFinancialAccount::retrieve($params, $defaults); if ($result) { $errorFlag = TRUE; } @@ -253,7 +253,7 @@ public static function formRule($values, $files, $self) { } else { $params['financial_account_id'] = $values['financial_account_id']; - $result = CRM_Financial_BAO_FinancialTypeAccount::retrieve($params, $defaults); + $result = CRM_Financial_BAO_EntityFinancialAccount::retrieve($params, $defaults); if ($result) { $errorFlag = TRUE; } @@ -272,7 +272,7 @@ public static function formRule($values, $files, $self) { */ public function postProcess() { if ($this->_action & CRM_Core_Action::DELETE) { - CRM_Financial_BAO_FinancialTypeAccount::del($this->_id, $this->_aid); + CRM_Financial_BAO_EntityFinancialAccount::del($this->_id, $this->_aid); CRM_Core_Session::setStatus(ts('Selected financial type account has been deleted.')); } else { @@ -291,7 +291,7 @@ public function postProcess() { $params['entity_id'] = $this->_aid; } try { - $financialTypeAccount = CRM_Financial_BAO_FinancialTypeAccount::add($params, $ids); + $financialTypeAccount = CRM_Financial_BAO_EntityFinancialAccount::add($params, $ids); CRM_Core_Session::setStatus(ts('The financial type Account has been saved.'), ts('Saved'), 'success'); } catch (CRM_Core_Exception $e) { diff --git a/CRM/Financial/Form/PaymentEdit.php b/CRM/Financial/Form/PaymentEdit.php index 9f0cae243709..751b830bf65e 100644 --- a/CRM/Financial/Form/PaymentEdit.php +++ b/CRM/Financial/Form/PaymentEdit.php @@ -210,7 +210,7 @@ protected function submit($submittedValues) { $newFinancialTrxn = $submittedValues; unset($newFinancialTrxn['id']); - $newFinancialTrxn['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($submittedValues['payment_instrument_id']); + $newFinancialTrxn['to_financial_account_id'] = CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($submittedValues['payment_instrument_id']); $newFinancialTrxn['total_amount'] = $this->_values['total_amount']; $newFinancialTrxn['currency'] = $this->_values['currency']; $newFinancialTrxn['contribution_id'] = $this->getContributionID(); diff --git a/CRM/Financial/Page/FinancialType.php b/CRM/Financial/Page/FinancialType.php index 2cb9fb903b27..00b44d0c6db1 100644 --- a/CRM/Financial/Page/FinancialType.php +++ b/CRM/Financial/Page/FinancialType.php @@ -100,7 +100,7 @@ public function browse() { $params['entity_id'] = $dao->id; $params['entity_table'] = 'civicrm_financial_type'; $null = []; - CRM_Financial_BAO_FinancialTypeAccount::retrieve($params, $null, $financialAccountIds); + CRM_Financial_BAO_EntityFinancialAccount::retrieve($params, $null, $financialAccountIds); foreach ($financialAccountIds as $key => $values) { if (!empty($financialAccounts[$values['financial_account_id']])) { diff --git a/CRM/Financial/Page/FinancialTypeAccount.php b/CRM/Financial/Page/FinancialTypeAccount.php index b451ee5a9800..75b4d47e2bbb 100644 --- a/CRM/Financial/Page/FinancialTypeAccount.php +++ b/CRM/Financial/Page/FinancialTypeAccount.php @@ -40,7 +40,7 @@ class CRM_Financial_Page_FinancialTypeAccount extends CRM_Core_Page { * Classname of BAO. */ public function getBAOName() { - return 'CRM_Financial_BAO_FinancialTypeAccount'; + return 'CRM_Financial_BAO_EntityFinancialAccount'; } /** diff --git a/tests/phpunit/CRM/Contribute/PseudoConstantTest.php b/tests/phpunit/CRM/Contribute/PseudoConstantTest.php index e8bdedddbad8..e7421c57b1d4 100644 --- a/tests/phpunit/CRM/Contribute/PseudoConstantTest.php +++ b/tests/phpunit/CRM/Contribute/PseudoConstantTest.php @@ -63,7 +63,7 @@ public function testGetRelationalFinancialAccountForPaymentInstrument() { $paymentInstruments = $this->callAPISuccess('Contribution', 'getoptions', ['field' => 'payment_instrument_id'])['values']; $financialAccounts = $this->callAPISuccess('FinancialAccount', 'get', [])['values']; foreach ($paymentInstruments as $paymentInstrumentID => $paymentInstrumentName) { - $financialAccountID = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($paymentInstrumentID); + $financialAccountID = CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($paymentInstrumentID); if (in_array($paymentInstrumentName, ['Credit Card', 'Debit Card'])) { $this->assertEquals('Payment Processor Account', $financialAccounts[$financialAccountID]['name']); } diff --git a/tests/phpunit/CRM/Core/PseudoConstantTest.php b/tests/phpunit/CRM/Core/PseudoConstantTest.php index 29f75600b1c3..469334dd5fa0 100644 --- a/tests/phpunit/CRM/Core/PseudoConstantTest.php +++ b/tests/phpunit/CRM/Core/PseudoConstantTest.php @@ -1028,7 +1028,7 @@ public function testGetTaxRates() { 'account_relationship' => 10, 'financial_account_id' => $financialAccountId, ]; - CRM_Financial_BAO_FinancialTypeAccount::add($financialAccountParams); + CRM_Financial_BAO_EntityFinancialAccount::add($financialAccountParams); $taxRates = CRM_Core_PseudoConstant::getTaxRates(); $this->assertEquals('5.00', round($taxRates[$financialType['id']], 2)); } diff --git a/tests/phpunit/CRM/Financial/BAO/FinancialTypeAccountTest.php b/tests/phpunit/CRM/Financial/BAO/FinancialTypeAccountTest.php index 4bb6d3b077e4..20e45f83ecca 100644 --- a/tests/phpunit/CRM/Financial/BAO/FinancialTypeAccountTest.php +++ b/tests/phpunit/CRM/Financial/BAO/FinancialTypeAccountTest.php @@ -47,7 +47,7 @@ public function testDel() { 'Expense Account is' ); - CRM_Financial_BAO_FinancialTypeAccount::del($financialAccountType->id); + CRM_Financial_BAO_EntityFinancialAccount::del($financialAccountType->id); $params = ['id' => $financialAccountType->id]; $result = CRM_Financial_BAO_FinancialType::retrieve($params, $defaults); $this->assertEquals(empty($result), TRUE, 'Verify financial types record deletion.'); @@ -70,7 +70,7 @@ public function testRetrieve() { ]; $defaults = []; - $financialAccountType = CRM_Financial_BAO_FinancialTypeAccount::retrieve($financialParams, $defaults); + $financialAccountType = CRM_Financial_BAO_EntityFinancialAccount::retrieve($financialParams, $defaults); $this->assertEquals($financialAccountType['entity_id'], $financialType->id, 'Verify Entity Id.'); $this->assertEquals($financialAccountType['financial_account_id'], $financialAccount->id, 'Verify Financial Account Id.'); } @@ -96,8 +96,8 @@ public function testGetInstrumentFinancialAccount() { 'financial_account_id' => $financialAccount->id, ]; - CRM_Financial_BAO_FinancialTypeAccount::add($financialParams); - $financialAccountId = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($paymentInstrumentValue); + CRM_Financial_BAO_EntityFinancialAccount::add($financialParams); + $financialAccountId = CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($paymentInstrumentValue); $this->assertEquals($financialAccountId, $financialAccount->id, 'Verify Payment Instrument'); } @@ -116,7 +116,7 @@ public function testValidateRelationship() { $financialAccountType->account_relationship = array_search('Credit/Contra Revenue Account is', $accountRelationships); $financialAccountType->financial_account_id = array_search('Liability', $financialAccount); try { - CRM_Financial_BAO_FinancialTypeAccount::validateRelationship($financialAccountType); + CRM_Financial_BAO_EntityFinancialAccount::validateRelationship($financialAccountType); $this->fail("Missed expected exception"); } catch (Exception $e) { @@ -167,7 +167,7 @@ public function createFinancialAccount($financialAccountType, $relationType = NU $financialParams['id'] = $dao->id; } $financialParams['financial_account_id'] = $financialAccount->id; - $financialAccountType = CRM_Financial_BAO_FinancialTypeAccount::add($financialParams); + $financialAccountType = CRM_Financial_BAO_EntityFinancialAccount::add($financialParams); } return [$financialAccount, $financialType, $financialAccountType]; } diff --git a/tests/phpunit/CRMTraits/Financial/TaxTrait.php b/tests/phpunit/CRMTraits/Financial/TaxTrait.php index 35da539dd63a..5ca4b2111e25 100644 --- a/tests/phpunit/CRMTraits/Financial/TaxTrait.php +++ b/tests/phpunit/CRMTraits/Financial/TaxTrait.php @@ -57,7 +57,7 @@ public function createFinancialTypeWithSalesTax(string $key = 'taxable', array $ 'account_relationship' => 10, 'financial_account_id' => $this->ids['FinancialAccount'][$key], ]; - CRM_Financial_BAO_FinancialTypeAccount::add($financialAccountParams); + CRM_Financial_BAO_EntityFinancialAccount::add($financialAccountParams); } } diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 6b0d304ee18a..f3a0cd29c2bc 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2988,7 +2988,7 @@ protected function createPaymentInstrument($params = [], $financialAccountName = 'account_relationship' => $relationTypeID, 'financial_account_id' => $this->callAPISuccess('FinancialAccount', 'getValue', ['name' => $financialAccountName, 'return' => 'id']), ]; - CRM_Financial_BAO_FinancialTypeAccount::add($financialAccountParams); + CRM_Financial_BAO_EntityFinancialAccount::add($financialAccountParams); return CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'payment_instrument_id', $params['label']); } @@ -3074,7 +3074,7 @@ protected function addTaxAccountToFinancialType(int $financialTypeID, array $acc } $entityParams['financial_account_id'] = $financialAccountID; - return CRM_Financial_BAO_FinancialTypeAccount::add($entityParams); + return CRM_Financial_BAO_EntityFinancialAccount::add($entityParams); } /** diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 547e7f472dd8..ff5eb784bfb8 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -4126,21 +4126,21 @@ public function checkFinancialTrxnPaymentInstrumentChange($contributionID, $orig $entityFinancialTrxns = $this->getFinancialTransactionsForContribution($contributionID); $originalTrxnParams = [ - 'to_financial_account_id' => CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($originalInstrumentID), + 'to_financial_account_id' => CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($originalInstrumentID), 'payment_instrument_id' => $originalInstrumentID, 'amount' => $amount, 'status_id' => 1, ]; $reversalTrxnParams = [ - 'to_financial_account_id' => CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($originalInstrumentID), + 'to_financial_account_id' => CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($originalInstrumentID), 'payment_instrument_id' => $originalInstrumentID, 'amount' => -$amount, 'status_id' => 1, ]; $newTrxnParams = [ - 'to_financial_account_id' => CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($newInstrumentID), + 'to_financial_account_id' => CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($newInstrumentID), 'payment_instrument_id' => $newInstrumentID, 'amount' => $amount, 'status_id' => 1, @@ -4233,7 +4233,7 @@ public function _checkFinancialTrxn($contribution, $context, $instrumentId = NUL ]; } if ($context === 'paymentInstrument') { - $compareParams['to_financial_account_id'] = CRM_Financial_BAO_FinancialTypeAccount::getInstrumentFinancialAccount($instrumentId); + $compareParams['to_financial_account_id'] = CRM_Financial_BAO_EntityFinancialAccount::getInstrumentFinancialAccount($instrumentId); $compareParams['payment_instrument_id'] = $instrumentId; } else { @@ -4273,7 +4273,7 @@ public function _addPaymentInstrument() { 'account_relationship' => $relationTypeId, 'financial_account_id' => 7, ]; - CRM_Financial_BAO_FinancialTypeAccount::add($financialParams); + CRM_Financial_BAO_EntityFinancialAccount::add($financialParams); $this->assertNotEmpty($optionValue['values'][$optionValue['id']]['value']); return $optionValue['values'][$optionValue['id']]['value']; } diff --git a/tests/phpunit/api/v3/TaxContributionPageTest.php b/tests/phpunit/api/v3/TaxContributionPageTest.php index 801c8fc5316d..11ecffabc736 100644 --- a/tests/phpunit/api/v3/TaxContributionPageTest.php +++ b/tests/phpunit/api/v3/TaxContributionPageTest.php @@ -94,7 +94,7 @@ public function setUp(): void { 'account_relationship' => 10, 'financial_account_id' => $this->financialAccountId, ]; - CRM_Financial_BAO_FinancialTypeAccount::add($financialRelationParams); + CRM_Financial_BAO_EntityFinancialAccount::add($financialRelationParams); // Financial type with 5% tax rate $financialAccountHalfTax = [ @@ -124,7 +124,7 @@ public function setUp(): void { 'financial_account_id' => $this->halfFinancialAccId, ]; - CRM_Financial_BAO_FinancialTypeAccount::add($financialRelationHalftax); + CRM_Financial_BAO_EntityFinancialAccount::add($financialRelationHalftax); // Enable component contribute setting $this->enableTaxAndInvoicing(); @@ -493,7 +493,7 @@ public function _getFinancialAccountId(int $financialTypeId): ?int { ]; $result = []; - CRM_Financial_BAO_FinancialTypeAccount::retrieve($searchParams, $result); + CRM_Financial_BAO_EntityFinancialAccount::retrieve($searchParams, $result); return $result['financial_account_id'] ?? NULL; } From bb33fc63f8e8894b2e4234675bc4dc68c3432bdc Mon Sep 17 00:00:00 2001 From: Coleman Watts Date: Wed, 23 Nov 2022 15:22:45 -0500 Subject: [PATCH 2/2] APIv3 - Don't use deprecated del() methods This follows the same pattern as the create action - to only use BAO methods if they are not @deprecated. Note that APIv4 already does this in the DAODeleteAction class. --- api/v3/utils.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/v3/utils.php b/api/v3/utils.php index 648defa7e1f2..37fb251ee534 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -1395,7 +1395,7 @@ function _civicrm_api3_basic_create_fallback($bao_name, $params) { function _civicrm_api3_basic_delete($bao_name, &$params) { civicrm_api3_verify_mandatory($params, NULL, ['id']); _civicrm_api3_check_edit_permissions($bao_name, ['id' => $params['id']]); - if (method_exists($bao_name, 'del')) { + if (method_exists($bao_name, 'del') && !\Civi\Api4\Utils\ReflectionUtils::isMethodDeprecated($bao_name, 'del')) { $args = [&$params['id']]; $dao = new $bao_name(); $dao->id = $params['id'];