Towards CRM-19815 Comment fixes & test tidy-up

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 347a9f42a5e60edacff1f6a10db93f8a680e3570..f0bda01fa8fba8737079693764539d35d72269e2 100644 (file)
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -639,12 +639,17 @@ class CRM_Contact_BAO_Query {
    */
   public function selectClause($apiEntity = NULL) {
 
+    // @todo Tidy up this. This arises because 1) we are ignoring the $mode & adding a new
+    // param ($apiEntity) instead - presumably an oversight & 2 because
+    // contact is not implemented as a component.
     $this->addSpecialFields($apiEntity);
 
     foreach ($this->_fields as $name => $field) {
       // skip component fields
       // there are done by the alter query below
       // and need not be done on every field
+      // @todo remove these & handle using metadata - only obscure fields
+      // that are hack-added should need to be excluded from the main loop.
       if (
         (substr($name, 0, 12) == 'participant_') ||
         (substr($name, 0, 7) == 'pledge_') ||
@@ -736,18 +741,21 @@ class CRM_Contact_BAO_Query {
                 $this->_pseudoConstantsSelect[$name]['element'] = $name;
 
                 if ($tableName == 'email_greeting') {
+                  // @todo bad join.
                   $this->_pseudoConstantsSelect[$name]['join']
                     = " LEFT JOIN civicrm_option_group option_group_email_greeting ON (option_group_email_greeting.name = 'email_greeting')";
                   $this->_pseudoConstantsSelect[$name]['join'] .=
                     " LEFT JOIN civicrm_option_value email_greeting ON (contact_a.email_greeting_id = email_greeting.value AND option_group_email_greeting.id = email_greeting.option_group_id ) ";
                 }
                 elseif ($tableName == 'postal_greeting') {
+                  // @todo bad join.
                   $this->_pseudoConstantsSelect[$name]['join']
                     = " LEFT JOIN civicrm_option_group option_group_postal_greeting ON (option_group_postal_greeting.name = 'postal_greeting')";
                   $this->_pseudoConstantsSelect[$name]['join'] .=
                     " LEFT JOIN civicrm_option_value postal_greeting ON (contact_a.postal_greeting_id = postal_greeting.value AND option_group_postal_greeting.id = postal_greeting.option_group_id ) ";
                 }
                 elseif ($tableName == 'addressee') {
+                  // @todo bad join.
                   $this->_pseudoConstantsSelect[$name]['join']
                     = " LEFT JOIN civicrm_option_group option_group_addressee ON (option_group_addressee.name = 'addressee')";
                   $this->_pseudoConstantsSelect[$name]['join'] .=
@@ -5848,6 +5856,7 @@ AND   displayRelType.is_active = 1
         // FIX ME: we should potentially move this to component Query and write a wrapper function that
         // handles pseudoconstant fixes for all component
         elseif (in_array($value['pseudoField'], array('participant_role_id', 'participant_role'))) {
+          // @todo define bao on this & merge into the above condition.
           $viewValues = explode(CRM_Core_DAO::VALUE_SEPARATOR, $val);
 
           if ($value['pseudoField'] == 'participant_role') {

diff --git a/CRM/Contribute/BAO/Query.php b/CRM/Contribute/BAO/Query.php
index d275b2ceecfb1cb9cc75dc815f83e92f05f181f1..c323b4f7a8471d485c767ed7d70d0e618f9b081d 100644 (file)
--- a/CRM/Contribute/BAO/Query.php
+++ b/CRM/Contribute/BAO/Query.php
@@ -297,6 +297,7 @@ class CRM_Contribute_BAO_Query extends CRM_Core_BAO_Query {
       case 'contribution_status':
         $name .= '_id';
       case 'financial_type_id':
+        // @todo we need to make this resemble a hook approach.
         CRM_Financial_BAO_FinancialType::getAvailableFinancialTypes($financialTypes);
         $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("civicrm_contribution.$name", 'IN', array_keys($financialTypes), 'String');
       case 'invoice_id':

diff --git a/CRM/Core/BAO/Mapping.php b/CRM/Core/BAO/Mapping.php
index f19b49839b0689f94620df9303cdda9c09872881..4b0d9106ed24a02ccf0debe2b865c11c25aa8d2d 100644 (file)
--- a/CRM/Core/BAO/Mapping.php
+++ b/CRM/Core/BAO/Mapping.php
@@ -1126,7 +1126,7 @@ class CRM_Core_BAO_Mapping extends CRM_Core_DAO_Mapping {
    * @return NULL
    */
   public static function saveMappingFields(&$params, $mappingId) {
-    //delete mapping fields records for exixting mapping
+    //delete mapping fields records for existing mapping
     $mappingFields = new CRM_Core_DAO_MappingField();
     $mappingFields->mapping_id = $mappingId;
     $mappingFields->delete();

diff --git a/tests/phpunit/CRM/Contact/BAO/QueryTest.php b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
index 8bffbf8c80d4c562ed7bde7373699ae1490176b5..5c014f30b8e676405b6ca9318c064beaacc726e4 100644 (file)
--- a/tests/phpunit/CRM/Contact/BAO/QueryTest.php
+++ b/tests/phpunit/CRM/Contact/BAO/QueryTest.php
@@ -31,8 +31,10 @@ class CRM_Contact_BAO_QueryTest extends CiviUnitTestCase {
   }
 
   /**
-   *  Test CRM_Contact_BAO_Query::searchQuery()
+   *  Test CRM_Contact_BAO_Query::searchQuery().
+   *
    * @dataProvider dataProvider
+   *
    * @param $fv
    * @param $count
    * @param $ids
@@ -331,7 +333,7 @@ class CRM_Contact_BAO_QueryTest extends CiviUnitTestCase {
   }
 
   /**
-   * CRM-19562 ensure that only ids are used for contactid searching.
+   * CRM-19562 ensure that only ids are used for contact_id searching.
    */
   public function testContactIDClause() {
     $params = array(

diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php
index 2d7b1db8ca1dd9360cbad6fca1d4680a7816501d..9c83d5ef7867d4d7376d7f51fc82220a9dbaf39d 100644 (file)
--- a/tests/phpunit/api/v3/ContributionTest.php
+++ b/tests/phpunit/api/v3/ContributionTest.php
@@ -147,25 +147,26 @@ class api_v3_ContributionTest extends CiviUnitTestCase {
       'contribution_id' => $this->_contribution['id'],
     );
 
-    $contribution = $this->callAPIAndDocument('contribution', 'get', $params, __FUNCTION__, __FILE__);
+    $contributions = $this->callAPIAndDocument('contribution', 'get', $params, __FUNCTION__, __FILE__);
     $financialParams['id'] = $this->_financialTypeId;
     $default = NULL;
     CRM_Financial_BAO_FinancialType::retrieve($financialParams, $default);
 
-    $this->assertEquals(1, $contribution['count']);
-    $this->assertEquals($contribution['values'][$contribution['id']]['contact_id'], $this->_individualId);
+    $this->assertEquals(1, $contributions['count']);
+    $contribution = $contributions['values'][$contributions['id']];
+    $this->assertEquals($contribution['contact_id'], $this->_individualId);
     // Note there was an assertion converting financial_type_id to 'Donation' which wasn't working.
     // Passing back a string rather than an id seems like an error/cruft.
     // If it is to be introduced we should discuss.
-    $this->assertEquals($contribution['values'][$contribution['id']]['financial_type_id'], 1);
-    $this->assertEquals($contribution['values'][$contribution['id']]['total_amount'], 100.00);
-    $this->assertEquals($contribution['values'][$contribution['id']]['non_deductible_amount'], 10.00);
-    $this->assertEquals($contribution['values'][$contribution['id']]['fee_amount'], 5.00);
-    $this->assertEquals($contribution['values'][$contribution['id']]['net_amount'], 95.00);
-    $this->assertEquals($contribution['values'][$contribution['id']]['trxn_id'], 23456);
-    $this->assertEquals($contribution['values'][$contribution['id']]['invoice_id'], 78910);
-    $this->assertEquals($contribution['values'][$contribution['id']]['contribution_source'], 'SSF');
-    $this->assertEquals($contribution['values'][$contribution['id']]['contribution_status'], 'Completed');
+    $this->assertEquals($contribution['financial_type_id'], 1);
+    $this->assertEquals($contribution['total_amount'], 100.00);
+    $this->assertEquals($contribution['non_deductible_amount'], 10.00);
+    $this->assertEquals($contribution['fee_amount'], 5.00);
+    $this->assertEquals($contribution['net_amount'], 95.00);
+    $this->assertEquals($contribution['trxn_id'], 23456);
+    $this->assertEquals($contribution['invoice_id'], 78910);
+    $this->assertEquals($contribution['contribution_source'], 'SSF');
+    $this->assertEquals($contribution['contribution_status'], 'Completed');
     // Create a second contribution - we are testing that 'id' gets the right contribution id (not the contact id).
     $p['trxn_id'] = '3847';
     $p['invoice_id'] = '3847';

diff --git a/tests/phpunit/api/v3/JobTest.php b/tests/phpunit/api/v3/JobTest.php
index 8ab80d447c11429b3fbdfd9b536cf373e21b92e9..d0f78cbddcc962990d74fa91802576a5ad711836 100644 (file)
--- a/tests/phpunit/api/v3/JobTest.php
+++ b/tests/phpunit/api/v3/JobTest.php
@@ -32,8 +32,6 @@
  * @subpackage API_Job
  *
  * @copyright CiviCRM LLC (c) 2004-2017
- * @version $Id: Job.php 30879 2010-11-22 15:45:55Z shot $
- *
  */
 
 /**