From 7d543448c609a05fc006bd40fd431b2fa83a2ec7 Mon Sep 17 00:00:00 2001 From: monishdeb Date: Wed, 28 Jan 2015 20:15:30 +0530 Subject: [PATCH] optimization and QA changes --- CRM/Contribute/BAO/Query.php | 6 ++- api/v3/utils.php | 48 ++++++------------- tests/phpunit/api/v3/ContributionTest.php | 6 +-- .../phpunit/api/v3/SyntaxConformanceTest.php | 12 ++--- 4 files changed, 28 insertions(+), 44 deletions(-) diff --git a/CRM/Contribute/BAO/Query.php b/CRM/Contribute/BAO/Query.php index 6b564a81a7..c026ac5f73 100644 --- a/CRM/Contribute/BAO/Query.php +++ b/CRM/Contribute/BAO/Query.php @@ -140,8 +140,9 @@ class CRM_Contribute_BAO_Query { // get payment instrument id if (!empty($query->_returnProperties['payment_instrument_id'])) { + $query->_select['instrument_id'] = "contribution_payment_instrument.value as instrument_id"; $query->_select['payment_instrument_id'] = "contribution_payment_instrument.value as payment_instrument_id"; - $query->_element['payment_instrument_id'] = 1; + $query->_element['instrument_id'] = $query->_element['payment_instrument_id'] = 1; $query->_tables['civicrm_contribution'] = 1; $query->_tables['contribution_payment_instrument'] = 1; } @@ -378,6 +379,9 @@ class CRM_Contribute_BAO_Query { $query->_tables['civicrm_contribution'] = $query->_whereTables['civicrm_contribution'] = 1; return; + case 'contribution_payment_instrument': + case 'contribution_payment_instrument_id': + $name = str_replace('contribution_', '', $name); case 'payment_instrument': case 'payment_instrument_id': if ($name == 'payment_instrument') { diff --git a/api/v3/utils.php b/api/v3/utils.php index ef950a98c9..80536cc841 100644 --- a/api/v3/utils.php +++ b/api/v3/utils.php @@ -1425,15 +1425,8 @@ function _civicrm_api3_validate_fields($entity, $action, &$params, $fields, $err if (strpos($op, 'NULL') !== FALSE || strpos($op, 'EMPTY') !== FALSE) { break; } - if (is_array($fieldValue)) { - foreach($fieldValue as $fieldvalue) { - if (!CRM_Utils_Rule::money($fieldvalue) && !empty($fieldvalue)) { - throw new Exception($fieldName . " is not a valid amount: " . $params[$fieldName]); - } - } - } - else { - if (!CRM_Utils_Rule::money($fieldValue) && !empty($fieldValue)) { + foreach((array)$fieldValue as $fieldvalue) { + if (!CRM_Utils_Rule::money($fieldvalue) && !empty($fieldvalue)) { throw new Exception($fieldName . " is not a valid amount: " . $params[$fieldName]); } } @@ -1951,29 +1944,22 @@ function _civicrm_api3_validate_string(&$params, &$fieldName, &$fieldInfo, $enti } if ($fieldName == 'currency') { //When using IN operator $fieldValue is a array of currency codes - if (is_array($fieldValue)) { - foreach ($fieldValue as $currency) { - if (!CRM_Utils_Rule::currencyCode($currency)) { - throw new Exception("Currency not a valid code: $currency"); - } + foreach ((array)$fieldValue as $currency) { + if (!CRM_Utils_Rule::currencyCode($currency)) { + throw new Exception("Currency not a valid code: $currency"); } } - else { - if (!CRM_Utils_Rule::currencyCode($fieldValue)) { - throw new Exception("Currency not a valid code: $fieldValue"); - } - } - } - if (!empty($fieldInfo['pseudoconstant']) || !empty($fieldInfo['options'])) { - _civicrm_api3_api_match_pseudoconstant($fieldValue, $entity, $fieldName, $fieldInfo); - } - // Check our field length - elseif (is_string($fieldValue) && !empty($fieldInfo['maxlength']) && strlen(utf8_decode($fieldValue)) > $fieldInfo['maxlength']) { - throw new API_Exception("Value for $fieldName is " . strlen(utf8_decode($value)) . " characters - This field has a maxlength of {$fieldInfo['maxlength']} characters.", - 2100, array('field' => $fieldName) - ); } } + if (!empty($fieldInfo['pseudoconstant']) || !empty($fieldInfo['options'])) { + _civicrm_api3_api_match_pseudoconstant($fieldValue, $entity, $fieldName, $fieldInfo); + } + // Check our field length + elseif (is_string($fieldValue) && !empty($fieldInfo['maxlength']) && strlen(utf8_decode($fieldValue)) > $fieldInfo['maxlength']) { + throw new API_Exception("Value for $fieldName is " . strlen(utf8_decode($value)) . " characters - This field has a maxlength of {$fieldInfo['maxlength']} characters.", + 2100, array('field' => $fieldName) + ); + } if (!empty($op)) { $params[$fieldName][$op] = $fieldValue; @@ -2002,12 +1988,6 @@ function _civicrm_api3_api_match_pseudoconstant(&$fieldValue, $entity, $fieldNam } $options = civicrm_api($entity, 'getoptions', array('version' => 3, 'field' => $fieldInfo['name'], 'context' => 'validate')); $options = CRM_Utils_Array::value('values', $options, array()); - - if (count($options) == 0 && isset($pseudoconstant['table'])) { - $pseudoParams = $pseudoconstant; - unset($pseudoParams['table']); - $options = CRM_Core_PseudoConstant::get(CRM_Core_DAO_AllCoreTables::getClassForTable($pseudoconstant['table']), $fieldName, $pseudoParams); - } } // If passed a value-separated string, explode to an array, then re-implode after matching values diff --git a/tests/phpunit/api/v3/ContributionTest.php b/tests/phpunit/api/v3/ContributionTest.php index 67a08de701..d1734ba3df 100644 --- a/tests/phpunit/api/v3/ContributionTest.php +++ b/tests/phpunit/api/v3/ContributionTest.php @@ -460,7 +460,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { $this->callAPISuccess('contribution', 'create', $params2); $contribution = $this->callAPISuccess('contribution', 'get', array( 'sequential' => 1, - 'contribution_payment_instrument_id' => 'Cash', + 'contribution_payment_instrument' => 'Cash', )); $this->assertArrayHasKey('payment_instrument', $contribution['values'][0]); $this->assertEquals('Cash',$contribution['values'][0]['payment_instrument']); @@ -1119,7 +1119,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { //This should not be required on update: $old_contact_id = $original['values'][$contributionID]['contact_id']; - $old_payment_instrument = $original['values'][$contributionID]['payment_instrument_id']; + $old_payment_instrument = $original['values'][$contributionID]['instrument_id']; $old_fee_amount = $original['values'][$contributionID]['fee_amount']; $old_source = $original['values'][$contributionID]['contribution_source']; @@ -1155,7 +1155,7 @@ class api_v3_ContributionTest extends CiviUnitTestCase { $this->assertEquals($contribution['values'][$contributionID]['contact_id'], $this->_individualId); $this->assertEquals($contribution['values'][$contributionID]['total_amount'], 110.00); $this->assertEquals($contribution['values'][$contributionID]['financial_type_id'],$this->_financialTypeId ); - $this->assertEquals($contribution['values'][$contributionID]['payment_instrument_id'], $old_payment_instrument); + $this->assertEquals($contribution['values'][$contributionID]['instrument_id'], $old_payment_instrument); $this->assertEquals($contribution['values'][$contributionID]['non_deductible_amount'], 10.00); $this->assertEquals($contribution['values'][$contributionID]['fee_amount'], $old_fee_amount); $this->assertEquals($contribution['values'][$contributionID]['net_amount'], 100.00); diff --git a/tests/phpunit/api/v3/SyntaxConformanceTest.php b/tests/phpunit/api/v3/SyntaxConformanceTest.php index 049de1efd5..c3fb74a9ea 100644 --- a/tests/phpunit/api/v3/SyntaxConformanceTest.php +++ b/tests/phpunit/api/v3/SyntaxConformanceTest.php @@ -516,6 +516,12 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase { 'entity_id', ), ), + 'ContributionSoft' => array( + 'cant_update' => array( + // can't be changed through api + 'pcp_id', + ), + ), 'Pledge' => array( 'cant_update' => array( 'pledge_original_installment_amount', @@ -1138,12 +1144,6 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase { )); $options['values'][] = $optionValue['id']; } - if (isset($specs['pseudoconstant']['table'])) { - //some psedoconstant property is based on other entity attributes - //as the there may or maynot be records for such entity, so we are considering empty records for now - //e.g. pcp_id property will be table=civicrm_pcp, keyColumn=id and labelColumn=title - continue; - } } $entity[$field] = array_rand($options['values']); } -- 2.25.1