Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 2397c27 The changes in this PR will be included in the next version bump. This PR includes changesets to release 36 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3950 +/- ##
==========================================
- Coverage 71.95% 71.91% -0.05%
==========================================
Files 263 263
Lines 6797 6801 +4
Branches 2075 2101 +26
==========================================
Hits 4891 4891
- Misses 1885 1889 +4
Partials 21 21
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
ByronDWall
left a comment
ByronDWall
left a comment
There was a problem hiding this comment.
I think that adding the fractionDigits value to the currency data is reasonable. I'm really wary of adding non-ISO-4217 currency codes to the currencies localization data though.
The following two objects describe amounts in the same currency, and so should use the same currency code.
{
"currencyCode": "HUF",
"centAmount": 1235,
"fractionDigits": 0
},
{
"currencyCode": "HUF",
"centAmount": 123500,
"fractionDigits": 2
} Fraction digits is metadata for that currency, and should not modify the currency code itself.
| }; | ||
|
|
||
| // Mapping of currencies that have a 0-fraction-digit variant (same label/symbol, fractionDigits 0). | ||
| export const ZERO_FRACTION_DIGITS_CURRENCY_MAPPING = { |
There was a problem hiding this comment.
My concern is that we are introducing currency codes into our base currency data that are not ISO 4217 currency codes.
As an example, application-project-settings has a currency selector for all project currencies. The methods that map the currency values would return HUF and HUF0 in the dropdown, but would not have any other way for the user to differentiate between the two.
Moreover, any code that passes these currency codes to Intl.NumberFormat, commercetools APIs, payment providers, or any ISO 4217-aware system will fail or behave unexpectedly with HUF0.
The fundamental tension is: this PR is encoding a display/formatting preference (fraction digits) into the currency code itself, but currency codes are meant to identify currencies, not formatting rules.
@nima-ct could you perhaps give a little more context about what your team is trying to achieve here? I think that allowing multiple fraction digits for these currencies makes sense, but it seems to me like it should be handled separately from the currency code itself. Perhaps a project-level setting or formatting option?
… in localization files
… localization files
…SO" designation in localization files
| }; | ||
|
|
||
| // Per-locale suffix for zero-fraction currency label | ||
| const ZERO_DECIMAL_LABEL_SUFFIX_BY_LOCALE = { |
There was a problem hiding this comment.
Additional context has been added for awareness
…ecimal support in localization files
3 participants
Summary
In Application Discounts, we want to support both versions of the same currency with different fraction digits.
For more context:
https://commercetools.atlassian.net/wiki/spaces/MCF/pages/3016556878/Handling+Non-ISO+Currency+Variants
https://commercetools.atlassian.net/wiki/spaces/PAD/pages/2992570378/FO+Lower+Precision+Digits+Frontend+Impact+Proposed+Solution
This PR first updates the generated currency locale data to include
fractionDigits(decimal precision) for each currency. It then creates explicit zero-fraction-digit currency code variants for a selected list of base currencies:CZK → CZK0
HUF → HUF0
TWD → TWD0
TRY → TRY0
ILS → ILS0
KZT → KZT0
Locale-specific files have been regenerated using
pnpm run generate-datahttps://commercetools.atlassian.net/browse/FEC-354