From 40ae3126acc9f1b21dc88f5212024f7bd1935aca Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 18 Jul 2025 13:55:45 -0400 Subject: [PATCH] Fill in balance reconciliation for entry group --- .../UI/account/activity_feed.html.erb | 97 ++++++++++ app/components/UI/account/activity_feed.rb | 43 +++++ .../UI/account/entries_date_group.html.erb | 78 ++++++++ .../UI/account/entries_date_group.rb | 68 +++++++ app/components/UI/account_page.html.erb | 6 +- app/components/UI/account_page.rb | 20 +- app/controllers/accounts_controller.rb | 2 + app/models/account/activity_feed_data.rb | 135 ++++++++++---- app/views/accounts/show.html.erb | 4 +- .../models/account/activity_feed_data_test.rb | 171 ++++++++++++++++++ test/support/entries_test_helper.rb | 56 +++--- 11 files changed, 610 insertions(+), 70 deletions(-) create mode 100644 app/components/UI/account/activity_feed.html.erb create mode 100644 app/components/UI/account/activity_feed.rb create mode 100644 app/components/UI/account/entries_date_group.html.erb create mode 100644 app/components/UI/account/entries_date_group.rb create mode 100644 test/models/account/activity_feed_data_test.rb diff --git a/app/components/UI/account/activity_feed.html.erb b/app/components/UI/account/activity_feed.html.erb new file mode 100644 index 00000000..894e25e7 --- /dev/null +++ b/app/components/UI/account/activity_feed.html.erb @@ -0,0 +1,97 @@ +<%= turbo_frame_tag dom_id(account, "entries") do %> +
+
+ <%= tag.h2 t(".title"), class: "font-medium text-lg" %> + + <% if account.manual? %> + <%= render MenuComponent.new(variant: "button") do |menu| %> + <% menu.with_button(text: "New", variant: "secondary", icon: "plus") %> + + <% menu.with_item( + variant: "link", + text: "New balance", + icon: "circle-dollar-sign", + href: new_valuation_path(account_id: account.id), + data: { turbo_frame: :modal }) %> + + <% unless account.crypto? %> + <% menu.with_item( + variant: "link", + text: "New transaction", + icon: "credit-card", + href: account.investment? ? new_trade_path(account_id: account.id) : new_transaction_path(account_id: account.id), + data: { turbo_frame: :modal }) %> + <% end %> + <% end %> + <% end %> +
+ +
+ <%= form_with url: account_path(account), + id: "entries-search", + scope: :q, + method: :get, + data: { controller: "auto-submit-form" } do |form| %> +
+
+
+ <%= helpers.icon("search") %> + + <%= hidden_field_tag :account_id, account.id %> + + <%= form.search_field :search, + placeholder: "Search entries by name", + value: search, + class: "form-field__input placeholder:text-sm placeholder:text-secondary", + "data-auto-submit-form-target": "auto" %> +
+
+
+ <% end %> +
+ + <% if grouped_entries.empty? %> +

No entries yet

+ <% else %> + <%= tag.div id: dom_id(account, "entries_bulk_select"), + data: { + controller: "bulk-select", + bulk_select_singular_label_value: "entry", + bulk_select_plural_label_value: "entries" + } do %> + + +
+
+ <%= check_box_tag "selection_entry", + class: "checkbox checkbox--light", + data: { action: "bulk-select#togglePageSelection" } %> +

Date

+
+ + <%= tag.p "Amount", class: "col-span-4 justify-self-end" %> +
+ +
+
+ <% grouped_entries.each do |date, entries| %> + <%= render UI::Account::EntriesDateGroup.new( + account: account, + date: date, + entries: entries, + balance_trend: balance_trend_for_date(date), + transfers: transfers_for_date(date) + ) %> + <% end %> +
+ +
+ <%= render "shared/pagination", pagy: pagy %> +
+
+ <% end %> + <% end %> +
+<% end %> \ No newline at end of file diff --git a/app/components/UI/account/activity_feed.rb b/app/components/UI/account/activity_feed.rb new file mode 100644 index 00000000..d58906cc --- /dev/null +++ b/app/components/UI/account/activity_feed.rb @@ -0,0 +1,43 @@ +class UI::Account::ActivityFeed < ApplicationComponent + attr_reader :feed_data, :pagy, :search + + def initialize(feed_data:, pagy:, search: nil) + @feed_data = feed_data + @pagy = pagy + @search = search + end + + def id + dom_id(account, :activity_feed) + end + + def broadcast_channel + account + end + + def broadcast_refresh! + Turbo::StreamsChannel.broadcast_replace_to( + broadcast_channel, + target: id, + renderable: self, + layout: false + ) + end + + def grouped_entries + feed_data.entries.group_by(&:date).sort.reverse + end + + def balance_trend_for_date(date) + feed_data.trend_for_date(date) + end + + def transfers_for_date(date) + feed_data.transfers_for_date(date) + end + + private + def account + feed_data.account + end +end diff --git a/app/components/UI/account/entries_date_group.html.erb b/app/components/UI/account/entries_date_group.html.erb new file mode 100644 index 00000000..ddff1114 --- /dev/null +++ b/app/components/UI/account/entries_date_group.html.erb @@ -0,0 +1,78 @@ +<%= tag.div id: id, data: { bulk_select_target: "group" }, class: "bg-container-inset rounded-xl p-1 w-full" do %> +
+ +
+
+ <%= check_box_tag "#{date}_entries_selection", + class: ["checkbox checkbox--light", "hidden": entries.size == 0], + id: "selection_entry_#{date}", + data: { action: "bulk-select#toggleGroupSelection" } %> + +

+ <%= tag.span I18n.l(date, format: :long) %> + · + <%= tag.span entries.size %> +

+
+ +
+ <%= helpers.icon "chevron-down", class: "group-open:rotate-180" %> +
+
+ + +
+
+
Start of day balance
+
+
<%= start_balance_money.format %>
+
+ + <% if account.balance_type == :investment %> +
+
Cash Δ
+
+
<%= transaction_totals_money.format %>
+
+ +
+
Holdings Δ
+
+
<%= holding_change_money.format %>
+
+ <% else %> +
+
Transaction totals
+
+
<%= transaction_totals_money.format %>
+
+ <% end %> + +
+
End of day balance
+
+
<%= end_balance_before_adjustments_money.format %>
+
+ +
+
+
Value adjustments Δ
+
+
<%= adjustments_money.format %>
+
+ +
+
Closing balance (incl. adjustments)
+
+
<%= end_balance_money.format %>
+
+
+
+
+ +
+ <% entries.each do |entry| %> + <%= render entry, view_ctx: "account" %> + <% end %> +
+<% end %> diff --git a/app/components/UI/account/entries_date_group.rb b/app/components/UI/account/entries_date_group.rb new file mode 100644 index 00000000..985b04ae --- /dev/null +++ b/app/components/UI/account/entries_date_group.rb @@ -0,0 +1,68 @@ +class UI::Account::EntriesDateGroup < ApplicationComponent + attr_reader :account, :date, :entries, :balance_trend, :transfers + + def initialize(account:, date:, entries:, balance_trend:, transfers:) + @account = account + @date = date + @entries = entries + @balance_trend = balance_trend + @transfers = transfers + end + + def id + dom_id(account, "entries_#{date}") + end + + def broadcast_channel + account + end + + def valuation_entry + entries.find { |entry| entry.entryable_type == "Valuation" } + end + + def start_balance_money + balance_trend.previous + end + + def end_balance_before_adjustments_money + balance_trend.previous + transaction_totals_money - holding_change_money + end + + def adjustments_money + end_balance_money - end_balance_before_adjustments_money + end + + def transaction_totals_money + transactions = entries.select { |e| e.transaction? } + + if transactions.any? + transactions.sum { |e| e.amount_money } * -1 + else + Money.new(0, account.currency) + end + end + + def holding_change_money + trades = entries.select { |e| e.trade? } + + if trades.any? + trades.sum { |e| e.amount_money } * -1 + else + Money.new(0, account.currency) + end + end + + def end_balance_money + balance_trend.current + end + + def broadcast_refresh! + Turbo::StreamsChannel.broadcast_replace_to( + broadcast_channel, + target: id, + renderable: self, + layout: false + ) + end +end diff --git a/app/components/UI/account_page.html.erb b/app/components/UI/account_page.html.erb index 7b02d30f..18f5c4d9 100644 --- a/app/components/UI/account_page.html.erb +++ b/app/components/UI/account_page.html.erb @@ -1,6 +1,6 @@ <%= turbo_stream_from account %> -<%= turbo_frame_tag dom_id(account, :container) do %> +<%= turbo_frame_tag id do %> <%= tag.div class: "space-y-4 pb-32" do %> <%= render "accounts/show/header", account: account, title: title, subtitle: subtitle %> @@ -17,12 +17,12 @@ <% tabs.each do |tab| %> <% tabs_container.with_panel(tab_id: tab) do %> - <%= render tab_partial_name(tab), account: account %> + <%= tab_content_for(tab) %> <% end %> <% end %> <% end %> <% else %> - <%= render tab_partial_name(tabs.first), account: account %> + <%= tab_content_for(tabs.first) %> <% end %> <% end %> diff --git a/app/components/UI/account_page.rb b/app/components/UI/account_page.rb index 159ed56f..5d3dd54a 100644 --- a/app/components/UI/account_page.rb +++ b/app/components/UI/account_page.rb @@ -1,6 +1,8 @@ class UI::AccountPage < ApplicationComponent attr_reader :account, :chart_view, :chart_period + renders_one :activity_feed, ->(feed_data:, pagy:, search:) { UI::Account::ActivityFeed.new(feed_data: feed_data, pagy: pagy, search: search) } + def initialize(account:, chart_view: nil, chart_period: nil, active_tab: nil) @account = account @chart_view = chart_view @@ -8,6 +10,18 @@ class UI::AccountPage < ApplicationComponent @active_tab = active_tab end + def id + dom_id(account, :container) + end + + def broadcast_channel + account + end + + def broadcast_refresh! + Turbo::StreamsChannel.broadcast_replace_to(broadcast_channel, target: id, renderable: self, layout: false) + end + def title account.name end @@ -33,13 +47,13 @@ class UI::AccountPage < ApplicationComponent end end - def tab_partial_name(tab) + def tab_content_for(tab) case tab when :activity - "accounts/show/activity" + activity_feed when :holdings, :overview # Accountable is responsible for implementing the partial in the correct folder - "#{account.accountable_type.downcase.pluralize}/tabs/#{tab}" + render "#{account.accountable_type.downcase.pluralize}/tabs/#{tab}", account: account end end end diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 7bf4470c..0b2252d7 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -16,6 +16,8 @@ class AccountsController < ApplicationController entries = @account.entries.search(@q).reverse_chronological @pagy, @entries = pagy(entries, limit: params[:per_page] || "10") + + @activity_feed_data = Account::ActivityFeedData.new(@account, @entries) end def sync diff --git a/app/models/account/activity_feed_data.rb b/app/models/account/activity_feed_data.rb index 4b2de075..24971149 100644 --- a/app/models/account/activity_feed_data.rb +++ b/app/models/account/activity_feed_data.rb @@ -2,58 +2,72 @@ # This data object is useful for avoiding N+1 queries and having an easy way to pass around the required data to the # activity feed component in controllers and background jobs that refresh it. class Account::ActivityFeedData - attr_reader :account + attr_reader :account, :entries def initialize(account, entries) @account = account @entries = entries.to_a end - # We read balances so we can show "start of day" -> "end of day" balances for each entry date group in the feed - def balances - @balances ||= begin - min_date = entries.min_by(&:date).date.prev_day - max_date = entries.max_by(&:date).date - - account.balances.where(date: min_date..max_date, currency: account.currency).order(:date) - end + def trend_for_date(date) + start_balance = start_balance_for_date(date) + end_balance = end_balance_for_date(date) + + Trend.new( + current: end_balance.balance_money, + previous: start_balance.balance_money + ) end + def transfers_for_date(date) + date_entries = entries_by_date[date] || [] + return [] if date_entries.empty? - def transfers - return [] unless has_transfers? + date_transaction_ids = date_entries.select(&:transaction?).map(&:entryable_id) + return [] if date_transaction_ids.empty? - @transfers ||= Transfer.where(inflow_transaction_id: transaction_ids).or(Transfer.where(outflow_transaction_id: transaction_ids)) + # Convert to Set for O(1) lookups + date_transaction_id_set = Set.new(date_transaction_ids) + + transfers.select { |txfr| + date_transaction_id_set.include?(txfr.inflow_transaction_id) || + date_transaction_id_set.include?(txfr.outflow_transaction_id) + } end - # If the account has entries denominated in a different currency than the main account, we attach necessary - # exchange rates required to "roll up" the entry group balance into the normal account currency. - def exchange_rates - return [] unless needs_exchange_rates? - - @exchange_rates ||= begin - rate_requirements = required_exchange_rates - return [] if rate_requirements.empty? - - # Build a single SQL query with all date/currency pairs - conditions = rate_requirements.map do |req| - "(date = ? AND from_currency = ? AND to_currency = ?)" - end.join(" OR ") - - # Flatten the parameters array in the same order - params = rate_requirements.flat_map do |req| - [ req.date, req.from, req.to ] - end - - ExchangeRate.where(conditions, *params) - end + def exchange_rates_for_date(date) + exchange_rates.select { |rate| rate.date == date } end private - attr_reader :entries + # Finds the balance on date, or the most recent balance before it ("last observation carried forward") + def start_balance_for_date(date) + locf_balance_for_date(date.prev_day) || generate_fallback_balance(date) + end + + # Finds the balance on date, or the most recent balance before it ("last observation carried forward") + def end_balance_for_date(date) + locf_balance_for_date(date) || generate_fallback_balance(date) + end RequiredExchangeRate = Data.define(:date, :from, :to) + def entries_by_date + @entries_by_date ||= entries.group_by(&:date) + end + + # We read balances so we can show "start of day" -> "end of day" balances for each entry date group in the feed + def balances + @balances ||= begin + return [] if entries.empty? + + min_date = entries.min_by(&:date).date.prev_day + max_date = entries.max_by(&:date).date + + account.balances.where(date: min_date..max_date, currency: account.currency).order(:date).to_a + end + end + def needs_exchange_rates? entries.any? { |entry| entry.currency != account.currency } end @@ -66,11 +80,60 @@ class Account::ActivityFeedData end.uniq end + # If the account has entries denominated in a different currency than the main account, we attach necessary + # exchange rates required to "roll up" the entry group balance into the normal account currency. + def exchange_rates + return [] unless needs_exchange_rates? + + @exchange_rates ||= begin + rate_requirements = required_exchange_rates + return [] if rate_requirements.empty? + + # Build a single SQL query with all date/currency pairs + conditions = rate_requirements.map do |req| + "(date = ? AND from_currency = ? AND to_currency = ?)" + end.join(" OR ") + + # Flatten the parameters array in the same order + params = rate_requirements.flat_map do |req| + [ req.date, req.from, req.to ] + end + + ExchangeRate.where(conditions, *params).to_a + end + end + + def transaction_ids + entries.select { |entry| entry.transaction? }.map(&:entryable_id) + end + def has_transfers? entries.any? { |entry| entry.transaction? && entry.transaction.transfer? } end - def transaction_ids - entries.select { |entry| entry.transaction? }.pluck(:entryable_id) + def transfers + return [] unless has_transfers? + + @transfers ||= Transfer.where(inflow_transaction_id: transaction_ids).or(Transfer.where(outflow_transaction_id: transaction_ids)).to_a + end + + # Use binary search since balances are sorted by date + def locf_balance_for_date(date) + idx = balances.bsearch_index { |b| b.date > date } + + if idx + idx > 0 ? balances[idx - 1] : nil + else + balances.last + end + end + + def generate_fallback_balance(date) + Balance.new( + account: account, + date: date, + balance: 0, + currency: account.currency + ) end end diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index a24e8b0b..170f1298 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -3,4 +3,6 @@ chart_view: @chart_view, chart_period: @period, active_tab: @tab - ) %> + ) do |account_page| %> + <%= account_page.with_activity_feed(feed_data: @activity_feed_data, pagy: @pagy, search: @q[:search]) %> +<% end %> diff --git a/test/models/account/activity_feed_data_test.rb b/test/models/account/activity_feed_data_test.rb new file mode 100644 index 00000000..21a65e2f --- /dev/null +++ b/test/models/account/activity_feed_data_test.rb @@ -0,0 +1,171 @@ +require "test_helper" + +class Account::ActivityFeedDataTest < ActiveSupport::TestCase + include EntriesTestHelper + + setup do + @family = families(:empty) + @checking = @family.accounts.create!(name: "Test Checking", accountable: Depository.new, currency: "USD", balance: 0) + @savings = @family.accounts.create!(name: "Test Savings", accountable: Depository.new, currency: "USD", balance: 0) + @investment = @family.accounts.create!(name: "Test Investment", accountable: Investment.new, currency: "USD", balance: 0) + + @test_period_start = Date.current - 4.days + @test_period_end = Date.current + + setup_test_data + end + + test "calculates correct trend for a given date when all balances exist" do + entries = @checking.entries.includes(:entryable).to_a + feed_data = Account::ActivityFeedData.new(@checking, entries) + + # Trend for day 2 should show change from end of day 1 to end of day 2 + trend = feed_data.trend_for_date(@test_period_start + 1.day) + assert_equal 1100, trend.current.amount.to_i # End of day 2 + assert_equal 1000, trend.previous.amount.to_i # End of day 1 + assert_equal 100, trend.value.amount.to_i + assert_equal "up", trend.direction.to_s + end + + test "calculates trend with correct start and end values" do + entries = @checking.entries.includes(:entryable).to_a + feed_data = Account::ActivityFeedData.new(@checking, entries) + + # First day trend (no previous day balance) + trend = feed_data.trend_for_date(@test_period_start) + assert_equal 1000, trend.current.amount.to_i # End of first day + assert_equal 0, trend.previous.amount.to_i # Fallback to 0 + assert_equal 1000, trend.value.amount.to_i + end + + test "uses last observation carried forward when intermediate balances are missing" do + @checking.balances.where(date: [ @test_period_start + 1.day, @test_period_start + 3.days ]).destroy_all + + entries = @checking.entries.includes(:entryable).to_a + feed_data = Account::ActivityFeedData.new(@checking, entries) + + # When day 2 balance is missing, both start and end use day 1 balance + trend = feed_data.trend_for_date(@test_period_start + 1.day) + assert_equal 1000, trend.current.amount.to_i # LOCF from day 1 + assert_equal 1000, trend.previous.amount.to_i # LOCF from day 1 + assert_equal 0, trend.value.amount.to_i + assert_equal "flat", trend.direction.to_s + + # When day 4 balance is missing, uses last available (day 1) + trend = feed_data.trend_for_date(@test_period_start + 3.days) + assert_equal 1000, trend.current.amount.to_i # LOCF from day 1 + assert_equal 1000, trend.previous.amount.to_i # LOCF from day 1 + end + + test "returns zero-balance fallback when no prior balances exist" do + @checking.balances.destroy_all + + entries = @checking.entries.includes(:entryable).to_a + feed_data = Account::ActivityFeedData.new(@checking, entries) + + trend = feed_data.trend_for_date(@test_period_start + 2.days) + assert_equal 0, trend.current.amount.to_i # Fallback to 0 + assert_equal 0, trend.previous.amount.to_i # Fallback to 0 + assert_equal 0, trend.value.amount.to_i + assert_equal "flat", trend.direction.to_s + end + + test "identifies transfers for a specific date" do + entries = @checking.entries.includes(:entryable).to_a + feed_data = Account::ActivityFeedData.new(@checking, entries) + + # Day 2 has the transfer + transfers = feed_data.transfers_for_date(@test_period_start + 1.day) + assert_equal 1, transfers.size + assert_equal @transfer, transfers.first + + # Other days have no transfers + transfers = feed_data.transfers_for_date(@test_period_start) + assert_empty transfers + end + + test "loads exchange rates only for entries with foreign currencies" do + entries = @investment.entries.includes(:entryable).to_a + feed_data = Account::ActivityFeedData.new(@investment, entries) + + rates = feed_data.exchange_rates_for_date(@test_period_start + 2.days) + assert_equal 1, rates.size + assert_equal "EUR", rates.first.from_currency + assert_equal "USD", rates.first.to_currency + assert_equal 1.1, rates.first.rate + + rates = feed_data.exchange_rates_for_date(@test_period_start) + assert_empty rates + end + + test "returns empty exchange rates when no foreign currency entries exist" do + entries = @checking.entries.includes(:entryable).to_a + feed_data = Account::ActivityFeedData.new(@checking, entries) + + rates = feed_data.exchange_rates_for_date(@test_period_start + 2.days) + assert_empty rates + end + + private + + def setup_test_data + # Create daily balances for checking account + 5.times do |i| + date = @test_period_start + i.days + @checking.balances.create!( + date: date, + balance: 1000 + (i * 100), + currency: "USD" + ) + end + + # Day 1: Regular transaction + create_transaction( + account: @checking, + date: @test_period_start, + amount: -50, + name: "Grocery Store" + ) + + # Day 2: Transfer between accounts + @transfer = create_transfer( + from_account: @checking, + to_account: @savings, + amount: 200, + date: @test_period_start + 1.day + ) + + # Day 3: Trade in investment account + create_trade( + securities(:aapl), + account: @investment, + qty: 10, + date: @test_period_start + 2.days, + price: 150 + ) + + # Day 3: Foreign currency transaction + create_transaction( + account: @investment, + date: @test_period_start + 2.days, + amount: -100, + currency: "EUR", + name: "International Wire" + ) + + # Create exchange rate for foreign currency + ExchangeRate.create!( + date: @test_period_start + 2.days, + from_currency: "EUR", + to_currency: "USD", + rate: 1.1 + ) + + # Day 4: Valuation + create_valuation( + account: @investment, + date: @test_period_start + 3.days, + amount: 25 + ) + end +end diff --git a/test/support/entries_test_helper.rb b/test/support/entries_test_helper.rb index 35e5450f..f586b148 100644 --- a/test/support/entries_test_helper.rb +++ b/test/support/entries_test_helper.rb @@ -15,33 +15,6 @@ module EntriesTestHelper Entry.create! entry_defaults.merge(entry_attributes) end - def create_opening_anchor_valuation(account:, balance:, date:) - create_valuation( - account: account, - kind: "opening_anchor", - amount: balance, - date: date - ) - end - - def create_reconciliation_valuation(account:, balance:, date:) - create_valuation( - account: account, - kind: "reconciliation", - amount: balance, - date: date - ) - end - - def create_current_anchor_valuation(account:, balance:, date: Date.current) - create_valuation( - account: account, - kind: "current_anchor", - amount: balance, - date: date - ) - end - def create_valuation(attributes = {}) entry_attributes = attributes.except(:kind) valuation_attributes = attributes.slice(:kind) @@ -77,4 +50,33 @@ module EntriesTestHelper currency: currency, entryable: trade end + + def create_transfer(from_account:, to_account:, amount:, date: Date.current, currency: "USD") + outflow_transaction = Transaction.create!(kind: "funds_movement") + inflow_transaction = Transaction.create!(kind: "funds_movement") + + transfer = Transfer.create!( + outflow_transaction: outflow_transaction, + inflow_transaction: inflow_transaction + ) + + # Create entries for both accounts + from_account.entries.create!( + name: "Transfer to #{to_account.name}", + date: date, + amount: -amount.abs, + currency: currency, + entryable: outflow_transaction + ) + + to_account.entries.create!( + name: "Transfer from #{from_account.name}", + date: date, + amount: amount.abs, + currency: currency, + entryable: inflow_transaction + ) + + transfer + end end