Address luckyPipewrench feedback in model-layer outflow aggregation

Agent-Logs-Url: https://github.com/we-promise/sure/sessions/485959be-a7fd-4d5d-83c5-90fedb15be34

Co-authored-by: jjmata <187772+jjmata@users.noreply.github.com>

app/controllers/pages_controller.rb

@@ -24,7 +24,7 @@ class PagesController < ApplicationController
     @cashflow_sankey_data = build_cashflow_sankey_data(net_totals, income_totals, expense_totals, family_currency)
     @outflows_data = build_outflows_donut_data(
       net_totals,
-      investment_contributions_total: investment_contributions_outflow_total(@period)
+      investment_contributions_total: income_statement.investment_contributions_outflow_total(period: @period)
     )
 
     @dashboard_sections = build_dashboard_sections
@@ -354,6 +354,8 @@ class PagesController < ApplicationController
       categories.reject! { |category| category[:amount].zero? }
       categories.sort_by! { |category| -category[:amount] }
 
+      # Includes transfer outflows in the same donut, so percentages here represent
+      # share of total outflows (expenses + investment contribution transfers).
       categories.each do |category|
         category[:percentage] = donut_total.zero? ? 0 : ((category[:amount] / donut_total) * 100).round(1)
       end
@@ -361,27 +363,6 @@ class PagesController < ApplicationController
       { categories: categories, total: donut_total.to_f.round(2), currency: net_totals.currency, currency_symbol: currency_symbol }
     end
 
-    # Total transfer outflows to investment/crypto accounts for dashboard outflow visibility.
-    # These transactions are excluded from budget/report expense analytics, but still shown
-    # in outflows so users can track where cash moved during the selected period.
-    def investment_contributions_outflow_total(period)
-      scope = Current.family.transactions
-        .visible
-        .excluding_pending
-        .in_period(period)
-        .where(kind: "investment_contribution")
-        .joins(entry: :account)
-        .joins(ApplicationRecord.sanitize_sql_array(
-          [
-            "LEFT JOIN exchange_rates er ON er.date = entries.date AND er.from_currency = entries.currency AND er.to_currency = ?",
-            Current.family.currency
-          ]
-        ))
-        .merge(Account.included_in_finances_for(Current.user))
-
-      scope.sum("ABS(entries.amount * COALESCE(er.rate, 1))")
-    end
-
     def ensure_intro_guest!
       return if Current.user&.guest?
 

app/models/income_statement.rb

@@ -116,6 +116,58 @@ class IncomeStatement
     family_stats(interval: interval).find { |stat| stat.classification == "income" }&.median || 0
   end
 
+  # Dashboard-specific transfer outflow visibility.
+  # These transfers are excluded from income/expense analytics, but shown in outflows
+  # so users can track cash moving to investment/crypto accounts.
+  def investment_contributions_outflow_total(period: Period.current_month)
+    account_ids = if included_account_ids
+      included_account_ids
+    elsif user
+      family.accounts.visible.included_in_finances_for(user).pluck(:id)
+    else
+      family.accounts.visible.pluck(:id)
+    end
+    return 0 if account_ids.empty?
+
+    scope = family.transactions
+      .visible
+      .excluding_pending
+      .in_period(period)
+      .where(kind: "investment_contribution")
+      .joins(:entry)
+      .where(entries: { excluded: false, account_id: account_ids })
+      .joins(ApplicationRecord.sanitize_sql_array(
+        [
+          "LEFT JOIN exchange_rates er ON er.date = entries.date AND er.from_currency = entries.currency AND er.to_currency = ?",
+          family.currency
+        ]
+      ))
+
+    rows = ActiveRecord::Base.connection.select_all(
+      ActiveRecord::Base.sanitize_sql_array([
+        <<~SQL,
+          SELECT
+            COALESCE(e.transfer_id, e.id) AS group_id,
+            GREATEST(
+              SUM(CASE WHEN e.amount > 0 THEN e.amount * COALESCE(er.rate, 1) ELSE 0 END),
+              ABS(SUM(CASE WHEN e.amount < 0 THEN e.amount * COALESCE(er.rate, 1) ELSE 0 END))
+            ) AS total
+          FROM (#{scope.select("entries.id").to_sql}) filtered
+          JOIN entries e ON e.id = filtered.id
+          LEFT JOIN exchange_rates er ON (
+            er.date = e.date
+            AND er.from_currency = e.currency
+            AND er.to_currency = :target_currency
+          )
+          GROUP BY COALESCE(e.transfer_id, e.id)
+        SQL
+        { target_currency: family.currency }
+      ])
+    )
+
+    rows.sum { |row| row["total"].to_d }
+  end
+
   private
     ScopeTotals = Data.define(:transactions_count, :income_money, :expense_money)
     PeriodTotal = Data.define(:classification, :total, :currency, :category_totals)

app/models/transaction.rb

@@ -34,7 +34,7 @@ class Transaction < ApplicationRecord
     cc_payment: "cc_payment", # A CC payment, excluded from budget analytics (CC payments offset the sum of expense transactions)
     loan_payment: "loan_payment", # A payment to a Loan account, treated as an expense in budgets
     one_time: "one_time", # A one-time expense/income, excluded from budget analytics
-    investment_contribution: "investment_contribution" # Transfer to investment/crypto account, treated as an expense in budgets
+    investment_contribution: "investment_contribution" # Transfer to investment/crypto account, excluded from budget/income-statement analytics
   }
 
   # All kinds where money moves between accounts (transfer? returns true).

test/models/income_statement_test.rb

@@ -330,6 +330,40 @@ class IncomeStatementTest < ActiveSupport::TestCase
     assert_equal Money.new(900, @family.currency), totals.expense_money
   end
 
+  test "investment contribution outflow total excludes hidden entries and avoids transfer double-counting" do
+    investment_account = @family.accounts.create!(
+      name: "Brokerage",
+      currency: @family.currency,
+      balance: 5000,
+      accountable: Investment.new
+    )
+
+    hidden_entry = create_transaction(
+      account: @checking_account,
+      amount: 125,
+      kind: "investment_contribution",
+      category: @family.investment_contributions_category
+    )
+    hidden_entry.update!(excluded: true)
+
+    transfer = Transfer::Creator.new(
+      family: @family,
+      source_account_id: @checking_account.id,
+      destination_account_id: investment_account.id,
+      date: Date.current,
+      amount: 300
+    ).create
+
+    # Simulate a provider-created investment_contribution inflow leg linked to the same transfer.
+    # Grouping by transfer_id should ensure this does not increase total outflow.
+    transfer.inflow_transaction.update!(kind: "investment_contribution")
+
+    income_statement = IncomeStatement.new(@family)
+    outflow_total = income_statement.investment_contributions_outflow_total(period: Period.last_30_days)
+
+    assert_equal 300, outflow_total.to_i
+  end
+
   # Tax-Advantaged Account Exclusion Tests
   test "excludes transactions from tax-advantaged Roth IRA accounts" do
     # Create a Roth IRA (tax-exempt) investment account