Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CRM-21656: Backend Membership with Priceset applies taxes twice to line_item #11521

Merged
merged 1 commit into from
Jan 18, 2018
Merged

CRM-21656: Backend Membership with Priceset applies taxes twice to line_item #11521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CRM/Contribute/Form/Contribution.php
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3742,6 +3742,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 @@ -3774,6 +3777,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;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no other way where we can update this static variable (which holds the tax rates of all FT) after setting a Taxable financial account to a financial type.


//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