Skip to content

Conversation

@Konamiman
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

#30692 introduced a change in the wc_get_price_excluding_tax function: if the woocommerce_adjust_non_base_location_prices is set to return false, the function is supposed to take the location of the customer from the order for the taxes calculation, but wasn't doing so when the order was being created directly from the admin area because no customer was being passed to WC_Tax::get_rates; thus, the location from the currently logged in user was being used instead. With the fix from that PR, the order was being passed to woocommerce_adjust_non_base_location_prices and its customer location was used for the taxes calculation.

The problem is that a newly created order with no customer whatsowever has actually a "null customer" with a zero customer id. The code was taking that to calculate taxes, which resulted in no taxes being deducted; adding a new line item would result in the full product price being added, with taxes; and pressing "Recalculate" would add the taxes again.

This PR fixes that: if an order is passed to woocommerce_adjust_non_base_location_prices, a check is done for the cusomer id, and if it's 0 then the shop base location is used for the tax calculation as a fallback.

Closes #31013.

How to test the changes in this Pull Request:

Note: These instructions are an extended version of the ones for #30692.

  1. Add the following snippet to your store: add_filter( 'woocommerce_adjust_non_base_location_prices', '__return_false' );
  2. Make sure that you have the "Prices entered with tax" option set as "Yes, I will enter prices inclusive of tax" (WooCommerce - Settings - Tax - Tax options).
  3. Let A be the country where your currently logged in user is based, and B the country where the shop is based. Create an user U based on a different country C.
  4. Go to WooCommerce - Settings - Tax - Standard rates and set 30% for country A, 20% for country B and 10% for country C.
  5. Create a simple product, taxable with standard rates, with a price of 110.
  6. Create an order from admin area, don't set any customer for it and save it ("Create" button in "Order actions").
  7. Add the product you just created to the order. You should see that the price that will appear in the line item is 91,67 (110 minus 20% of taxes), and if you click "Recalculate", the tax for country B is added and the final order price is 110.
  8. Remove the line item from the order
  9. Set U as the order customer and save the order ("Update" button in "Order actions").
  10. Add the same product to the order again. This time the price that will appear in the line item is 100 (110 minus 10% of taxes), and if you click "Recalculate", the tax for country C is added and the final order price is again 110.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

N/A (it's a fix for #30692, which was included in 5.9 beta)

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

PR #30692 modified 'wc_get_price_excluding_tax' so that if an order
is passed its customer will be passed to WC_Tax::get_rates in order
to use the proper location for the taxes to be discounted. The problem
is that when the order has no customer (it's "Guest") an invalid
customer (id=0) is passed, which has no location, and thus no taxes
are deducted whatsoever.

The fix consists of checking if the customer id from the order is 0,
and in that case no customer is passed to WC_Tax::get_rates, thus
the shop location is used for the taxes.
…g_tax

Modify wc_get_price_excluding_tax so that when there's no user
available from a passed order and the
'woocommerce_adjust_non_base_location_prices' filter returns false,
the shop base location is used for the tax calculation instead of
the location of the currently logged in user.
@Konamiman Konamiman requested a review from peterfabian October 22, 2021 09:23
Copy link
Contributor

@peterfabian peterfabian left a comment

Choose a reason for hiding this comment

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

Side note for future myself:

  • woocommerce_adjust_non_base_location_prices filter set to false means that all customers irrespective of their billing/shipping country should pay the same price incl tax.

Overall, I think it works fine, although the UX is not super nice since the merchant needs to save/update orders in between.

Great job with those tests, I think it should cover all the eventualities.

@Konamiman Konamiman added this to the 5.9.0 milestone Oct 25, 2021
@Konamiman Konamiman merged commit 50d7553 into release/5.9 Oct 25, 2021
@Konamiman Konamiman deleted the fix/get_price_excluding_tax_with_zero_customer_id branch October 25, 2021 06:59
@Konamiman Konamiman added release: cherry-pick Commits from this PR also needs to be added to current release branch. release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Oct 25, 2021
@github-actions
Copy link
Contributor

Hi @Konamiman, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the status: needs changelog label
  • Add the status: needs testing instructions label

@Konamiman
Copy link
Contributor Author

Merged into release/5.9 for inclusion in WooCommerce 5.9 RC. Needs to be cherry-picked into trunk.

@Konamiman Konamiman added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Oct 25, 2021
@Konamiman Konamiman added cherry picked and removed release: cherry-pick Commits from this PR also needs to be added to current release branch. labels Nov 5, 2021
@Konamiman
Copy link
Contributor Author

Cherry-picked into trunk for inclusion in 6.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants