From 3af96592238e7cc7246907f56b394559bebfeff5 Mon Sep 17 00:00:00 2001 From: eileen Date: Tue, 24 Jan 2017 13:25:12 +1300 Subject: [PATCH] Towards CRM-19815 Comment fixes & test tidy-up --- CRM/Contact/BAO/Query.php | 9 ++++++++ CRM/Contribute/BAO/Query.php | 1 + CRM/Core/BAO/Mapping.php | 2 +- tests/phpunit/CRM/Contact/BAO/QueryTest.php | 6 +++-- tests/phpunit/api/v3/ContributionTest.php | 25 +++++++++++---------- tests/phpunit/api/v3/JobTest.php | 2 -- 6 files changed, 28 insertions(+), 17 deletions(-) diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php index 347a9f42a5..f0bda01fa8 100644 --- 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 d275b2ceec..c323b4f7a8 100644 --- 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 f19b49839b..4b0d9106ed 100644 --- 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 8bffbf8c80..5c014f30b8 100644 --- 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 2d7b1db8ca..9c83d5ef78 100644 --- 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 8ab80d447c..d0f78cbddc 100644 --- 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 $ - * */ /** -- 2.25.1