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.

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

misc(currency) add IRR #1575

Closed
wants to merge 1 commit into from
Closed

misc(currency) add IRR #1575

wants to merge 1 commit into from

Conversation

AnonC0DER
Copy link
Contributor

Adding IRR (Iranian Rial) currency

@ansmonjol
Copy link
Collaborator

Hey @AnonC0DER thanks for adding this new currency.

I made few search, but can you confirm this currency does have a 2 digit precision for the cents?
We have a particular handling for currencies with different digit precision and I would love to make sure we're ok for this one.

@AnonC0DER
Copy link
Contributor Author

Hello @ansmonjol, thank you for checking.

No, all amounts in IRR should be treated as whole numbers without any decimal places.
I wasn't aware there is a specific area for handling these types of currencies.
Could you direct me to it so I can modify the PR?

@ansmonjol
Copy link
Collaborator

Ok I did check those infos

This list that present IRR with 2 decimal, but I agree I could find other resources that says 0 decimal too 😕

But most important, this is the specification of the lib we use on Backend side for all currencies, and IRR shows with "subunit_to_unit": 100,, just like EUR.
JPY that have no decimal is defined as "subunit_to_unit": 1, for example.

Even if that is a specification mistake, we'll have to follow this package definition and handle it as a 2 decimal currency, otherwise it would add buggy behaviour in the app for you (sending amount as 0 decimal but handled as 2 decimal on Backend)

In such case, for 2 decimal currencies, there is no other change to add to your PR!
FTR, this is the place we define currencies precision. Very custom and specific implem, don't feel bad not finding it 😄

Ok, sorry for this long text but we should be all good to merge your PR haha.

Probably the PR's CI will fail because of GH token error, but in such case I'll re-open it and tag you as co-author if you don't mind.

@AnonC0DER
Copy link
Contributor Author

Great!
Thank you for the quick response.

@ansmonjol ansmonjol mentioned this pull request Jul 1, 2024
vincent-pochet added a commit to getlago/lago-api that referenced this pull request Jul 1, 2024
Reopened from #2229 in order to
fix the currency order in the GraphQL schema

Related to getlago/lago-front#1575

---------

Co-authored-by: Hesam Norin <hesam.norin@yahoo.com>
@ansmonjol
Copy link
Collaborator

Re-opened here

Thanks for your contribution @AnonC0DER

@ansmonjol ansmonjol closed this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants