Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/models/holding/forward_calculator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ def update_cost_basis_tracker(trade_entries)
security_id = trade.security_id
tracker = @cost_basis_tracker[security_id]

# Convert trade price to account currency if needed
# Convert trade price to account currency using the trade-date rate
trade_price = Money.new(trade.price, trade.currency)
begin
converted_price = trade_price.exchange_to(account.currency).amount
converted_price = trade_price.exchange_to(account.currency, date: trade_entry.date, custom_rate: trade.exchange_rate).amount
rescue Money::ConversionError
converted_price = trade.price
end
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down
2 changes: 1 addition & 1 deletion app/models/holding/reverse_calculator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def precompute_cost_basis
security_id = trade.security_id
trade_price = Money.new(trade.price, trade.currency)
begin
converted_price = trade_price.exchange_to(account.currency).amount
converted_price = trade_price.exchange_to(account.currency, date: trade_entry.date, custom_rate: trade.exchange_rate).amount
rescue Money::ConversionError
converted_price = trade.price
end
Expand Down
48 changes: 48 additions & 0 deletions test/models/holding/forward_calculator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,54 @@ class Holding::ForwardCalculatorTest < ActiveSupport::TestCase
assert_holdings(expected, calculated)
end

# Cost basis FX: trade-date rate must be used, not today's rate
test "cost_basis uses trade-date exchange rate for foreign-currency buy" do
travel_to Date.new(2025, 6, 1) do
eur_stock = Security.create!(ticker: "EURST", name: "EUR Stock")
buy_date = Date.new(2025, 5, 27)

Security::Price.create!(security: eur_stock, date: buy_date, price: 100, currency: "EUR")
Security::Price.create!(security: eur_stock, date: Date.current, price: 100, currency: "EUR")

# EUR/USD was 1.10 on trade date; today it is 1.50 — cost basis must use 1.10
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: buy_date, rate: 1.10)
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: Date.current, rate: 1.50)

create_trade(eur_stock, qty: 10, date: buy_date, price: 100, currency: "EUR", account: @account)

calculated = Holding::ForwardCalculator.new(@account).calculate
today_holding = calculated.find { |h| h.date == Date.current && h.security == eur_stock }

# cost_basis per share = 100 EUR × 1.10 (trade-date rate) = 110 USD
assert_in_delta 110.0, today_holding.cost_basis.to_f, 0.01,
"cost_basis must use the trade-date FX rate (1.10), not today's rate (1.50)"
end
end

test "cost_basis uses provider-supplied exchange_rate when present" do
travel_to Date.new(2025, 6, 1) do
eur_stock = Security.create!(ticker: "EURST2", name: "EUR Stock 2")
buy_date = Date.new(2025, 5, 27)

Security::Price.create!(security: eur_stock, date: buy_date, price: 100, currency: "EUR")
Security::Price.create!(security: eur_stock, date: Date.current, price: 100, currency: "EUR")

# DB has a different rate — the provider-supplied rate (1.20) must win
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: buy_date, rate: 1.10)
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: Date.current, rate: 1.50)

create_trade(eur_stock, qty: 10, date: buy_date, price: 100, currency: "EUR",
exchange_rate: 1.20, account: @account)

calculated = Holding::ForwardCalculator.new(@account).calculate
today_holding = calculated.find { |h| h.date == Date.current && h.security == eur_stock }

# cost_basis per share = 100 EUR × 1.20 (provider rate) = 120 USD
assert_in_delta 120.0, today_holding.cost_basis.to_f, 0.01,
"cost_basis must prefer the provider-supplied exchange_rate over the DB lookup"
end
end

private
def assert_holdings(expected, calculated)
expected.each do |expected_entry|
Expand Down
56 changes: 56 additions & 0 deletions test/models/holding/reverse_calculator_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,62 @@ class Holding::ReverseCalculatorTest < ActiveSupport::TestCase
assert_in_delta 100.0, cost_basis_for(calc, security, Date.current).to_f, 1e-6
end

# Cost basis FX: trade-date rate must be used, not today's rate
test "cost_basis uses trade-date exchange rate for foreign-currency buy" do
travel_to Date.new(2025, 6, 1) do
eur_stock = Security.create!(ticker: "EURST", name: "EUR Stock")
buy_date = Date.new(2025, 5, 27)

Security::Price.create!(security: eur_stock, date: buy_date, price: 100, currency: "EUR")
Security::Price.create!(security: eur_stock, date: Date.current, price: 100, currency: "EUR")

@account.holdings.create!(security: eur_stock, date: Date.current,
qty: 10, price: 100, amount: 1000, currency: "EUR")

# EUR/USD was 1.10 on trade date; today it is 1.50 — cost basis must use 1.10
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: buy_date, rate: 1.10)
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: Date.current, rate: 1.50)

create_trade(eur_stock, qty: 10, date: buy_date, price: 100, currency: "EUR", account: @account)

snapshot = OpenStruct.new(to_h: { eur_stock.id => 10 })
calculated = Holding::ReverseCalculator.new(@account, portfolio_snapshot: snapshot).calculate
today_holding = calculated.find { |h| h.date == Date.current && h.security == eur_stock }

# cost_basis per share = 100 EUR × 1.10 (trade-date rate) = 110 USD
assert_in_delta 110.0, today_holding.cost_basis.to_f, 0.01,
"cost_basis must use the trade-date FX rate (1.10), not today's rate (1.50)"
end
end

test "cost_basis uses provider-supplied exchange_rate when present" do
travel_to Date.new(2025, 6, 1) do
eur_stock = Security.create!(ticker: "EURST2", name: "EUR Stock 2")
buy_date = Date.new(2025, 5, 27)

Security::Price.create!(security: eur_stock, date: buy_date, price: 100, currency: "EUR")
Security::Price.create!(security: eur_stock, date: Date.current, price: 100, currency: "EUR")

@account.holdings.create!(security: eur_stock, date: Date.current,
qty: 10, price: 100, amount: 1000, currency: "EUR")

# DB has a different rate — the provider-supplied rate (1.20) must win
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: buy_date, rate: 1.10)
ExchangeRate.create!(from_currency: "EUR", to_currency: "USD", date: Date.current, rate: 1.50)

create_trade(eur_stock, qty: 10, date: buy_date, price: 100, currency: "EUR",
exchange_rate: 1.20, account: @account)

snapshot = OpenStruct.new(to_h: { eur_stock.id => 10 })
calculated = Holding::ReverseCalculator.new(@account, portfolio_snapshot: snapshot).calculate
today_holding = calculated.find { |h| h.date == Date.current && h.security == eur_stock }

# cost_basis per share = 100 EUR × 1.20 (provider rate) = 120 USD
assert_in_delta 120.0, today_holding.cost_basis.to_f, 0.01,
"cost_basis must prefer the provider-supplied exchange_rate over the DB lookup"
end
end

private
def assert_holdings(expected, calculated)
expected.each do |expected_entry|
Expand Down
4 changes: 3 additions & 1 deletion test/support/entries_test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def create_valuation(attributes = {})
Entry.create! entry_defaults.merge(entry_attributes)
end

def create_trade(security, account:, qty:, date:, price: nil, currency: "USD")
def create_trade(security, account:, qty:, date:, price: nil, currency: "USD", exchange_rate: nil)
trade_price = price || Security::Price.find_by!(security: security, date: date).price

trade = Trade.new \
Expand All @@ -44,6 +44,8 @@ def create_trade(security, account:, qty:, date:, price: nil, currency: "USD")
currency: currency,
investment_activity_label: qty > 0 ? "Buy" : "Sell"

trade.exchange_rate = exchange_rate if exchange_rate

account.entries.create! \
name: "Trade",
date: date,
Expand Down
Loading