diff --git a/app/models/account/provider_import_adapter.rb b/app/models/account/provider_import_adapter.rb index a207468aa4..6b2dbf9322 100644 --- a/app/models/account/provider_import_adapter.rb +++ b/app/models/account/provider_import_adapter.rb @@ -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 @@ -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 diff --git a/app/models/enable_banking_account/transactions/processor.rb b/app/models/enable_banking_account/transactions/processor.rb index fb50333e0e..2809c6f9e5 100644 --- a/app/models/enable_banking_account/transactions/processor.rb +++ b/app/models/enable_banking_account/transactions/processor.rb @@ -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 diff --git a/test/models/account/provider_import_adapter_test.rb b/test/models/account/provider_import_adapter_test.rb index bbfa31b081..cff61d8669 100644 --- a/test/models/account/provider_import_adapter_test.rb +++ b/test/models/account/provider_import_adapter_test.rb @@ -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( diff --git a/test/models/enable_banking_account/transactions/processor_test.rb b/test/models/enable_banking_account/transactions/processor_test.rb index 00982bdef3..81426dce3c 100644 --- a/test/models/enable_banking_account/transactions/processor_test.rb +++ b/test/models/enable_banking_account/transactions/processor_test.rb @@ -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