Skip to content

Commit

Permalink
Merge pull request #11521 from JMAConsulting/CRM-21656
Browse files Browse the repository at this point in the history
CRM-21656: Backend Membership with Priceset applies taxes twice to line_item
  • Loading branch information
eileenmcnaughton authored Jan 18, 2018
2 parents 91da8b6 + f436577 commit e0cdb02
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 10 deletions.
2 changes: 1 addition & 1 deletion CRM/Contribute/Form/Contribution.php
Original file line number Diff line number Diff line change
Expand Up @@ -1382,7 +1382,7 @@ protected function submit($submittedValues, $action, $pledgePaymentID) {
// 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.
Expand Down
2 changes: 1 addition & 1 deletion CRM/Contribute/Form/Contribution/Confirm.php
Original file line number Diff line number Diff line change
Expand Up @@ -1991,7 +1991,7 @@ public static function submit($params) {

$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) {
Expand Down
2 changes: 1 addition & 1 deletion CRM/Contribute/Form/Contribution/Main.php
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ public function submit($params) {
$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']);
}
Expand Down
12 changes: 6 additions & 6 deletions CRM/Price/BAO/PriceField.php
Original file line number Diff line number Diff line change
Expand Up @@ -645,17 +645,16 @@ public static function addQuickFormElement(
* 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();
Expand All @@ -669,9 +668,10 @@ public static function getOptions($fieldId, $inactiveNeeded = FALSE, $reset = FA
$options[$fieldId][$priceFieldId]['tax_amount'] = $taxAmount['tax_amount'];
}
}
Civi::$statics[__CLASS__]['priceOptions'][$fieldId] = $options[$fieldId];
}

return $options[$fieldId];
return Civi::$statics[__CLASS__]['priceOptions'][$fieldId];
}

/**
Expand Down
2 changes: 1 addition & 1 deletion CRM/Price/BAO/PriceSet.php
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ public static function processAmount($fields, &$params, &$lineItem, $component =
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']);
}
}
Expand Down
49 changes: 49 additions & 0 deletions tests/phpunit/CRM/Member/Form/MembershipTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1247,4 +1247,53 @@ public function testContributionFormStatusUpdate() {
)));
}

/**
* 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();
}

}
6 changes: 6 additions & 0 deletions tests/phpunit/CiviTest/CiviUnitTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -3738,6 +3738,9 @@ protected function enableTaxAndInvoicing($params = array()) {
* 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(
Expand Down Expand Up @@ -3770,6 +3773,9 @@ protected function relationForFinancialTypeWithFinancialAccount($financialTypeId
'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();
Expand Down

0 comments on commit e0cdb02

Please sign in to comment.