Skip to content

Conversation

@Sidsector9
Copy link
Contributor

@Sidsector9 Sidsector9 commented Mar 2, 2022

All Submissions:

Changes proposed in this Pull Request:

This PR fixes the order in which the coupon is applied.

Closes #31971

How to test the changes in this Pull Request:

Please follow the instruction provided in the issue.

I have tested 2 (P)roducts, 3 Standard Tax rates (inclusive/exclusive) and 2 (C)oupons with the following sequences in the order page and the results have been consistent.

  • P1 - C1 - Recalculate - P2 - C2
  • P1 - P2 - Recalculate - C1 - C2
  • P1 - P2 - C1 - C2 - Recalculate
  • P1 - C1 - P2 - C2 - Recalculate

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: order total inconsistency between checkout and manual order pages.

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.

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Mar 2, 2022
@jeffpaul
Copy link
Contributor

jeffpaul commented Mar 2, 2022

@barryhughes @swatipawarGS note that this PR is up for review, please assign reviewer(s) as necessary, thanks!

@barryhughes barryhughes requested review from a team and jeffstieler and removed request for a team March 3, 2022 20:43
@jeffstieler jeffstieler requested review from a team and vedanshujain and removed request for a team and jeffstieler March 3, 2022 23:42
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.

Looks good, great work! Can you also add a unit test for this please, these kind of issues are pretty tricky to solve, and we can easily end up accidentally breaking this fix in the future if it's not covered by automated tests.

@jeffpaul
Copy link
Contributor

jeffpaul commented Mar 7, 2022

@Sidsector9 note the request for a unit test to help cover this, thanks!

@Sidsector9
Copy link
Contributor Author

Hi @vedanshujain

I am facing issues while writing tests. In the first attempt, I was able to write the test case.
After running the test individually a few times, the following started to happen:

Screen Shot 2022-04-14 at 12 58 32 PM

I confirmed that the site was available. Visiting localhost opened the WordPress site but yet the "waiting for testing environment" error was shown.

To fix this, I deleted everything and attempted to write from scratch. But now I see this error:

Screen Shot 2022-04-14 at 10 39 57 PM

I am not sure what the problem is. I followed the exact steps outlined here.

Any idea how to proceed?

@vedanshujain
Copy link
Contributor

hey @Sidsector9, please write a php unit test for this instead of an end-to-end test. PHPUnit test should be relatively much simpler. See this file on instructions for setting it up: https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/tests/README.md

You would probably want to add your test to this file: https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/tests/php/includes/class-wc-ajax-test.php . Some other examples of calculation-based tests - https://github.com/woocommerce/woocommerce/blob/trunk/plugins/woocommerce/tests/php/includes/class-wc-cart-totals-test.php.

Don't hesitate to ask if there are any issues in setup, PHPUnit is generally used internally so it's quite possible that there are gaps in documentation.

@Sidsector9
Copy link
Contributor Author

@vedanshujain thanks for the guidance.

I wrote the skeleton of the test:

  • Created a Product
  • Created a Coupon
  • Created an Order

Where I am stuck at is testing these AJAX calls.

I don't have much experience with testing Ajax calls in unit tests. I saw a similar test in the same file but the method being tested is refactored for better unit testing.

My aim for this test is that the Order Total should be same when these 2 methods are called in any order:

  • WC_AJAX::calc_line_taxes()
  • WC_AJAX::add_coupon_discount()

How can I achieve this?

@vedanshujain
Copy link
Contributor

hey @Sidsector9

Are you having issues with the post request? Feel free to refactor the original methods if they help in better testing, it will likely be more work than this issue scopes.

If you want to continue testing without refactoring, then perhaps you can write two different tests, and pass the required data in a POST variable from the test? Although it might be easier to refactor the parts where it does not interact with the POST variables into different methods and test them instead.

@Konamiman Konamiman added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Jul 11, 2022
@Sidsector9
Copy link
Contributor Author

Closing in favour of #33812

@Sidsector9 Sidsector9 closed this Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. 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

4 participants