diff --git a/app/javascript/controllers/time_series_chart_controller.js b/app/javascript/controllers/time_series_chart_controller.js index 3c358dd96..5a13d2c9c 100644 --- a/app/javascript/controllers/time_series_chart_controller.js +++ b/app/javascript/controllers/time_series_chart_controller.js @@ -408,11 +408,15 @@ export default class extends Controller { ${ - datum.trend.value === 0 + this._extractNumericValue(datum.trend.value) === 0 ? `` : ` - ${this._extractFormattedValue(datum.trend.value)} (${datum.trend.percent_formatted}) + ${this._extractFormattedValue(datum.trend.value)}${ + datum.trend.percent === null + ? "" + : ` (${datum.trend.percent_formatted})` + } ` } diff --git a/app/models/balance_sheet/net_worth_breakdown_series_builder.rb b/app/models/balance_sheet/net_worth_breakdown_series_builder.rb index 06397c4a5..6e21093f3 100644 --- a/app/models/balance_sheet/net_worth_breakdown_series_builder.rb +++ b/app/models/balance_sheet/net_worth_breakdown_series_builder.rb @@ -2,6 +2,7 @@ class BalanceSheet::NetWorthBreakdownSeriesBuilder # Monthly interval regardless of period length so the reports chart always # shows one point per month in the selected range. INTERVAL = "1 month" + CACHE_VERSION = "v2" def initialize(family, user: nil) @family = family @@ -41,18 +42,26 @@ class BalanceSheet::NetWorthBreakdownSeriesBuilder } end + # Built by hand rather than through `Series#as_json`, so it needs the same + # display rounding: the chart is drawn from the amount, and a sub-unit + # residue would plot as a visible move between two points that print the + # same value. The classification totals are left alone, since only their + # formatted string is rendered. + current = value.value.for_display + previous_value = (previous&.value || value.value).for_display + { date: value.date, date_formatted: value.date_formatted, - value: value.value, + value: current, # Month-over-month change between chart points. The trend on the raw # series value compares the underlying balance row's own start/end, # which at a monthly interval reflects only the last balance update # before the sample date. The first point has no prior month, so it # gets a flat trend. trend: Trend.new( - current: value.value, - previous: previous&.value || value.value, + current: current, + previous: previous_value, favorable_direction: "up" ), assets: classification_total(point_groups, "asset"), @@ -125,6 +134,7 @@ class BalanceSheet::NetWorthBreakdownSeriesBuilder shares_version = user ? AccountShare.where(user: user).maximum(:updated_at)&.to_i : nil key = [ "balance_sheet_net_worth_breakdown_series", + CACHE_VERSION, user&.id, shares_version, period.start_date, diff --git a/app/models/series.rb b/app/models/series.rb index 501d78fba..6c2800bf1 100644 --- a/app/models/series.rb +++ b/app/models/series.rb @@ -56,11 +56,14 @@ class Series @favorable_direction = favorable_direction end + # Rounded like the serialized values: this trend is only ever rendered next to + # them, so a change between two amounts that print identically has to read as + # no change here too. def trend return nil if values.blank? @trend ||= Trend.new( - current: values.last&.value, - previous: values.first&.value, + current: display_amount(values.last&.value), + previous: display_amount(values.first&.value), favorable_direction: favorable_direction ) end @@ -71,7 +74,27 @@ class Series end_date: end_date, interval: interval, trend: trend, - values: values.map { |v| { date: v.date, date_formatted: v.date_formatted, value: v.value, trend: v.trend } } + values: values.map do |v| + { date: v.date, date_formatted: v.date_formatted, value: display_amount(v.value), trend: display_trend(v.trend) } + end } end + + private + # A chart is drawn from these amounts, not from the formatted strings, so a + # sub-unit residue would plot as a visible move between two points that both + # print as the same value, and a change between them would read as non-zero. + def display_trend(trend) + return trend unless trend.is_a?(Trend) + + Trend.new( + current: display_amount(trend.current), + previous: display_amount(trend.previous), + favorable_direction: trend.favorable_direction + ) + end + + def display_amount(amount) + amount.is_a?(Money) ? amount.for_display : amount + end end diff --git a/app/views/accountable_sparklines/show.html.erb b/app/views/accountable_sparklines/show.html.erb index c090551a6..05b0339c0 100644 --- a/app/views/accountable_sparklines/show.html.erb +++ b/app/views/accountable_sparklines/show.html.erb @@ -4,8 +4,11 @@ <%= render "shared/sparkline", id: dom_id(@accountable, :sparkline_chart), series: @series %> - <%= tag.p @series.trend.percent_formatted, - style: "color: #{@series.trend.color}", - class: "font-mono text-right text-xs font-medium text-primary" %> + <%# A previous value of zero makes the percentage infinite, with nothing useful to print %> + <% if @series.trend.percent.finite? %> + <%= tag.p @series.trend.percent_formatted, + style: "color: #{@series.trend.color}", + class: "font-mono text-right text-xs font-medium text-primary" %> + <% end %> <% end %> diff --git a/app/views/accounts/sparkline.html.erb b/app/views/accounts/sparkline.html.erb index 0a07c653d..ba8339049 100644 --- a/app/views/accounts/sparkline.html.erb +++ b/app/views/accounts/sparkline.html.erb @@ -4,8 +4,11 @@ <%= render "shared/sparkline", id: dom_id(@account, :sparkline_chart), series: @sparkline_series %> - <%= tag.p @sparkline_series.trend.percent_formatted, - style: "color: #{@sparkline_series.trend.color}", - class: "font-mono text-right text-xs font-medium text-primary" %> + <%# A previous value of zero makes the percentage infinite, with nothing useful to print %> + <% if @sparkline_series.trend.percent.finite? %> + <%= tag.p @sparkline_series.trend.percent_formatted, + style: "color: #{@sparkline_series.trend.color}", + class: "font-mono text-right text-xs font-medium text-primary" %> + <% end %> <% end %> diff --git a/app/views/reports/_net_worth.html.erb b/app/views/reports/_net_worth.html.erb index 0d6bc74b0..6c87fafd6 100644 --- a/app/views/reports/_net_worth.html.erb +++ b/app/views/reports/_net_worth.html.erb @@ -17,9 +17,11 @@

<%= trend.value.format(signify_positive: true) %>

-

- <%= trend.value >= 0 ? "+" : "" %><%= trend.percent_formatted %> -

+ <% if trend.percent.finite? %> +

+ <%= trend.value >= 0 ? "+" : "" %><%= trend.percent_formatted %> +

+ <% end %> <% else %>

--

<% end %> diff --git a/app/views/reports/print.html.erb b/app/views/reports/print.html.erb index d92698ab4..bfbe76ddc 100644 --- a/app/views/reports/print.html.erb +++ b/app/views/reports/print.html.erb @@ -82,9 +82,16 @@ <%= @net_worth_metrics[:current_net_worth].format %> <% if @net_worth_metrics[:trend] %> - - <%= @net_worth_metrics[:trend].value >= 0 ? "+" : "" %><%= @net_worth_metrics[:trend].value.format %> (<%= @net_worth_metrics[:trend].percent_formatted %>) <%= t("reports.print.net_worth.this_period") %> - + <% trend = @net_worth_metrics[:trend] %> + <% unless trend.direction.flat? %> + + <%= trend.value >= 0 ? "+" : "" %><%= trend.value.format %> + <% if trend.percent.finite? %> + (<%= trend.percent_formatted %>) + <% end %> + <%= t("reports.print.net_worth.this_period") %> + + <% end %> <% end %> <% if has_sparkline_data?(@trends_data) %> diff --git a/lib/money.rb b/lib/money.rb index 0d1e73a88..4f5887761 100644 --- a/lib/money.rb +++ b/lib/money.rb @@ -68,6 +68,11 @@ class Money end end + # Rounded to the precision `format` actually prints. + def for_display + self.class.new(amount.round(currency.default_precision), currency) + end + def as_json { amount: amount, currency: currency.iso_code, formatted: format }.as_json end diff --git a/test/lib/money_test.rb b/test/lib/money_test.rb index 8232de506..1ee6b9745 100644 --- a/test/lib/money_test.rb +++ b/test/lib/money_test.rb @@ -90,6 +90,11 @@ class MoneyTest < ActiveSupport::TestCase assert_equal "€ 1.000,12", Money.new(1000.12, :eur).format(locale: :nl) end + test "rounds for display using currency precision" do + assert_equal Money.new(1000.9, :usd), Money.new(1000.899, :usd).for_display + assert_equal Money.new(1001, :jpy), Money.new(1000.6, :jpy).for_display + end + test "formats correctly for French locale" do # French uses non-breaking spaces (NBSP = \u00A0) between thousands and before currency symbol assert_equal "1\u00A0000,12\u00A0€", Money.new(1000.12, :eur).format(locale: :fr) diff --git a/test/models/balance_sheet/net_worth_breakdown_series_builder_test.rb b/test/models/balance_sheet/net_worth_breakdown_series_builder_test.rb index eace99634..efe513025 100644 --- a/test/models/balance_sheet/net_worth_breakdown_series_builder_test.rb +++ b/test/models/balance_sheet/net_worth_breakdown_series_builder_test.rb @@ -62,6 +62,43 @@ class BalanceSheet::NetWorthBreakdownSeriesBuilderTest < ActiveSupport::TestCase assert groups.all? { |g| g[:color].present? } end + test "does not serialize a sub-unit residue the chart would plot as a move" do + period = Period.custom(start_date: Date.new(2026, 6, 15), end_date: Date.new(2026, 7, 15)) + + # Both amounts print as $0.00, so the chart has to draw a flat line + create_balance(account: @asset_account, date: period.start_date, balance: 0.0001) + create_balance(account: @asset_account, date: period.end_date, balance: 0.0002) + + series = builder.breakdown_series(period: period) + + assert series[:values].size >= 2 + series[:values].each do |point| + assert_equal 0, point[:value].amount + assert point[:trend].direction.flat?, "expected no change between values printing as $0.00" + assert point[:trend].percent.finite?, "expected a finite percentage" + end + end + + test "serializes a nil trend percentage when displayed value increases from zero" do + period = Period.custom(start_date: Date.new(2026, 6, 15), end_date: Date.new(2026, 7, 15)) + + create_balance(account: @asset_account, date: period.start_date, balance: 0.0001) + create_balance(account: @asset_account, date: period.end_date, balance: 1) + + series = builder.breakdown_series(period: period) + parsed = JSON.parse(series.to_json) + + assert_equal 0, series[:values].first[:value].amount + assert_equal 1, series[:values].last[:value].amount + assert_nil parsed["values"].last["trend"]["percent"] + end + + test "cache key includes payload version" do + period = Period.custom(start_date: Date.new(2026, 6, 15), end_date: Date.new(2026, 7, 15)) + + assert_includes builder.send(:cache_key, period), BalanceSheet::NetWorthBreakdownSeriesBuilder::CACHE_VERSION + end + private def builder BalanceSheet::NetWorthBreakdownSeriesBuilder.new(@family) diff --git a/test/models/series_test.rb b/test/models/series_test.rb new file mode 100644 index 000000000..40ccf4151 --- /dev/null +++ b/test/models/series_test.rb @@ -0,0 +1,79 @@ +require "test_helper" + +class SeriesTest < ActiveSupport::TestCase + test "a change between two values that print identically reads as no change" do + series = build_series(previous: 100.001, current: 100.004, currency: "USD") + + trend = series.as_json[:values].last[:trend] + + # Both amounts format as $100.00 + assert_equal 0, trend.value.amount + assert trend.direction.flat? + end + + test "rounds the trend at the currency's display precision" do + series = build_series(previous: 1000.4, current: 1000.6, currency: "JPY") + + trend = series.as_json[:values].last[:trend] + + # ¥1,000 -> ¥1,001 + assert_equal 1, trend.value.amount + assert trend.direction.up? + end + + test "does not serialize a sub-unit residue the chart would plot as a move" do + # A leftover cent fraction (400 - 199.9999 - 200) prints as $0.00, but the + # chart is drawn from the amount, so an unrounded 0.0001 fills the y axis + series = build_series(previous: 0, current: 0.0001, currency: "USD") + + assert_equal 0, series.as_json[:values].last[:value].amount + end + + test "leaves the series values themselves exact" do + series = build_series(previous: 100.001, current: 100.004, currency: "USD") + + # Insights, goals and the assistant read the values directly + assert_equal 100.004, series.values.last.value.amount + assert_equal 0.003, series.values.last.trend.value.amount + end + + test "the series trend ignores a change that is not visible" do + # The account chart renders this trend right above the chart itself + series = build_series(previous: 0, current: 0.0001, currency: "USD") + + assert series.trend.direction.flat? + assert_equal 0, series.trend.value.amount + assert series.trend.percent.finite? + end + + private + def build_series(previous:, current:, currency:) + Series.new( + start_date: Date.new(2026, 6, 1), + end_date: Date.new(2026, 6, 2), + interval: "1 day", + values: [ + Series::Value.new( + date: Date.new(2026, 6, 1), + date_formatted: "June 1, 2026", + value: Money.new(previous, currency), + trend: Trend.new( + current: Money.new(previous, currency), + previous: Money.new(previous, currency), + favorable_direction: "up" + ) + ), + Series::Value.new( + date: Date.new(2026, 6, 2), + date_formatted: "June 2, 2026", + value: Money.new(current, currency), + trend: Trend.new( + current: Money.new(current, currency), + previous: Money.new(previous, currency), + favorable_direction: "up" + ) + ) + ] + ) + end +end