Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 4 additions & 17 deletions app/models/account/provider_import_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -726,26 +726,9 @@ def find_duplicate_transaction(date:, amount:, currency:, name: nil, exclude_ent
query.order(created_at: :asc).first
end

# Finds a pending transaction that likely matches a newly posted transaction
# Used to reconcile pending→posted when SimpleFIN gives different IDs for the same transaction
#
# @param date [Date, String] Posted transaction date
# @param amount [BigDecimal, Numeric] Transaction amount (must match exactly)
# @param currency [String] Currency code
# @param source [String] Provider name (e.g., "simplefin")
# @param date_window [Integer] Days to search around the posted date (default: 8)
# @return [Entry, nil] The pending entry or nil if not found
def find_pending_transaction(date:, amount:, currency:, source:, date_window: 8)
date = Date.parse(date.to_s) unless date.is_a?(Date)

# Look for entries that:
# 1. Same account (implicit via account.entries)
# 2. Same source (simplefin)
# 3. Same amount (exact match - this is the strongest signal)
# 4. Same currency
# 5. Date within window (pending can post days later)
# 6. Is a Transaction (not Trade or Valuation)
# 7. Has pending=true in transaction.extra["simplefin"]["pending"] or extra["plaid"]["pending"]
candidates = account.entries
.joins("INNER JOIN transactions ON transactions.id = entries.entryable_id AND entries.entryable_type = 'Transaction'")
.where(source: source)
Expand All @@ -759,6 +742,10 @@ def find_pending_transaction(date:, amount:, currency:, source:, date_window: 8)
OR (transactions.extra -> 'enable_banking' ->> 'pending')::boolean = true
SQL
.order(date: :desc) # Prefer most recent pending transaction
.limit(2)
.to_a

return nil if candidates.size != 1 # ambiguous same-amount pendings must not be auto-claimed (#2013)

candidates.first
end
Expand Down
101 changes: 93 additions & 8 deletions test/models/account/provider_import_adapter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1179,8 +1179,9 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase
end
end

test "reconciles most recent pending when multiple exist" do
# Create two pending transactions with same amount
test "does not auto-claim pending when multiple same-amount candidates exist" do
# Two pending transactions with the same amount — this is the ambiguous case.
# Claiming either one could silently destroy a distinct transaction (issue #2013).
older_pending = @adapter.import_transaction(
external_id: "simplefin_older_pending",
amount: 60.00,
Expand All @@ -1201,8 +1202,8 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase
extra: { "simplefin" => { "pending" => true } }
)

# Import posted - should match the most recent pending (by date)
assert_no_difference "@account.entries.count" do
# Posted with same amount — ambiguous, so a NEW entry must be created
assert_difference "@account.entries.count", 1 do
posted_entry = @adapter.import_transaction(
external_id: "simplefin_posted_recurring",
amount: 60.00,
Expand All @@ -1213,11 +1214,14 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase
extra: { "simplefin" => { "pending" => false } }
)

# Should match the newer pending entry
assert_equal newer_pending.id, posted_entry.id
# Older pending should remain untouched
assert_equal "simplefin_older_pending", older_pending.reload.external_id
# Must be a brand-new entry, not a hijacked pending
assert_not_equal older_pending.id, posted_entry.id
assert_not_equal newer_pending.id, posted_entry.id
end

# Both original pendings must be untouched
assert_equal "simplefin_older_pending", older_pending.reload.external_id
assert_equal "simplefin_newer_pending", newer_pending.reload.external_id
end

test "find_pending_transaction returns nil when no pending transactions exist" do
Expand All @@ -1242,6 +1246,87 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase
assert_nil result
end

# ============================================================================
# Same-amount collision guard (issue #2013)
# ============================================================================

test "two same-amount ATM withdrawals both pending — posting one does not steal the other" do
# Exact scenario from issue #2013:
# Two $20 ATM withdrawals are both pending at the same time. When the first
# one posts, the system must NOT auto-claim the second pending (ambiguous),
# so both pending entries survive intact and a new posted entry is created.
pending_atm1 = @adapter.import_transaction(
external_id: "simplefin_pending_atm_1",
amount: 20.00,
currency: "USD",
date: Date.today - 5.days,
name: "ATM Withdrawal",
source: "simplefin",
extra: { "simplefin" => { "pending" => true } }
)

pending_atm2 = @adapter.import_transaction(
external_id: "simplefin_pending_atm_2",
amount: 20.00,
currency: "USD",
date: Date.today - 2.days,
name: "ATM Withdrawal",
source: "simplefin",
extra: { "simplefin" => { "pending" => true } }
)

# First ATM withdrawal posts — two same-amount pendings in window → ambiguous → new entry
assert_difference "@account.entries.count", 1 do
posted_entry = @adapter.import_transaction(
external_id: "simplefin_posted_atm_1",
amount: 20.00,
currency: "USD",
date: Date.today,
name: "ATM Withdrawal",
source: "simplefin",
extra: { "simplefin" => { "pending" => false } }
)

assert_not_equal pending_atm1.id, posted_entry.id
assert_not_equal pending_atm2.id, posted_entry.id
end

# Both pending entries must remain untouched — no silent destruction
assert_equal "simplefin_pending_atm_1", pending_atm1.reload.external_id
assert pending_atm1.transaction.pending?
assert_equal "simplefin_pending_atm_2", pending_atm2.reload.external_id
assert pending_atm2.transaction.pending?
end

test "posted transaction still claims the pending when it is the only same-amount candidate" do
# When there is exactly ONE matching pending, auto-claim must still work.
pending_entry = @adapter.import_transaction(
external_id: "simplefin_pending_coffee",
amount: 5.50,
currency: "USD",
date: Date.today - 2.days,
name: "Coffee Shop",
source: "simplefin",
extra: { "simplefin" => { "pending" => true } }
)

assert_no_difference "@account.entries.count" do
posted_entry = @adapter.import_transaction(
external_id: "simplefin_posted_coffee",
amount: 5.50,
currency: "USD",
date: Date.today,
name: "Coffee Shop",
source: "simplefin",
extra: { "simplefin" => { "pending" => false } }
)

assert_equal pending_entry.id, posted_entry.id
assert_equal "simplefin_posted_coffee", posted_entry.external_id
assert_not posted_entry.transaction.pending?
end
end

# ============================================================================
# Critical Direction Fix Tests (CITGO Bug Prevention)
# ============================================================================
Expand Down