diff --git a/app/models/concerns/currency_normalizable.rb b/app/models/concerns/currency_normalizable.rb index 3d1f62331..743392912 100644 --- a/app/models/concerns/currency_normalizable.rb +++ b/app/models/concerns/currency_normalizable.rb @@ -1,10 +1,10 @@ # Provides currency normalization and validation for provider data imports # # This concern provides a shared method to parse and normalize currency codes -# from external providers (Plaid, SimpleFIN, LunchFlow), ensuring: +# from external providers (Plaid, SimpleFIN, LunchFlow, Enable Banking), ensuring: # - Consistent uppercase formatting (e.g., "eur" -> "EUR") -# - Validation of 3-letter ISO currency codes -# - Proper handling of nil, empty, and invalid values +# - Validation against Money gem's known currencies (not just 3-letter format) +# - Proper handling of nil, empty, and invalid values (e.g., "XXX") # # Usage: # include CurrencyNormalizable @@ -23,6 +23,7 @@ module CurrencyNormalizable # parse_currency("usd") # => "USD" # parse_currency("EUR") # => "EUR" # parse_currency(" gbp ") # => "GBP" + # parse_currency("XXX") # => nil (not a valid Money currency) # parse_currency("invalid") # => nil (logs warning) # parse_currency(nil) # => nil # parse_currency("") # => nil @@ -33,8 +34,15 @@ module CurrencyNormalizable # Normalize to uppercase 3-letter code normalized = currency_value.to_s.strip.upcase - # Validate it's a reasonable currency code (3 letters) - if normalized.match?(/\A[A-Z]{3}\z/) + # Validate it's a 3-letter format first + unless normalized.match?(/\A[A-Z]{3}\z/) + log_invalid_currency(currency_value) + return nil + end + + # Validate against Money gem's known currencies + # This catches codes like "XXX" which are 3 letters but not valid for monetary operations + if valid_money_currency?(normalized) normalized else log_invalid_currency(currency_value) @@ -42,6 +50,17 @@ module CurrencyNormalizable end end + # Check if a currency code is valid in the Money gem + # + # @param code [String] Uppercase 3-letter currency code + # @return [Boolean] true if the Money gem recognizes this currency + def valid_money_currency?(code) + Money::Currency.new(code) + true + rescue Money::Currency::UnknownCurrencyError + false + end + # Log warning for invalid currency codes # Override this method in including classes to provide context-specific logging def log_invalid_currency(currency_value) diff --git a/db/migrate/20260108000000_cleanup_orphaned_currency_balances.rb b/db/migrate/20260108000000_cleanup_orphaned_currency_balances.rb index c481723aa..e493e0578 100644 --- a/db/migrate/20260108000000_cleanup_orphaned_currency_balances.rb +++ b/db/migrate/20260108000000_cleanup_orphaned_currency_balances.rb @@ -9,6 +9,9 @@ # going forward, but existing data needs to be cleaned up. class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2] def up + # Skip in test environment with empty database (CI) + return say "Skipping in test environment - no data to clean" if Rails.env.test? && account_count.zero? + # First, identify affected accounts for logging affected_accounts = execute(<<~SQL).to_a SELECT DISTINCT @@ -37,7 +40,7 @@ class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2] # Delete orphaned balances where currency doesn't match account currency # Only for linked accounts (provider-connected accounts) - deleted_result = execute(<<~SQL) + execute(<<~SQL) DELETE FROM balances WHERE id IN ( SELECT b.id @@ -57,15 +60,18 @@ class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2] # Get unique account IDs that need re-sync account_ids = affected_accounts.map { |row| row["id"] }.uniq - say "Scheduling re-sync for #{account_ids.size} affected accounts..." - # Schedule re-sync for affected accounts to regenerate correct balances - # Use find_each to handle large datasets efficiently - Account.where(id: account_ids).find_each do |account| - account.sync_later + # Only if Account model is available and responds to sync_later + if defined?(Account) && Account.respond_to?(:where) + say "Scheduling re-sync for #{account_ids.size} affected accounts..." + Account.where(id: account_ids).find_each do |account| + account.sync_later if account.respond_to?(:sync_later) + end + say "Scheduled re-sync for #{account_ids.size} affected accounts" + else + say "Skipping re-sync scheduling (Account model not available)" + say "Please manually sync affected accounts: #{account_ids.join(', ')}" end - - say "Scheduled re-sync for #{account_ids.size} affected accounts" else say "No orphaned currency balances found - database is clean" end @@ -76,4 +82,12 @@ class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2] say "The deleted balances will be regenerated by the scheduled syncs." say "If syncs haven't run yet, you may need to manually trigger them." end + + private + + def account_count + execute("SELECT COUNT(*) FROM accounts").first["count"].to_i + rescue + 0 + end end diff --git a/test/models/concerns/currency_normalizable_test.rb b/test/models/concerns/currency_normalizable_test.rb new file mode 100644 index 000000000..7e916e03b --- /dev/null +++ b/test/models/concerns/currency_normalizable_test.rb @@ -0,0 +1,69 @@ +require "test_helper" + +class CurrencyNormalizableTest < ActiveSupport::TestCase + # Create a test class that includes the concern + class TestClass + include CurrencyNormalizable + + # Expose private method for testing + def test_parse_currency(value) + parse_currency(value) + end + end + + setup do + @parser = TestClass.new + end + + test "parse_currency normalizes lowercase to uppercase" do + assert_equal "USD", @parser.test_parse_currency("usd") + assert_equal "EUR", @parser.test_parse_currency("eur") + assert_equal "GBP", @parser.test_parse_currency("gbp") + end + + test "parse_currency handles whitespace" do + assert_equal "USD", @parser.test_parse_currency(" usd ") + assert_equal "EUR", @parser.test_parse_currency("\teur\n") + end + + test "parse_currency returns nil for blank values" do + assert_nil @parser.test_parse_currency(nil) + assert_nil @parser.test_parse_currency("") + assert_nil @parser.test_parse_currency(" ") + end + + test "parse_currency returns nil for invalid format" do + assert_nil @parser.test_parse_currency("US") # Too short + assert_nil @parser.test_parse_currency("USDD") # Too long + assert_nil @parser.test_parse_currency("123") # Numbers + assert_nil @parser.test_parse_currency("US1") # Mixed + end + + test "parse_currency returns nil for XXX (no currency code)" do + # XXX is ISO 4217 for "no currency" but not valid for monetary operations + assert_nil @parser.test_parse_currency("XXX") + assert_nil @parser.test_parse_currency("xxx") + end + + test "parse_currency returns nil for unknown 3-letter codes" do + # These are 3 letters but not recognized currencies + assert_nil @parser.test_parse_currency("ZZZ") + assert_nil @parser.test_parse_currency("ABC") + end + + test "parse_currency accepts valid ISO currencies" do + # Common currencies + assert_equal "USD", @parser.test_parse_currency("USD") + assert_equal "EUR", @parser.test_parse_currency("EUR") + assert_equal "GBP", @parser.test_parse_currency("GBP") + assert_equal "JPY", @parser.test_parse_currency("JPY") + assert_equal "CHF", @parser.test_parse_currency("CHF") + assert_equal "CAD", @parser.test_parse_currency("CAD") + assert_equal "AUD", @parser.test_parse_currency("AUD") + + # Less common but valid currencies + assert_equal "PLN", @parser.test_parse_currency("PLN") + assert_equal "SEK", @parser.test_parse_currency("SEK") + assert_equal "NOK", @parser.test_parse_currency("NOK") + end +end