Skip to content

Use tax location from order while computing tax in discount. #26983

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.

Sign up for GitHub

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

Jump to bottom
Merged
claudiosanches merged 4 commits into from
Jan 28, 2021
Merged

Use tax location from order while computing tax in discount. #26983

claudiosanches merged 4 commits into from
Jan 28, 2021

Conversation

@vedanshujain
Copy link
Contributor

@vedanshujain vedanshujain commented Jul 8, 2020 •
edited
Loading

All Submissions:

Changes proposed in this Pull Request:

We were not passing tax location while computing discount in orders, hence it was defaulting to the shop's base address resulting in incorrect tax calculation.

This was causing #25082 since the order's address is not passed, it was defaulting to address with what current admin user had.

This commit refactors get_rates method into another method that allows getting rates from the location directly.

Closes #25082.

How to test the changes in this Pull Request:

  1. Set price inclusive of taxes and any rate for a specific country other than your shop base address. For example, shop's base address could be US, then enter prices for India (@25%), make sure that there is no other tax applied like so:
    Screenshot 2020-07-09 at 1 48 47 AM
  2. Add a coupon for 10% and add a product for a price of 242. (This could be any price -- if it's different then modify numbers in next step accordingly).
  3. Complete an order for this product as a guest user (in an incognito window). Make sure to use Indian address
  4. Open this order in the admin view, set the order status to Pending Payment and update.
  5. Add the coupon, without this patch, discount will be calculated on the final amount instead of pre-tax amount (so the discount will be more than it should be). With this patch, the discount will be calculated before taxes are applied.
    In above case, after the patch, the discount should be 19.36 (193.6 @ 10%), whereas before this patch discount will be 24.20 (242.2 @ 10%, which is incorrect as in this case, discount on tax will be applied twice).

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

Fix - Calculate discount based on order location in the admin view.

@vedanshujain vedanshujain added status: in progress This is being worked on. status: needs review and removed status: in progress This is being worked on. labels Jul 8, 2020
claudiosanches
claudiosanches suggested changes Jul 21, 2020
View reviewed changes
@rrennick
Copy link
Contributor

rrennick commented Jul 23, 2020

@vedanshujain This might not be related to this PR but some of the tax calculations remain local to the shop. Shop is in NB (15% HST). Customer is in the US (10%).
Screen Shot 2020-07-23 at 10 45 58 AM

@rrennick rrennick added needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. and removed status: needs review labels Jul 24, 2020
@vedanshujain
Copy link
Contributor Author

vedanshujain commented Aug 7, 2020

@rrennick Sorry for the late reply, this somehow skipped me. Can you please post more detailed steps, I tried creating a manual order with a guest customer, and always rates depending upon customer's billing address are being picked.

@rrennick
Copy link
Contributor

rrennick commented Aug 7, 2020

I tried creating a manual order with a guest customer

Was that from the dashboard or an checkout in an incognito window?

@vedanshujain
Copy link
Contributor Author

vedanshujain commented Aug 7, 2020

Was that from the dashboard or an checkout in an incognito window?

This is manual order from dashboard using admin account, but creating order for a guest user.

@rrennick
Copy link
Contributor

rrennick commented Aug 13, 2020

This is manual order from dashboard using admin account, but creating order for a guest user.

I was following the testing instructions.

@vedanshujain
Use tax location from order while computing tax in discount.
74de152 
We were not passing tax location while computing discount in orders, hence it was defaulting to shop's base address resulting in incorrect tax calculation.

This commit refactors `get_rates` method into another method that allows getting rates from location directly.
@vedanshujain
Add return param comment
24a69fd
@vedanshujain
Add method to get rates based on order.
00b7b40 
This method will prioritize getting rates from billing/shipping address instead of `WC()->customer` which in irrevelant in context of editing order from admin screen.
@vedanshujain
Add unit test
a2231c4
@vedanshujain
Copy link
Contributor Author

vedanshujain commented Nov 14, 2020

@rrennick I am not able to reproduce the issue you are mentioning, I always seeing the correct taxes applied with this fix. Perhaps some setting somewhere is taking the base shop tax?

@vedanshujain vedanshujain requested review from a team and claudiosanches and removed request for ObliviousHarmony and claudiosanches November 14, 2020 06:22
@rrennick
Copy link
Contributor

rrennick commented Dec 3, 2020

I am not able to reproduce the issue you are mentioning

@vedanshujain Sorry for the delay in responding. I'm not seeing the issue now either.

@vedanshujain vedanshujain removed the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Dec 8, 2020
claudiosanches
claudiosanches approved these changes Jan 28, 2021
View reviewed changes
Copy link
Contributor

@claudiosanches claudiosanches left a comment

Choose a reason for hiding this comment

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

Great work!

@claudiosanches claudiosanches merged commit 6701ce9 into master Jan 28, 2021
@claudiosanches claudiosanches deleted the fix/25082 branch January 28, 2021 20:00
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Jan 28, 2021
@claudiosanches claudiosanches added this to the 5.1.0 milestone Jan 28, 2021
@zhongruige zhongruige added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@claudiosanches claudiosanches claudiosanches approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release: add changelog Mark all PRs that have not had their changelog entries added. [auto]

Projects

None yet

Milestone

5.1.0

Development

Successfully merging this pull request may close these issues.

Coupons manually added to orders via the admin are calculating discounts based on logged in user's shipping address

7 participants

@vedanshujain @rrennick @claudiosanches @Konamiman @woocommercebot @zhongruige