From faba82fcdd5b99364b6b838f0b418a2151237a06 Mon Sep 17 00:00:00 2001 From: eileen Date: Fri, 15 Feb 2019 11:50:56 +1300 Subject: [PATCH] dev/core#720 add unit test, remove legacy code style. This PR adds tests to the summaryQuery code in preparation for changes. The one non-test change is to remove the & from summaryContribution fn. It is only called from one place in the code & that place does not return as reference. ``` /** * @return mixed */ public function getSummary() { return $this->_query->summaryContribution($this->_context); } ``` --- CRM/Contact/BAO/Query.php | 2 +- tests/phpunit/CRM/Contact/BAO/QueryTest.php | 103 +++++++++++++++++- .../CRMTraits/Financial/FinancialACLTrait.php | 2 +- .../CRMTraits/Financial/PriceSetTrait.php | 7 +- tests/phpunit/CiviTest/CiviUnitTestCase.php | 2 +- 5 files changed, 107 insertions(+), 9 deletions(-) diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index 2db3cb9616..5c7d814c9f 100644 --- a/CRM/Contact/BAO/Query.php +++ b/CRM/Contact/BAO/Query.php @@ -5087,7 +5087,7 @@ civicrm_relationship.start_date > {$today} * * @return array */ - public function &summaryContribution($context = NULL) { + public function summaryContribution($context = NULL) { list($innerselect, $from, $where, $having) = $this->query(TRUE); // hack $select diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php index d6a038bd9e..c0f4af3740 100644 --- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php +++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php @@ -6,6 +6,7 @@ */ class CRM_Contact_BAO_QueryTest extends CiviUnitTestCase { use CRMTraits_Financial_FinancialACLTrait; + use CRMTraits_Financial_PriceSetTrait; /** * @return CRM_Contact_BAO_QueryTestDataProvider @@ -19,6 +20,7 @@ class CRM_Contact_BAO_QueryTest extends CiviUnitTestCase { } public function tearDown() { + $this->quickCleanUpFinancialEntities(); $tablesToTruncate = array( 'civicrm_group_contact', 'civicrm_group', @@ -714,12 +716,34 @@ civicrm_relationship.is_active = 1 AND * Test the summary query does not add an acl clause when acls not enabled.. */ public function testGetSummaryQueryWithFinancialACLDisabled() { + $this->createContributionsForSummaryQueryTests(); + + // Test the function directly $where = $from = NULL; $queryObject = new CRM_Contact_BAO_Query(); - $query = $queryObject->appendFinancialTypeWhereAndFromToQueryStrings($where, + $queryObject->appendFinancialTypeWhereAndFromToQueryStrings($where, $from); - $this->assertEquals($where, $query[0]); - $this->assertEquals($from, $query[1]); + $this->assertEquals(NULL, $where); + $this->assertEquals(NULL, $from); + + // Test the function in action + $queryObject = new CRM_Contact_BAO_Query([['contribution_source', '=', 'SSF', '', '']]); + $summary = $queryObject->summaryContribution(); + $this->assertEquals([ + 'total' => [ + 'avg' => '$ 233.33', + 'amount' => '$ 1,400.00', + 'count' => 6, + 'mode' => '$ 300.00', + 'median' => '$ 300.00', + 'currencyCount' => 1, + ], + 'cancel' => [ + 'count' => 2, + 'amount' => '$ 100.00', + 'avg' => '$ 50.00', + ], + ], $summary); } /** @@ -727,16 +751,38 @@ civicrm_relationship.is_active = 1 AND */ public function testGetSummaryQueryWithFinancialACLEnabled() { $where = $from = NULL; + $this->createContributionsForSummaryQueryTests(); $this->enableFinancialACLs(); $this->createLoggedInUserWithFinancialACL(); + + // Test the function directly $queryObject = new CRM_Contact_BAO_Query(); - $query = $queryObject->appendFinancialTypeWhereAndFromToQueryStrings($where, + $queryObject->appendFinancialTypeWhereAndFromToQueryStrings($where, $from); $donationTypeID = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'); $this->assertEquals( " LEFT JOIN civicrm_line_item li ON civicrm_contribution.id = li.contribution_id AND li.entity_table = 'civicrm_contribution' AND li.financial_type_id NOT IN ({$donationTypeID}) ", $from); + + // Test the function in action + $queryObject = new CRM_Contact_BAO_Query([['contribution_source', '=', 'SSF', '', '']]); + $summary = $queryObject->summaryContribution(); + $this->assertEquals([ + 'total' => [ + 'avg' => '$ 200.00', + 'amount' => '$ 400.00', + 'count' => 2, + 'mode' => 'N/A', + 'median' => '$ 200.00', + 'currencyCount' => 1, + ], + 'cancel' => [ + 'count' => 1, + 'amount' => '$ 50.00', + 'avg' => '$ 50.00', + ], + ], $summary); $this->disableFinancialACLs(); } @@ -778,4 +824,53 @@ civicrm_relationship.is_active = 1 AND $this->assertEquals($modparams['member_is_primary'][2], $fv_orig['member_is_primary']); } + /** + * Create contributions to test summary calculations. + * + * financial type | cancel_date |total_amount| source | line_item_financial_types |number_line_items| line_amounts + * Donation |NULL | 100.00 |SSF | Donation | 1 | 100.00 + * Member Dues |NULL | 100.00 |SSF | Member Dues | 1 | 100.00 + * Donation |NULL | 300.00 |SSF | Event Fee,Event Fee | 2 | 200.00,100.00 + * Donation |NULL | 300.00 |SSF | Event Fee,Donation | 2 | 200.00,100.00 + * Donation |NULL | 300.00 |SSF | Donation,Donation | 2 | 200.00,100.00 + * Donation |2019-02-13 00:00:00 | 50.00 |SSF | Donation | 1 | 50.00 + * Member Dues |2019-02-13 00:00:00 | 50.00 |SSF | Member Dues | 1 | 50.00 + */ + protected function createContributionsForSummaryQueryTests() { + $contactID = $this->individualCreate(); + $this->contributionCreate(['contact_id' => $contactID]); + $this->contributionCreate([ + 'contact_id' => $contactID, + 'total_amount' => 100, + 'financial_type_id' => 'Member Dues', + ]); + $eventFeeType = CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Event Fee'); + $this->createContributionWithTwoLineItemsAgainstPriceSet(['contact_id' => $contactID, 'source' => 'SSF']); + $this->createContributionWithTwoLineItemsAgainstPriceSet(['contact_id' => $contactID, 'source' => 'SSF'], [ + CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'), + $eventFeeType, + ]); + $this->createContributionWithTwoLineItemsAgainstPriceSet(['contact_id' => $contactID, 'source' => 'SSF'], [ + CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'), + CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'), + ]); + $this->createContributionWithTwoLineItemsAgainstPriceSet(['contact_id' => $contactID, 'source' => 'SSF', 'financial_type_id' => $eventFeeType], [ + CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'), + CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'financial_type_id', 'Donation'), + ]); + $this->contributionCreate([ + 'contact_id' => $contactID, + 'total_amount' => 50, + 'contribution_status_id' => 'Cancelled', + 'cancel_date' => 'yesterday', + ]); + $this->contributionCreate([ + 'contact_id' => $contactID, + 'total_amount' => 50, + 'contribution_status_id' => 'Cancelled', + 'cancel_date' => 'yesterday', + 'financial_type_id' => 'Member Dues', + ]); + } + } diff --git a/tests/phpunit/CRMTraits/Financial/FinancialACLTrait.php b/tests/phpunit/CRMTraits/Financial/FinancialACLTrait.php index b3f57b62b1..9a7b49da9b 100644 --- a/tests/phpunit/CRMTraits/Financial/FinancialACLTrait.php +++ b/tests/phpunit/CRMTraits/Financial/FinancialACLTrait.php @@ -66,7 +66,7 @@ trait CRMTraits_Financial_FinancialACLTrait { * @return int Contact ID */ protected function createLoggedInUserWithFinancialACL($aclPermissions = [['view', 'Donation']]) { - CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM']; + CRM_Core_Config::singleton()->userPermissionClass->permissions = ['access CiviCRM', 'view all contacts']; $contactID = $this->createLoggedInUser(); $this->addFinancialAclPermissions($aclPermissions); return $contactID; diff --git a/tests/phpunit/CRMTraits/Financial/PriceSetTrait.php b/tests/phpunit/CRMTraits/Financial/PriceSetTrait.php index 48d1020669..082881b032 100644 --- a/tests/phpunit/CRMTraits/Financial/PriceSetTrait.php +++ b/tests/phpunit/CRMTraits/Financial/PriceSetTrait.php @@ -38,11 +38,14 @@ trait CRMTraits_Financial_PriceSetTrait { * This also involves creating t * * @param $params + * @param array $lineItemFinancialTypes + * Financial Types, if an override is intended. */ - protected function createContributionWithTwoLineItemsAgainstPriceSet($params) { + protected function createContributionWithTwoLineItemsAgainstPriceSet($params, $lineItemFinancialTypes = []) { $params = array_merge(['total_amount' => 300, 'financial_type_id' => 'Donation'], $params); $priceFields = $this->createPriceSet('contribution'); foreach ($priceFields['values'] as $key => $priceField) { + $financialTypeID = (!empty($lineItemFinancialTypes) ? array_shift($lineItemFinancialTypes) : $priceField['financial_type_id']); $params['line_items'][]['line_item'][$key] = [ 'price_field_id' => $priceField['price_field_id'], 'price_field_value_id' => $priceField['id'], @@ -51,7 +54,7 @@ trait CRMTraits_Financial_PriceSetTrait { 'qty' => 1, 'unit_price' => $priceField['amount'], 'line_total' => $priceField['amount'], - 'financial_type_id' => $priceField['financial_type_id'], + 'financial_type_id' => $financialTypeID, 'entity_table' => 'civicrm_contribution', ]; } diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 50e363fc40..14e252f4da 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3133,7 +3133,7 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) $paramsSet['title'] = 'Price Set' . substr(sha1(rand()), 0, 7); $paramsSet['name'] = CRM_Utils_String::titleToVar($paramsSet['title']); $paramsSet['is_active'] = TRUE; - $paramsSet['financial_type_id'] = 4; + $paramsSet['financial_type_id'] = 'Event Fee'; $paramsSet['extends'] = 1; $priceSet = $this->callAPISuccess('price_set', 'create', $paramsSet); $priceSetId = $priceSet['id']; -- 2.25.1