Skip to content

Conversation

@Konamiman
Copy link
Contributor

@Konamiman Konamiman commented Jul 11, 2022

All Submissions:

Changes proposed in this Pull Request:

Fixes inconsistent order total on checkout vs manual order page when the shop is configured for tax-inclusive prices and a coupon is applied. Also refactors the add_coupon_discount and calc_line_taxes methods in the WC_AJAX class to make the code more unit testable.

Fix originally submitted by @Sidsector9 in #32010, work redone in a new pull request in order to easily apply ther required refactoring.

Note for reviewers: It's obscured by the refactor, but the main change in this pull request is executing $order->calculate_taxes and $order->calculate_totals before $order->apply_coupon, instead of after, inside add_coupon_discount.

Closes #31971.

How to test the changes in this Pull Request:

See #31971 and #32010

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?
  • Have you created a changelog file for each project being changed, ie pnpm changelog add --filter=<project>?

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.

Sidsector9 and others added 3 commits July 11, 2022 10:37
The methods are moved to separate classes in src/Internal,
and two new "core" methods that exclude HTTP processing are added.
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jul 11, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 11, 2022

Test Results Summary

Commit SHA: fbfbd6f

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests11500201170m 40s
E2E Tests185001018615m 23s
To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@Konamiman Konamiman changed the title Fix/31971 Fix inconsistent order total on checkout vs manual order page Jul 15, 2022
@Konamiman Konamiman marked this pull request as ready for review July 15, 2022 15:51
@Konamiman Konamiman requested review from a team and vedanshujain and removed request for a team July 15, 2022 15:51
@jeffpaul
Copy link
Contributor

@vedanshujain checking in to see if you've got any code review feedback on this PR (as it will help unblock some downstream work in the EU VAT extension)?

Copy link
Contributor

@vedanshujain vedanshujain left a comment

Choose a reason for hiding this comment

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

LGTM!

@vedanshujain vedanshujain merged commit 5f257ed into trunk Aug 17, 2022
@vedanshujain vedanshujain deleted the fix/31971 branch August 17, 2022 09:26
@github-actions github-actions bot added this to the 6.9.0 milestone Aug 17, 2022
@github-actions
Copy link
Contributor

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

  • Add the release: add testing instructions label

vedanshujain pushed a commit that referenced this pull request Aug 18, 2022
* fix: tax calculation and coupon sequence

* add phpunit skeleton

* Refactor calc_line_taxes and add_coupon_discount

The methods are moved to separate classes in src/Internal,
and two new "core" methods that exclude HTTP processing are added.

* The test partially passes now

* Final fix to unit tests, and fix remaining formatting issues

* Add changelog file

* Fix path to html-order-items.php

Co-authored-by: Siddharth Thevaril <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent order total on Checkout vs manual order page

5 participants