From 52083d5774d09c58f45cd585af28e0472a67cb86 Mon Sep 17 00:00:00 2001 From: CrossDrain <32982516+CrossDrain@users.noreply.github.com> Date: Thu, 28 May 2026 12:49:04 +0000 Subject: [PATCH] feat(reports): add Period Return card to Investment Performance (#1962) * feat(reports): add Period Return card to Investment Performance tab Surfaces market-only return (absolute + %) for the selected period using net_market_flows from the balances table, excluding contributions and withdrawals. Appears in both the interactive report and the print view. * docs: remove TODOS.md; fold FX fallback caveat into PR description The single V2 item (Period Return's 1:1 FX fallback on missing rates) is now documented under Known Limitations in the PR description, so a tracked file in the repo root is redundant. * fix(investment_statement): align start_value denominator scope and FX handling Add status filter to match absolute_return, and move FX conversion into SQL so pre-period balances are found even when an account's currency was changed after balances were recorded. --- app/controllers/reports_controller.rb | 1 + app/models/investment_statement.rb | 71 +++++++++++++++++++ .../reports/_investment_performance.html.erb | 18 ++++- app/views/reports/print.html.erb | 11 +++ config/locales/views/reports/en.yml | 2 + test/models/investment_statement_test.rb | 45 ++++++++++++ 6 files changed, 147 insertions(+), 1 deletion(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 26a03f23d..9c5bd46a5 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -471,6 +471,7 @@ class ReportsController < ApplicationController has_investments: true, portfolio_value: investment_statement.portfolio_value_money, unrealized_trend: investment_statement.unrealized_gains_trend, + period_return_trend: investment_statement.period_return_trend(period: @period), period_contributions: period_totals.contributions, period_withdrawals: period_totals.withdrawals, top_holdings: investment_statement.top_holdings(limit: 5), diff --git a/app/models/investment_statement.rb b/app/models/investment_statement.rb index e4f79fee1..bb7a836a9 100644 --- a/app/models/investment_statement.rb +++ b/app/models/investment_statement.rb @@ -156,6 +156,77 @@ class InvestmentStatement ) end + def period_return_trend(period: Period.current_month) + currency = family.currency + account_ids = investment_account_ids + return nil if account_ids.empty? + + absolute_return = ActiveRecord::Base.connection.select_value( + ActiveRecord::Base.sanitize_sql_array([ + <<~SQL.squish, + SELECT COALESCE(SUM(b.net_market_flows * COALESCE(er.rate, 1)), 0) + FROM balances b + JOIN accounts a ON a.id = b.account_id + LEFT JOIN exchange_rates er ON ( + er.date = b.date + AND er.from_currency = b.currency + AND er.to_currency = :currency + ) + WHERE a.id IN (:account_ids) + AND a.family_id = :family_id + AND a.status IN ('draft', 'active') + AND b.date BETWEEN :start_date AND :end_date + SQL + { + currency: currency, + account_ids: account_ids, + family_id: family.id, + start_date: period.date_range.begin, + end_date: period.date_range.end + } + ]) + ).to_d + + period_start = period.date_range.begin + + # Single query for all accounts' most recent pre-period balance (strict < to avoid + # double-counting the first day's net_market_flows in both the denominator and absolute_return). + # FX conversion is done in SQL (matching absolute_return) so balance rows whose currency + # differs from the account's current currency (e.g. after a currency change) are still picked up. + start_value = ActiveRecord::Base.connection.select_value( + ActiveRecord::Base.sanitize_sql_array([ + <<~SQL.squish, + SELECT COALESCE(SUM(b.end_balance * COALESCE(er.rate, 1)), 0) + FROM accounts a + INNER JOIN balances b ON b.account_id = a.id + LEFT JOIN exchange_rates er ON ( + er.date = :period_start + AND er.from_currency = b.currency + AND er.to_currency = :currency + ) + INNER JOIN ( + SELECT b2.account_id, MAX(b2.date) AS max_date + FROM balances b2 + WHERE b2.account_id IN (:account_ids) + AND b2.date < :period_start + GROUP BY b2.account_id + ) latest ON latest.account_id = b.account_id AND b.date = latest.max_date + WHERE a.id IN (:account_ids) + AND a.family_id = :family_id + AND a.status IN ('draft', 'active') + SQL + { account_ids: account_ids, period_start: period_start, family_id: family.id, currency: currency } + ]) + ).to_d + + return nil if start_value.zero? + + Trend.new( + current: Money.new(start_value + absolute_return, currency), + previous: Money.new(start_value, currency) + ) + end + # Day change across portfolio, summed in family currency def day_change changes = current_holdings.to_a.filter_map do |h| diff --git a/app/views/reports/_investment_performance.html.erb b/app/views/reports/_investment_performance.html.erb index 23a922ebc..96f9fc8e6 100644 --- a/app/views/reports/_investment_performance.html.erb +++ b/app/views/reports/_investment_performance.html.erb @@ -3,7 +3,7 @@ <% if investment_metrics[:has_investments] %>
<%# Investment Summary Cards %> -
+
<%# Portfolio Value Card %>
@@ -31,6 +31,22 @@ <% end %>
+ <%# Period Return Card %> +
+
+ <%= icon("bar-chart-2", size: "sm") %> + <%= t("reports.investment_performance.period_return") %> +
+ <% if investment_metrics[:period_return_trend] %> +

+ <%= format_money(Money.new(investment_metrics[:period_return_trend].value, Current.family.currency)) %> + (<%= investment_metrics[:period_return_trend].percent_formatted %>) +

+ <% else %> +

<%= t("reports.investment_performance.no_data") %>

+ <% end %> +
+ <%# Period Contributions Card %>
diff --git a/app/views/reports/print.html.erb b/app/views/reports/print.html.erb index 1de2a4cd2..d92698ab4 100644 --- a/app/views/reports/print.html.erb +++ b/app/views/reports/print.html.erb @@ -210,6 +210,17 @@
<% end %> + <% if @investment_metrics[:period_return_trend] %> +
+ <%= t("reports.print.investments.period_return") %> + + <%= @investment_metrics[:period_return_trend].value >= 0 ? "+" : "" %><%= format_money(Money.new(@investment_metrics[:period_return_trend].value, Current.family.currency)) %> + + + <%= @investment_metrics[:period_return_trend].percent_formatted %> + +
+ <% end %>
<%= t("reports.print.investments.contributions") %> <%= format_money(@investment_metrics[:period_contributions]) %> diff --git a/config/locales/views/reports/en.yml b/config/locales/views/reports/en.yml index be792678d..826924ba4 100644 --- a/config/locales/views/reports/en.yml +++ b/config/locales/views/reports/en.yml @@ -142,6 +142,7 @@ en: title: Investment Performance portfolio_value: Portfolio Value total_return: Total Return + period_return: Period Return contributions: Period Contributions withdrawals: Period Withdrawals top_holdings: Top Holdings @@ -228,6 +229,7 @@ en: title: Investments portfolio_value: Portfolio Value total_return: Total Return + period_return: Period Return contributions: Contributions withdrawals: Withdrawals this_period: this period diff --git a/test/models/investment_statement_test.rb b/test/models/investment_statement_test.rb index 3bb50dff8..064553969 100644 --- a/test/models/investment_statement_test.rb +++ b/test/models/investment_statement_test.rb @@ -177,6 +177,51 @@ class InvestmentStatementTest < ActiveSupport::TestCase assert_in_delta 3980, trend.previous.amount, 0.001 end + test "period_return_trend returns nil when no balance data in period" do + period = Period.custom(start_date: 10.years.ago.to_date, end_date: 9.years.ago.to_date) + assert_nil @statement.period_return_trend(period: period) + end + + test "period_return_trend returns nil when start portfolio value is zero" do + account = create_investment_account(balance: 5000) + period = Period.custom(start_date: Date.current.beginning_of_month, end_date: Date.current) + # Balance only inside the period — nothing strictly before period_start means start_value = 0 + account.balances.create!( + date: period.date_range.begin, + balance: 5000, + currency: @family.currency, + net_market_flows: 200 + ) + assert_nil @statement.period_return_trend(period: period) + end + + test "period_return_trend returns Trend with correct absolute and percent return" do + account = create_investment_account(balance: 10_500) + period = Period.custom(start_date: Date.current.beginning_of_month, end_date: Date.current) + + # Pre-period row: start_non_cash_balance drives end_balance (virtual stored column) + account.balances.create!( + date: period.date_range.begin - 1.day, + balance: 10_000, + currency: @family.currency, + start_non_cash_balance: 10_000, + net_market_flows: 0 + ) + # In-period row: 500 of market gains + account.balances.create!( + date: period.date_range.begin, + balance: 10_500, + currency: @family.currency, + start_non_cash_balance: 10_000, + net_market_flows: 500 + ) + + trend = @statement.period_return_trend(period: period) + assert_not_nil trend + assert_in_delta 500, trend.value.amount, 1 + assert_in_delta 5.0, trend.percent, 0.1 + end + private def create_investment_account(balance:, cash_balance: 0, currency: "USD") @family.accounts.create!(