From 5ee3275f9830ab9dd3db2e9457ac1d8ea25f7a8f Mon Sep 17 00:00:00 2001 From: Bishal Shrestha <95735295+shrestha-bishal@users.noreply.github.com> Date: Thu, 30 Jul 2026 09:11:56 +1000 Subject: [PATCH] feat: show counterpart account in transfer transaction list row (#2643) * Show counterpart account in transfer transaction list row * feat: show transfer counterpart account with access and nil guards * - Gate counterpart name behind accessible_accounts check. - Add nested transfer includes to TransactionsController and AccountsController to prevent N+1 queries. - Use precomputed @accessible_account_ids Set for O(1) lookups. * test: add view tests for transfer counterpart rendering Cover outflow arrow, inflow arrow, and unmatched transfer fallback using ActionView::TestCase following existing merged_badge pattern. * Fix transfer eager loading for polymorphic entryables * Keep accessible_account_ids as Array to fix mock test expectations --- app/controllers/accounts_controller.rb | 13 +++ app/controllers/transactions_controller.rb | 13 +-- app/views/transactions/_transaction.html.erb | 16 +++- .../transfer_counterpart_view_test.rb | 85 +++++++++++++++++++ 4 files changed, 120 insertions(+), 7 deletions(-) create mode 100644 test/views/transactions/transfer_counterpart_view_test.rb diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index c745d3caa..330312dc6 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -57,8 +57,10 @@ class AccountsController < ApplicationController def show @chart_view = params[:chart_view] || "balance" @tab = params[:tab] + @accessible_account_ids = Current.user.accessible_accounts.pluck(:id).to_set @q = params.fetch(:q, {}).permit(:search, status: []) entries = @account.entries.where(excluded: false).search(@q).reverse_chronological.includes(:entryable) + if statement_tab_active? build_statement_tab_data return render_statement_tab_frame if statement_tab_frame_request? @@ -69,6 +71,17 @@ class AccountsController < ApplicationController limit: safe_per_page, params: request.query_parameters.except("tab").merge("tab" => "activity") ) + + # Preload transfer associations only for Transaction entries + txn_entryables = @entries.filter_map { |e| e.entryable if e.entryable_type == "Transaction" } + ActiveRecord::Associations::Preloader.new( + records: txn_entryables, + associations: { + transfer_as_outflow: { inflow_transaction: { entry: :account } }, + transfer_as_inflow: { outflow_transaction: { entry: :account } } + } + ).call + Transaction::ActivitySecurityPreloader.new(@entries).preload @activity_feed_data = Account::ActivityFeedData.new(@account, @entries) diff --git a/app/controllers/transactions_controller.rb b/app/controllers/transactions_controller.rb index 9387d7550..b57a10654 100644 --- a/app/controllers/transactions_controller.rb +++ b/app/controllers/transactions_controller.rb @@ -14,16 +14,17 @@ class TransactionsController < ApplicationController def index @q = search_params - accessible_account_ids = Current.user.accessible_accounts.pluck(:id) - @search = Transaction::Search.new(Current.family, filters: @q, accessible_account_ids: accessible_account_ids) + @accessible_account_ids = Current.user.accessible_accounts.pluck(:id) + @search = Transaction::Search.new(Current.family, filters: @q, accessible_account_ids: @accessible_account_ids) base_scope = @search.transactions_scope .reverse_chronological .includes( - { entry: :account }, - :category, :merchant, :tags, - :transfer_as_inflow, :transfer_as_outflow - ) + { entry: :account }, + :category, :merchant, :tags, + transfer_as_outflow: { inflow_transaction: { entry: :account } }, + transfer_as_inflow: { outflow_transaction: { entry: :account } } + ) @pagy, @transactions = pagy(base_scope, limit: safe_per_page) Transaction::ActivitySecurityPreloader.new(@transactions).preload diff --git a/app/views/transactions/_transaction.html.erb b/app/views/transactions/_transaction.html.erb index 9752d6117..213e11964 100644 --- a/app/views/transactions/_transaction.html.erb +++ b/app/views/transactions/_transaction.html.erb @@ -153,7 +153,21 @@
<% if transaction.transfer? %> - <%= transaction.loan_payment? ? t("transactions.show.loan_payment") : t("transactions.show.transfer") %> • <%= entry.account.name %> + <%= transaction.loan_payment? ? t("transactions.show.loan_payment") : t("transactions.show.transfer") %> • + <% if transaction.transfer.present? %> + <% counterpart = transaction.transfer_as_outflow.present? ? transaction.transfer.to_account : transaction.transfer.from_account %> + <% if counterpart.present? && @accessible_account_ids&.include?(counterpart.id) %> + <% if transaction.transfer_as_outflow.present? %> + <%= entry.account.name %> → <%= counterpart.name %> + <% else %> + <%= entry.account.name %> ← <%= counterpart.name %> + <% end %> + <% else %> + <%= entry.account.name %> + <% end %> + <% else %> + <%= entry.account.name %> + <% end %> <% else %> <% if transaction.merchant&.present? %> diff --git a/test/views/transactions/transfer_counterpart_view_test.rb b/test/views/transactions/transfer_counterpart_view_test.rb new file mode 100644 index 000000000..1c1de6016 --- /dev/null +++ b/test/views/transactions/transfer_counterpart_view_test.rb @@ -0,0 +1,85 @@ +require "test_helper" + +class Transactions::TransferCounterpartViewTest < ActionView::TestCase + setup do + @family = families(:dylan_family) + @user = users(:family_admin) + Current.session = Session.create!(user: @user) + + @checking = accounts(:depository) # "from" account + @savings = accounts(:credit_card) # "to" account + + @accessible_account_ids = @user.accessible_accounts.pluck(:id).to_set + @split_parent_entry_ids = Set.new + end + + test "renders outflow transfer with arrow to destination account" do + outflow_tx = Transaction.create!(kind: "funds_movement") + outflow_entry = Entry.create!( + account: @checking, entryable: outflow_tx, + name: "Transfer to Savings", amount: 100, currency: "USD", date: Date.today + ) + + inflow_tx = Transaction.create!(kind: "funds_movement") + inflow_entry = Entry.create!( + account: @savings, entryable: inflow_tx, + name: "Transfer from Checking", amount: -100, currency: "USD", date: Date.today + ) + + Transfer.create!( + inflow_transaction: inflow_tx, + outflow_transaction: outflow_tx, + status: "confirmed" + ) + + html = render(partial: "transactions/transaction", locals: { + entry: outflow_entry, balance_trend: nil, view_ctx: "global" + }) + + assert_includes html, "→" + assert_includes html, @savings.name + end + + test "renders inflow transfer with arrow from source account" do + outflow_tx = Transaction.create!(kind: "funds_movement") + outflow_entry = Entry.create!( + account: @checking, entryable: outflow_tx, + name: "Transfer to Savings", amount: 100, currency: "USD", date: Date.today + ) + + inflow_tx = Transaction.create!(kind: "funds_movement") + inflow_entry = Entry.create!( + account: @savings, entryable: inflow_tx, + name: "Transfer from Checking", amount: -100, currency: "USD", date: Date.today + ) + + Transfer.create!( + inflow_transaction: inflow_tx, + outflow_transaction: outflow_tx, + status: "confirmed" + ) + + html = render(partial: "transactions/transaction", locals: { + entry: inflow_entry, balance_trend: nil, view_ctx: "global" + }) + + assert_includes html, "←" + assert_includes html, @checking.name + end + + test "falls back to account name when transfer has no counterpart" do + tx = Transaction.create!(kind: "funds_movement") + entry = Entry.create!( + account: @checking, entryable: tx, + name: "Unmatched Transfer", amount: 100, currency: "USD", date: Date.today + ) + + html = render(partial: "transactions/transaction", locals: { + entry: entry, balance_trend: nil, view_ctx: "global" + }) + + assert_includes html, @checking.name + assert_not_includes html, "→" + assert_not_includes html, "←" + end +end