From f436577a6c524a11bd75cc467487b1b33449bded Mon Sep 17 00:00:00 2001 From: "deb.monish" Date: Mon, 15 Jan 2018 18:11:57 +0530 Subject: [PATCH] CRM-21656: Backend Membership with Priceset applies taxes twice to line_item --- CRM/Contribute/Form/Contribution.php | 2 +- CRM/Contribute/Form/Contribution/Confirm.php | 2 +- CRM/Contribute/Form/Contribution/Main.php | 2 +- CRM/Price/BAO/PriceField.php | 12 ++--- CRM/Price/BAO/PriceSet.php | 2 +- .../CRM/Member/Form/MembershipTest.php | 49 +++++++++++++++++++ tests/phpunit/CiviTest/CiviUnitTestCase.php | 6 +++ 7 files changed, 65 insertions(+), 10 deletions(-) diff --git a/CRM/Contribute/Form/Contribution.php b/CRM/Contribute/Form/Contribution.php index 773f80a4a5..47ecce7922 100644 --- a/CRM/Contribute/Form/Contribution.php +++ b/CRM/Contribute/Form/Contribution.php @@ -1382,7 +1382,7 @@ class CRM_Contribute_Form_Contribution extends CRM_Contribute_Form_AbstractEditP // as a point of fragility rather than a logical 'if' clause. if ($priceSetId) { CRM_Price_BAO_PriceSet::processAmount($this->_priceSet['fields'], - $submittedValues, $lineItem[$priceSetId]); + $submittedValues, $lineItem[$priceSetId], NULL, $priceSetId); // Unset tax amount for offline 'is_quick_config' contribution. // @todo WHY - quick config was conceived as a quick way to configure contribution forms. // this is an example of 'other' functionality being hung off it. diff --git a/CRM/Contribute/Form/Contribution/Confirm.php b/CRM/Contribute/Form/Contribution/Confirm.php index a8ba2af41f..4018b85863 100644 --- a/CRM/Contribute/Form/Contribution/Confirm.php +++ b/CRM/Contribute/Form/Contribution/Confirm.php @@ -1991,7 +1991,7 @@ class CRM_Contribute_Form_Contribution_Confirm extends CRM_Contribute_Form_Contr $priceFields = $priceFields[$priceSetID]['fields']; $lineItems = array(); - CRM_Price_BAO_PriceSet::processAmount($priceFields, $paramsProcessedForForm, $lineItems, 'civicrm_contribution'); + CRM_Price_BAO_PriceSet::processAmount($priceFields, $paramsProcessedForForm, $lineItems, 'civicrm_contribution', $priceSetID); $form->_lineItem = array($priceSetID => $lineItems); $membershipPriceFieldIDs = array(); foreach ((array) $lineItems as $lineItem) { diff --git a/CRM/Contribute/Form/Contribution/Main.php b/CRM/Contribute/Form/Contribution/Main.php index 9cf20054ec..f7b04a1ac8 100644 --- a/CRM/Contribute/Form/Contribution/Main.php +++ b/CRM/Contribute/Form/Contribution/Main.php @@ -1165,7 +1165,7 @@ class CRM_Contribute_Form_Contribution_Main extends CRM_Contribute_Form_Contribu $component = 'membership'; } - CRM_Price_BAO_PriceSet::processAmount($this->_values['fee'], $params, $lineItem[$priceSetId], $component); + CRM_Price_BAO_PriceSet::processAmount($this->_values['fee'], $params, $lineItem[$priceSetId], $component, $priceSetId); if ($params['tax_amount']) { $this->set('tax_amount', $params['tax_amount']); } diff --git a/CRM/Price/BAO/PriceField.php b/CRM/Price/BAO/PriceField.php index 000b977e23..04d3f39f39 100644 --- a/CRM/Price/BAO/PriceField.php +++ b/CRM/Price/BAO/PriceField.php @@ -645,17 +645,16 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { * array of options */ public static function getOptions($fieldId, $inactiveNeeded = FALSE, $reset = FALSE, $isDefaultContributionPriceSet = FALSE) { - static $options = array(); - if ($reset) { - $options = array(); + if ($reset || !isset(Civi::$statics[__CLASS__]['priceOptions'])) { + Civi::$statics[__CLASS__]['priceOptions'] = array(); // This would happen if the function was only called to clear the cache. if (empty($fieldId)) { return array(); } } - if (empty($options[$fieldId])) { - $values = array(); + if (empty(Civi::$statics[__CLASS__]['priceOptions'][$fieldId])) { + $values = $options = array(); CRM_Price_BAO_PriceFieldValue::getValues($fieldId, $values, 'weight', !$inactiveNeeded); $options[$fieldId] = $values; $taxRates = CRM_Core_PseudoConstant::getTaxRates(); @@ -669,9 +668,10 @@ class CRM_Price_BAO_PriceField extends CRM_Price_DAO_PriceField { $options[$fieldId][$priceFieldId]['tax_amount'] = $taxAmount['tax_amount']; } } + Civi::$statics[__CLASS__]['priceOptions'][$fieldId] = $options[$fieldId]; } - return $options[$fieldId]; + return Civi::$statics[__CLASS__]['priceOptions'][$fieldId]; } /** diff --git a/CRM/Price/BAO/PriceSet.php b/CRM/Price/BAO/PriceSet.php index 3af46ffbde..ffcd36795d 100644 --- a/CRM/Price/BAO/PriceSet.php +++ b/CRM/Price/BAO/PriceSet.php @@ -747,7 +747,7 @@ WHERE id = %1"; CRM_Price_BAO_LineItem::format($id, $params, $field, $lineItem, $amount_override); if (CRM_Utils_Array::value('tax_rate', $field['options'][$optionValueId])) { $lineItem = self::setLineItem($field, $lineItem, $optionValueId, $totalTax); - if (CRM_Utils_Array::value('field_title', $lineItem[$optionValueId]) == 'Membership Amount') { + if ($amount_override) { $lineItem[$optionValueId]['line_total'] = $lineItem[$optionValueId]['unit_price'] = CRM_Utils_Rule::cleanMoney($lineItem[$optionValueId]['line_total'] - $lineItem[$optionValueId]['tax_amount']); } } diff --git a/tests/phpunit/CRM/Member/Form/MembershipTest.php b/tests/phpunit/CRM/Member/Form/MembershipTest.php index 41463c1450..066407ab4b 100644 --- a/tests/phpunit/CRM/Member/Form/MembershipTest.php +++ b/tests/phpunit/CRM/Member/Form/MembershipTest.php @@ -1247,4 +1247,53 @@ Expires: ', ))); } + /** + * CRM-21656: Test the submit function of the membership form if Sale Tax is enabled. + * Check that the tax rate isn't reapplied to line item's unit price and total amount + */ + public function testLineItemAmountOnSaleTax() { + $this->enableTaxAndInvoicing(); + $this->relationForFinancialTypeWithFinancialAccount(2); + $form = $this->getForm(); + $form->preProcess(); + $this->mut = new CiviMailUtils($this, TRUE); + $this->createLoggedInUser(); + $priceSet = $this->callAPISuccess('PriceSet', 'Get', array("extends" => "CiviMember")); + $form->set('priceSetId', $priceSet['id']); + // clean the price options static variable to repopulate the options, in order to fetch tax information + \Civi::$statics['CRM_Price_BAO_PriceField']['priceOptions'] = NULL; + CRM_Price_BAO_PriceSet::buildPriceSet($form); + // rebuild the price set form variable to include the tax information against each price options + $form->_priceSet = current(CRM_Price_BAO_PriceSet::getSetDetail($priceSet['id'])); + $params = array( + 'cid' => $this->_individualId, + 'join_date' => date('m/d/Y', time()), + 'start_date' => '', + 'end_date' => '', + // This format reflects the 23 being the organisation & the 25 being the type. + 'membership_type_id' => array(23, $this->membershipTypeAnnualFixedID), + 'record_contribution' => 1, + 'total_amount' => 55, + 'receive_date' => date('m/d/Y', time()), + 'receive_date_time' => '08:36PM', + 'payment_instrument_id' => array_search('Check', $this->paymentInstruments), + 'contribution_status_id' => CRM_Core_PseudoConstant::getKey('CRM_Contribute_BAO_Contribution', 'contribution_status_id', 'Completed'), + 'financial_type_id' => 2, //Member dues, see data.xml + 'payment_processor_id' => $this->_paymentProcessorID, + ); + $form->_contactID = $this->_individualId; + $form->testSubmit($params); + + $membership = $this->callAPISuccessGetSingle('Membership', array('contact_id' => $this->_individualId)); + $lineItem = $this->callAPISuccessGetSingle('LineItem', array('entity_id' => $membership['id'], 'entity_table' => 'civicrm_membership')); + $this->assertEquals(1, $lineItem['qty']); + $this->assertEquals(50.00, $lineItem['unit_price']); + $this->assertEquals(50.00, $lineItem['line_total']); + $this->assertEquals(5.00, $lineItem['tax_amount']); + + // reset the price options static variable so not leave any dummy data, that might hamper other unit tests + \Civi::$statics['CRM_Price_BAO_PriceField']['priceOptions'] = NULL; + $this->disableTaxAndInvoicing(); + } + } diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index a1df399f31..dfc16ebb9b 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -3742,6 +3742,9 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) * Enable Tax and Invoicing */ protected function disableTaxAndInvoicing($params = array()) { + if (!empty(\Civi::$statics['CRM_Core_PseudoConstant']) && isset(\Civi::$statics['CRM_Core_PseudoConstant']['taxRates'])) { + unset(\Civi::$statics['CRM_Core_PseudoConstant']['taxRates']); + } // Enable component contribute setting $contributeSetting = array_merge($params, array( @@ -3774,6 +3777,9 @@ AND ( TABLE_NAME LIKE 'civicrm_value_%' ) 'account_relationship' => key(CRM_Core_PseudoConstant::accountOptionValues('account_relationship', NULL, " AND v.name LIKE 'Sales Tax Account is' ")), ); + // set tax rate (as 10) for provided financial type ID to static variable, later used to fetch tax rates of all financial types + \Civi::$statics['CRM_Core_PseudoConstant']['taxRates'][$financialTypeId] = 10; + //CRM-20313: As per unique index added in civicrm_entity_financial_account table, // first check if there's any record on basis of unique key (entity_table, account_relationship, entity_id) $dao = new CRM_Financial_DAO_EntityFinancialAccount(); -- 2.25.1