feat: Add crypto currency support with global toggle #401
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
193878d to
1a371a5
Compare
davidpatters0n
commented
May 14, 2025
9047b40 to
c64f55d
Compare
elfassy
reviewed
May 14, 2025
elfassy
reviewed
May 14, 2025
Add support for cryptocurrencies with USDC as the first implementation. This feature is opt-in and disabled by default. - Add crypto_currencies config option (default: false) - Add crypto.yml for cryptocurrency definitions - Implement crypto currency lookup in Currency class - Add load_crypto_currencies method to Currency::Loader - Add test - Update README with usage documentationt
c44bcb1 to
f86bbe0
Compare
davidpatters0n
commented
May 15, 2025
| disambiguate_symbol: USDC | ||
| alternate_symbols: [] | ||
| subunit: Cent | ||
| subunit_to_unit: 100 |
There was a problem hiding this comment.
Why two decimal places see; #398 (comment) for more context. In addition if there is a need to override we can do as mentioned on the same comment.
antw
reviewed
May 15, 2025
lib/money/currency/loader.rb
Outdated
| @@ -5,14 +5,20 @@ | |||
| class Money | |||
| class Currency | |||
| module Loader | |||
| CURRENCY_DATA_PATH = File.expand_path("../../../../config", __FILE__) | |||
There was a problem hiding this comment.
nit:
Suggested change
| CURRENCY_DATA_PATH = File.expand_path("../../../../config", __FILE__) | |
| CURRENCY_DATA_PATH = File.expand_path("../../../config", __dir__) |
There was a problem hiding this comment.
Addressed 👍🏽
spec/currency_spec.rb
Outdated
| expect(currency.iso_code).to eq('USDC') | ||
| expect(currency.symbol).to eq('USDC') | ||
|
|
||
| Money.config = old_config |
There was a problem hiding this comment.
question: Does this been to be in an ensure block?
If the test fails I'm assuming the config won't be restored to the previous state and the custom config might affect other tests. I'm guessing we need something like:
old_config = Money.config
begin
# the test
ensure
Money.config = old_config
endI wonder if it's worth adding a Edit: My suggestion isn't thread-safe, and refactoring with_config method here?Money.config to be thread-safe is likely not worthwhile. Maybe a test helper?
def with_config(config)
old_config = Money.config
Money.config = config
yield
ensure
Money.config = old_config
end
spec/spec_helper.rb
Outdated
| old_config = Money.config | ||
| Money.config = Money::Config.new.tap do |config| | ||
| config.default_currency = default_currency if default_currency | ||
| config.legacy_json_format! if legacy_json_format | ||
| config.legacy_deprecations! if legacy_deprecations | ||
| config.legacy_default_currency! if legacy_default_currency | ||
| config.crypto_currencies = true if crypto_currencies |
There was a problem hiding this comment.
nit:
Suggested change
| config.crypto_currencies = true if crypto_currencies | |
| config.crypto_currencies = crypto_currencies unless crypto_currencies.nil? |
There was a problem hiding this comment.
Shouldn't this be a boolean and then just config.crypto_currencies = crypto_currencies?
robinbrandt
reviewed
May 15, 2025
spec/spec_helper.rb
Outdated
| old_config = Money.config | ||
| Money.config = Money::Config.new.tap do |config| | ||
| config.default_currency = default_currency if default_currency | ||
| config.legacy_json_format! if legacy_json_format | ||
| config.legacy_deprecations! if legacy_deprecations | ||
| config.legacy_default_currency! if legacy_default_currency | ||
| config.crypto_currencies = true if crypto_currencies |
There was a problem hiding this comment.
Shouldn't this be a boolean and then just config.crypto_currencies = crypto_currencies?
robinbrandt
approved these changes
May 15, 2025
antw
reviewed
May 15, 2025
e41a0a0 to
cdb107a
Compare
antw
approved these changes
May 15, 2025
elfassy
approved these changes
May 15, 2025
90c56f8 to
716b09d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for cryptocurrencies to the Money gem, starting with USDC stablecoin. It's implemented through a clean configuration system that makes crypto handling seamless while maintaining backward compatibility.
Changes
Breaking changes
None! The feature is opt-in and disabled by default, so existing code will continue to work without any changes.
Configuration System
Before
After
Example Usage