diff --git a/app/models/holding.rb b/app/models/holding.rb index 2b8e73ba5..0b18b3e9c 100644 --- a/app/models/holding.rb +++ b/app/models/holding.rb @@ -112,7 +112,9 @@ class Holding < ApplicationRecord return new_source == "calculated" end - new_priority > cost_basis_source_priority + # Allow refreshes from the same source (e.g., new trades change calculated cost basis, + # or providers send updated cost basis). + new_priority >= cost_basis_source_priority end # Set cost_basis from user input (locks the value) diff --git a/app/models/holding/cost_basis_reconciler.rb b/app/models/holding/cost_basis_reconciler.rb index 88bd9b635..3a12bebed 100644 --- a/app/models/holding/cost_basis_reconciler.rb +++ b/app/models/holding/cost_basis_reconciler.rb @@ -40,6 +40,15 @@ class Holding::CostBasisReconciler # Check priority - can the incoming source replace the existing? if existing_holding.cost_basis_replaceable_by?(incoming_source) if incoming_cost_basis.present? + # Avoid writes when nothing would change (common when re-materializing) + if existing_holding.cost_basis_source == incoming_source && existing_holding.cost_basis == incoming_cost_basis + return { + cost_basis: existing_holding.cost_basis, + cost_basis_source: existing_holding.cost_basis_source, + should_update: false + } + end + return { cost_basis: incoming_cost_basis, cost_basis_source: incoming_source, diff --git a/test/models/holding/materializer_test.rb b/test/models/holding/materializer_test.rb index 5130743e8..4ab68e10f 100644 --- a/test/models/holding/materializer_test.rb +++ b/test/models/holding/materializer_test.rb @@ -75,4 +75,22 @@ class Holding::MaterializerTest < ActiveSupport::TestCase assert_equal BigDecimal("180.00"), holding.cost_basis, "Trade-derived cost_basis should override provider cost_basis when available" end + + test "recalculates calculated cost_basis when new trades are added" do + date = Date.current + + create_trade(@aapl, account: @account, qty: 1, price: 3000, date: date) + Holding::Materializer.new(@account, strategy: :forward).materialize_holdings + + holding = @account.holdings.find_by!(security: @aapl, date: date, currency: "USD") + assert_equal "calculated", holding.cost_basis_source + assert_equal BigDecimal("3000.0"), holding.cost_basis + + create_trade(@aapl, account: @account, qty: 1, price: 2500, date: date) + Holding::Materializer.new(@account, strategy: :forward).materialize_holdings + + holding.reload + assert_equal "calculated", holding.cost_basis_source + assert_equal BigDecimal("2750.0"), holding.cost_basis + end end diff --git a/test/models/holding_test.rb b/test/models/holding_test.rb index 7f6985767..be58ed84f 100644 --- a/test/models/holding_test.rb +++ b/test/models/holding_test.rb @@ -129,17 +129,18 @@ class HoldingTest < ActiveSupport::TestCase assert_not @amzn.cost_basis_replaceable_by?("manual") end - test "cost_basis_replaceable_by? respects priority hierarchy" do - # Provider data can be replaced by calculated or manual + test "cost_basis_replaceable_by? respects priority hierarchy and allows refreshes" do + # Provider data can be replaced by higher-priority sources (calculated/manual) + # and can be refreshed by provider again. @amzn.update!(cost_basis: 200, cost_basis_source: "provider", cost_basis_locked: false) assert @amzn.cost_basis_replaceable_by?("calculated") assert @amzn.cost_basis_replaceable_by?("manual") - assert_not @amzn.cost_basis_replaceable_by?("provider") + assert @amzn.cost_basis_replaceable_by?("provider") - # Calculated data can be replaced by manual only + # Calculated data can be replaced by manual and can be refreshed by calculated again. @amzn.update!(cost_basis: 200, cost_basis_source: "calculated", cost_basis_locked: false) assert @amzn.cost_basis_replaceable_by?("manual") - assert_not @amzn.cost_basis_replaceable_by?("calculated") + assert @amzn.cost_basis_replaceable_by?("calculated") assert_not @amzn.cost_basis_replaceable_by?("provider") # Manual data when LOCKED cannot be replaced by anything