From 5868fe9e4d5d1977c440861ae11b96c8e1d1e944 Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Wed, 30 Nov 2022 12:31:04 +1300 Subject: [PATCH] Test cleanup, stop using legacy financial bao methods --- CRM/Financial/BAO/FinancialItem.php | 3 + tests/phpunit/CiviTest/CiviUnitTestCase.php | 53 +++++++- tests/phpunit/api/v3/ContributionTest.php | 11 +- .../phpunit/api/v3/ParticipantPaymentTest.php | 6 +- .../api/v3/TaxContributionPageTest.php | 126 ++++++++---------- 5 files changed, 115 insertions(+), 84 deletions(-) diff --git a/CRM/Financial/BAO/FinancialItem.php b/CRM/Financial/BAO/FinancialItem.php index 506214447c..4e1680d59a 100644 --- a/CRM/Financial/BAO/FinancialItem.php +++ b/CRM/Financial/BAO/FinancialItem.php @@ -182,9 +182,12 @@ class CRM_Financial_BAO_FinancialItem extends CRM_Financial_DAO_FinancialItem { * @param bool $maxId * To retrieve max id. * + * @deprecated + * * @return array */ public static function retrieveEntityFinancialTrxn($params, $maxId = FALSE) { + CRM_Core_Error::deprecatedFunctionWarning('api'); $financialItem = new CRM_Financial_DAO_EntityFinancialTrxn(); $financialItem->copyValues($params); // retrieve last entry from civicrm_entity_financial_trxn diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index f3a0cd29c2..0267d64048 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2827,11 +2827,11 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { ]); $this->assertEquals($contribution['total_amount'] - $contribution['fee_amount'], $contribution['net_amount']); if ($context === 'pending') { - $trxn = CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams); + $trxn = $this->retrieveEntityFinancialTrxn($entityParams); $this->assertNull($trxn, 'No Trxn to be created until IPN callback'); return; } - $trxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams)); + $trxn = current($this->retrieveEntityFinancialTrxn($entityParams)); $trxnParams = [ 'id' => $trxn['financial_trxn_id'], ]; @@ -2865,7 +2865,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { 'financial_trxn_id' => $trxn['financial_trxn_id'], 'entity_table' => 'civicrm_financial_item', ]; - $entityTrxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams)); + $entityTrxn = current($this->retrieveEntityFinancialTrxn($entityParams)); $fitemParams = [ 'id' => $entityTrxn['entity_id'], ]; @@ -2887,7 +2887,7 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { 'entity_id' => $params['id'], 'entity_table' => 'civicrm_contribution', ]; - $maxTrxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($maxParams, TRUE)); + $maxTrxn = current($this->retrieveEntityFinancialTrxn($maxParams, TRUE)); $trxnParams = [ 'id' => $maxTrxn['financial_trxn_id'], ]; @@ -3945,4 +3945,49 @@ WHERE table_schema = '$dataObject->_database'"); return $data; } + /** + * Retrieve entity financial trxn details. + * + * @deprecated - only called by tests - to be replaced with + * $trxn = (array) EntityFinancialTrxn::get() + * ->addWhere('id', '=', $contributionID) + * ->addWhere('entity_table', '=', 'civicrm_contribution') + * ->addSelect('*')->execute(); + * + * @param array $params + * an assoc array of name/value pairs. + * @param bool $maxId + * To retrieve max id. + * + * @deprecated - this was used so much in tests I copied it over - but + * it should go.... the tests just need to use the api. + * + * @return array + */ + public function retrieveEntityFinancialTrxn($params, $maxId = FALSE) { + $financialItem = new CRM_Financial_DAO_EntityFinancialTrxn(); + $financialItem->copyValues($params); + // retrieve last entry from civicrm_entity_financial_trxn + if ($maxId) { + $financialItem->orderBy('id DESC'); + $financialItem->limit(1); + } + $financialItem->find(); + while ($financialItem->fetch()) { + $financialItems[$financialItem->id] = [ + 'id' => $financialItem->id, + 'entity_table' => $financialItem->entity_table, + 'entity_id' => $financialItem->entity_id, + 'financial_trxn_id' => $financialItem->financial_trxn_id, + 'amount' => $financialItem->amount, + ]; + } + if (!empty($financialItems)) { + return $financialItems; + } + else { + return NULL; + } + } + } diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 4e454b39e7..a47aca1ec8 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -4058,12 +4058,12 @@ class api_v3_ContributionTest extends CiviUnitTestCase { 'entity_id' => $contId, 'entity_table' => 'civicrm_contribution', ]; - $trxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($params, TRUE)); + $trxn = current($this->retrieveEntityFinancialTrxn($params, TRUE)); $entityParams = [ 'financial_trxn_id' => $trxn['financial_trxn_id'], 'entity_table' => 'civicrm_financial_item', ]; - $entityTrxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams)); + $entityTrxn = current($this->retrieveEntityFinancialTrxn($entityParams)); $params = [ 'id' => $entityTrxn['entity_id'], ]; @@ -4219,7 +4219,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { 'entity_table' => 'civicrm_contribution', 'amount' => -100, ]; - $trxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams)); + $trxn = current($this->retrieveEntityFinancialTrxn($entityParams)); $trxnParams1 = [ 'id' => $trxn['financial_trxn_id'], ]; @@ -4770,10 +4770,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { 'entity_id' => $contributionID, 'entity_table' => 'civicrm_contribution', ]; - // @todo the following function has naming errors & has a weird signature & appears to - // only be called from test classes. Move into test suite & maybe just use api - // from this function. - return array_merge(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($trxnParams)); + return array_merge($this->retrieveEntityFinancialTrxn($trxnParams)); } /** diff --git a/tests/phpunit/api/v3/ParticipantPaymentTest.php b/tests/phpunit/api/v3/ParticipantPaymentTest.php index 56c09e4f55..33b2ae888b 100644 --- a/tests/phpunit/api/v3/ParticipantPaymentTest.php +++ b/tests/phpunit/api/v3/ParticipantPaymentTest.php @@ -253,7 +253,7 @@ class api_v3_ParticipantPaymentTest extends CiviUnitTestCase { 'entity_id' => $params['id'], 'entity_table' => 'civicrm_contribution', ]; - $trxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams)); + $trxn = current($this->retrieveEntityFinancialTrxn($entityParams)); $trxnParams = [ 'id' => $trxn['financial_trxn_id'], ]; @@ -289,11 +289,11 @@ class api_v3_ParticipantPaymentTest extends CiviUnitTestCase { 'financial_trxn_id' => $trxn['financial_trxn_id'], 'entity_table' => 'civicrm_financial_item', ]; - $entityTrxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams)); + $entityTrxn = current($this->retrieveEntityFinancialTrxn($entityParams)); $fitemParams = [ 'id' => $entityTrxn['entity_id'], ]; - if ($context == 'offline' || $context == 'online') { + if ($context === 'offline' || $context === 'online') { $compareParams = [ 'amount' => 100, 'status_id' => 1, diff --git a/tests/phpunit/api/v3/TaxContributionPageTest.php b/tests/phpunit/api/v3/TaxContributionPageTest.php index 11ecffabc7..d350adf4c6 100644 --- a/tests/phpunit/api/v3/TaxContributionPageTest.php +++ b/tests/phpunit/api/v3/TaxContributionPageTest.php @@ -9,7 +9,11 @@ +--------------------------------------------------------------------+ */ +use Civi\Api4\EntityFinancialAccount; use Civi\Api4\EntityFinancialTrxn; +use Civi\Api4\FinancialAccount; +use Civi\Api4\FinancialType; +use Civi\Api4\Generic\Result; /** * Class api_v3_TaxContributionPageTest @@ -19,16 +23,14 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { protected $params; protected $financialTypeID; protected $financialAccountId; - protected $_entity = 'contribution_page'; protected $_priceSetParams = []; protected $_paymentProcessorType; protected $payParams = []; protected $settingValue = []; protected $setInvoiceSettings; - protected $_individualId; - protected $financialAccHalftax; - protected $financialtypeHalftax; - protected $financialRelationHalftax; + protected $financialAccountHalfTax; + protected $financialTypeHalfTax; + protected $financialRelationHalfTax; protected $halfFinancialAccId; protected $halfFinancialTypeId; @@ -39,8 +41,8 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { */ public function setUp(): void { parent::setUp(); - $this->_individualId = $this->individualCreate(); - $this->_orgId = $this->organizationCreate(NULL); + $this->ids['Contact']['individual'] = $this->individualCreate(); + $this->ids['Contact']['organization'] = $this->organizationCreate(NULL); $this->ids['PaymentProcessor'] = $this->paymentProcessorCreate(); $this->params = [ @@ -67,19 +69,18 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { 'is_quick_config' => 0, 'is_reserved' => 0, ]; + // Financial Account with 20% tax rate - $financialAccountSetparams = [ + $financialAccount = $this->callAPISuccess('financial_account', 'create', [ 'name' => 'vat full tax rate account', - 'contact_id' => $this->_orgId, + 'contact_id' => $this->ids['Contact']['organization'], 'financial_account_type_id' => 2, 'is_tax' => 1, 'tax_rate' => 20.00, 'is_reserved' => 0, 'is_active' => 1, 'is_default' => 0, - ]; - - $financialAccount = $this->callAPISuccess('financial_account', 'create', $financialAccountSetparams); + ]); $this->financialAccountId = $financialAccount['id']; // Financial type having 'Sales Tax Account is' with liability financial account @@ -94,12 +95,12 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { 'account_relationship' => 10, 'financial_account_id' => $this->financialAccountId, ]; - CRM_Financial_BAO_EntityFinancialAccount::add($financialRelationParams); + EntityFinancialAccount::create()->setValues($financialRelationParams)->execute(); // Financial type with 5% tax rate $financialAccountHalfTax = [ 'name' => 'vat half tax_rate account', - 'contact_id' => $this->_orgId, + 'contact_id' => $this->ids['Contact']['organization'], 'financial_account_type_id' => 2, 'is_tax' => 1, 'tax_rate' => 5.00, @@ -107,24 +108,21 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { 'is_active' => 1, 'is_default' => 0, ]; - $halfFinancialAccount = CRM_Financial_BAO_FinancialAccount::add($financialAccountHalfTax); - $this->halfFinancialAccId = $halfFinancialAccount->id; + $this->halfFinancialAccId = FinancialAccount::create()->setValues($financialAccountHalfTax)->execute()->first()['id']; $halfFinancialTypeHalfTax = [ 'name' => 'grass_variety_2', 'is_reserved' => 0, 'is_active' => 1, ]; - $halfFinancialType = CRM_Financial_BAO_FinancialType::add($halfFinancialTypeHalfTax); - $this->halfFinancialTypeId = $halfFinancialType->id; - $financialRelationHalftax = [ + $this->halfFinancialTypeId = FinancialType::create()->setValues($halfFinancialTypeHalfTax)->execute()->first()['id']; + + EntityFinancialAccount::create()->setValues([ 'entity_table' => 'civicrm_financial_type', 'entity_id' => $this->halfFinancialTypeId, 'account_relationship' => 10, 'financial_account_id' => $this->halfFinancialAccId, - ]; - - CRM_Financial_BAO_EntityFinancialAccount::add($financialRelationHalftax); + ])->execute(); // Enable component contribute setting $this->enableTaxAndInvoicing(); @@ -132,8 +130,6 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { /** * Cleanup after function. - * - * @throws \CRM_Core_Exception */ public function tearDown(): void { $this->quickCleanUpFinancialEntities(); @@ -141,15 +137,15 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { } /** - * @throws \CRM_Core_Exception + * Set up a page for test use. */ public function setUpContributionPage(): void { - $contributionPageResult = $this->callAPISuccess($this->_entity, 'create', $this->params); + $contributionPageResult = $this->callAPISuccess('ContributionPage', 'create', $this->params); if (empty($this->ids['price_set'])) { $priceSet = $this->callAPISuccess('price_set', 'create', $this->_priceSetParams); $this->ids['price_set'][] = $priceSet['id']; } - $priceSetID = $this->_price = reset($this->ids['price_set']); + $priceSetID = reset($this->ids['price_set']); CRM_Price_BAO_PriceSet::addTo('civicrm_contribution_page', $contributionPageResult['id'], $priceSetID); if (empty($this->ids['price_field'])) { @@ -194,7 +190,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { $this->setCurrencySeparators($thousandSeparator); $this->setUpContributionPage(); $params = [ - 'contact_id' => $this->_individualId, + 'contact_id' => $this->ids['Contact']['individual'], 'receive_date' => '20120511', 'total_amount' => $this->formatMoneyInput(100.00), 'financial_type_id' => $this->financialTypeID, @@ -208,7 +204,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { ]; $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; - $this->assertEquals($this->_individualId, $contribution['contact_id']); + $this->assertEquals($this->ids['Contact']['individual'], $contribution['contact_id']); $this->assertEquals(120.00, $contribution['total_amount']); $this->assertEquals($this->financialTypeID, $contribution['financial_type_id']); $this->assertEquals(12345, $contribution['trxn_id']); @@ -226,14 +222,12 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { * punctuation used to refer to thousands. * * @dataProvider getThousandSeparators - * - * @throws \CRM_Core_Exception */ public function testCreateContributionChainedLineItems(string $thousandSeparator): void { $this->setCurrencySeparators($thousandSeparator); $this->setUpContributionPage(); $params = [ - 'contact_id' => $this->_individualId, + 'contact_id' => $this->ids['Contact']['individual'], 'receive_date' => '20120511', 'total_amount' => 400.00, 'financial_type_id' => $this->financialTypeID, @@ -277,7 +271,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { public function testCreateContributionPayLaterOnline(): void { $this->setUpContributionPage(); $params = [ - 'contact_id' => $this->_individualId, + 'contact_id' => $this->ids['Contact']['individual'], 'receive_date' => '20120511', 'total_amount' => 100.00, 'financial_type_id' => $this->financialTypeID, @@ -290,7 +284,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { 'sequential' => 1, ]; $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; - $this->assertEquals($contribution['contact_id'], $this->_individualId); + $this->assertEquals($contribution['contact_id'], $this->ids['Contact']['individual']); $this->assertEquals(120.00, $contribution['total_amount']); $this->assertEquals($this->financialTypeID, $contribution['financial_type_id']); $this->assertEquals(12345, $contribution['trxn_id']); @@ -316,7 +310,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { $this->setCurrencySeparators($thousandSeparator); $this->setUpContributionPage(); $params = [ - 'contact_id' => $this->_individualId, + 'contact_id' => $this->ids['Contact']['individual'], 'receive_date' => '20120511', 'total_amount' => $this->formatMoneyInput(100.00), 'financial_type_id' => $this->financialTypeID, @@ -329,7 +323,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { ]; $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; - $this->assertEquals($this->_individualId, $contribution['contact_id']); + $this->assertEquals($this->ids['Contact']['individual'], $contribution['contact_id']); $this->assertEquals(120.00, $contribution['total_amount']); $this->assertEquals($this->financialTypeID, $contribution['financial_type_id']); $this->assertEquals(12345, $contribution['trxn_id']); @@ -348,13 +342,11 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { * * Function tests that line items, financial records are updated when * contribution amount is changed - * - * @throws \CRM_Core_Exception */ public function testCreateUpdateContributionChangeTotal(): void { $this->setUpContributionPage(); $contributionParams = [ - 'contact_id' => $this->_individualId, + 'contact_id' => $this->ids['Contact']['individual'], 'receive_date' => '20120511', 'total_amount' => 100.00, 'financial_type_id' => $this->financialTypeID, @@ -369,7 +361,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { 'return' => 'line_total', ]); $this->assertEquals('100.00', $lineItems); - $trxnAmount = $this->_getFinancialTrxnAmount($contribution['id']); + $trxnAmount = $this->getFinancialTrxnAmount($contribution['id']); $this->assertEquals('120.00', $trxnAmount); $newParams = [ 'id' => $contribution['id'], @@ -387,22 +379,22 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { ]); $this->assertEquals('300.00', $lineItems); - $this->assertEquals('300.00', $this->_getFinancialTrxnAmount($contribution['id'])); + $this->assertEquals('300.00', $this->getFinancialTrxnAmount($contribution['id'])); $this->assertEquals('320.00', $this->_getFinancialItemAmount($contribution['id'])); } /** - * @param int $contId + * @param int $contributionID * * @return null|string */ - public function _getFinancialTrxnAmount(int $contId): ?string { + public function getFinancialTrxnAmount(int $contributionID): ?string { $query = "SELECT SUM( ft.total_amount ) AS total FROM civicrm_financial_trxn AS ft LEFT JOIN civicrm_entity_financial_trxn AS ceft ON ft.id = ceft.financial_trxn_id WHERE ceft.entity_table = 'civicrm_contribution' - AND ceft.entity_id = {$contId}"; + AND ceft.entity_id = $contributionID"; return CRM_Core_DAO::singleValueQuery($query); } @@ -417,7 +409,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { SUM(amount) FROM civicrm_financial_item WHERE entity_table = 'civicrm_line_item' - AND entity_id = {$lineItem}"; + AND entity_id = $lineItem"; return CRM_Core_DAO::singleValueQuery($query); } @@ -430,37 +422,37 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { public function _checkFinancialRecords($params, $context): void { $contributionID = $params['id']; $trxn = $this->getFinancialTransactionsForContribution($contributionID); - $trxnParams = [ 'id' => $trxn->first()['financial_trxn_id'], ]; if ($context !== 'online' && $context !== 'payLater') { - $compareParams = [ + $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $trxnParams, [ 'to_financial_account_id' => 6, 'total_amount' => 120, 'status_id' => 1, - ]; + ]); } if ($context === 'online') { - $compareParams = [ + $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $trxnParams, [ 'to_financial_account_id' => 12, 'total_amount' => 120, 'status_id' => 1, - ]; + ]); } elseif ($context === 'payLater') { - $compareParams = [ + $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $trxnParams, [ 'to_financial_account_id' => 7, 'total_amount' => 120, 'status_id' => 2, - ]; + ]); } - $this->assertDBCompareValues('CRM_Financial_DAO_FinancialTrxn', $trxnParams, $compareParams); + $entityParams = [ 'financial_trxn_id' => $trxn->first()['financial_trxn_id'], 'entity_table' => 'civicrm_financial_item', ]; - $entityTrxn = current(CRM_Financial_BAO_FinancialItem::retrieveEntityFinancialTrxn($entityParams)); + $entityTrxn = current($this->retrieveEntityFinancialTrxn($entityParams)); + $compareParams = [ 'amount' => 100, 'status_id' => 1, @@ -481,20 +473,16 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { /** * @param int $financialTypeId * - * @return int + * @return int|null + * + * @throws \CRM_Core_Exception */ public function _getFinancialAccountId(int $financialTypeId): ?int { - $accountRel = key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE 'Income Account is' ")); - - $searchParams = [ - 'entity_table' => 'civicrm_financial_type', - 'entity_id' => $financialTypeId, - 'account_relationship' => $accountRel, - ]; - - $result = []; - CRM_Financial_BAO_EntityFinancialAccount::retrieve($searchParams, $result); - return $result['financial_account_id'] ?? NULL; + return EntityFinancialAccount::get() + ->addWhere('entity_table', '=', 'civicrm_financial_type') + ->addWhere('entity_id', '=', $financialTypeId) + ->addWhere('account_relationship', '=', key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE 'Income Account is' "))) + ->addSelect('financial_account_id')->execute()->first()['financial_account_id']; } /** @@ -503,12 +491,10 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { * (It is unclear why this is in this class - it seems like maybe it doesn't * test anything not on the contribution test class & might be copy and * paste....). - * - * @throws \CRM_Core_Exception */ public function testDeleteContribution(): void { $contributionID = $this->contributionCreate([ - 'contact_id' => $this->_individualId, + 'contact_id' => $this->ids['Contact']['individual'], 'trxn_id' => 12389, 'financial_type_id' => $this->financialTypeID, 'invoice_id' => 'abc', @@ -523,7 +509,7 @@ class api_v3_TaxContributionPageTest extends CiviUnitTestCase { * @throws \CRM_Core_Exception * @throws \Civi\API\Exception\UnauthorizedException */ - protected function getFinancialTransactionsForContribution($contributionID): \Civi\Api4\Generic\Result { + protected function getFinancialTransactionsForContribution($contributionID): Result { return EntityFinancialTrxn::get() ->addWhere('id', '=', $contributionID) ->addWhere('entity_table', '=', 'civicrm_contribution') -- 2.25.1