-
Notifications
You must be signed in to change notification settings - Fork 232
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.
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
Add a section clarifying currency units; specify behavior when no currency is given #388
Add a section clarifying currency units; specify behavior when no currency is given #388
Conversation
@thekidder #379 didn't necessarily introduce a bug of integer-only costs; this has been in the spec/schema since the beginning as the cost fields were defined as USD cents. Your proposal here does highlight some ambiguity. Allowing fractional costs only works if we're defining the cost at the "dollar" level, vs. the "cents/centavos/etc." level. The currency code carries no information about the scale of the value. The other way to do this is to define the cost fields in terms of the 1/100 equivalent ("cent") for the identified currency code. I don't know enough about international currencies to know if this would handle all cases. |
If no scale is given, I would assume the major unit is implied (i.e. dollars for USD). MDS could specify a different scale, but given that different currencies may have different minor units, this is less clear to me. But perhaps keeping integers is a better solution to avoid having money amounts as a floating point value and the resulting potential nondeterminism. As an example, Stripe's API documentation specifies using integers in the "currency's smallest unit": https://stripe.com/docs/currencies#zero-decimal |
The Stripe example you referenced sounds like a good approach - do you want to add that wording into the descriptions? |
Yes, agreed. Will update this shortly. |
@thekidder any update on the the update? |
6bdebbc
to
08b21a1
Compare
08b21a1
to
99e3c36
Compare
Updated the wording here, using Stripe's behavior as an example. There may still be some ambiguity here - I am not a currency expert - but I think this is an improvement. |
Explain pull request
This PR fixes a couple of bugs introduced in #379.
Firstly, the schema only allows integral costs; this is insufficient for many currencies that contain a fractional part (including USD). This PR changes the types of the cost fields to
number
instead ofinteger
. Sincenumber
is a superset ofinteger
, this should not break backwards compatibility.Additionally, the wording does not specify the behavior when the
currency
field is not present. This change clarifies the language to use the old definition (cents of USD) to keep backwards compatibility.Is this a breaking change
Provider
oragency
provider