From bf9bcae600df84ccbd3dd0eb5a95acb76d4429d2 Mon Sep 17 00:00:00 2001 From: LPW Date: Mon, 19 Jan 2026 09:44:49 -0500 Subject: [PATCH] Add gains by tax treatment to investment report with grouped subtype dropdown (#701) * Add tax treatment metrics to reports, forms, and models - Implement `build_gains_by_tax_treatment` for grouping gains by tax treatment - Update investment performance view with tax treatment breakdown - Add tax treatment field to crypto and investments forms - Introduce `realized_gain_loss` calculation in the Trade model - Group investment subtypes by region for improved dropdown organization * Optimize investment performance report by reducing N+1 queries - Eager-load associations in `build_gains_by_tax_treatment` to minimize database queries - Preload holdings for realized gain/loss calculations in trades - Refactor views to standardize "no data" placeholder using translations - Adjust styling in tax treatment breakdown for improved layout * Enhance investment performance translations and optimize holdings lookup logic - Update `holdings_count` and `sells_count` translations to handle pluralization - Refactor views to use pluralized translation keys with count interpolation - Optimize preloaded holdings lookup in `Trade` to ensure deterministic selection using `select` and `max_by` * Refine preloaded holdings logic in `Trade` model - Treat empty preloaded holdings as authoritative to prevent unnecessary DB queries - Add explicit fallback behavior for database query when holdings are not preloaded --------- Co-authored-by: luckyPipewrench --- app/controllers/cryptos_controller.rb | 2 + app/controllers/reports_controller.rb | 67 +++++++++++++- app/helpers/reports_helper.rb | 14 +++ app/models/investment.rb | 26 ++++++ app/models/trade.rb | 37 ++++++++ app/views/cryptos/_form.html.erb | 10 ++- app/views/investments/_form.html.erb | 2 +- .../reports/_investment_performance.html.erb | 89 ++++++++++++++++++- config/locales/views/cryptos/en.yml | 3 + config/locales/views/reports/en.yml | 16 ++++ 10 files changed, 261 insertions(+), 5 deletions(-) diff --git a/app/controllers/cryptos_controller.rb b/app/controllers/cryptos_controller.rb index 2df7d041c..87bd53414 100644 --- a/app/controllers/cryptos_controller.rb +++ b/app/controllers/cryptos_controller.rb @@ -1,3 +1,5 @@ class CryptosController < ApplicationController include AccountableResource + + permitted_accountable_attributes :id, :tax_treatment end diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 9a076701d..6d22b7971 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -415,10 +415,75 @@ class ReportsController < ApplicationController period_contributions: period_totals.contributions, period_withdrawals: period_totals.withdrawals, top_holdings: investment_statement.top_holdings(limit: 5), - accounts: investment_accounts.to_a + accounts: investment_accounts.to_a, + gains_by_tax_treatment: build_gains_by_tax_treatment(investment_statement) } end + def build_gains_by_tax_treatment(investment_statement) + currency = Current.family.currency + # Eager-load account and accountable to avoid N+1 when accessing tax_treatment + current_holdings = investment_statement.current_holdings + .includes(account: :accountable) + .to_a + + # Group holdings by tax treatment (from account) + holdings_by_treatment = current_holdings.group_by { |h| h.account.tax_treatment || :taxable } + + # Get sell trades in period with realized gains + # Eager-load security, account, and accountable to avoid N+1 + sell_trades = Current.family.trades + .joins(:entry) + .where(entries: { date: @period.date_range }) + .where("trades.qty < 0") + .includes(:security, entry: { account: :accountable }) + .to_a + + # Preload holdings for all accounts that have sell trades to avoid N+1 in realized_gain_loss + account_ids = sell_trades.map { |t| t.entry.account_id }.uniq + holdings_by_account = Holding + .where(account_id: account_ids) + .where("date <= ?", @period.date_range.end) + .order(date: :desc) + .group_by(&:account_id) + + # Inject preloaded holdings into trades for realized_gain_loss calculation + sell_trades.each do |trade| + trade.instance_variable_set(:@preloaded_holdings, holdings_by_account[trade.entry.account_id] || []) + end + + trades_by_treatment = sell_trades.group_by { |t| t.entry.account.tax_treatment || :taxable } + + # Build metrics per treatment + %i[taxable tax_deferred tax_exempt tax_advantaged].each_with_object({}) do |treatment, hash| + holdings = holdings_by_treatment[treatment] || [] + trades = trades_by_treatment[treatment] || [] + + # Sum unrealized gains from holdings (only those with known cost basis) + unrealized = holdings.sum do |h| + trend = h.trend + trend ? trend.value : 0 + end + + # Sum realized gains from sell trades + realized = trades.sum do |t| + gain = t.realized_gain_loss + gain ? gain.value : 0 + end + + # Only include treatment groups that have some activity + next if holdings.empty? && trades.empty? + + hash[treatment] = { + holdings: holdings, + sell_trades: trades, + unrealized_gain: Money.new(unrealized, currency), + realized_gain: Money.new(realized, currency), + total_gain: Money.new(unrealized + realized, currency) + } + end + end + def build_net_worth_metrics balance_sheet = Current.family.balance_sheet currency = Current.family.currency diff --git a/app/helpers/reports_helper.rb b/app/helpers/reports_helper.rb index 52b3016b5..d79535146 100644 --- a/app/helpers/reports_helper.rb +++ b/app/helpers/reports_helper.rb @@ -1,4 +1,18 @@ module ReportsHelper + # Returns CSS classes for tax treatment badge styling + def tax_treatment_badge_classes(treatment) + case treatment.to_sym + when :tax_exempt + "bg-green-500/10 text-green-600 theme-dark:text-green-400" + when :tax_deferred + "bg-blue-500/10 text-blue-600 theme-dark:text-blue-400" + when :tax_advantaged + "bg-purple-500/10 text-purple-600 theme-dark:text-purple-400" + else + "bg-gray-500/10 text-secondary" + end + end + # Generate SVG polyline points for a sparkline chart # Returns empty string if fewer than 2 data points (can't draw a line with 1 point) def sparkline_points(values, width: 60, height: 16) diff --git a/app/models/investment.rb b/app/models/investment.rb index e701b2f80..c0c9c918f 100644 --- a/app/models/investment.rb +++ b/app/models/investment.rb @@ -74,5 +74,31 @@ class Investment < ApplicationRecord def region_label_for(region) I18n.t("accounts.subtype_regions.#{region || 'generic'}") end + + # Maps currency codes to regions for prioritizing user's likely region + CURRENCY_REGION_MAP = { + "USD" => "us", + "GBP" => "uk", + "CAD" => "ca", + "AUD" => "au", + "EUR" => "eu", + "CHF" => "eu" + }.freeze + + # Returns subtypes grouped by region for use with grouped_options_for_select + # Optionally accepts currency to prioritize user's region first + def subtypes_grouped_for_select(currency: nil) + user_region = CURRENCY_REGION_MAP[currency] + grouped = SUBTYPES.group_by { |_, v| v[:region] } + + # Build region order: user's region first (if known), then Generic, then others + other_regions = %w[us uk ca au eu] - [ user_region ].compact + region_order = [ user_region, nil, *other_regions ].compact.uniq + + region_order.filter_map do |region| + next unless grouped[region] + [ region_label_for(region), grouped[region].map { |k, v| [ v[:long], k ] } ] + end + end end end diff --git a/app/models/trade.rb b/app/models/trade.rb index 0692fae96..5357b0453 100644 --- a/app/models/trade.rb +++ b/app/models/trade.rb @@ -39,9 +39,46 @@ class Trade < ApplicationRecord Trend.new(current: current_value, previous: cost_basis) end + # Calculates realized gain/loss for sell trades based on avg_cost at time of sale + # Returns nil for buy trades or when cost basis cannot be determined + def realized_gain_loss + return @realized_gain_loss if defined?(@realized_gain_loss) + + @realized_gain_loss = calculate_realized_gain_loss + end + # Trades are always excluded from expense budgets # They represent portfolio management, not living expenses def excluded_from_budget? true end + + private + + def calculate_realized_gain_loss + return nil unless sell? + + # Use preloaded holdings if available (set by reports controller to avoid N+1) + # Treat defined-but-empty preload as authoritative to prevent DB fallback + holding = if defined?(@preloaded_holdings) + # Use select + max_by for deterministic selection regardless of array order + (@preloaded_holdings || []) + .select { |h| h.security_id == security_id && h.date <= entry.date } + .max_by(&:date) + else + # Fall back to database query only when not preloaded + entry.account.holdings + .where(security_id: security_id) + .where("date <= ?", entry.date) + .order(date: :desc) + .first + end + + return nil unless holding&.avg_cost + + cost_basis = holding.avg_cost * qty.abs + sale_proceeds = price_money * qty.abs + + Trend.new(current: sale_proceeds, previous: cost_basis) + end end diff --git a/app/views/cryptos/_form.html.erb b/app/views/cryptos/_form.html.erb index 8895b27ab..6b401b19c 100644 --- a/app/views/cryptos/_form.html.erb +++ b/app/views/cryptos/_form.html.erb @@ -1,3 +1,11 @@ <%# locals: (account:, url:) %> -<%= render "accounts/form", account: account, url: url %> +<%= render "accounts/form", account: account, url: url do |form| %> + <%= form.fields_for :accountable do |crypto_form| %> + <%= crypto_form.select :tax_treatment, + Crypto.tax_treatments.keys.map { |k| [t("accounts.tax_treatments.#{k}"), k] }, + { label: t("cryptos.form.tax_treatment_label"), include_blank: false }, + {} %> +

<%= t("cryptos.form.tax_treatment_hint") %>

+ <% end %> +<% end %> diff --git a/app/views/investments/_form.html.erb b/app/views/investments/_form.html.erb index a9387af10..4fc706e33 100644 --- a/app/views/investments/_form.html.erb +++ b/app/views/investments/_form.html.erb @@ -2,6 +2,6 @@ <%= render "accounts/form", account: account, url: url do |form| %> <%= form.select :subtype, - Investment::SUBTYPES.map { |k, v| [v[:long], k] }, + grouped_options_for_select(Investment.subtypes_grouped_for_select(currency: Current.family.currency), account.subtype), { label: true, prompt: t("investments.form.subtype_prompt"), include_blank: t("investments.form.none") } %> <% end %> diff --git a/app/views/reports/_investment_performance.html.erb b/app/views/reports/_investment_performance.html.erb index c727c819a..97c76fca7 100644 --- a/app/views/reports/_investment_performance.html.erb +++ b/app/views/reports/_investment_performance.html.erb @@ -29,7 +29,7 @@ (<%= investment_metrics[:unrealized_trend].percent_formatted %>)

<% else %> -

-

+

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

<% end %> @@ -101,7 +101,7 @@ <%= holding.trend.percent_formatted %> <% else %> - - + <%= t("reports.investment_performance.no_data") %> <% end %> @@ -112,6 +112,91 @@ <% end %> + <%# Gains by Tax Treatment %> + <% if investment_metrics[:gains_by_tax_treatment].present? %> +
+

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

+ +
+ <% investment_metrics[:gains_by_tax_treatment].each do |treatment, data| %> +
+
+ "> + <%= t("accounts.tax_treatments.#{treatment}") %> + + + <%= format_money(data[:total_gain]) %> + +
+ +
+
+ <%= t("reports.investment_performance.unrealized_gains") %> + <%= format_money(data[:unrealized_gain]) %> +
+
+ <%= t("reports.investment_performance.realized_gains") %> + <%= format_money(data[:realized_gain]) %> +
+
+ + <% if treatment == :taxable && data[:realized_gain].amount > 0 %> +

+ <%= icon("alert-triangle", size: "sm") %> + <%= t("reports.investment_performance.taxable_realized_note") %> +

+ <% end %> + +
+ + <%= icon "chevron-right", size: "sm", class: "group-open:rotate-90 transition-transform" %> + <%= t("reports.investment_performance.view_details") %> + (<%= t("reports.investment_performance.holdings_count", count: data[:holdings].count) %>, <%= t("reports.investment_performance.sells_count", count: data[:sell_trades].count) %>) + + +
+ <% if data[:holdings].any? %> +
+

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

+ <% data[:holdings].first(5).each do |holding| %> +
+ <%= holding.ticker %> + " style="<%= holding.trend ? "color: #{holding.trend.color}" : "" %>"> + <%= holding.trend ? format_money(Money.new(holding.trend.value, Current.family.currency)) : t("reports.investment_performance.no_data") %> + +
+ <% end %> + <% if data[:holdings].count > 5 %> +

<%= t("reports.investment_performance.and_more", count: data[:holdings].count - 5) %>

+ <% end %> +
+ <% end %> + + <% if data[:sell_trades].any? %> +
+

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

+ <% data[:sell_trades].first(5).each do |trade| %> + <% gain = trade.realized_gain_loss %> +
+ <%= trade.security.ticker %> + " style="<%= gain ? "color: #{gain.color}" : "" %>"> + <%= gain ? format_money(Money.new(gain.value, Current.family.currency)) : t("reports.investment_performance.no_data") %> + +
+ <% end %> + <% if data[:sell_trades].count > 5 %> +

<%= t("reports.investment_performance.and_more", count: data[:sell_trades].count - 5) %>

+ <% end %> +
+ <% end %> +
+
+
+ <% end %> +
+
+ <% end %> + <%# Investment Accounts Summary %> <% if investment_metrics[:accounts].any? %>
diff --git a/config/locales/views/cryptos/en.yml b/config/locales/views/cryptos/en.yml index 298cc9cc7..eb3324b12 100644 --- a/config/locales/views/cryptos/en.yml +++ b/config/locales/views/cryptos/en.yml @@ -3,5 +3,8 @@ en: cryptos: edit: edit: Edit %{account} + form: + tax_treatment_label: Tax Treatment + tax_treatment_hint: Most cryptocurrency is held in taxable accounts. Select a different option if held in a tax-advantaged account like a self-directed IRA. new: title: Enter account balance diff --git a/config/locales/views/reports/en.yml b/config/locales/views/reports/en.yml index 189a99ebe..81bb96d91 100644 --- a/config/locales/views/reports/en.yml +++ b/config/locales/views/reports/en.yml @@ -134,6 +134,22 @@ en: value: Value return: Return accounts: Investment Accounts + gains_by_tax_treatment: Gains by Tax Treatment + unrealized_gains: Unrealized Gains + realized_gains: Realized Gains + total_gains: Total Gains + taxable_realized_note: These gains may be subject to taxes + no_data: "-" + view_details: View details + holdings_count: + one: "%{count} holding" + other: "%{count} holdings" + sells_count: + one: "%{count} sell" + other: "%{count} sells" + holdings: Holdings + sell_trades: Sell Trades + and_more: "+%{count} more" investment_flows: title: Investment Flows description: Track money flowing into and out of your investment accounts