From 5220bd527b22900843d72a6b4a5bddf7ad9e6a3e Mon Sep 17 00:00:00 2001 From: Guillem Arias Fauste Date: Fri, 1 May 2026 00:59:48 +0200 Subject: [PATCH] fix(budgets): stop auto-matched transfers leaking into category cards (#1059) (#1588) Refs #1059. When you auto-match a $500 expense from your checking account against the matching deposit on your credit card, the resulting transfer pair was leaving traces in the per-card "Recent transactions" list under each budget category card, even though the aggregate `Budget#actual_spending` (via `IncomeStatement`) already excluded `BUDGET_EXCLUDED_KINDS` (funds_movement / one_time / cc_payment) from the totals. The user saw $X under the card while the totals showed $X less. Fix: extend the same exclusion to the drilldown list. The aggregate and the list now agree. ```ruby # app/controllers/budget_categories_controller.rb @recent_transactions = @budget.transactions .where.not(transactions: { kind: Transaction::BUDGET_EXCLUDED_KINDS }) ``` `loan_payment` and `investment_contribution` are intentionally NOT in `BUDGET_EXCLUDED_KINDS`, so those transfers still appear (they are budget-tracked). What this PR does NOT do: - It does not clear the matched transactions' `category_id` in the matcher itself. An earlier draft of this PR did, but codex correctly flagged that doing so causes data loss when a user rejects an incorrect auto-match: `Transfer#reject!` resets `kind` to `standard` but does not restore the previously-cleared category, permanently dropping the user's original categorisation. The controller filter alone is sufficient to fix the user-visible bug, and the inconsistency between `kind = funds_movement` and a retained category is harmless because every relevant view filters one or the other. - The mortgage scenario in #1059 (a `loan_payment` match showing as "Uncategorised" in the budget) isn't a leak; it is a missing feature. The matcher doesn't auto-assign a category to `loan_payment` rows the way #924 does for `investment_contribution`. The natural follow-up is a parallel `loan_payments_category` plus matcher / import-adapter auto-assignment, which deserves a maintainer signoff first. Tests: - `BudgetCategoriesControllerTest#show drilldown excludes BUDGET_EXCLUDED_KINDS transfers from recent transactions`: a matched depository <-> CC pair does not appear in the Uncategorised drilldown after the matcher runs. - `BudgetCategoriesControllerTest#show drilldown still lists loan_payment transfers (intentionally budget-tracked)`: a matched depository <-> loan pair stays visible in the drilldown. Suite: 3239 / 0 / 0 / 24 on the latest upstream/main. Lint clean. --- .../budget_categories_controller.rb | 8 +++ .../budget_categories_controller_test.rb | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/app/controllers/budget_categories_controller.rb b/app/controllers/budget_categories_controller.rb index 0490a5f61..65880ea05 100644 --- a/app/controllers/budget_categories_controller.rb +++ b/app/controllers/budget_categories_controller.rb @@ -7,7 +7,15 @@ class BudgetCategoriesController < ApplicationController end def show + # The aggregate `Budget#actual_spending` already excludes transactions + # whose kind is in BUDGET_EXCLUDED_KINDS (funds_movement, one_time, + # cc_payment) via IncomeStatement. The drilldown list must apply the + # same filter, otherwise a matched transfer (post-#874 the matcher + # correctly tags inflow as funds_movement and outflow per destination + # account) shows under the Uncategorized card -- or any retained + # category -- even though the aggregate ignores it. See issue #1059. @recent_transactions = @budget.transactions + .where.not(transactions: { kind: Transaction::BUDGET_EXCLUDED_KINDS }) if params[:id] == BudgetCategory.uncategorized.id @budget_category = @budget.uncategorized_budget_category diff --git a/test/controllers/budget_categories_controller_test.rb b/test/controllers/budget_categories_controller_test.rb index c810baa83..5bea22d96 100644 --- a/test/controllers/budget_categories_controller_test.rb +++ b/test/controllers/budget_categories_controller_test.rb @@ -2,6 +2,7 @@ require "test_helper" class BudgetCategoriesControllerTest < ActionDispatch::IntegrationTest include ActionView::RecordIdentifier + include EntriesTestHelper setup do sign_in users(:family_admin) @@ -107,4 +108,58 @@ class BudgetCategoriesControllerTest < ActionDispatch::IntegrationTest assert_equal 0.0, @electric_budget_category.reload.budgeted_spending.to_f end + + test "show drilldown excludes BUDGET_EXCLUDED_KINDS transfers from recent transactions" do + # Issue #1059: a matched depository <-> CC pair becomes + # (cc_payment outflow + funds_movement inflow). Both kinds are in + # BUDGET_EXCLUDED_KINDS so the budget aggregate excludes them, but + # the per-category drilldown previously listed them anyway -- + # appearing under whatever category they retained (or under + # Uncategorized once the matcher cleared the category). Filter + # them out so the drilldown matches the aggregate. + create_transaction( + date: @budget.start_date, + account: accounts(:depository), + amount: 500, + name: "BUG_1059_REPRO_OUTFLOW" + ) + create_transaction( + date: @budget.start_date, + account: accounts(:credit_card), + amount: -500, + name: "BUG_1059_REPRO_INFLOW" + ) + @family.auto_match_transfers! + + get budget_budget_category_path(@budget, BudgetCategory.uncategorized.id) + assert_response :success + refute_includes @response.body, "BUG_1059_REPRO_OUTFLOW", + "matched cc_payment outflow must not appear in Uncategorized drilldown" + refute_includes @response.body, "BUG_1059_REPRO_INFLOW", + "matched funds_movement inflow must not appear in Uncategorized drilldown" + end + + test "show drilldown still lists loan_payment transfers (intentionally budget-tracked)" do + # loan_payment is NOT in BUDGET_EXCLUDED_KINDS. The drilldown should + # keep showing loan_payment transfers so the user can see what's + # under Uncategorized (or whichever category they manually set). + create_transaction( + date: @budget.start_date, + account: accounts(:depository), + amount: 500, + name: "MORTGAGE_REPRO_OUTFLOW" + ) + create_transaction( + date: @budget.start_date, + account: accounts(:loan), + amount: -500, + name: "MORTGAGE_REPRO_INFLOW" + ) + @family.auto_match_transfers! + + get budget_budget_category_path(@budget, BudgetCategory.uncategorized.id) + assert_response :success + assert_includes @response.body, "MORTGAGE_REPRO_OUTFLOW", + "loan_payment outflow remains visible (kind is not BUDGET_EXCLUDED)" + end end