fix(family): include HSA depository accounts in tax-advantaged exclusion (#2004)

* fix(family): include HSA depository accounts in tax-advantaged exclusion

`Family#tax_advantaged_account_ids` is the ID set the budget engine uses
to exclude tax-advantaged account activity from income / expense /
cashflow totals. PR #724 originated this method and explicitly listed HSA
in scope ("401k, IRA, HSA, Roth IRA, etc."), but the implementation only
joined `investments` and `cryptos`. `Depository::SUBTYPES["hsa"]` already
exists and Plaid routes `depository.hsa` accounts to `Depository` (not
`Investment`) via `PlaidAccount::TypeMappable`, so HSA cash accounts were
silently absent from the filter and HSA contributions/withdrawals showed
up in household expense totals.

- Add `Depository::TAX_ADVANTAGED_SUBTYPES = %w[hsa]` + a `tax_treatment`
  instance method (mirrors `Investment#tax_treatment`).
  `TaxTreatable#tax_advantaged?` picks it up via the existing `respond_to?`
  check, so `Account#tax_advantaged?` now flips to true for HSA depositories
  without touching the concern.
- Extract `Family#tax_advantaged_depository_account_ids` (private) that
  joins `depositories` and filters by `Depository::TAX_ADVANTAGED_SUBTYPES`,
  mirroring the existing `investment_ids` / `crypto_ids` extraction style.
  Append it to the union in `tax_advantaged_account_ids`.

Behavior change is scoped: HSA depositories now exit the budget engine via
the same path as 401k / IRA / Roth IRA. Non-HSA depositories continue to
report `tax_treatment: :taxable` (was `nil`), so `Account#taxable?` returns
true for them via the existing `== :taxable` clause — no expense-total
change for Checking / Savings / CD / Money Market.

Tests:
- `test/models/account_test.rb` — rewrite "tax_treatment returns nil for
  non-investment accounts" (was implicitly testing the bug) into two tests:
  one asserting `:taxable` for non-HSA depositories and a new sibling
  asserting `nil` for accountables that genuinely lack `tax_treatment`
  (CreditCard). Add an HSA-depository test asserting `tax_advantaged?`.
- `test/models/income_statement_test.rb` — new test asserting an HSA
  depository is included in `tax_advantaged_account_ids` and a `savings`
  depository is not.

No schema migration, no controller change, no provider integration change.

* [200~fix(family): return nil for non-HSA depository tax_treatment
This commit is contained in:
galuis116
2026-05-31 07:24:01 -07:00
committed by GitHub
parent 8251b7e4d6
commit 61a235f765
4 changed files with 107 additions and 5 deletions

View File

@@ -11,6 +11,28 @@ class Depository < ApplicationRecord
"money_market" => { short: "MM", long: "Money Market" }
}.freeze
# Depository subtypes that carry tax-advantaged treatment in the budget /
# cashflow / income-statement filters (`Family#tax_advantaged_account_ids`,
# `TaxTreatable#tax_advantaged?`). HSA cash sits here because Plaid routes
# `depository.hsa` to `Depository` (not `Investment`) via
# `PlaidAccount::TypeMappable`, so a real-world Plaid-linked HSA cash account
# was previously invisible to the tax-advantaged filter PR #724 introduced.
TAX_ADVANTAGED_SUBTYPES = %w[hsa].freeze
# `TaxTreatable` (the `Account` concern) reads this via `respond_to?` so
# adding it here transparently flips `Account#tax_advantaged?` for HSA
# depositories without touching the concern itself.
#
# Returns `nil` (not `:taxable`) for ordinary depository subtypes. `nil`
# already reads as taxable everywhere it matters: `TaxTreatable#taxable?`
# treats `nil` as taxable and `#tax_advantaged?` excludes it. Returning
# `nil` also keeps `tax_treatment.present?` false so the header tax badge
# (`app/views/accounts/show/_header.html.erb`) stays hidden on checking,
# savings, CD, and money-market accounts that never displayed it before.
def tax_treatment
:tax_advantaged if TAX_ADVANTAGED_SUBTYPES.include?(subtype)
end
class << self
def color
"#875BF7"

View File

@@ -258,7 +258,7 @@ class Family < ApplicationRecord
.where(cryptos: { tax_treatment: %w[tax_deferred tax_exempt] })
.pluck(:id)
investment_ids + crypto_ids
investment_ids + crypto_ids + tax_advantaged_depository_account_ids
end
end
@@ -344,6 +344,18 @@ class Family < ApplicationRecord
end
private
# Mirrors the inline `investment_ids` / `crypto_ids` SQL blocks in
# `tax_advantaged_account_ids`. Joins `depositories` and filters by
# `Depository::TAX_ADVANTAGED_SUBTYPES` (currently `%w[hsa]`). Extracted
# rather than inlined because the existing two blocks are already long
# enough; the extraction keeps `tax_advantaged_account_ids` readable.
def tax_advantaged_depository_account_ids
accounts
.joins("INNER JOIN depositories ON depositories.id = accounts.accountable_id AND accounts.accountable_type = 'Depository'")
.where(depositories: { subtype: Depository::TAX_ADVANTAGED_SUBTYPES })
.pluck(:id)
end
def normalize_enabled_currencies!
if enabled_currencies.blank?
self.enabled_currencies = nil

View File

@@ -155,10 +155,32 @@ class AccountTest < ActiveSupport::TestCase
assert_equal I18n.t("accounts.tax_treatments.taxable"), account.tax_treatment_label
end
test "tax_treatment returns nil for non-investment accounts" do
# Depository accounts don't have tax_treatment
test "tax_treatment returns nil for non-HSA depository accounts" do
# Depository exposes a `tax_treatment` method so HSA cash flips
# tax-advantaged, but non-HSA subtypes (checking, savings, cd,
# money_market) return nil. nil still reads as taxable via `taxable?`,
# and keeps `tax_treatment.present?` false so the header tax badge does
# not appear on ordinary bank accounts that never displayed it before.
assert_nil @account.tax_treatment
assert_nil @account.tax_treatment_label
assert_not @account.tax_treatment.present?
assert @account.taxable?
end
test "tax_treatment returns nil for accountables that do not implement it" do
# CreditCard / Loan / Property / OtherAsset / OtherLiability do not
# implement `tax_treatment`, so the `TaxTreatable#respond_to?` short-
# circuit still returns nil for them.
credit_card_account = @family.accounts.create!(
owner: @admin,
name: "Test Credit Card",
balance: 100,
currency: "USD",
accountable: CreditCard.new
)
assert_nil credit_card_account.tax_treatment
assert_nil credit_card_account.tax_treatment_label
end
test "tax_advantaged? returns true for tax-advantaged accounts" do
@@ -175,6 +197,20 @@ class AccountTest < ActiveSupport::TestCase
assert_not account.taxable?
end
test "tax_advantaged? returns true for HSA depository accounts" do
hsa_depository = @family.accounts.create!(
owner: @admin,
name: "Fidelity HSA Cash",
balance: 3_000,
currency: "USD",
accountable: Depository.new(subtype: "hsa")
)
assert_equal :tax_advantaged, hsa_depository.tax_treatment
assert hsa_depository.tax_advantaged?
assert_not hsa_depository.taxable?
end
test "tax_advantaged? returns false for taxable accounts" do
investment = Investment.new(subtype: "brokerage")
account = @family.accounts.create!(
@@ -189,8 +225,9 @@ class AccountTest < ActiveSupport::TestCase
assert account.taxable?
end
test "taxable? returns true for accounts without tax_treatment" do
# Depository accounts
test "taxable? returns true for non-HSA depository accounts" do
# `@account` is the checking depository fixture; `tax_treatment` is
# `nil` (no subtype override), which `taxable?` reads as true.
assert @account.taxable?
assert_not @account.tax_advantaged?
end

View File

@@ -513,6 +513,37 @@ class IncomeStatementTest < ActiveSupport::TestCase
refute_includes tax_advantaged_ids, @credit_card_account.id
end
test "family.tax_advantaged_account_ids includes HSA depository accounts and excludes non-HSA depositories" do
# Plaid routes `depository.hsa` to `Depository` (not Investment), so HSA
# cash accounts created from a Plaid sync end up as Depository(subtype:
# "hsa") rows. They are semantically tax-advantaged but were previously
# invisible to this filter because it only joined `investments` and
# `cryptos`.
hsa_depository = @family.accounts.create!(
name: "Fidelity HSA Cash",
currency: @family.currency,
balance: 3_000,
accountable: Depository.new(subtype: "hsa")
)
savings_depository = @family.accounts.create!(
name: "Emergency Savings",
currency: @family.currency,
balance: 8_000,
accountable: Depository.new(subtype: "savings")
)
# Clear the memoized value (the setup-block @checking_account is also
# a depository, so we exercise both inclusion and exclusion paths).
@family.instance_variable_set(:@tax_advantaged_account_ids, nil)
tax_advantaged_ids = @family.tax_advantaged_account_ids
assert_includes tax_advantaged_ids, hsa_depository.id
refute_includes tax_advantaged_ids, savings_depository.id
refute_includes tax_advantaged_ids, @checking_account.id
end
# net_category_totals tests
test "net_category_totals nets expense and refund in the same category" do
Entry.joins(:account).where(accounts: { family_id: @family.id }).destroy_all