Fix creditmemo tax calculation bug when using multiple discounts #88
Conversation
Replace
$creditmemo->setGrandTotal($creditmemo->getGrandTotal() + $subtotal);
$creditmemo->setBaseGrandTotal($creditmemo->getBaseGrandTotal() + $baseSubtotal);
with
$creditmemo->setGrandTotal($creditmemo->getGrandTotal() + $subtotalInclTax);
$creditmemo->setBaseGrandTotal($creditmemo->getBaseGrandTotal() + $baseSubtotalInclTax);
|
Can you give a detail description of the problem and how this code solves the issue? |
|
When adding several product types and utilizing several (more than 1) discount code the credit memo calculation is not correct. Before the fix the order value is 184,40 and credit memo is 181,41 So in essence it seems to solves an tax issue when utilizing cart discounts |
|
Does this merge cleanly with 1.9.3.0? |
|
I've changed the base to 1.9.3.x, however a confirmation is needed whether the bug occures in 1.9.3 still. |
| public function collectTotals() | ||
| { | ||
| foreach ($this->getConfig()->getTotalModels() as $model) { | ||
| public function collectTotals() |
There was a problem hiding this comment.
whitespace issue here (space)
| { | ||
| $models = $this->getConfig()->getTotalModels(); | ||
| $res = array(); | ||
| $i=0; |
| $i=0; | ||
| $discount = 0; | ||
| foreach ($models as $name =>$model) { | ||
| if ($name=='discount') $discount = $i; |
There was a problem hiding this comment.
whitespaces around == needed, and please wrap $discount = $i; with brackets {}
| $discount = 0; | ||
| foreach ($models as $name =>$model) { | ||
| if ($name=='discount') $discount = $i; | ||
| if ($name=='tax') { |
|
@seansan can you improve the pull request and confirm that the issue still exists? |
|
Feel free to change pull. But more importantly there needs to be more than 1 test .... |
|
@seansan I have no access to this branch, probably you forgot to select a checkbox like "allow contributors to edit..." when making pull request |
Fix creditmemo tax calculation bug when using multiple discounts
Needs code review and additional testing
But this has been tested in production and solved our errors with creditmemo calculation missing value