This repository has been archived by the owner on Feb 23, 2024. It is now read-only.
Fix: Handle taxes for local pickup shipping #6631
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rubikuserbot
requested review from
a team and
nielslange
and removed request for
a team
senadir
added
type: bug
Remove custom local pickup handling once WooCommerce 6.8....Remove custom local pickup handling once WooCommerce 6.8.0 is the minimum version.
woocommerce-blocks/src/StoreApi/Utilities/OrderController.php Lines 443 to 455 in 56e4b9a 🚀 This comment was generated by the automations bot based on a
|
senadir
commented
Jun 26, 2022
| @@ -44,6 +44,9 @@ public function create_order_from_cart() { | |||
| * @param \WC_Order $order The order object to update. | |||
| */ | |||
| public function update_order_from_cart( \WC_Order $order ) { | |||
| // Ensures Local pickups are accounted for. | |||
| add_filter( 'woocommerce_order_get_tax_location', array( $this, 'handle_local_pickup_taxes' ) ); | |||
There was a problem hiding this comment.
We want to limit where we call this logic, this function matters because this is where we call calculate_totals.
senadir
commented
Jun 26, 2022
Comment on lines
+440
to
+456
| /** | ||
| * Passes the correct base for local pick orders | ||
| * | ||
| * @todo: Remove custom local pickup handling once WooCommerce 6.8.0 is the minimum version. | ||
| * | ||
| * @param array $location Taxes location. | ||
| * @return array updated location that accounts for local pickup. | ||
| */ | ||
| public function handle_local_pickup_taxes( $location ) { | ||
| $customer = wc()->customer; | ||
|
|
||
| if ( ! empty( $customer ) ) { | ||
| return $customer->get_taxable_address(); | ||
| } | ||
|
|
||
| return $location; | ||
| } |
There was a problem hiding this comment.
get_tax_location has a filter in which we're using. In this case, we check if we have a user (we always do at the submit point) and get the address from there (which accounts for local pickup).
|
Size Change: 0 B Total Size: 874 kB ℹ️ View Unchanged
|
tarhi-saad
approved these changes
Jun 27, 2022
There was a problem hiding this comment.
Thank you, @senadir, for fixing the local pickup tax calculation bug! You've done an excellent job! I've done multiple tests, including the above user-facing test, and all passed successfully! 🎉 So, it's looking good to me. I think we can ship it. But, please, @nielslange, if you can look into this as well, it would be fantastic! 🙌
EDIT:
Thank you, @nielslange, for looking into this PR!! I will merge it.
nielslange
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an issue in which taxes are calculated against the user address for local pickups orders. A niche issue, but an important one.
Fixes #6480
Why did this issue happen in Store API
Shortcode Checkout has a function called
process_checkoutthat handles submitting an order, that functions passes$_POSTdata around, it also creates an order just when you finish an order. Store API can't that function, because we can't support$_POSTvia a rest api, and because we create an order early on.For this purpose, Store API has its own
process_checkoutfunction, theget_route_post_responsefunction of /checkout route.WooCommerce Shortcode also calculates the order totals using
wc()->cart->calculate_totalsit then copies that result to the created order.Store API augments the order with all correct informations, than calls
WC_Order::calculate_totals.The issue here is that those two functions are slightly different, exactly around how they calculate taxes.
The cart version decides on what address it should calculate taxes using the
WC_Customer::get_taxable_addressfunction.The order version uses
WC_Abstract_Order::get_tax_location.The problem stems from the fact that
WC_Customer::get_taxable_addresschecks if the cart has local pickup selected (in which it defaults tobaselocation), butWC_Abstract_Order::get_tax_locationdoesn't do any of that, and just checks the value ofwoocommerce_tax_based_onsetting.This is a long standing issue, that is simply hard to discover because most Checkout block users. who have local pickup probably have their customers locations in the same place as the shop, so the taxes are the same.
Good news is that @iamdharmesh discovered the same issue from WooCommerce Admin side and fixed the issue in WooCommerce core woocommerce/woocommerce#33330, the fix will land in WooCommerce Core 6.8.0 which is still 3 months away for us (we do L-1 for WC core when we can).
This PR does a workaround by calling
WC_Customer::get_taxable_addressinstead, and we should remove it once WC Core 6.8.0 is the min version.Testing
User Facing Testing
123 Test Street, 90210 Beverly Hills, CA, USA"Standard" tax ratesin US, CA. Set it to:10%60 29th Street #343, 35005 birmingham, AL, USA)Local pickupshipping method. Check that we have a tax of 10% applied.Local pickupshipping method. Check that we have a tax of 10% applied.Order receivedpage that the displayed tax is actually added to the total.Edit Order. Check that taxes were applied correctly there as well.Changelog