FIX providers invalid currency handling (#589)

This commit is contained in:
soky srm
2026-01-09 11:54:38 +01:00
committed by GitHub
parent 701742e218
commit d185c6161c
3 changed files with 115 additions and 13 deletions

View File

@@ -1,10 +1,10 @@
# Provides currency normalization and validation for provider data imports # Provides currency normalization and validation for provider data imports
# #
# This concern provides a shared method to parse and normalize currency codes # 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") # - Consistent uppercase formatting (e.g., "eur" -> "EUR")
# - Validation of 3-letter ISO currency codes # - Validation against Money gem's known currencies (not just 3-letter format)
# - Proper handling of nil, empty, and invalid values # - Proper handling of nil, empty, and invalid values (e.g., "XXX")
# #
# Usage: # Usage:
# include CurrencyNormalizable # include CurrencyNormalizable
@@ -23,6 +23,7 @@ module CurrencyNormalizable
# parse_currency("usd") # => "USD" # parse_currency("usd") # => "USD"
# parse_currency("EUR") # => "EUR" # parse_currency("EUR") # => "EUR"
# parse_currency(" gbp ") # => "GBP" # parse_currency(" gbp ") # => "GBP"
# parse_currency("XXX") # => nil (not a valid Money currency)
# parse_currency("invalid") # => nil (logs warning) # parse_currency("invalid") # => nil (logs warning)
# parse_currency(nil) # => nil # parse_currency(nil) # => nil
# parse_currency("") # => nil # parse_currency("") # => nil
@@ -33,8 +34,15 @@ module CurrencyNormalizable
# Normalize to uppercase 3-letter code # Normalize to uppercase 3-letter code
normalized = currency_value.to_s.strip.upcase normalized = currency_value.to_s.strip.upcase
# Validate it's a reasonable currency code (3 letters) # Validate it's a 3-letter format first
if normalized.match?(/\A[A-Z]{3}\z/) 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 normalized
else else
log_invalid_currency(currency_value) log_invalid_currency(currency_value)
@@ -42,6 +50,17 @@ module CurrencyNormalizable
end end
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 # Log warning for invalid currency codes
# Override this method in including classes to provide context-specific logging # Override this method in including classes to provide context-specific logging
def log_invalid_currency(currency_value) def log_invalid_currency(currency_value)

View File

@@ -9,6 +9,9 @@
# going forward, but existing data needs to be cleaned up. # going forward, but existing data needs to be cleaned up.
class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2] class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2]
def up 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 # First, identify affected accounts for logging
affected_accounts = execute(<<~SQL).to_a affected_accounts = execute(<<~SQL).to_a
SELECT DISTINCT SELECT DISTINCT
@@ -37,7 +40,7 @@ class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2]
# Delete orphaned balances where currency doesn't match account currency # Delete orphaned balances where currency doesn't match account currency
# Only for linked accounts (provider-connected accounts) # Only for linked accounts (provider-connected accounts)
deleted_result = execute(<<~SQL) execute(<<~SQL)
DELETE FROM balances DELETE FROM balances
WHERE id IN ( WHERE id IN (
SELECT b.id SELECT b.id
@@ -57,15 +60,18 @@ class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2]
# Get unique account IDs that need re-sync # Get unique account IDs that need re-sync
account_ids = affected_accounts.map { |row| row["id"] }.uniq 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 # Schedule re-sync for affected accounts to regenerate correct balances
# Use find_each to handle large datasets efficiently # Only if Account model is available and responds to sync_later
Account.where(id: account_ids).find_each do |account| if defined?(Account) && Account.respond_to?(:where)
account.sync_later 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 end
say "Scheduled re-sync for #{account_ids.size} affected accounts"
else else
say "No orphaned currency balances found - database is clean" say "No orphaned currency balances found - database is clean"
end end
@@ -76,4 +82,12 @@ class CleanupOrphanedCurrencyBalances < ActiveRecord::Migration[7.2]
say "The deleted balances will be regenerated by the scheduled syncs." 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." say "If syncs haven't run yet, you may need to manually trigger them."
end end
private
def account_count
execute("SELECT COUNT(*) FROM accounts").first["count"].to_i
rescue
0
end
end end

View File

@@ -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