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);
  }
```

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 2db3cb961640b7d3816388f28a3c09d4a2f82199..5c7d814c9f6f99609f07786dcd6b089dd8fac4dd 100644 (file)
--- 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 d6a038bd9e897f9cdbd5f422965d63089cd8be5f..c0f4af3740abb1119ff6bcdfbb341dcea6cc4e29 100644 (file)
--- 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 b3f57b62b11ba9da6c66cdc82b152f1f68ca3656..9a7b49da9b98398a3d2c32cdce504aab9a1ee4d5 100644 (file)
--- 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 48d102066960bb4f71fae0abcd468ae139a8358f..082881b0320685541bd5bf4e44bab12e5d32118a 100644 (file)
--- 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 50e363fc40a4d4f320e39758d9a73abf1927f8f8..14e252f4dafce745d2e12f72aad2403419dfc97a 100644 (file)
--- 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'];