diff --git a/app/models/simplefin_account/investments/holdings_processor.rb b/app/models/simplefin_account/investments/holdings_processor.rb index 3274b4b9d..8cb4882ef 100644 --- a/app/models/simplefin_account/investments/holdings_processor.rb +++ b/app/models/simplefin_account/investments/holdings_processor.rb @@ -47,7 +47,8 @@ class SimplefinAccount::Investments::HoldingsProcessor # which would cause the system to display average cost as current price. (GH #1182) qty = parse_decimal(any_of(simplefin_holding, %w[shares quantity qty units])) market_value = parse_decimal(any_of(simplefin_holding, %w[market_value current_value])) - cost_basis = parse_decimal(any_of(simplefin_holding, %w[cost_basis basis total_cost value])) + raw_cost_basis, cost_basis_source_key = cost_basis_from(simplefin_holding) + cost_basis = normalize_cost_basis(raw_cost_basis, qty, cost_basis_source_key) # Derive price from market_value when possible; otherwise fall back to any price field fallback_price = parse_decimal(any_of(simplefin_holding, %w[purchase_price price unit_price average_cost avg_cost])) @@ -112,6 +113,32 @@ class SimplefinAccount::Investments::HoldingsProcessor simplefin_account.raw_holdings_payload || [] end + def cost_basis_from(simplefin_holding) + %w[cost_basis basis total_cost value].each do |key| + raw = simplefin_holding[key] + next if raw.nil? || raw.to_s.strip.empty? + + return [ parse_decimal(raw), key ] + end + + [ nil, nil ] + end + + # Sure stores holding cost_basis as per-share average cost. Some SimpleFIN + # providers expose total position basis via total_cost/value, so normalize only + # when the selected provider field is known to represent total position basis. + def normalize_cost_basis(raw_cost_basis, qty, source_key) + return nil if raw_cost_basis.nil? + + if %w[total_cost value].include?(source_key) + return nil unless qty.to_d.positive? + + raw_cost_basis / qty + else + raw_cost_basis + end + end + def resolve_security(symbol, description) # Normalize crypto tickers to a distinct namespace so they don't collide with equities sym = symbol.to_s.upcase diff --git a/test/jobs/simplefin_holdings_apply_job_test.rb b/test/jobs/simplefin_holdings_apply_job_test.rb index 7cc7b4c6c..1826e2495 100644 --- a/test/jobs/simplefin_holdings_apply_job_test.rb +++ b/test/jobs/simplefin_holdings_apply_job_test.rb @@ -124,7 +124,7 @@ class SimplefinHoldingsApplyJobTest < ActiveSupport::TestCase # Price derived from market_value assert_in_delta 500.0, holding.price.to_f, 0.01 - # cost_basis should fall back to "value" field (45000) - assert_in_delta 45000.0, holding.cost_basis.to_f, 0.01 + # cost_basis should fall back to "value" field and normalize total basis to per-share basis + assert_in_delta 450.0, holding.cost_basis.to_f, 0.01 end end diff --git a/test/models/simplefin_account/investments/holdings_processor_test.rb b/test/models/simplefin_account/investments/holdings_processor_test.rb new file mode 100644 index 000000000..122d908c0 --- /dev/null +++ b/test/models/simplefin_account/investments/holdings_processor_test.rb @@ -0,0 +1,95 @@ +require "test_helper" + +class SimplefinAccount::Investments::HoldingsProcessorTest < ActiveSupport::TestCase + setup do + @processor = SimplefinAccount::Investments::HoldingsProcessor.new(nil) + end + + test "cost_basis source is used unchanged as per share basis" do + payload = { + "cost_basis" => "16.61", + "total_cost" => "9588.61", + "value" => "10108.16" + } + + raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload) + cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key) + + assert_equal BigDecimal("16.61"), cost_basis + assert_equal "cost_basis", source_key + end + + test "basis source is used unchanged as per share basis" do + payload = { + "basis" => "16.61", + "total_cost" => "9588.61" + } + + raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload) + cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key) + + assert_equal BigDecimal("16.61"), cost_basis + assert_equal "basis", source_key + end + + test "total_cost source is normalized to per share basis" do + payload = { + "total_cost" => "9588.61" + } + + raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload) + cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key) + + assert_equal BigDecimal("9588.61") / BigDecimal("577.279"), cost_basis + assert_equal "total_cost", source_key + end + + test "value source is normalized to per share basis" do + payload = { + "value" => "9588.61" + } + + raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload) + cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key) + + assert_equal BigDecimal("9588.61") / BigDecimal("577.279"), cost_basis + assert_equal "value", source_key + end + + test "total cost source with zero quantity returns nil" do + payload = { + "total_cost" => "9588.61" + } + + raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload) + cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("0"), source_key) + + assert_nil cost_basis + assert_equal "total_cost", source_key + end + + test "total cost source with nil quantity returns nil" do + payload = { + "total_cost" => "9588.61" + } + + raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload) + cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, nil, source_key) + + assert_nil cost_basis + assert_equal "total_cost", source_key + end + + test "missing cost basis fields return nil" do + payload = { + "market_value" => "10108.16" + } + + raw_cost_basis, source_key = @processor.send(:cost_basis_from, payload) + cost_basis = @processor.send(:normalize_cost_basis, raw_cost_basis, BigDecimal("577.279"), source_key) + + assert_nil raw_cost_basis + assert_nil source_key + assert_nil cost_basis + end +end