From 3ed4be1fb014953be8893b0a3bab0f55282fbecc Mon Sep 17 00:00:00 2001 From: Eileen McNaughton Date: Thu, 8 Jun 2023 08:05:50 +1200 Subject: [PATCH] Cleanup on ContributionTest Remove calls to deprecated functions, typos, undeclared properties, use Event functions --- .../CRM/Contribute/BAO/ContributionTest.php | 406 ++++++++---------- .../CRM/Event/BAO/ChangeFeeSelectionTest.php | 4 +- .../phpunit/CRM/Member/BAO/MembershipTest.php | 2 +- tests/phpunit/CRM/Price/Form/FieldTest.php | 2 +- tests/phpunit/CiviTest/CiviUnitTestCase.php | 6 +- tests/phpunit/api/v3/MembershipTypeTest.php | 2 +- 6 files changed, 194 insertions(+), 228 deletions(-) diff --git a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php index d6c8eb28ea62..e3f25529c2ec 100644 --- a/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php +++ b/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php @@ -10,7 +10,10 @@ */ use Civi\Api4\Activity; use Civi\Api4\Contribution; +use Civi\Api4\CustomField; +use Civi\Api4\FinancialTrxn; use Civi\Api4\PledgePayment; +use Civi\Api4\Product; /** * Class CRM_Contribute_BAO_ContributionTest @@ -21,13 +24,6 @@ class CRM_Contribute_BAO_ContributionTest extends CiviUnitTestCase { use CRMTraits_Financial_FinancialACLTrait; use CRMTraits_Financial_PriceSetTrait; - /** - * Should financials be checked after the test but before tear down. - * - * @var bool - */ - protected $isValidateFinancialsOnPostAssert = TRUE; - /** * Clean up after tests. */ @@ -39,14 +35,12 @@ public function tearDown(): void { /** * Test create method (create and update modes). - * - * @throws \CRM_Core_Exception */ - public function testCreate() { - $contactId = $this->individualCreate(); + public function testCreate(): void { + $contactID = $this->individualCreate(); $params = [ - 'contact_id' => $contactId, + 'contact_id' => $contactID, 'currency' => 'USD', 'financial_type_id' => 1, 'contribution_status_id' => 1, @@ -58,8 +52,8 @@ public function testCreate() { 'total_amount' => 200.00, 'fee_amount' => 5, 'net_amount' => 195, - 'trxn_id' => '22ereerwww444444', - 'invoice_id' => '86ed39c9e9ee6ef6031621ce0eafe7eb81', + 'trxn_id' => '22444444', + 'invoice_id' => '86ed39c9e9ee6ef6031621ce07eb81', 'thankyou_date' => '20080522', 'sequential' => TRUE, ]; @@ -67,41 +61,43 @@ public function testCreate() { $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; $this->assertEquals($params['trxn_id'], $contribution['trxn_id'], 'Check for transaction id creation.'); - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); + $this->assertEquals($contactID, $contribution['contact_id'], 'Check for contact id creation.'); - //update contribution amount + // Update contribution amount. $params['id'] = $contribution['id']; $params['fee_amount'] = 10; $params['net_amount'] = 190; $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; - $this->assertEquals($params['trxn_id'], $contribution['trxn_id'], 'Check for transcation id .'); - $this->assertEquals($params['net_amount'], $contribution['net_amount'], 'Check for Amount updation.'); + $this->assertEquals($params['trxn_id'], $contribution['trxn_id'], 'Check for transaction id .'); + $this->assertEquals($params['net_amount'], $contribution['net_amount'], 'Check for Amount update.'); } /** * Create() method with custom data. + * + * @throws \CRM_Core_Exception + * @throws \Exception */ - public function testCreateWithCustomData() { - $contactId = $this->individualCreate(); + public function testCreateWithCustomData(): void { + $this->individualCreate(); //create custom data - $customGroup = $this->customGroupCreate(['extends' => 'Contribution']); + $customGroup = $this->customGroupCreate(['extends' => 'Contribution', 'name', 'group']); $customGroupID = $customGroup['id']; $customGroup = $customGroup['values'][$customGroupID]; - - $fields = [ - 'label' => 'testFld', + $customFieldID = CustomField::create()->setValues([ + 'label' => 'test Field', 'data_type' => 'String', 'html_type' => 'Text', 'is_active' => 1, + 'column_name' => 'field_column', 'custom_group_id' => $customGroupID, - ]; - $customField = CRM_Core_BAO_CustomField::create($fields); + ])->execute()->first()['id']; $params = [ - 'contact_id' => $contactId, + 'contact_id' => $this->ids['Contact']['individual_0'], 'currency' => 'USD', 'financial_type_id' => 1, 'contribution_status_id' => 1, @@ -114,21 +110,21 @@ public function testCreateWithCustomData() { 'total_amount' => 200.00, 'fee_amount' => 5, 'net_amount' => 195, - 'trxn_id' => '22ereerwww322323', - 'invoice_id' => '22ed39c9e9ee6ef6031621ce0eafe6da70', + 'trxn_id' => '22w322323', + 'invoice_id' => '22ed39c9e9ee6ef6031621ce0a6da70', 'thankyou_date' => '20080522', 'skipCleanMoney' => TRUE, ]; $params['custom'] = [ - $customField->id => [ + $customFieldID => [ -1 => [ 'value' => 'Test custom value', 'type' => 'String', - 'custom_field_id' => $customField->id, + 'custom_field_id' => $customFieldID, 'custom_group_id' => $customGroupID, 'table_name' => $customGroup['table_name'], - 'column_name' => $customField->column_name, + 'column_name' => 'field_column', 'file_id' => NULL, ], ], @@ -139,22 +135,22 @@ public function testCreateWithCustomData() { // Check that the custom field value is saved $customValueParams = [ 'entityID' => $contribution->id, - 'custom_' . $customField->id => 1, + 'custom_' . $customFieldID => 1, ]; $values = CRM_Core_BAO_CustomValueTable::getValues($customValueParams); - $this->assertEquals('Test custom value', $values['custom_' . $customField->id], 'Check the custom field value'); + $this->assertEquals('Test custom value', $values['custom_' . $customFieldID], 'Check the custom field value'); - $this->assertEquals($params['trxn_id'], $contribution->trxn_id, 'Check for transcation id creation.'); - $this->assertEquals($contactId, $contribution->contact_id, 'Check for contact id for Conribution.'); + $this->assertEquals($params['trxn_id'], $contribution->trxn_id, 'Check for transaction id creation.'); + $this->assertEquals($this->ids['Contact']['individual_0'], $contribution->contact_id); } /** * CRM-21026 Test ContributionCount after contribution created with disabled FT */ - public function testContributionCountDisabledFinancialType() { + public function testContributionCountDisabledFinancialType(): void { $contactId = $this->individualCreate(); $financialType = [ - 'name' => 'grassvariety1' . substr(sha1(rand()), 0, 7), + 'name' => 'grass_variety_1', 'is_reserved' => 0, 'is_active' => 0, ]; @@ -173,8 +169,8 @@ public function testContributionCountDisabledFinancialType() { 'total_amount' => 200.00, 'fee_amount' => 5, 'net_amount' => 195, - 'trxn_id' => '22ereerwww322323', - 'invoice_id' => '22ed39c9e9ee6ef6031621ce0eafe6da70', + 'trxn_id' => '22322323', + 'invoice_id' => '22ed39c9e9ee6ef6031621ce06da70', 'thankyou_date' => '20080522', ]; $this->callAPISuccess('Contribution', 'create', $params); @@ -219,7 +215,10 @@ public function testDeleteContribution(int $version): void { } /** - * Test that financial type data is not added to the annual query if acls not enabled. + * Test that financial type data is not added to the annual query if acls not + * enabled. + * + * @throws \Civi\Core\Exception\DBQueryException */ public function testAnnualQueryWithFinancialACLsEnabled(): void { $this->enableFinancialACLs(); @@ -235,7 +234,10 @@ public function testAnnualQueryWithFinancialACLsEnabled(): void { } /** - * Test the annual query returns a correct result when multiple line items are present. + * Test the annual query returns a correct result when multiple line items + * are present. + * + * @throws \Civi\Core\Exception\DBQueryException */ public function testAnnualWithMultipleLineItems(): void { $contactID = $this->createLoggedInUserWithFinancialACL(); @@ -253,23 +255,28 @@ public function testAnnualWithMultipleLineItems(): void { } /** - * Test that financial type data is not added to the annual query if acls not enabled. + * Test that financial type data is not added to the annual query if acls not + * enabled. + * + * @throws \Civi\Core\Exception\DBQueryException */ public function testAnnualQueryWithFinancialACLsDisabled(): void { $sql = CRM_Contribute_BAO_Contribution::getAnnualQuery([1, 2, 3]); $this->assertStringContainsString('SUM(total_amount) as amount,', $sql); $this->assertStringContainsString('b.contact_id IN (1,2,3)', $sql); $this->assertStringNotContainsString('b.financial_type_id', $sql); - //$this->assertNotContains('line_item', $sql); // Run it to make sure it's not bad sql. CRM_Core_DAO::executeQuery($sql); } /** - * Test that financial type data is not added to the annual query if acls not enabled. + * Test that financial type data is not added to the annual query if acls not + * enabled. + * + * @throws \Civi\Core\Exception\DBQueryException */ public function testAnnualQueryWithFinancialHook(): void { - $this->hookClass->setHook('civicrm_selectWhereClause', [$this, 'aclIdNoZero']); + $this->hookClass->setHook('civicrm_selectWhereClause', [$this, 'aclNoZero']); $sql = CRM_Contribute_BAO_Contribution::getAnnualQuery([1, 2, 3]); $this->assertStringContainsString('SUM(total_amount) as amount,', $sql); $this->assertStringContainsString('b.contact_id IN (1,2,3)', $sql); @@ -282,10 +289,10 @@ public function testAnnualQueryWithFinancialHook(): void { * Add ACL denying values LIKE '0'. * * @param string $entity - * @param string $clauses + * @param array $clauses */ - public function aclIdNoZero($entity, &$clauses) { - if ($entity != 'Contribution') { + public function aclNoZero(string $entity, array &$clauses): void { + if ($entity !== 'Contribution') { return; } $clauses['id'] = 'NOT IN (0)'; @@ -296,21 +303,14 @@ public function aclIdNoZero($entity, &$clauses) { * Update multiple contributions * sortName(); */ - public function testsortName() { - $params = [ + public function testSortName(): void { + $this->individualCreate([ 'first_name' => 'Shane', - 'last_name' => 'Whatson', - 'contact_type' => 'Individual', - ]; - - $contact = CRM_Contact_BAO_Contact::add($params); - - //Now check $contact is object of contact DAO.. - $this->assertInstanceOf('CRM_Contact_DAO_Contact', $contact, 'Check for created object'); + 'last_name' => 'Watson', + ]); - $contactId = $contact->id; $param = [ - 'contact_id' => $contactId, + 'contact_id' => $this->ids['Contact']['individual_0'], 'currency' => 'USD', 'financial_type_id' => 1, 'contribution_status_id' => 1, @@ -323,29 +323,25 @@ public function testsortName() { 'total_amount' => 300.00, 'fee_amount' => 5, 'net_amount' => 295, - 'trxn_id' => '22ereerwww323', - 'invoice_id' => '22ed39c9e9ee621ce0eafe6da70', + 'trxn_id' => '22323', + 'invoice_id' => '22ed39c9e9ee621ce06da70', 'thankyou_date' => '20080522', 'sequential' => TRUE, ]; - $contribution = $this->callAPISuccess('Contribution', 'create', $param)['values'][0]; - - $this->assertEquals($param['trxn_id'], $contribution['trxn_id'], 'Check for transcation id creation.'); - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); - - //display sort name during Update multiple contributions - $sortName = CRM_Contribute_BAO_Contribution::sortName($contribution['id']); - - $this->assertEquals('Whatson, Shane', $sortName, 'Check for sort name.'); + $contributionID = $this->callAPISuccess('Contribution', 'create', $param)['id']; + // Display sort name during Update multiple contributions. + $this->assertEquals('Watson, Shane', CRM_Contribute_BAO_Contribution::sortName($contributionID)); } /** * Add premium during online Contribution. * * AddPremium(); + * + * @throws \CRM_Core_Exception */ - public function testAddPremium() { + public function testAddPremium(): void { $contactId = $this->individualCreate(); $params = [ @@ -376,16 +372,13 @@ public function testAddPremium() { 'total_amount' => 300.00, 'fee_amount' => 5, 'net_amount' => 295, - 'trxn_id' => '33erdfrwvw434', - 'invoice_id' => '98ed34f7u9hh672ce0eafe8fb92', + 'trxn_id' => '33er434', + 'invoice_id' => '98ed34f7u9hh672ce0e8fb92', 'thankyou_date' => '20080522', 'sequential' => TRUE, ]; $contribution = $this->callAPISuccess('Contribution', 'create', $contributionParams)['values'][0]; - $this->assertEquals($contributionParams['trxn_id'], $contribution['trxn_id'], 'Check for transcation id creation.'); - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); - //parameter for adding premium to contribution $data = [ 'product_id' => $premium->id, @@ -396,8 +389,8 @@ public function testAddPremium() { $contributionProduct = CRM_Contribute_BAO_Contribution::addPremium($data); $this->assertEquals($contributionProduct->product_id, $premium->id, 'Check for Product id .'); - //Delete Product - CRM_Contribute_BAO_Product::deleteRecord(['id' => $premium->id]); + // Delete Product. + Product::delete()->addWhere('id', '=', $premium->id)->execute(); $this->assertDBNull('CRM_Contribute_DAO_Product', $premium->name, 'id', 'name', 'Database check for deleted Product.' ); @@ -405,14 +398,10 @@ public function testAddPremium() { /** * Check duplicate contribution id. - * during the contribution import - * checkDuplicateIds(); */ - public function testcheckDuplicateIds() { - $contactId = $this->individualCreate(); - + public function testCheckDuplicateIDs(): void { $param = [ - 'contact_id' => $contactId, + 'contact_id' => $this->individualCreate(), 'currency' => 'USD', 'financial_type_id' => 1, 'contribution_status_id' => 1, @@ -425,33 +414,29 @@ public function testcheckDuplicateIds() { 'total_amount' => 300.00, 'fee_amount' => 5, 'net_amount' => 295, - 'trxn_id' => '76ereeswww835', - 'invoice_id' => '93ed39a9e9hd621bs0eafe3da82', + 'trxn_id' => '76er835', + 'invoice_id' => '93ed39a9e9hd621bs0f3da82', 'thankyou_date' => '20080522', 'sequential' => TRUE, ]; $contribution = $this->callAPISuccess('Contribution', 'create', $param)['values'][0]; - $this->assertEquals($param['trxn_id'], $contribution['trxn_id'], 'Check for transcation id creation.'); - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); $data = [ 'id' => $contribution['id'], 'trxn_id' => $contribution['trxn_id'], 'invoice_id' => $contribution['invoice_id'], ]; $contributionID = CRM_Contribute_BAO_Contribution::checkDuplicateIds($data); - $this->assertEquals($contributionID, $contribution['id'], 'Check for duplicate transcation id .'); + $this->assertEquals($contributionID, $contribution['id'], 'Check for duplicate transaction id .'); } /** * Create() method (create and update modes). */ - public function testIsPaymentFlag() { - $contactId = $this->individualCreate(); - - $params = [ - 'contact_id' => $contactId, + public function testIsPaymentFlag(): void { + $contributionID = $this->callAPISuccess('Contribution', 'create', [ + 'contact_id' => $this->individualCreate(), 'currency' => 'USD', 'financial_type_id' => 1, 'contribution_status_id' => 1, @@ -463,46 +448,26 @@ public function testIsPaymentFlag() { 'total_amount' => 200.00, 'fee_amount' => 5, 'net_amount' => 195, - 'trxn_id' => '22ereerwww4444xx', - 'invoice_id' => '86ed39c9e9ee6ef6541621ce0eafe7eb81', + 'trxn_id' => '12345', + 'invoice_id' => '86ed39c9e9ee6ef6541621ce0fe7eb81', 'thankyou_date' => '20080522', 'sequential' => TRUE, - ]; - $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; - - $this->assertEquals($params['trxn_id'], $contribution['trxn_id'], 'Check for transcation id creation.'); - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); + ])['id']; - $trxnArray = [ - 'trxn_id' => $params['trxn_id'], - 'is_payment' => 1, - ]; - $defaults = []; - $financialTrxn = CRM_Core_BAO_FinancialTrxn::retrieve($trxnArray, $defaults); - $this->assertEquals(1, $financialTrxn->N, 'Mismatch count for is payment flag.'); - //update contribution amount - $params['id'] = $contribution['id']; + $this->assertFinancialTransactionCount(1, 12345, 1); + // Update contribution amount. + $params['id'] = $contributionID; $params['total_amount'] = 150; - $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; + $this->callAPISuccess('Contribution', 'create', $params); - $this->assertEquals($params['trxn_id'], $contribution['trxn_id'], 'Check for transcation id .'); - $this->assertEquals($params['total_amount'], $contribution['total_amount'], 'Check for Amount updation.'); - $trxnArray = [ - 'trxn_id' => $params['trxn_id'], - 'is_payment' => 1, - ]; - $defaults = []; - $financialTrxn = CRM_Core_BAO_FinancialTrxn::retrieve($trxnArray, $defaults); - $this->assertEquals(2, $financialTrxn->N, 'Mismatch count for is payment flag.'); - $trxnArray['is_payment'] = 0; - $financialTrxn = CRM_Core_BAO_FinancialTrxn::retrieve($trxnArray, $defaults); - $this->assertEquals(1, $financialTrxn->N, 'Mismatch count for is payment flag.'); + $this->assertFinancialTransactionCount(2, 12345, TRUE); + $this->assertFinancialTransactionCount(1, 12345, FALSE); } /** * Create() method (create and update modes). */ - public function testIsPaymentFlagForPending() { + public function testIsPaymentFlagForPending(): void { $contactId = $this->individualCreate(); $params = [ @@ -519,8 +484,8 @@ public function testIsPaymentFlagForPending() { 'total_amount' => 200.00, 'fee_amount' => 5, 'net_amount' => 195, - 'trxn_id' => '22ereerwww4444yy', - 'invoice_id' => '86ed39c9e9yy6ef6541621ce0eafe7eb81', + 'trxn_id' => '2345', + 'invoice_id' => '86ed39c9e9yy6ef6541621ce0e7eb81', 'thankyou_date' => '20080522', 'sequential' => TRUE, ]; @@ -530,47 +495,31 @@ public function testIsPaymentFlagForPending() { $this->assertEquals($params['trxn_id'], $contribution['trxn_id'], 'Check for transaction id creation.'); $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); - $trxnArray = [ - 'trxn_id' => $params['trxn_id'], - 'is_payment' => 0, - ]; - $defaults = []; - $financialTrxn = CRM_Core_BAO_FinancialTrxn::retrieve($trxnArray, $defaults); - $this->assertEquals(2, $financialTrxn->N, 'Mismatch count for is payment flag.'); - $trxnArray['is_payment'] = 1; - $financialTrxn = CRM_Core_BAO_FinancialTrxn::retrieve($trxnArray, $defaults); - $this->assertEquals(NULL, $financialTrxn, 'Mismatch count for is payment flag.'); - //update contribution amount + $this->assertFinancialTransactionCount(2, 2345, FALSE); + $this->assertFinancialTransactionCount(0, 2345, TRUE); + // Update contribution amount. $params['id'] = $contribution['id']; $params['contribution_status_id'] = 1; $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; - $this->assertEquals($params['trxn_id'], $contribution['trxn_id'], 'Check for transcation id .'); - $this->assertEquals($params['contribution_status_id'], $contribution['contribution_status_id'], 'Check for status updation.'); - $trxnArray = [ - 'trxn_id' => $params['trxn_id'], - 'is_payment' => 1, - ]; - $defaults = []; - $financialTrxn = CRM_Core_BAO_FinancialTrxn::retrieve($trxnArray, $defaults); - $this->assertEquals(1, $financialTrxn->N, 'Mismatch count for is payment flag.'); - $trxnArray['is_payment'] = 0; - $financialTrxn = CRM_Core_BAO_FinancialTrxn::retrieve($trxnArray, $defaults); - $this->assertEquals(2, $financialTrxn->N, 'Mismatch count for is payment flag.'); + $this->assertEquals($params['contribution_status_id'], $contribution['contribution_status_id'], 'Check for status update.'); + $this->assertFinancialTransactionCount(2, 2345, FALSE); + $this->assertFinancialTransactionCount(1, 2345, TRUE); } /** * checks db values for financial item + * + * @throws \Civi\Core\Exception\DBQueryException */ - public function checkItemValues($contribution) { - $toFinancialAccount = CRM_Contribute_PseudoConstant::getRelationalFinancialAccount(4, 'Accounts Receivable Account is'); + public function checkItemValues($contribution): void { $query = "SELECT eft1.entity_id, ft.total_amount, eft1.amount FROM civicrm_financial_trxn ft INNER JOIN civicrm_entity_financial_trxn eft ON (eft.financial_trxn_id = ft.id AND eft.entity_table = 'civicrm_contribution') INNER JOIN civicrm_entity_financial_trxn eft1 ON (eft1.financial_trxn_id = eft.financial_trxn_id AND eft1.entity_table = 'civicrm_financial_item') WHERE eft.entity_id = %1 AND ft.to_financial_account_id <> %2"; $queryParams[1] = [$contribution->id, 'Integer']; - $queryParams[2] = [$toFinancialAccount, 'Integer']; + $queryParams[2] = [CRM_Financial_BAO_FinancialAccount::getFinancialAccountForFinancialTypeByRelationship(4, 'Accounts Receivable Account is'), 'Integer']; $dao = CRM_Core_DAO::executeQuery($query, $queryParams); $amounts = [100.00, 50.00]; @@ -611,12 +560,11 @@ public function testAssignProportionalLineItems(): void { * * @throws \CRM_Core_Exception */ - public function addParticipantWithContribution() { - // creating price set, price field - $this->_contactId = $this->individualCreate(); - $event = $this->legacyEventCreatePaid([]); - $this->_eventId = $event['id']; - $priceSetID = $this->ids['PriceSet']['PaidEvent']; + public function addParticipantWithContribution(): CRM_Contribute_BAO_Contribution { + // Creating price set, price field. + $this->individualCreate(); + $this->eventCreatePaid([]); + $priceSetID = $this->getPriceSetID('PaidEvent'); $paramsField = [ 'label' => 'Price Field', 'name' => CRM_Utils_String::titleToVar('Price Field'), @@ -636,27 +584,27 @@ public function addParticipantWithContribution() { ]; $priceField = CRM_Price_BAO_PriceField::create($paramsField); $eventParams = [ - 'id' => $this->_eventId, + 'id' => $this->getEventID(), 'financial_type_id' => 4, 'is_monetary' => 1, ]; CRM_Event_BAO_Event::create($eventParams); - CRM_Price_BAO_PriceSet::addTo('civicrm_event', $this->_eventId, $priceSetID); + CRM_Price_BAO_PriceSet::addTo('civicrm_event', $this->getEventID(), $priceSetID); $priceFields = $this->callAPISuccess('PriceFieldValue', 'get', ['price_field_id' => $priceField->id]); $participantParams = [ 'financial_type_id' => 4, - 'event_id' => $this->_eventId, + 'event_id' => $this->getEventID(), 'role_id' => 1, 'status_id' => 14, 'fee_currency' => 'USD', - 'contact_id' => $this->_contactId, + 'contact_id' => $this->ids['Contact']['individual_0'], ]; $participant = CRM_Event_BAO_Participant::add($participantParams); $contributionParams = [ 'total_amount' => 300, 'currency' => 'USD', - 'contact_id' => $this->_contactId, + 'contact_id' => $this->ids['Contact']['individual_0'], 'financial_type_id' => 4, 'contribution_status_id' => 'Pending', 'participant_id' => $participant->id, @@ -694,12 +642,12 @@ public function addParticipantWithContribution() { /** * Test activity amount updates activity subject. + * + * @throws \CRM_Core_Exception */ - public function testActivityCreate() { - $contactId = $this->individualCreate(); - + public function testActivityCreate(): void { $params = [ - 'contact_id' => $contactId, + 'contact_id' => $this->individualCreate(), 'currency' => 'USD', 'financial_type_id' => 1, 'contribution_status_id' => 1, @@ -709,7 +657,7 @@ public function testActivityCreate() { 'receipt_date' => '20080522000000', 'non_deductible_amount' => 0.00, 'total_amount' => 100.00, - 'invoice_id' => '86ed39c9e9ee6ef6031621ce0eafe7eb81', + 'invoice_id' => '86ed39c9e9ee6ef6031621ce0e7eb81', 'thankyou_date' => '20160519', 'sequential' => 1, ]; @@ -717,7 +665,6 @@ public function testActivityCreate() { $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; $this->assertEquals($params['total_amount'], $contribution['total_amount'], 'Check for total amount in contribution.'); - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); $activityWhere = [ ['source_record_id', '=', $contribution['id']], ['activity_type_id:name', '=', 'Contribution'], @@ -734,7 +681,6 @@ public function testActivityCreate() { $contribution = $this->callAPISuccess('Contribution', 'create', $params)['values'][0]; $this->assertEquals($params['total_amount'], $contribution['total_amount'], 'Check for total amount in contribution.'); - $this->assertEquals($contactId, $contribution['contact_id'], 'Check for contact id creation.'); // Retrieve activity again. $activity = Activity::get()->setWhere($activityWhere)->setSelect(['source_record_id', 'subject', 'campaign_id'])->execute()->first(); @@ -760,7 +706,7 @@ public function testAllowUpdateRevenueRecognitionDate(): void { $allowUpdate = CRM_Contribute_BAO_Contribution::allowUpdateRevenueRecognitionDate($order['id']); $this->assertTrue($allowUpdate); - $event = $this->eventCreate(); + $event = $this->eventCreatePaid(); $params = [ 'contact_id' => $contactID, 'receive_date' => '2010-01-20', @@ -801,7 +747,7 @@ public function testAllowUpdateRevenueRecognitionDate(): void { 'contact_id' => $contactID, 'receive_date' => '2010-01-20', 'total_amount' => 200, - 'financial_type_id' => $this->getFinancialTypeId('Member Dues'), + 'financial_type_id' => $this->getFinancialTypeID('Member Dues'), 'contribution_status_id' => 'Pending', ]; $membershipType = $this->membershipTypeCreate(); @@ -843,10 +789,8 @@ public function testAllowUpdateRevenueRecognitionDate(): void { * Test recording of amount with comma separator. */ public function testCommaSeparatorAmount(): void { - $contactId = $this->individualCreate(); - $params = [ - 'contact_id' => $contactId, + 'contact_id' => $this->individualCreate(), 'currency' => 'USD', 'financial_type_id' => 1, 'contribution_status_id' => 'Pending', @@ -867,7 +811,7 @@ public function testCommaSeparatorAmount(): void { 'return' => ['total_amount'], ] ); - $this->assertEquals($financialTrxn['total_amount'], 8000, 'Invalid amount.'); + $this->assertEquals(8000, $financialTrxn['total_amount'], 'Invalid amount.'); } /** @@ -892,8 +836,9 @@ public function testGetSalesTaxFinancialAccounts(): void { * punctuation used to refer to thousands. * * @dataProvider getThousandSeparators + * @throws \CRM_Core_Exception */ - public function testCreateProportionalEntry($thousandSeparator) { + public function testCreateProportionalEntry(string $thousandSeparator): void { $this->setCurrencySeparators($thousandSeparator); [$contribution, $financialAccount] = $this->createContributionWithTax(); $params = [ @@ -965,13 +910,13 @@ public function testCreateProportionalEntryZeroAmount(string $thousandSeparator) /** * Test for function getLastFinancialItemIds(). */ - public function testgetLastFinancialItemIds() { - [$contribution, $financialAccount] = $this->createContributionWithTax(); + public function testGetLastFinancialItemIDs(): void { + [$contribution] = $this->createContributionWithTax(); [$ftIds, $taxItems] = CRM_Contribute_BAO_Contribution::getLastFinancialItemIds($contribution['id']); - $this->assertEquals(count($ftIds), 1, 'Invalid count.'); - $this->assertEquals(count($taxItems), 1, 'Invalid count.'); + $this->assertCount(1, $ftIds, 'Invalid count.'); + $this->assertCount(1, $taxItems, 'Invalid count.'); foreach ($taxItems as $value) { - $this->assertEquals($value['amount'], 10, 'Invalid tax amount.'); + $this->assertEquals(10, $value['amount'], 'Invalid tax amount.'); } } @@ -980,7 +925,7 @@ public function testgetLastFinancialItemIds() { * * In this test we create a pending contribution for $110 consisting of $100 contribution and $10 tax. * - * We pay $50, resulting in it being allocated as $45.45 paymnt & $4.55 tax. This is in equivalent proportions + * We pay $50, resulting in it being allocated as $45.45 payment & $4.55 tax. This is in equivalent proportions * to the original payment - ie. .0909 of the $110 is 10 & that * 50 is $4.54 (note the rounding seems wrong as it should be * saved un-rounded). */ @@ -1009,10 +954,10 @@ public function testCreateProportionalFinancialEntriesViaPaymentCreate(): void { } /** - * Test to check if amount is proportionally asigned for PI change. + * Test to check if amount is proportionally assigned for PI change. */ - public function testProportionallyAssignedForPIChange() { - [$contribution, $financialAccount] = $this->createContributionWithTax(); + public function testProportionallyAssignedForPIChange(): void { + [$contribution] = $this->createContributionWithTax(); $params = [ 'id' => $contribution['id'], 'payment_instrument_id' => 3, @@ -1024,7 +969,7 @@ public function testProportionallyAssignedForPIChange() { 'financial_trxn_id' => $lastFinancialTrxnId['financialTrxnId'], ]; $entityFinancialTrxn = $this->callAPISuccess('EntityFinancialTrxn', 'Get', $eftParams); - $this->assertEquals($entityFinancialTrxn['count'], 2, 'Invalid count.'); + $this->assertEquals(2, $entityFinancialTrxn['count'], 'Invalid count.'); $testAmount = [10, 100]; foreach ($entityFinancialTrxn['values'] as $value) { $this->assertEquals($value['amount'], array_pop($testAmount), 'Invalid amount stored in civicrm_entity_financial_trxn.'); @@ -1063,8 +1008,10 @@ public function createContributionWithTax($params = [], $isCompleted = TRUE): ar /** * Test processOnBehalfOrganization() function. + * + * @throws \CRM_Core_Exception */ - public function testProcessOnBehalfOrganization() { + public function testProcessOnBehalfOrganization(): void { $orgInfo = [ 'phone' => '11111111', 'email' => 'testorg@gmail.com', @@ -1075,16 +1022,16 @@ public function testProcessOnBehalfOrganization() { 'country' => 'United States', ]; $originalContactId = $contactID = $this->individualCreate(); - $orgId = $this->organizationCreate(['organization_name' => 'testorg1']); + $orgId = $this->organizationCreate(['organization_name' => 'test organization 1']); $orgCount = $this->callAPISuccessGetCount('Contact', [ 'contact_type' => 'Organization', - 'organization_name' => 'testorg1', + 'organization_name' => 'test organization 1', ]); - $this->assertEquals($orgCount, 1); + $this->assertEquals(1, $orgCount); $values = $params = []; $originalBehalfOrganization = $behalfOrganization = [ - 'organization_name' => 'testorg1', + 'organization_name' => 'test organization 1', 'phone' => [ 1 => [ 'phone' => $orgInfo['phone'], @@ -1125,9 +1072,9 @@ public function testProcessOnBehalfOrganization() { //Check whether new organisation is created. $result = $this->callAPISuccess('Contact', 'get', [ 'contact_type' => 'Organization', - 'organization_name' => 'testorg1', + 'organization_name' => 'test organization 1', ]); - $this->assertEquals($result['count'], 1); + $this->assertEquals(1, $result['count']); //Assert all org values are updated. foreach ($orgInfo as $key => $val) { @@ -1135,9 +1082,9 @@ public function testProcessOnBehalfOrganization() { } //Check if alert is assigned to params if more than 1 dupe exists. - $orgId = $this->organizationCreate(['organization_name' => 'testorg1', 'email' => 'testorg@gmail.com']); + $this->organizationCreate(['organization_name' => 'test organization 1', 'email' => 'testorg@gmail.com']); CRM_Contribute_Form_Contribution_Confirm::processOnBehalfOrganization($originalBehalfOrganization, $originalContactId, $values, $params, $fields); - $this->assertEquals($params['onbehalf_dupe_alert'], 1); + $this->assertEquals(1, $params['onbehalf_dupe_alert']); } /** @@ -1190,7 +1137,7 @@ public function testContributionWithDeferredRevenue(): void { 'financial_account_id.name' => 'Deferred Revenue - Event Fee', 'id' => $result['entity_id'], ]; - $result = $this->callAPISuccessGetSingle('FinancialItem', [ + $this->callAPISuccessGetSingle('FinancialItem', [ 'id' => $result['entity_id'], 'return' => ['financial_account_id.name'], ], $checkAgainst); @@ -1200,8 +1147,7 @@ public function testContributionWithDeferredRevenue(): void { * https://lab.civicrm.org/dev/financial/issues/56 * Changing financial type on a contribution records correct financial items */ - public function testChangingFinancialTypeWithoutTax() { - $ids = $values = []; + public function testChangingFinancialTypeWithoutTax(): void { $contactId = $this->individualCreate(); $params = [ 'contact_id' => $contactId, @@ -1230,14 +1176,14 @@ public function testChangingFinancialTypeWithoutTax() { ]); $this->assertEquals( - $lineItem['line_total'], 100.00, + $lineItem['line_total'], 'Invalid line amount.' ); $this->assertEquals( - $lineItem['financial_type_id.name'], 'Event Fee', + $lineItem['financial_type_id.name'], 'Invalid Financial Type stored.' ); @@ -1250,7 +1196,7 @@ public function testChangingFinancialTypeWithoutTax() { 'return' => ['financial_account_id.name', 'amount', 'description'], ]); - $this->assertEquals($financialItems['count'], 3, 'Count mismatch.'); + $this->assertEquals(3, $financialItems['count'], 'Count mismatch.'); $toCheck = [ ['Donation', 100.00], @@ -1270,8 +1216,8 @@ public function testChangingFinancialTypeWithoutTax() { 'Amount mismatch.' ); $this->assertEquals( - $values['description'], 'Contribution Amount', + $values['description'], 'Description mismatch.' ); } @@ -1283,7 +1229,7 @@ public function testChangingFinancialTypeWithoutTax() { 'entity_id' => $contributionId, 'sequential' => 1, ]); - $this->assertEquals($financialTransactions['count'], 3, 'Count mismatch.'); + $this->assertEquals(3, $financialTransactions['count'], 'Count mismatch.'); foreach ($financialTransactions['values'] as $key => $values) { $this->callAPISuccessGetCount('EntityFinancialTrxn', [ @@ -1295,9 +1241,12 @@ public function testChangingFinancialTypeWithoutTax() { } /** - * CRM-21424 Check if the receipt update is set after composing the receipt message + * CRM-21424 Check if the receipt update is set after composing the receipt + * message + * + * @throws \CRM_Core_Exception */ - public function testSendMailUpdateReceiptDate() { + public function testSendMailUpdateReceiptDate(): void { $ids = $values = []; $contactId = $this->individualCreate(); $params = [ @@ -1320,16 +1269,16 @@ public function testSendMailUpdateReceiptDate() { $this->assertDBNotNull('CRM_Contribute_BAO_Contribution', $contributionId, 'receipt_date', 'id', 'After sendMail with the permission to allow update receipt date must be set'); /* repeat the same scenario for downloading a pdf */ - $contribution = $this->callAPISuccess('contribution', 'create', $params); - $contributionId = $contribution['id']; - $this->assertDBNull('CRM_Contribute_BAO_Contribution', $contributionId, 'receipt_date', 'id', 'After creating receipt date must be null'); + $contributionID = $this->callAPISuccess('Contribution', 'create', $params)['id']; + + $this->assertDBNull('CRM_Contribute_BAO_Contribution', $contributionID, 'receipt_date', 'id', 'After creating receipt date must be null'); $input = ['receipt_update' => 0]; - /* setting the lasast parameter (returnmessagetext) to TRUE is done by the download of the pdf */ - CRM_Contribute_BAO_Contribution::sendMail($input, $ids, $contributionId, $values, TRUE); - $this->assertDBNull('CRM_Contribute_BAO_Contribution', $contributionId, 'receipt_date', 'id', 'After sendMail, with the explicit instruction not to update receipt date stays null'); + /* setting the last parameter (returnMessageText) to TRUE is done by the download of the pdf */ + CRM_Contribute_BAO_Contribution::sendMail($input, $ids, $contributionID, $values, TRUE); + $this->assertDBNull('CRM_Contribute_BAO_Contribution', $contributionID, 'receipt_date', 'id', 'After sendMail, with the explicit instruction not to update receipt date stays null'); $input = ['receipt_update' => 1]; - CRM_Contribute_BAO_Contribution::sendMail($input, $ids, $contributionId, $values, TRUE); - $this->assertDBNotNull('CRM_Contribute_BAO_Contribution', $contributionId, 'receipt_date', 'id', 'After sendMail with the permission to allow update receipt date must be set'); + CRM_Contribute_BAO_Contribution::sendMail($input, $ids, $contributionID, $values, TRUE); + $this->assertDBNotNull('CRM_Contribute_BAO_Contribution', $contributionID, 'receipt_date', 'id', 'After sendMail with the permission to allow update receipt date must be set'); } /** @@ -1426,7 +1375,6 @@ public function testContributionQuickConfigTwoLineItems(): void { 'api.Payment.create' => ['total_amount' => 150], ]; - $now = date('Ymd'); foreach ($priceFields as $priceField) { $lineItems = []; $contactId = array_search($priceField['membership_type_id'], $contactIds); @@ -1520,7 +1468,7 @@ public function testUpdateActivityContactOnContributionContactChange(): void { $this->assertEquals($activityContact['contact_id'], $contactId_1, 'Check source contact ID matches the first contact'); - $contribution = $this->callAPISuccess('Contribution', 'create', array_merge( + $this->callAPISuccess('Contribution', 'create', array_merge( $contributionParams, [ 'id' => $contribution['id'], @@ -1553,7 +1501,7 @@ public function testUpdateActivityContactOnContributionContactChange(): void { $this->assertEquals($activityContact['contact_id'], $contactId_1, 'Check target contact ID matches first contact'); - $contribution = $this->callAPISuccess('Contribution', 'create', array_merge( + $this->callAPISuccess('Contribution', 'create', array_merge( $contributionParams, [ 'id' => $contribution['id'], @@ -1572,7 +1520,7 @@ public function testUpdateActivityContactOnContributionContactChange(): void { * @throws \CRM_Core_Exception * @throws \Civi\API\Exception\UnauthorizedException */ - public function testContributionStatusUpdateActivityPropagation() { + public function testContributionStatusUpdateActivityPropagation(): void { $contactId = $this->individualCreate(); $campaignId = $this->campaignCreate(); $contribution = Contribution::create() @@ -1616,4 +1564,22 @@ public function testContributionStatusUpdateActivityPropagation() { $this->assertNull($activity['campaign_id'], 'Should have removed campaign from contribution activity'); } + /** + * @param int $expectedCount + * @param string $transactionID + * @param bool $isPayment + */ + protected function assertFinancialTransactionCount(int $expectedCount, string $transactionID, bool $isPayment): void { + try { + $this->assertCount($expectedCount, FinancialTrxn::get() + ->addWhere('trxn_id', '=', $transactionID) + ->addWhere('is_payment', '=', $isPayment) + ->execute() + , 'Financial transaction count is incorrect'); + } + catch (CRM_Core_Exception $e) { + $this->fail($e->getMessage()); + } + } + } diff --git a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php index e37ba37f5116..5c39795e54a7 100644 --- a/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php +++ b/tests/phpunit/CRM/Event/BAO/ChangeFeeSelectionTest.php @@ -120,7 +120,7 @@ protected function priceSetCreate(string $type = 'Radio', array $params = []): i 'is_active' => ['1' => 1], 'price_set_id' => $priceSetID, 'is_enter_qty' => 1, - 'financial_type_id' => $this->getFinancialTypeId('Event Fee'), + 'financial_type_id' => $this->getFinancialTypeID('Event Fee'), ]; } else { @@ -142,7 +142,7 @@ protected function priceSetCreate(string $type = 'Radio', array $params = []): i 'is_active' => ['1' => 1], 'price_set_id' => $priceSetID, 'is_enter_qty' => 1, - 'financial_type_id' => $this->getFinancialTypeId('Event Fee'), + 'financial_type_id' => $this->getFinancialTypeID('Event Fee'), ]; } $field = CRM_Price_BAO_PriceField::create($paramsField); diff --git a/tests/phpunit/CRM/Member/BAO/MembershipTest.php b/tests/phpunit/CRM/Member/BAO/MembershipTest.php index 0cc6299bf9cc..4368d25acad0 100644 --- a/tests/phpunit/CRM/Member/BAO/MembershipTest.php +++ b/tests/phpunit/CRM/Member/BAO/MembershipTest.php @@ -672,7 +672,7 @@ public function testUpdateAllMembershipStatusDoesNotConvertOverriddenMembershipW public function testMembershipPaymentForSingleContributionMultipleMembership(): void { $membershipTypeID1 = $this->membershipTypeCreate(['name' => 'Parent']); $membershipTypeID2 = $this->membershipTypeCreate(['name' => 'Child']); - $financialTypeId = $this->getFinancialTypeId('Member Dues'); + $financialTypeId = $this->getFinancialTypeID('Member Dues'); $priceSet = $this->callAPISuccess('price_set', 'create', [ 'is_quick_config' => 0, 'extends' => 'CiviMember', diff --git a/tests/phpunit/CRM/Price/Form/FieldTest.php b/tests/phpunit/CRM/Price/Form/FieldTest.php index 8d710564d284..0314289cbbd8 100644 --- a/tests/phpunit/CRM/Price/Form/FieldTest.php +++ b/tests/phpunit/CRM/Price/Form/FieldTest.php @@ -95,7 +95,7 @@ private function initializeFieldParameters($params) { 'weight' => 1, 'options_per_line' => 1, 'is_enter_qty' => 1, - 'financial_type_id' => $this->getFinancialTypeId('Event Fee'), + 'financial_type_id' => $this->getFinancialTypeID('Event Fee'), 'visibility_id' => $this->visibilityOptionsKeys['public'], 'price' => 10, 'active_on' => date('Y-m-d'), diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 82a4451df827..9cf01e84354b 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -2496,7 +2496,7 @@ protected function eventPriceSetCreate(float $feeTotal, float $minAmt = 0, strin 'is_active' => ['1' => 1], 'price_set_id' => $priceSetID, 'is_enter_qty' => 1, - 'financial_type_id' => $this->getFinancialTypeId('Event Fee'), + 'financial_type_id' => $this->getFinancialTypeID('Event Fee'), ]; if ($type === 'Radio') { foreach ($options as $index => $option) { @@ -2795,8 +2795,8 @@ public function _checkFinancialRecords($params, $context) { * * @return int */ - public function getFinancialTypeId($name) { - return CRM_Core_DAO::getFieldValue('CRM_Financial_DAO_FinancialType', $name, 'id', 'name'); + public function getFinancialTypeID(string $name): int { + return CRM_Core_PseudoConstant::getKey('CRM_Financial_DAO_FinancialType', 'financial_type_id', $name); } /** diff --git a/tests/phpunit/api/v3/MembershipTypeTest.php b/tests/phpunit/api/v3/MembershipTypeTest.php index d1eb1671066a..977a89315468 100644 --- a/tests/phpunit/api/v3/MembershipTypeTest.php +++ b/tests/phpunit/api/v3/MembershipTypeTest.php @@ -67,7 +67,7 @@ public function testGet($version) { $membershipType = $this->callAPIAndDocument('membership_type', 'get', $params, __FUNCTION__, __FILE__); $this->assertEquals($membershipType['values'][$id]['name'], 'General'); $this->assertEquals($membershipType['values'][$id]['member_of_contact_id'], $this->_contactID); - $this->assertEquals($membershipType['values'][$id]['financial_type_id'], $this->getFinancialTypeId('Member Dues')); + $this->assertEquals($membershipType['values'][$id]['financial_type_id'], $this->getFinancialTypeID('Member Dues')); $this->assertEquals($membershipType['values'][$id]['duration_unit'], 'year'); $this->assertEquals($membershipType['values'][$id]['duration_interval'], '1'); $this->assertEquals($membershipType['values'][$id]['period_type'], 'rolling');