fix: pricing decimal formatting for tokenized cart #8992
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +47 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
frosso
commented
Jun 20, 2024
Comment on lines
+17
to
+19
| export const transformPrice = ( price, priceObject ) => | ||
| // making sure the decimals are always correctly represented for GooglePay/ApplePay, since they don't allow us to specify the decimals. | ||
| price * 10 ** ( 2 - priceObject.currency_minor_unit ); |
There was a problem hiding this comment.
This is similar to this formatting function: https://github.com/woocommerce/woocommerce/blob/e5fafb455eded795d66ae0f75b2cc8a3a36320ca/plugins/woocommerce-blocks/assets/js/extensions/google-analytics/utils.ts#L59
instead of ({price} / 10^{currency_minor_unit}) * 100, it's simplified to {price} * 10 ^ (2 - {currency_minor_unit}), because 100==10^2.
mdmoore
approved these changes
Jun 21, 2024
There was a problem hiding this comment.
This tests well with various currencies and decimal places. The changes all make sense as well. 🚢
I do have one question that's not really part of this PR. What's the cartData.totals.total_refund that's being subtracted from each amount? Is there some flow a shopper could reach that allows the use of a refund? Just curious really and wasn't able to find this in the original PRB implementation.
In the pay-for-order flow, an order can be created & subsequently updated with refunded amounts (all while the order is still "pending payment"). We have a WooPayments-wide issue I created about this: #8933 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes partially #8373
Changes proposed in this Pull Request
The store's currency number of decimals are not taken into account when providing the data to GooglePay/ApplePay.
We need to ensure we're providing cents to the GooglePay/ApplePay API.
Fixing.
Before:
After:
Testing instructions
2to0(for example - you could also try1)npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge