-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[Intl] Add methods to filter currencies more precisely #61431
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
base: 7.4
Are you sure you want to change the base?
Conversation
Hey! To help keep things organized, we don't allow "Draft" pull requests. Could you please click the "ready for review" button or close this PR and open a new one when you are done? Note that a pull request does not have to be "perfect" or "ready for merge" when you first open it. We just want it to be ready for a first review. Cheers! Carsonbot |
Currencies::isActive()
to ensure the currency is active in at least 1 countryCurrencies::isActive()
to ensure the currency is active in at least 1 country
I think it's pretty much ready for a first review @javiereguiluz. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps an alternative API could be getCurrencyCodesForCountry(string $country, ?bool $active = null)
+ existsForCountry(string $country, ?bool $active = null)
?
I will add this ASAP 👍🏻 |
I added the requested methods + tender filter but I need to add fews tests to ensure all uses cases are covered. I also need to update the CHANGELOG. I'll continue this ASAP. |
Currencies::isActive()
to ensure the currency is active in at least 1 country…cases to achieve 100% code coverage
I’ve applied the requested changes, this should be ready for a second review. |
$regions = iterator_to_array($supplementalDataBundle['CurrencyMap']); | ||
|
||
foreach ($regions as $regionId => $region) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just foreach ($supplementalDataBundle['CurrencyMap']
?
Description
In the ICU dataset, there are values that are no longer relevant / no longer used today (e.g. the BEF currency). This PR add a method that check the metadata in the ICU metadata to check if the currency is active in at least 1 country.
Limitations
$active
).