diff --git a/app/models/simplefin_item/importer.rb b/app/models/simplefin_item/importer.rb index 76ae68390..8e4adfe66 100644 --- a/app/models/simplefin_item/importer.rb +++ b/app/models/simplefin_item/importer.rb @@ -158,6 +158,16 @@ class SimplefinItem::Importer return end + # Skip zero balance detection for liability accounts (CreditCard, Loan) where + # 0 balance with no holdings is normal (paid off card/loan) + account_type = simplefin_account.current_account&.accountable_type + return if %w[CreditCard Loan].include?(account_type) + + # Only count each account once per sync run to avoid false positives during + # chunked imports (which process the same account multiple times) + zero_balance_seen_keys << key if zeroish_balance && no_holdings + return if zero_balance_seen_keys.count(key) > 1 + if zeroish_balance && no_holdings stats["zero_runs"][key] = stats["zero_runs"][key].to_i + 1 # Cap to avoid unbounded growth @@ -173,6 +183,11 @@ class SimplefinItem::Importer end end + # Track accounts that have been flagged for zero balance in this sync run + def zero_balance_seen_keys + @zero_balance_seen_keys ||= [] + end + # Track seen error fingerprints during a single importer run to avoid double counting def seen_errors @seen_errors ||= Set.new diff --git a/test/models/simplefin_item/importer_inactive_test.rb b/test/models/simplefin_item/importer_inactive_test.rb index 4ae83db6f..158e325af 100644 --- a/test/models/simplefin_item/importer_inactive_test.rb +++ b/test/models/simplefin_item/importer_inactive_test.rb @@ -19,37 +19,33 @@ class SimplefinItem::ImporterInactiveTest < ActiveSupport::TestCase assert stats.dig("inactive", "a1"), "should be inactive when closed flag present" end - test "marks inactive after three consecutive zero runs with no holdings" do + test "counts zero runs once per sync even with multiple imports" do account_data = { id: "a2", name: "Dormant", balance: 0, "available-balance": 0, currency: "USD" } - 2.times { importer.send(:import_account, account_data) } - stats = @sync.reload.sync_stats - assert_equal 2, stats.dig("zero_runs", "a2"), "should count zero runs" - assert_equal false, stats.dig("inactive", "a2"), "should not be inactive before threshold" + # Multiple imports in the same sync (simulating chunked imports) should only count once + 5.times { importer.send(:import_account, account_data) } - importer.send(:import_account, account_data) stats = @sync.reload.sync_stats - assert_equal true, stats.dig("inactive", "a2"), "should be inactive at threshold" + assert_equal 1, stats.dig("zero_runs", "a2"), "should only count once per sync despite multiple imports" + assert_equal false, stats.dig("inactive", "a2"), "should not be inactive after single count" end - test "resets zero_runs_count and inactive when activity returns" do + test "resets zero_runs and inactive when activity returns" do account_data = { id: "a3", name: "Dormant", balance: 0, "available-balance": 0, currency: "USD" } - 3.times { importer.send(:import_account, account_data) } - stats = @sync.reload.sync_stats - assert_equal true, stats.dig("inactive", "a3") + importer.send(:import_account, account_data) - # Activity returns: non-zero balance or holdings + stats = @sync.reload.sync_stats + assert_equal 1, stats.dig("zero_runs", "a3") + + # Activity returns: non-zero balance active_data = { id: "a3", name: "Dormant", balance: 10, currency: "USD" } importer.send(:import_account, active_data) + stats = @sync.reload.sync_stats assert_equal 0, stats.dig("zero_runs", "a3") assert_equal false, stats.dig("inactive", "a3") end -end - -# Additional regression: no balances present should not increment zero_runs or mark inactive -class SimplefinItem::ImporterInactiveTest < ActiveSupport::TestCase test "does not count zero run when both balances are missing and no holdings" do account_data = { id: "a4", name: "Unknown", currency: "USD" } # no balance keys, no holdings @@ -59,4 +55,64 @@ class SimplefinItem::ImporterInactiveTest < ActiveSupport::TestCase assert_equal 0, stats.dig("zero_runs", "a4").to_i assert_equal false, stats.dig("inactive", "a4") end + + test "skips zero balance detection for credit cards" do + # Create a SimplefinAccount linked to a CreditCard account + sfa = SimplefinAccount.create!( + simplefin_item: @item, + name: "Paid Off Card", + account_id: "cc1", + account_type: "credit", + currency: "USD", + current_balance: 0 + ) + + credit_card = CreditCard.create! + account = @family.accounts.create!( + name: "Paid Off Card", + balance: 0, + currency: "USD", + accountable: credit_card, + simplefin_account_id: sfa.id + ) + + account_data = { id: "cc1", name: "Paid Off Card", balance: 0, "available-balance": 0, currency: "USD" } + + # Even with zero balance and no holdings, credit cards should not trigger the counter + importer.send(:import_account, account_data) + stats = @sync.reload.sync_stats + + assert_nil stats.dig("zero_runs", "cc1"), "should not count zero runs for credit cards" + assert_equal false, stats.dig("inactive", "cc1") + end + + test "skips zero balance detection for loans" do + # Create a SimplefinAccount linked to a Loan account + sfa = SimplefinAccount.create!( + simplefin_item: @item, + name: "Paid Off Loan", + account_id: "loan1", + account_type: "loan", + currency: "USD", + current_balance: 0 + ) + + loan = Loan.create! + account = @family.accounts.create!( + name: "Paid Off Loan", + balance: 0, + currency: "USD", + accountable: loan, + simplefin_account_id: sfa.id + ) + + account_data = { id: "loan1", name: "Paid Off Loan", balance: 0, "available-balance": 0, currency: "USD" } + + # Even with zero balance and no holdings, loans should not trigger the counter + importer.send(:import_account, account_data) + stats = @sync.reload.sync_stats + + assert_nil stats.dig("zero_runs", "loan1"), "should not count zero runs for loans" + assert_equal false, stats.dig("inactive", "loan1") + end end