Skip to content
Merged
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
22 changes: 21 additions & 1 deletion app/models/account/provider_import_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,28 @@ def import_transaction(external_id:, amount:, currency:, date:, name:, source:,
end

if pending_match
old_pending_external_id = pending_match.external_id
pending_entry_date = pending_match.date
entry = pending_match
entry.assign_attributes(external_id: external_id)

# Clear the pending flag so this entry no longer shows as pending after being claimed
# by a booked transaction. Also record the old external_id so the sync engine can
# exclude it from re-import (preventing the old pending from being recreated on the
# next sync when the stored raw payload still contains the pending transaction data).
if entry.entryable.is_a?(Transaction)
ex = (entry.transaction.extra || {}).deep_dup
Transaction::PENDING_PROVIDERS.each do |provider|
next unless ex.key?(provider)
ex[provider].delete("pending")
ex.delete(provider) if ex[provider].empty?
end
if old_pending_external_id.present?
existing_claims = Array.wrap(ex["auto_claimed_pending_ids"])
ex["auto_claimed_pending_ids"] = (existing_claims + [ old_pending_external_id ]).uniq
end
entry.transaction.extra = ex
end
end
end

Expand All @@ -120,7 +140,7 @@ def import_transaction(external_id:, amount:, currency:, date:, name:, source:,
entry.assign_attributes(
amount: amount,
currency: currency,
date: date
date: pending_entry_date || date
)

# Use enrichment pattern to respect user overrides
Expand Down
66 changes: 44 additions & 22 deletions app/models/enable_banking_account/transactions/processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,51 @@ def process
Account::ProviderImportAdapter.new(enable_banking_account.current_account)
end

# Pre-fetch external_ids that were manually merged and must not be re-imported.
# One query per sync; O(1) Set lookup per transaction — avoids N+1.
# Uses a lateral jsonb_array_elements join to extract only the ID strings in SQL,
# avoiding loading full extra blobs into Ruby. Handles both Array (current) and
# Hash (legacy) formats via jsonb_typeof.
# Pre-fetch external_ids that must not be re-imported.
# One query per category per sync; O(1) Set lookup per transaction — avoids N+1.
excluded_ids = if enable_banking_account.current_account
Transaction.joins(:entry)
.where(entries: { account_id: enable_banking_account.current_account.id })
.where("transactions.extra ? 'manual_merge'")
.joins(
Arel.sql(<<~SQL.squish)
CROSS JOIN LATERAL jsonb_array_elements(
CASE jsonb_typeof(transactions.extra->'manual_merge')
WHEN 'array' THEN transactions.extra->'manual_merge'
WHEN 'object' THEN jsonb_build_array(transactions.extra->'manual_merge')
ELSE '[]'::jsonb
END
) AS merge_elem
SQL
)
.pluck(Arel.sql("merge_elem->>'merged_from_external_id'"))
.compact
.to_set
account_id = enable_banking_account.current_account.id

# 1. Manually merged: pending entries the user explicitly merged into a posted transaction.
# Uses a lateral join to extract merged_from_external_id from the manual_merge JSON
# (handles both Array current format and legacy Hash format via jsonb_typeof).
manually_merged_ids = Transaction.joins(:entry)
.where(entries: { account_id: account_id })
.where("transactions.extra ? 'manual_merge'")
.joins(
Arel.sql(<<~SQL.squish)
CROSS JOIN LATERAL jsonb_array_elements(
CASE jsonb_typeof(transactions.extra->'manual_merge')
WHEN 'array' THEN transactions.extra->'manual_merge'
WHEN 'object' THEN jsonb_build_array(transactions.extra->'manual_merge')
ELSE '[]'::jsonb
END
) AS merge_elem
SQL
)
.pluck(Arel.sql("merge_elem->>'merged_from_external_id'"))
.compact
.to_set

# 2. Auto-claimed: pending entries that were automatically matched to a booked transaction
# by the amount/date heuristic. Their old external_ids are stored in
# extra["auto_claimed_pending_ids"] so they are not re-imported as new pending entries
# on subsequent syncs (the stored raw payload still contains the old pending data).
auto_claimed_ids = Transaction.joins(:entry)
.where(entries: { account_id: account_id })
.where("transactions.extra ? 'auto_claimed_pending_ids'")
.joins(
Arel.sql(<<~SQL.squish)
CROSS JOIN LATERAL jsonb_array_elements_text(
transactions.extra->'auto_claimed_pending_ids'
) AS claimed_id
SQL
)
.pluck(Arel.sql("claimed_id"))
.compact
.to_set

manually_merged_ids | auto_claimed_ids
else
Set.new
end
Expand Down
45 changes: 45 additions & 0 deletions test/models/account/provider_import_adapter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,51 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase
end
end

test "clears pending flag, preserves pending date, and records old external_id when claiming pending entry with nil extra" do
# Enable Banking booked transactions often have nil extra (no FX, no MCC).
# The deep_merge path is skipped, so we must clear the pending flag explicitly.
pending_date = Date.today - 2.days
pending_entry = @adapter.import_transaction(
external_id: "eb_pending_nil_extra",
amount: 42.00,
currency: "EUR",
date: pending_date,
name: "Supermarket",
source: "enable_banking",
extra: { "enable_banking" => { "pending" => true } }
)

assert pending_entry.transaction.pending?, "should be pending before claim"

assert_no_difference "@account.entries.count" do
posted_entry = @adapter.import_transaction(
external_id: "eb_booked_nil_extra",
amount: 42.00,
currency: "EUR",
date: Date.today, # booked date is later than pending date
name: "Supermarket Posted",
source: "enable_banking",
extra: nil # typical for simple Enable Banking booked transactions
)

assert_equal pending_entry.id, posted_entry.id, "should claim the pending entry"
assert_equal "eb_booked_nil_extra", posted_entry.external_id

posted_entry.reload

# Pending flag must be cleared so the entry no longer shows a pending badge
assert_not posted_entry.transaction.pending?, "pending flag should be cleared after claim"

# Date must be the original pending date, not the later booked date
assert_equal pending_date, posted_entry.date, "pending date should be preserved, not overwritten with booked date"

# Old pending external_id must be stored so the sync engine can skip re-importing it
claimed_ids = posted_entry.transaction.extra&.dig("auto_claimed_pending_ids") || []
assert_includes claimed_ids, "eb_pending_nil_extra",
"auto_claimed_pending_ids should record the old pending external_id"
end
end

test "does not reconcile when posted transaction has same external_id as pending" do
# When external_id matches, normal dedup should handle it
pending_entry = @adapter.import_transaction(
Expand Down
83 changes: 83 additions & 0 deletions test/models/enable_banking_account/transactions/processor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,89 @@ def raw_pending_transaction(transaction_id:)
result = EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process

assert_equal 0, result[:failed]
end

test "does not re-import a pending transaction whose external_id was auto-claimed" do
# When a pending entry is automatically matched to a booked transaction by the
# amount/date heuristic (find_pending_transaction), the old pending external_id
# is stored in auto_claimed_pending_ids so subsequent syncs don't recreate it.
pending_ext_id = "enable_banking_PDNG_AUTO_CLAIMED"

booked_entry = create_transaction(
account: @account,
name: "Grocery Store",
date: 1.day.ago.to_date,
amount: 55,
currency: "EUR",
external_id: "enable_banking_BOOK_SETTLED",
source: "enable_banking"
)
booked_entry.transaction.update!(
extra: {
"auto_claimed_pending_ids" => [ pending_ext_id ]
}
)

@enable_banking_account.update!(
raw_transactions_payload: [
raw_pending_transaction(transaction_id: "PDNG_AUTO_CLAIMED")
]
)

assert_no_difference "@account.entries.count" do
EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process
end
end

test "does not re-import when both manual_merge and auto_claimed_pending_ids exclusions are present" do
manually_merged_ext_id = "enable_banking_PDNG_MANUAL"
auto_claimed_ext_id = "enable_banking_PDNG_AUTO"

manual_entry = create_transaction(
account: @account,
name: "Manual Merge Entry",
date: 2.days.ago.to_date,
amount: 20,
currency: "EUR",
external_id: "enable_banking_BOOK_MANUAL",
source: "enable_banking"
)
manual_entry.transaction.update!(
extra: {
"manual_merge" => {
"merged_from_external_id" => manually_merged_ext_id,
"merged_at" => Time.current.iso8601,
"source" => "enable_banking"
}
}
)

auto_entry = create_transaction(
account: @account,
name: "Auto Claimed Entry",
date: 1.day.ago.to_date,
amount: 30,
currency: "EUR",
external_id: "enable_banking_BOOK_AUTO",
source: "enable_banking"
)
auto_entry.transaction.update!(
extra: { "auto_claimed_pending_ids" => [ auto_claimed_ext_id ] }
)

@enable_banking_account.update!(
raw_transactions_payload: [
raw_pending_transaction(transaction_id: "PDNG_MANUAL"), # excluded via manual_merge
raw_pending_transaction(transaction_id: "PDNG_AUTO"), # excluded via auto_claimed_pending_ids
raw_pending_transaction(transaction_id: "PDNG_BRAND_NEW_XXXX") # new — should import
]
)

result = nil
assert_difference "@account.entries.count", 1 do
result = EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process
end
assert_equal 2, result[:skipped]
assert_equal 1, result[:imported]
end

Expand Down
Loading