-
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
magento/magento2#12396: Total Amount cart rule without tax #21288
magento/magento2#12396: Total Amount cart rule without tax #21288
Conversation
- Added new condition type to give user opportunity to choose the configuration.
Hi @AleksLi. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
@@ -61,6 +61,7 @@ public function __construct( | |||
public function loadAttributeOptions() | |||
{ | |||
$attributes = [ | |||
'base_subtotal_with_discount' => __('Subtotal (Excl. Tax)'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure that the base_subtotal_with_discount
is the Subtotal Excluding Tax?
Its name (and usage in the code) makes me think that it is not.
Maybe I am wrong, can you help me understand?
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure that that's the correct subtotal to apply a coupon. I've taken it from the collect method which is always running. All that data in any case will be in the Quote.
Hi @aleron75, thank you for the review. |
@magento-engcom-team, @aleron75 . I can't merge the changes, please let me know how to do that, or who is willing to do that. |
Hello, you can't merge your own PR, it will be merged by the repo maintainers. |
Thanks @aleron75 . Now, I will be aware of that. |
✔️ QA Passed |
Hi @AleksLi, thank you for your contribution! |
Why was this ever approved as this was clearly never properly tested... Both "base_subtotal" and "base_subtotal_with_discount" are ALWAYS excluding tax. The only real fix for this problem is adding "base_subtotal_total_incl_tax" as this is correct value. |
@koenner01 unfortunately, only members of the maintainers team are allowed to unassign developers from the pull request |
@koenner01: I can see in our preview access to Magento 2.3.7 that it will be fixed there (and was already fixed before in 2.4.2), just FYI. That info was also provided in this comment. |
Added new condition type to give user opportunity to choose the configuration.
Description (*)
I thought about the fix of what should be fixed in here. But I guess it's up to the user what to choose and what conditions to have to apply the rule.
I decided to add new Cart Attribute to Subtotal (Excl. Tax) which is basically one of the totals base_subtotal_with_discount
Fixed Issues (if relevant)
Manual testing scenarios (*)
Contribution checklist (*)