mirror of
https://github.com/we-promise/sure.git
synced 2026-05-27 14:34:56 +00:00
fix(enable-banking): import transactions missing transaction_id and entry_reference (#1767)
* fix(enable-banking): handle transactions missing transaction_id and entry_reference Some ASPSPs omit both transaction_id and entry_reference from their transaction payloads, which is valid per the PSD2/Berlin Group spec. Previously, every such transaction raised an ArgumentError and was silently dropped during sync. compute_external_id now falls back to a deterministic MD5 fingerprint (prefixed enable_banking_content_) derived from date, amount, currency, direction, counterparty, and remittance info. This fingerprint is stable across re-syncs, so duplicate imports are still correctly prevented. An ArgumentError is only raised for truly empty/unidentifiable payloads. The importer is updated in three places to use compute_external_id consistently: the pending pre-filter (before combining with booked), the C4 stored-pending cleanup, and the new_transactions dedup. This means ID-less pending entries are now also removed when their settled booked counterpart arrives. Tests cover compute_external_id directly (all 5 cases), end-to-end fingerprint import, idempotency, and importer storage/dedup behaviour for ID-less transactions including the pending→booked settlement path. * fix(enable-banking): implement dual-strategy matching for transaction settlement When a stored pending row had only entry_reference (no transaction_id) and the settled BOOK row arrived with a new transaction_id, compute_external_id produced different fingerprints for each side (enable_banking_<ref> vs enable_banking_<txn_id>). The fingerprint-only comparison introduced in the previous commit never matched, leaving the stale pending entry in raw_transactions_payload. Both rows were then imported as separate visible transactions. Restore a book_entry_refs set alongside book_fingerprints in both the pending pre-filter and the C4 stored-pending cleanup. A pending entry is now removed when either its fingerprint or its entry_reference matches a booked counterpart — covering same-ID settlement, content-fingerprint settlement, and the entry_reference cross-match settlement path. Also updates the ArgumentError message in external_id to accurately reflect that transaction_id, entry_reference, and content fingerprint are all accepted identifiers, and aligns build_transaction_content_key to use transaction_date as a fallback (matching compute_external_id). Adds a regression test that stores a pending-only row and asserts it is removed when the booked counterpart arrives with a new transaction_id.
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
145
test/models/enable_banking_item/importer_id_less_test.rb
Normal file
145
test/models/enable_banking_item/importer_id_less_test.rb
Normal file
@@ -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
|
||||
Reference in New Issue
Block a user