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-20676: Tax applied repeatedly on edits of price set events #11455

Merged
merged 1 commit into from
Jan 1, 2018
Merged

CRM-20676: Tax applied repeatedly on edits of price set events #11455

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
12 changes: 11 additions & 1 deletion CRM/Contribute/Form/Contribution.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1324,6 +1324,12 @@ public function testSubmit($params, $action, $creditCardMode = NULL) {
));
$this->_id = $params['id'];
$this->_values = $existingContribution;
if (CRM_Contribute_BAO_Contribution::checkContributeSettings('invoicing')) {
$this->_values['tax_amount'] = civicrm_api3('contribution', 'getvalue', array(
'id' => $params['id'],
'return' => 'tax_amount',
));
}
Copy link
Member

Choose a reason for hiding this comment

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

@Monish I'm curious to know why this is necessary. The old code was referencing this variable already, just as the new code is. Was it sometimes not present?

Copy link
Member Author

@monishdeb monishdeb Dec 29, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@colemanw for the added unit test it needs default tax_amount information in $this->_values, but unfortunately Contribution.getsingle API doesn't provide value for this attribute, so yes its something not present earlier and there was no UT written, that need tax_amount (until now). Another alternative to deal with it is to set this as one of the default return property as I did here But after discussing with Eileen I decided to fetch its value on demand.

}
else {
$existingContribution = array();
Expand Down Expand Up @@ -1522,7 +1528,11 @@ protected function submit($submittedValues, $action, $pledgePaymentID) {
}

if (!isset($submittedValues['total_amount'])) {
$submittedValues['total_amount'] = CRM_Utils_Array::value('total_amount', $this->_values) - CRM_Utils_Array::value('tax_amount', $this->_values);
$submittedValues['total_amount'] = CRM_Utils_Array::value('total_amount', $this->_values);
// Avoid tax amount deduction on edit form and keep it original, because this will lead to error described in CRM-20676
if (!$this->_id) {
$submittedValues['total_amount'] -= CRM_Utils_Array::value('tax_amount', $this->_values, 0);
}
}
$this->assign('lineItem', !empty($lineItem) && !$isQuickConfig ? $lineItem : FALSE);

Expand Down
17 changes: 17 additions & 0 deletions tests/phpunit/CRM/Contribute/Form/ContributionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,23 @@ public function testSubmitSaleTax() {
$lineItem = $this->callAPISuccessGetSingle('LineItem', array('contribution_id' => $contribution['id']));
$this->assertEquals(100, $lineItem['line_total']);
$this->assertEquals(10, $lineItem['tax_amount']);

// CRM-20423: Upon simple submit of 'Edit Contribution' form ensure that total amount is same
$form->testSubmit(array(
'id' => $contribution['id'],
'financial_type_id' => 3,
'receive_date' => '04/21/2015',
'receive_date_time' => '11:27PM',
'contact_id' => $this->_individualId,
'payment_instrument_id' => array_search('Check', $this->paymentInstruments),
'contribution_status_id' => 1,
),
CRM_Core_Action::UPDATE
);

$contribution = $this->callAPISuccessGetSingle('Contribution', array('contact_id' => $this->_individualId));
// Check if total amount is unchanged
$this->assertEquals(110, $contribution['total_amount']);
}

/**
Expand Down