Skip to content

Update the tax rate meta when recalculating and updating order tax items #27985

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 1 commit into from
Mar 19, 2021
Merged

Update the tax rate meta when recalculating and updating order tax items #27985

claudiosanches merged 1 commit into from
Mar 19, 2021

Conversation

@james-allan
Copy link
Contributor

@james-allan james-allan commented Oct 16, 2020 •
edited by claudiosanches
Loading

All Submissions:

Changes proposed in this Pull Request:

Updates the tax line item's rate_percent meta when recalculating order totals on the edit order screen.

How to test the changes in this Pull Request:

  1. Enable taxes and set an applicable 10% tax rate. https://d.pr/i/Rwuijd
  2. Manually create an order with a pending payment status and a simple product. Calculate totals and create order. https://d.pr/i/lzGjRq
  3. Check the rate_percent associated with the order's tax line item. https://d.pr/i/I3Bgq7
  4. Change the tax rate to 5%. https://d.pr/i/wPvs98
  5. Recalculate the order totals using the recalculate button in the edit order line item metabox. The tax amount is recalculated correctly. https://d.pr/i/aF0Ypt
  6. Check the rate_percent associated with the line item. It remains unchanged https://d.pr/i/5TIxaU.

The line this PR adds is the same line used when new taxes are added. This happens here, a couple of lines below,

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 - Update tax rate for existing taxes when recalculating order totals.

@mouligreenlaw
Copy link

mouligreenlaw commented Feb 9, 2021

Hey there,
Is there any update on this?
The user is back with the same issue (3716191-zen) because they have changed the tax rate again.
It would be nice to give them some idea of when a fix may be released ( I understand that that is an impossible question to answer though)

@roykho
Copy link
Contributor

roykho commented Feb 9, 2021

@mouligreenlaw - yes this is already fixed in master. Should be going into 5.1 release. @james-allan - sorry saw this PR late. It didn't have closes PR# and thus was not linked to original issue and was overlooked. It should be fixed already.

@roykho roykho closed this Feb 9, 2021
@roykho roykho deleted the set_rate_id_on_recalculate branch February 9, 2021 22:21
@vedanshujain vedanshujain restored the set_rate_id_on_recalculate branch March 19, 2021 07:41
@vedanshujain
Copy link
Contributor

vedanshujain commented Mar 19, 2021

Reopening because this PR addresses all the other rate props like rate_percent, compound, label, etc, whereas the current fix only addresses updating label.

@vedanshujain vedanshujain reopened this Mar 19, 2021
@vedanshujain vedanshujain requested review from a team and claudiosanches and removed request for a team March 19, 2021 07:42
@github-actions
Copy link
Contributor

github-actions bot commented Mar 19, 2021

📦 Artifacts ready for download!

claudiosanches
claudiosanches approved these changes Mar 19, 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.

Good catch!
Works great.

@claudiosanches claudiosanches added this to the 5.3.0 milestone Mar 19, 2021
@claudiosanches claudiosanches merged commit 9398c89 into master Mar 19, 2021
@claudiosanches claudiosanches deleted the set_rate_id_on_recalculate branch March 19, 2021 15:40
@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 Mar 19, 2021
@claudiosanches
Copy link
Contributor

claudiosanches commented Mar 31, 2021

For some reason it got merged into master and I fixed cherry picking to trunk.

@tammullen tammullen added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Apr 20, 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.3.0

Development

Successfully merging this pull request may close these issues.

8 participants

@james-allan @mouligreenlaw @roykho @vedanshujain @claudiosanches @tammullen @woocommercebot