diff --git a/app/models/enable_banking_entry/processor.rb b/app/models/enable_banking_entry/processor.rb index e08d37acc..4f41b21f8 100644 --- a/app/models/enable_banking_entry/processor.rb +++ b/app/models/enable_banking_entry/processor.rb @@ -13,7 +13,25 @@ class EnableBankingEntry::Processor def self.compute_external_id(raw_transaction_data) data = raw_transaction_data.with_indifferent_access id = data[:transaction_id].presence || data[:entry_reference].presence - id ? "enable_banking_#{id}" : nil + return "enable_banking_#{id}" if id + + # Some ASPSPs omit both transaction_id and entry_reference (both are optional + # in PSD2). Generate a deterministic content-based ID so these transactions + # can still be imported idempotently. Uses the same fields as the importer's + # dedup key so the two strategies stay in sync. + date = data[:booking_date].presence || data[:value_date].presence || data[:transaction_date] + amount = data.dig(:transaction_amount, :amount).presence || data[:amount] + currency = data.dig(:transaction_amount, :currency).presence || data[:currency] + direction = data[:credit_debit_indicator] + creditor = data.dig(:creditor, :name).presence || data[:creditor_name] + debtor = data.dig(:debtor, :name).presence || data[:debtor_name] + remittance = data[:remittance_information] + remittance_key = remittance.is_a?(Array) ? remittance.compact.map(&:to_s).sort.join("|") : remittance.to_s + + content = [ date, amount, currency, direction, creditor, debtor, remittance_key ].map(&:to_s).join("\x1F") + return nil if content.gsub("\x1F", "").blank? + + "enable_banking_content_#{Digest::MD5.hexdigest(content)}" end def initialize(enable_banking_transaction, enable_banking_account:, import_adapter: nil) @@ -75,7 +93,7 @@ class EnableBankingEntry::Processor def external_id id = self.class.compute_external_id(data) - raise ArgumentError, "Enable Banking transaction missing required field 'transaction_id'" unless id + raise ArgumentError, "Enable Banking transaction missing required identifier (transaction_id, entry_reference, or identifiable content)" unless id id end diff --git a/app/models/enable_banking_item/importer.rb b/app/models/enable_banking_item/importer.rb index 504b22c9a..708664212 100644 --- a/app/models/enable_banking_item/importer.rb +++ b/app/models/enable_banking_item/importer.rb @@ -251,18 +251,23 @@ class EnableBankingItem::Importer ) end - book_ids = all_transactions - .map { |tx| tx.with_indifferent_access[:transaction_id].presence } + book_fingerprints = all_transactions + .map { |tx| EnableBankingEntry::Processor.compute_external_id(tx) } .compact.to_set + # Also index all booked entry_references so a pending row that lacks + # transaction_id can still be matched when the settled BOOK row adds one + # (fingerprints differ; entry_reference stays the same across settlement). book_entry_refs = all_transactions - .select { |tx| tx.with_indifferent_access[:transaction_id].blank? } .map { |tx| tx.with_indifferent_access[:entry_reference].presence } .compact.to_set pending_transactions.reject! do |tx| - tx = tx.with_indifferent_access - tx[:transaction_id].present? ? book_ids.include?(tx[:transaction_id]) : book_entry_refs.include?(tx[:entry_reference].presence) + tx_ia = tx.with_indifferent_access + fp = EnableBankingEntry::Processor.compute_external_id(tx_ia) + entry_ref = tx_ia[:entry_reference].presence + (fp.present? && book_fingerprints.include?(fp)) || + (entry_ref.present? && book_entry_refs.include?(entry_ref)) end all_transactions = all_transactions + tag_as_pending(pending_transactions) @@ -291,14 +296,17 @@ class EnableBankingItem::Importer if all_transactions.any? # C4: Remove stored PDNG entries that have now settled as BOOK. - # When a BOOK transaction arrives with the same transaction_id as a stored - # PDNG entry, the pending entry is stale — drop it to avoid duplicates. - book_ids = all_transactions + # Two match strategies run in parallel: + # 1. Fingerprint: covers same-ID rows and ID-less rows matched by content. + # 2. Entry-reference cross-match: covers the case where a pending row had + # no transaction_id but the settled BOOK row gained one — fingerprints + # diverge (enable_banking_ vs enable_banking_) but the + # shared entry_reference is a reliable settlement signal. + book_fingerprints = all_transactions .reject { |tx| tx.with_indifferent_access[:_pending] } - .map { |tx| tx.with_indifferent_access[:transaction_id].presence } + .map { |tx| EnableBankingEntry::Processor.compute_external_id(tx) } .compact.to_set - # Fallback: collect entry_references for BOOK rows that have no transaction_id book_entry_refs = all_transactions .reject { |tx| tx.with_indifferent_access[:_pending] } .map { |tx| tx.with_indifferent_access[:entry_reference].presence } @@ -310,21 +318,20 @@ class EnableBankingItem::Importer pending_flag = tx.dig(:extra, :enable_banking, :pending) || tx[:_pending] next false unless pending_flag - tx[:transaction_id].present? ? - book_ids.include?(tx[:transaction_id]) : - book_entry_refs.include?(tx[:entry_reference].presence) + fp = EnableBankingEntry::Processor.compute_external_id(tx) + entry_ref = tx[:entry_reference].presence + (fp.present? && book_fingerprints.include?(fp)) || + (entry_ref.present? && book_entry_refs.include?(entry_ref)) end end existing_ids = existing_transactions.map { |tx| - tx = tx.with_indifferent_access - tx[:transaction_id].presence || tx[:entry_reference].presence + EnableBankingEntry::Processor.compute_external_id(tx) }.compact.to_set new_transactions = all_transactions.select do |tx| - # Use transaction_id if present, otherwise fall back to entry_reference - tx_id = tx[:transaction_id].presence || tx[:entry_reference].presence - tx_id.present? && !existing_ids.include?(tx_id) + ext_id = EnableBankingEntry::Processor.compute_external_id(tx) + ext_id.present? && !existing_ids.include?(ext_id) end if new_transactions.any? || removed_pending @@ -398,7 +405,7 @@ class EnableBankingItem::Importer # omit transaction_id rarely produce such exact duplicates in the same # API response; timestamps or remittance info usually differ. (Issue #954) def build_transaction_content_key(tx) - date = tx[:booking_date].presence || tx[:value_date] + date = tx[:booking_date].presence || tx[:value_date].presence || tx[:transaction_date] amount = tx.dig(:transaction_amount, :amount).presence || tx[:amount] currency = tx.dig(:transaction_amount, :currency).presence || tx[:currency] creditor = tx.dig(:creditor, :name).presence || tx[:creditor_name] diff --git a/test/models/enable_banking_account/transactions/processor_test.rb b/test/models/enable_banking_account/transactions/processor_test.rb index f136ab806..00982bdef 100644 --- a/test/models/enable_banking_account/transactions/processor_test.rb +++ b/test/models/enable_banking_account/transactions/processor_test.rb @@ -157,6 +157,38 @@ class EnableBankingAccount::Transactions::ProcessorTest < ActiveSupport::TestCas assert_equal 1, result[:imported] end + test "imports id-less transaction using content fingerprint" do + tx = { + "booking_date" => Date.current.to_s, + "transaction_amount" => { "amount" => "19.99", "currency" => "EUR" }, + "credit_debit_indicator" => "DBIT", + "creditor" => { "name" => "Spotify" } + } + @enable_banking_account.update!(raw_transactions_payload: [ tx ]) + + assert_difference "@account.entries.count", 1 do + EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + end + + expected_id = EnableBankingEntry::Processor.compute_external_id(tx) + assert @account.entries.exists?(external_id: expected_id, source: "enable_banking") + end + + test "id-less transaction does not appear in failed count" do + tx = { + "booking_date" => Date.current.to_s, + "transaction_amount" => { "amount" => "5.00", "currency" => "EUR" }, + "credit_debit_indicator" => "CRDT", + "debtor" => { "name" => "Employer" } + } + @enable_banking_account.update!(raw_transactions_payload: [ tx ]) + + result = EnableBankingAccount::Transactions::Processor.new(@enable_banking_account).process + + assert_equal 0, result[:failed] + assert_equal 1, result[:imported] + end + test "handles empty raw_transactions_payload gracefully" do @enable_banking_account.update!(raw_transactions_payload: nil) diff --git a/test/models/enable_banking_entry/processor_test.rb b/test/models/enable_banking_entry/processor_test.rb index 9a4623ee6..5b54b29be 100644 --- a/test/models/enable_banking_entry/processor_test.rb +++ b/test/models/enable_banking_entry/processor_test.rb @@ -83,17 +83,89 @@ class EnableBankingEntry::ProcessorTest < ActiveSupport::TestCase end end - test "raises ArgumentError when both transaction_id and entry_reference are nil" do + # --- compute_external_id unit tests --- + + test "compute_external_id returns transaction_id-based id when present" do + assert_equal "enable_banking_txn_abc", + EnableBankingEntry::Processor.compute_external_id(transaction_id: "txn_abc", entry_reference: "ref_xyz") + end + + test "compute_external_id falls back to entry_reference when transaction_id is blank" do + assert_equal "enable_banking_ref_xyz", + EnableBankingEntry::Processor.compute_external_id(transaction_id: nil, entry_reference: "ref_xyz") + end + + test "compute_external_id returns content fingerprint when both id fields are absent" do + tx = { + booking_date: "2026-03-15", + transaction_amount: { amount: "42.00", currency: "EUR" }, + credit_debit_indicator: "DBIT", + creditor: { name: "Spar" } + } + result = EnableBankingEntry::Processor.compute_external_id(tx) + assert result.start_with?("enable_banking_content_"), "Expected content fingerprint, got: #{result}" + end + + test "compute_external_id fingerprint is stable across calls" do + tx = { + booking_date: "2026-03-15", + transaction_amount: { amount: "42.00", currency: "EUR" }, + credit_debit_indicator: "DBIT", + creditor: { name: "Spar" } + } + assert_equal EnableBankingEntry::Processor.compute_external_id(tx), + EnableBankingEntry::Processor.compute_external_id(tx) + end + + test "compute_external_id returns nil for transaction with no identifiable content" do + assert_nil EnableBankingEntry::Processor.compute_external_id({}) + assert_nil EnableBankingEntry::Processor.compute_external_id(transaction_id: nil, entry_reference: nil) + end + + # --- ID-less transaction processing --- + + test "imports transaction using content fingerprint when transaction_id and entry_reference are absent" do tx = { transaction_id: nil, entry_reference: nil, booking_date: Date.current.to_s, transaction_amount: { amount: "10.00", currency: "EUR" }, - creditor: { name: "Test" }, + creditor: { name: "Lidl" }, credit_debit_indicator: "DBIT", status: "BOOK" } + assert_difference "@account.entries.count", 1 do + EnableBankingEntry::Processor.new(tx, enable_banking_account: @enable_banking_account).process + end + + expected_id = EnableBankingEntry::Processor.compute_external_id(tx) + assert @account.entries.exists?(external_id: expected_id, source: "enable_banking") + end + + test "does not create duplicate when same id-less transaction is processed twice" do + tx = { + transaction_id: nil, + entry_reference: nil, + booking_date: Date.current.to_s, + transaction_amount: { amount: "10.00", currency: "EUR" }, + creditor: { name: "Lidl" }, + credit_debit_indicator: "DBIT", + status: "BOOK" + } + + assert_difference "@account.entries.count", 1 do + EnableBankingEntry::Processor.new(tx, enable_banking_account: @enable_banking_account).process + end + + assert_no_difference "@account.entries.count" do + EnableBankingEntry::Processor.new(tx, enable_banking_account: @enable_banking_account).process + end + end + + test "raises ArgumentError for transaction with no identifiable content at all" do + tx = { transaction_id: nil, entry_reference: nil } + assert_raises(ArgumentError) do EnableBankingEntry::Processor.new(tx, enable_banking_account: @enable_banking_account).process end diff --git a/test/models/enable_banking_item/importer_id_less_test.rb b/test/models/enable_banking_item/importer_id_less_test.rb new file mode 100644 index 000000000..2ca068fcc --- /dev/null +++ b/test/models/enable_banking_item/importer_id_less_test.rb @@ -0,0 +1,145 @@ +require "test_helper" + +class EnableBankingItem::ImporterIdLessTest < ActiveSupport::TestCase + setup do + @family = families(:dylan_family) + @account = accounts(:depository) + + @enable_banking_item = EnableBankingItem.create!( + family: @family, + name: "Test EB", + country_code: "RO", + application_id: "test_app_id", + client_certificate: "test_cert", + session_id: "test_session", + session_expires_at: 1.day.from_now, + sync_start_date: 1.month.ago.to_date + ) + @enable_banking_account = EnableBankingAccount.create!( + enable_banking_item: @enable_banking_item, + name: "Current Account", + uid: "hash_idless_test", + account_id: "uuid-idless-1234-abcd", + currency: "RON" + ) + AccountProvider.create!(account: @account, provider: @enable_banking_account) + + @mock_provider = mock() + @importer = EnableBankingItem::Importer.new(@enable_banking_item, enable_banking_provider: @mock_provider) + end + + def id_less_tx(amount: "50.00", creditor: "Kaufland", date: Date.current.to_s) + { + booking_date: date, + transaction_amount: { amount: amount, currency: "RON" }, + credit_debit_indicator: "DBIT", + creditor: { name: creditor } + } + end + + test "stores id-less transactions in raw_transactions_payload on first sync" do + tx = id_less_tx + + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "BOOK")).returns([ tx ]) + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "PDNG")).returns([]) + @importer.stubs(:include_pending?).returns(false) + @importer.stubs(:determine_sync_start_date).returns(1.month.ago.to_date) + + @importer.send(:fetch_and_store_transactions, @enable_banking_account) + + @enable_banking_account.reload + assert_equal 1, @enable_banking_account.raw_transactions_payload.count + end + + test "does not re-store id-less transaction on second sync" do + tx = id_less_tx + + # First sync + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "BOOK")).returns([ tx ]) + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "PDNG")).returns([]) + @importer.stubs(:include_pending?).returns(false) + @importer.stubs(:determine_sync_start_date).returns(1.month.ago.to_date) + + @importer.send(:fetch_and_store_transactions, @enable_banking_account) + @enable_banking_account.reload + assert_equal 1, @enable_banking_account.raw_transactions_payload.count + + # Second sync with the same transaction + @importer.send(:fetch_and_store_transactions, @enable_banking_account) + @enable_banking_account.reload + assert_equal 1, @enable_banking_account.raw_transactions_payload.count + end + + test "stores multiple distinct id-less transactions separately" do + tx1 = id_less_tx(amount: "50.00", creditor: "Kaufland") + tx2 = id_less_tx(amount: "12.50", creditor: "Starbucks") + + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "BOOK")).returns([ tx1, tx2 ]) + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "PDNG")).returns([]) + @importer.stubs(:include_pending?).returns(false) + @importer.stubs(:determine_sync_start_date).returns(1.month.ago.to_date) + + @importer.send(:fetch_and_store_transactions, @enable_banking_account) + + @enable_banking_account.reload + assert_equal 2, @enable_banking_account.raw_transactions_payload.count + end + + test "removes stored id-less pending entry when its booked counterpart arrives" do + tx = id_less_tx(amount: "30.00", creditor: "Netflix") + pending_tx = tx.merge(_pending: true) + + @enable_banking_account.update!(raw_transactions_payload: [ pending_tx ]) + + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "BOOK")).returns([ tx ]) + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "PDNG")).returns([]) + @importer.stubs(:include_pending?).returns(true) + @importer.stubs(:determine_sync_start_date).returns(1.month.ago.to_date) + + @importer.send(:fetch_and_store_transactions, @enable_banking_account) + + @enable_banking_account.reload + stored = @enable_banking_account.raw_transactions_payload + assert_equal 1, stored.count + assert_nil stored.first["_pending"] + end + + # Regression: pending row has entry_reference only; booked counterpart gains + # transaction_id on settlement. Fingerprints diverge but entry_reference is + # stable — the pending entry must still be removed from stored payload. + test "removes stored pending entry when settled book row gains a transaction_id" do + entry_ref = "REF-SETTLE-123" + + pending_tx = { + "entry_reference" => entry_ref, + "booking_date" => Date.current.to_s, + "transaction_amount" => { "amount" => "15.00", "currency" => "RON" }, + "credit_debit_indicator" => "DBIT", + "creditor" => { "name" => "Bolt" }, + "_pending" => true + } + + booked_tx = { + transaction_id: "TXN-NEW-456", + entry_reference: entry_ref, + booking_date: Date.current.to_s, + transaction_amount: { amount: "15.00", currency: "RON" }, + credit_debit_indicator: "DBIT", + creditor: { name: "Bolt" } + } + + @enable_banking_account.update!(raw_transactions_payload: [ pending_tx ]) + + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "BOOK")).returns([ booked_tx ]) + @importer.stubs(:fetch_paginated_transactions).with(@enable_banking_account, has_entry(transaction_status: "PDNG")).returns([]) + @importer.stubs(:include_pending?).returns(true) + @importer.stubs(:determine_sync_start_date).returns(1.month.ago.to_date) + + @importer.send(:fetch_and_store_transactions, @enable_banking_account) + + @enable_banking_account.reload + stored = @enable_banking_account.raw_transactions_payload + assert_equal 1, stored.count, "Stale pending entry should have been removed" + assert_nil stored.first["_pending"], "Remaining entry should be the booked row" + end +end