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.
This commit is contained in:
Guillem Arias Fauste
2026-05-01 00:59:48 +02:00
committed by GitHub
parent 072f92c715
commit 5220bd527b
2 changed files with 63 additions and 0 deletions

View File

@@ -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

View File

@@ -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