Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
166 stars 67 forks source link

Balances in Payments > Overview change depending on Woo store location settings #6402

Open csmcneill opened 1 year ago

csmcneill commented 1 year ago

Describe the bug

If a store's location in WooCommerce > Settings > General is set to a Country / State that has a unique way of handling decimal points (e.g., Kuwait uses a , separator and three decimal places for Kuwaiti Dinar; Japan uses no decimal separator or decimal places for Japanese Yen), this will affect how the Available funds and Pending funds appear at Payments > Overview.

To Reproduce

  1. Log into a site that has had some live or test payments.
  2. Navigate to Payments > Overview.
  3. Take note of how the Available funds and Pending funds appear on the page.
  4. Navigate to WooCommerce > Settings > General.
  5. Change the Country / State to a region with non-decimal (Japan, Yemen) or three-decimal (Bahrain, Kuwait) display.
  6. Save these settings.
  7. Repeat steps 2 and 3.
  8. Repeat steps 5, 6, and 7 as necessary.

Actual behavior

Since some regions use multiple decimal places and different decimal separators, this can create an issue where €-1.85 can appear as though it is €-1,850—which makes it seem like the value is 1,000 times the expected amount.

This can create a clash between store locale and multicurrency.

Screenshots

Screenshot 2023-05-29 at 9 46 27 PM (1)

Markup on 2023-05-29 at 22_59_22

Markup on 2023-05-29 at 23_04_08

Expected behavior

Store location settings will not impact the display of deposit amounts so drastically. By displaying this information in a way that is more in line with the expectations of that particular currency (instead of the location of the store), it will make understanding deposit amounts much more clear.

Additional context

6341895-zen p1685411458115359-slack-C03EE4CPPK5

csmcneill commented 1 year ago

Flagging to @Automattic/helix. I discussed this at length with Rua, but is this on the team's radar?

Jinksi commented 1 year ago

Thanks for the ping @csmcneill. I've marked this as a high-priority maintenance issue for Helix.

Jinksi commented 1 year ago

This bug seems to be WCPay-wide, which makes sense, as we'd be using the client/utils/currency/formatCurrency() function when rendering currency amounts.

Transaction details screen: image

Account Balances card: image

WC Analytics, however, displays the correct format: image

Jinksi commented 1 year ago

this can create an issue where €-1.85 can appear as though it is €-1,850 — which makes it seem like the value is 1,000 times the expected amount.

@csmcneill I've discovered that this is the expected behaviour and was introduced in #3971.

As per the WCPay currency formatting guidelines (paJDYF-20W-p2, June 2021):

In the merchant’s admin we should always render non-store currencies using the store currency format. This will help a merchant scan and comprehend prices more quickly by displaying them in the format they’re most familiar with. It will also create consistency when viewing currencies with different formats.

Example: A US based merchant has enabled Euros as a non-store currency. On the front end of their store, a German customer would see a price of one thousand euros formated as 1.000,00 €. However, in the store admin, the merchant would see the same price rendered as €1,000.00.

So I’m interpreting this as – a merchant in Kuwait expects to see €1.96 displayed as 1,960 €.

Should I close this issue as "feature, not a bug"?

csmcneill commented 1 year ago

Should I close this issue as "feature, not a bug"?

I'm not sure. I read the following a bit differently:

In the merchant’s admin we should always render non-store currencies using the store currency format.

To me, this means that if a store's default is set to EUR, then any non-store currencies should adhere to the EUR format—not to the format of the region in which they are located.

As a hypothetical, if a store's default currency is set to USD (which has a $xx.xx format) but they are based in Japan (which is a zero-decimal currency), I would expect for EUR to show up as €xx.xx to be in line with the store's default currency and not €xx to adhere to the currency format of JPY.

Jinksi commented 1 year ago

Ah, I see your point @csmcneill, thanks.

Since this issue is not limited to deposits and affects currency formatting throughout WCPay admin pages, @Automattic/sigma would this issue be suitable for you to investigate?

jessepearson commented 1 year ago

I think this is going to need to turn into a P2 discussion with stakeholders making the decision on what formatting is going to be the correct one.

As mentioned above, when Multi-Currency was added, we were given the spec that backend currencies should be in the format of the store currency to allow merchants to quickly be able to comprehend what they were looking at. At the time of the MC project, it appears that #2251 was opened to change which currencies and formatting were being used for WCPay, but this issue was not related to the MC project. #2246 was related to the MC project and added currencies and formatting into WCPay and WooCommerce.

Close to a year after #2251 was created, #3948 was created to close it, and it changed how currencies were formatted in the admin.

It's now a year later and the formatting in the admin is coming up in question again, which is why I am suggesting this needs to go to P2 and allow stakeholders make a decision for how formatting should be in the admin for merchants. We need to make sure we have a defined format so that merchants are not caught off guard by changes to the formatting they may have become accustomed to over the course of months or years.

I will say, as someone from the US, that seeing 44,000 $ does make me think that it is 44 thousand dollars, but I also have to remember that for some countries they use commas as their decimal points and periods as thousand separators.

zmaglica commented 5 months ago

This issue impacts Multicurrency, so assigning to team Fractal (based on team responsibilities) @bborman22 . Assigning as part of Gamma Triage process PcreKM-yM-p2