From 61a235f765b6dc4b74c2ef0e4d9332ca599a484d Mon Sep 17 00:00:00 2001 From: galuis116 <116897328+galuis116@users.noreply.github.com> Date: Sun, 31 May 2026 07:24:01 -0700 Subject: [PATCH] fix(family): include HSA depository accounts in tax-advantaged exclusion (#2004) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- app/models/depository.rb | 22 ++++++++++++++ app/models/family.rb | 14 ++++++++- test/models/account_test.rb | 45 +++++++++++++++++++++++++--- test/models/income_statement_test.rb | 31 +++++++++++++++++++ 4 files changed, 107 insertions(+), 5 deletions(-) diff --git a/app/models/depository.rb b/app/models/depository.rb index 0ebeda390..96809cc61 100644 --- a/app/models/depository.rb +++ b/app/models/depository.rb @@ -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" diff --git a/app/models/family.rb b/app/models/family.rb index 3ebd72406..4313ab4a5 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -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 diff --git a/test/models/account_test.rb b/test/models/account_test.rb index de44d90cb..0d7013354 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -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 diff --git a/test/models/income_statement_test.rb b/test/models/income_statement_test.rb index 2752a46a8..9e2b350f6 100644 --- a/test/models/income_statement_test.rb +++ b/test/models/income_statement_test.rb @@ -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