-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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.
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
100% Discount cart rule results in Tax invoiced #30853
Comments
Hi @OldPlanets. Thank you for your report.
Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:
For more details, please, review the Magento Contributor Assistant documentation. Please, add a comment to assign the issue:
🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket. 🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
Hi @engcom-Delta. Thank you for working on this issue.
|
✅ Confirmed by @engcom-Delta Issue Available: @engcom-Delta, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself. |
I noticed in magento2/app/code/Magento/Sales/Model/Order/Invoice/Total/Tax.php Lines 121 to 122 in 445b0f1
|
@paulvandermeijs |
Hi All, As I think it was correct within Magento 2.3.5, I made a new module overriding the classes Tax.php, Subtotal.php, Shippment.php and Discount.php and took the code from Magento 2.3.5 The result of it is, that everything works as it should. Therefore, there must be some bugs in the new classes. As this is a mandatory and needed functionality, please be so kind and fix this as fast as possible and especially provide a patch because waiting for releases might be to long. Thanks and best regards, |
Hi
The invoice is wrong because $invoice->getGrandTotal() contains the amount without tax, but $totalDiscountAmount is with tax. Using the code from 2.3.5 at that point, invoices are correct again.
I hope this helps to fix the this bug. Best regards, |
Hi @engcom-Hotel. Thank you for working on this issue.
|
@magento I am working on this |
I might guess that this is of big concerns when discount is greater than 50%, as, if the GrandTotal gest negative along the overall computation, a recent Magento update f**s it up. Thanks to @chequille I've narrowed it down to this commit, which is completely wrong, and inside Magento since 2.3.6 The grand total is supposed to be negative as it's not concern of the discounting class to ensure it is greater than zero, as that is later used to calculate correct amounts. Statistically speaking, we've never had problems up to 2.3.5, and now both 2.4.1 and 2.3.6 reports computational problems. Quick solution for us, it's been patching the file directly, as the blame show no further modification was applied to it: https://github.com/magento/magento2/blame/2.4.1/app/code/Magento/Sales/Model/Order/Invoice/Total/Discount.php Here the patch for magento/module-sales: --- Model/Order/Invoice/Total/Discount.php
+++ Model/Order/Invoice/Total/Discount.php
@@ -71,12 +71,12 @@
$invoice->setDiscountAmount(-$totalDiscountAmount);
$invoice->setBaseDiscountAmount(-$baseTotalDiscountAmount);
- $grandTotal = $invoice->getGrandTotal() - $totalDiscountAmount < 0.0001
- ? 0 : $invoice->getGrandTotal() - $totalDiscountAmount;
- $baseGrandTotal = $invoice->getBaseGrandTotal() - $baseTotalDiscountAmount < 0.0001
- ? 0 : $invoice->getBaseGrandTotal() - $baseTotalDiscountAmount;
- $invoice->setGrandTotal($grandTotal);
- $invoice->setBaseGrandTotal($baseGrandTotal);
+ // Lines reverted from 2.3.5 as https://github.com/magento/magento2/commit/c710c9b38e0bcfbaedc6114bc5b4b9e62df95bbf
+ // bogs it up. Ref: https://github.com/magento/magento2/issues/30853
+ //
+ $invoice->setGrandTotal($invoice->getGrandTotal() - $totalDiscountAmount);
+ $invoice->setBaseGrandTotal($invoice->getBaseGrandTotal() - $baseTotalDiscountAmount);
+
return $this;
}
Hope this helps to better identify and put a containment to the original problem (which is how can these commits occur in the first place...) |
@angelomaragna Thanks, I will try this patch. The solution in #31556 does not work for us unfortunately. |
@engcom-Hotel Is there any ETA or progress? |
I can confirm the proposed change in #30853 (comment) works for discounts of 0%, 40%, 50%, 90% and 100% on M2.4.1. While the given patch could not be applied in my case I created a new (identical) one, see #32270 (comment) The here mentioned fix in PR #31556 does work for discounts of 100%, however, not for discounts less than 100% and larger than 50%. (Tested on M2.4.1) |
@engcom-Hotel is there any ETA on this issue? Looking forward to hearing from you! |
This is probably fixed by: MC-41438: 100% Discount cart rule results in Tax invoiced, which I'm guessing is going to be included in Magento 2.4.3 Maybe somebody can confirm this? |
Preconditions (*)
Steps to reproduce (*)
Expected result (*)
Actual result (*)
I'm suspect this bug was introduced in this commit (MC-35675):
2dbab41
It doesn't allow GrandTotals to go below zero, but the Tax Totals will add the discounted Tax, expecting that it was deducted before, resulting in a Invoice which is never smaller than the tax amount (instead of being 0)
Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.
The text was updated successfully, but these errors were encountered: