From 83bbabdc2d25bf359b02bec78397d55bb226f5fa Mon Sep 17 00:00:00 2001
From: Valeron <valerontoscano@gmail.com>
Date: Sun, 23 Nov 2025 20:43:55 +0530
Subject: [PATCH] Fixed average cost calculation

---
 app/models/holding.rb       | 17 ++++++++++++++---
 test/models/holding_test.rb | 29 +++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/app/models/holding.rb b/app/models/holding.rb
index b6cf379f0..d3c117b4e 100644
--- a/app/models/holding.rb
+++ b/app/models/holding.rb
@@ -29,7 +29,7 @@ class Holding < ApplicationRecord
 
   # Basic approximation of cost-basis
   def avg_cost
-    avg_cost = account.trades
+    trades = account.trades
       .with_entry
       .joins(ActiveRecord::Base.sanitize_sql_array([
         "LEFT JOIN exchange_rates ON (
@@ -40,9 +40,20 @@ class Holding < ApplicationRecord
       ]))
       .where(security_id: security.id)
       .where("trades.qty > 0 AND entries.date <= ?", date)
-      .average("trades.price * COALESCE(exchange_rates.rate, 1)")
 
-    Money.new(avg_cost || price, currency)
+    total_cost, total_qty = trades.pick(
+      Arel.sql("SUM(trades.price * trades.qty * COALESCE(exchange_rates.rate, 1))"),
+      Arel.sql("SUM(trades.qty)")
+    )
+
+    weighted_avg =
+      if total_qty && total_qty > 0
+        total_cost / total_qty
+      else
+        price
+      end
+
+    Money.new(weighted_avg || price, currency)
   end
 
   def trend
diff --git a/test/models/holding_test.rb b/test/models/holding_test.rb
index c3cc9a7ae..ea44d918b 100644
--- a/test/models/holding_test.rb
+++ b/test/models/holding_test.rb
@@ -1,5 +1,6 @@
 require "test_helper"
 require "ostruct"
+require "bigdecimal"
 
 class HoldingTest < ActiveSupport::TestCase
   include EntriesTestHelper, SecuritiesTestHelper
@@ -26,8 +27,17 @@ class HoldingTest < ActiveSupport::TestCase
     create_trade(@nvda.security, account: @account, qty: 5, price: 128.00, date: 1.day.ago.to_date)
     create_trade(@nvda.security, account: @account, qty: 30, price: 124.00, date: Date.current)
 
-    assert_equal Money.new((212.00 + 216.00).to_d / 2), @amzn.avg_cost
-    assert_equal Money.new((128.00 + 124.00).to_d / 2), @nvda.avg_cost
+    # expected weighted averages (quantity-weighted)
+    amzn_total = BigDecimal("10") * BigDecimal("212.00") + BigDecimal("15") * BigDecimal("216.00")
+    amzn_qty   = BigDecimal("10") + BigDecimal("15")
+    expected_amzn = amzn_total / amzn_qty
+
+    nvda_total = BigDecimal("5") * BigDecimal("128.00") + BigDecimal("30") * BigDecimal("124.00")
+    nvda_qty   = BigDecimal("5") + BigDecimal("30")
+    expected_nvda = nvda_total / nvda_qty
+
+    assert_equal Money.new(expected_amzn), @amzn.avg_cost
+    assert_equal Money.new(expected_nvda), @nvda.avg_cost
   end
 
   test "calculates average cost basis from another currency" do
@@ -37,8 +47,19 @@ class HoldingTest < ActiveSupport::TestCase
     create_trade(@nvda.security, account: @account, qty: 5, price: 128.00, date: 1.day.ago.to_date, currency: "CAD")
     create_trade(@nvda.security, account: @account, qty: 30, price: 124.00, date: Date.current, currency: "CAD")
 
-    assert_equal Money.new((212.00 + 216.00).to_d / 2, "CAD").exchange_to("USD", fallback_rate: 1), @amzn.avg_cost
-    assert_equal Money.new((128.00 + 124.00).to_d / 2, "CAD").exchange_to("USD", fallback_rate: 1), @nvda.avg_cost
+    # compute expected: sum(price * qty * rate) / sum(qty)
+    amzn_total_usd = BigDecimal("10") * BigDecimal("212.00") * BigDecimal("1") +
+                     BigDecimal("15") * BigDecimal("216.00") * BigDecimal("1")
+    amzn_qty = BigDecimal("10") + BigDecimal("15")
+    expected_amzn_usd = amzn_total_usd / amzn_qty
+
+    nvda_total_usd = BigDecimal("5") * BigDecimal("128.00") * BigDecimal("1") +
+                     BigDecimal("30") * BigDecimal("124.00") * BigDecimal("1")
+    nvda_qty = BigDecimal("5") + BigDecimal("30")
+    expected_nvda_usd = nvda_total_usd / nvda_qty
+
+    assert_equal Money.new(expected_amzn_usd, "CAD").exchange_to("USD", fallback_rate: 1), @amzn.avg_cost
+    assert_equal Money.new(expected_nvda_usd, "CAD").exchange_to("USD", fallback_rate: 1), @nvda.avg_cost
   end
 
   test "calculates total return trend" do