Skip to content
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

feature/currency-change-bug #367

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

feature/currency-change-bug #367

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a8882e7
fix: error while changing country
prachigarg19 Jan 17, 2023
5b0ca60
Merge branch 'develop' into feature/currency-change-bug
prachigarg19 Jan 17, 2023
4c7b72e
refactor: resolve es lint errors
prachigarg19 Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Utils/stripe/stripeHelpers.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export function formatAmountForStripe(
style: "currency",
currency,
currencyDisplay: "symbol",
maximumSignificantDigits: 21,
Copy link
Collaborator

@norbertschuler norbertschuler Jan 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change leads to returning an amount of 200 for e.g. 200€ (in fact all amounts having no decimal values) instead of 20000 (amount calculated in sub currency "Cent") as before. So probably only 2€ would get billed using native payments as Google or Apple Pay now, compare https://stripe.com/docs/js/appendix/payment_item_object.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As some currencies do not return a decimal part for numberFormat.formatToParts(amount), we might should consider rounding the result every time for this method, e.g. return zeroDecimalCurrency ? Math.round(amount) : Math.round(amount * 100);, but I am not sure if for AFN we would then try to bill factor 100 less then expected. We should clarify how to deal correctly with currencies like Afghani or Japanese yen in this method.

Copy link
Contributor

@mohitb35 mohitb35 Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@norbertschuler I wonder if we should use formatToParts to detect if the currency is a zero decimal currency. Sometimes, the results aren't as expected.

For example, stripe does not consider AFN a zero decimal currency, but the previous logic does.

const numberFormat = new Intl.NumberFormat(["en-US"], {
    style: "currency",
    currency,
    currencyDisplay: "symbol",
  });
  const parts = numberFormat.formatToParts(amount);
  let zeroDecimalCurrency = true;

  parts.forEach((part) => {
    if (part.type === "decimal") {
      zeroDecimalCurrency = false;
    }
  });
  return zeroDecimalCurrency ? amount : Math.round(amount * 100); //zeroDecimalCurrency is true for AFN.

Would it be better to keep a list of currency exceptions that are handled separately, and do Math.round(amount * 100) for the rest?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that if Stripe and Intl.NumberFormat differ they should not be used together to decide how to format the amount for Stripe.

});
const parts = numberFormat.formatToParts(amount);
let zeroDecimalCurrency = true;
Expand Down