mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 15:34:58 +00:00
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.
This commit is contained in:
@@ -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),
|
||||
|
||||
@@ -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|
|
||||
|
||||
@@ -3,7 +3,7 @@
|
||||
<% if investment_metrics[:has_investments] %>
|
||||
<div class="space-y-6">
|
||||
<%# Investment Summary Cards %>
|
||||
<div class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-4 gap-4">
|
||||
<div class="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-5 gap-4">
|
||||
<%# Portfolio Value Card %>
|
||||
<div class="bg-container-inset rounded-lg p-4">
|
||||
<div class="flex items-center gap-2 mb-3">
|
||||
@@ -31,6 +31,22 @@
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<%# Period Return Card %>
|
||||
<div class="bg-container-inset rounded-lg p-4">
|
||||
<div class="flex items-center gap-2 mb-3">
|
||||
<%= icon("bar-chart-2", size: "sm") %>
|
||||
<span class="text-sm text-secondary"><%= t("reports.investment_performance.period_return") %></span>
|
||||
</div>
|
||||
<% if investment_metrics[:period_return_trend] %>
|
||||
<p class="text-xl font-semibold privacy-sensitive" style="color: <%= investment_metrics[:period_return_trend].color %>">
|
||||
<%= format_money(Money.new(investment_metrics[:period_return_trend].value, Current.family.currency)) %>
|
||||
(<%= investment_metrics[:period_return_trend].percent_formatted %>)
|
||||
</p>
|
||||
<% else %>
|
||||
<p class="text-xl font-semibold text-secondary"><%= t("reports.investment_performance.no_data") %></p>
|
||||
<% end %>
|
||||
</div>
|
||||
|
||||
<%# Period Contributions Card %>
|
||||
<div class="bg-container-inset rounded-lg p-4">
|
||||
<div class="flex items-center gap-2 mb-3">
|
||||
|
||||
@@ -210,6 +210,17 @@
|
||||
</span>
|
||||
</div>
|
||||
<% end %>
|
||||
<% if @investment_metrics[:period_return_trend] %>
|
||||
<div class="tufte-metric-card tufte-metric-card-sm">
|
||||
<span class="tufte-metric-card-label"><%= t("reports.print.investments.period_return") %></span>
|
||||
<span class="tufte-metric-card-value" style="color: <%= @investment_metrics[:period_return_trend].color %>">
|
||||
<%= @investment_metrics[:period_return_trend].value >= 0 ? "+" : "" %><%= format_money(Money.new(@investment_metrics[:period_return_trend].value, Current.family.currency)) %>
|
||||
</span>
|
||||
<span class="tufte-metric-card-change" style="color: <%= @investment_metrics[:period_return_trend].color %>">
|
||||
<%= @investment_metrics[:period_return_trend].percent_formatted %>
|
||||
</span>
|
||||
</div>
|
||||
<% end %>
|
||||
<div class="tufte-metric-card tufte-metric-card-sm">
|
||||
<span class="tufte-metric-card-label"><%= t("reports.print.investments.contributions") %></span>
|
||||
<span class="tufte-metric-card-value tufte-income"><%= format_money(@investment_metrics[:period_contributions]) %></span>
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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!(
|
||||
|
||||
Reference in New Issue
Block a user