mirror of
https://github.com/we-promise/sure
synced 2026-04-25 17:15:07 +02:00
fix: Lunchflow pending transaction duplicates, missing from search and filter (#859)
* fix: lunchflow parity with simplefin/plaid pending behaviour * fix: don't suggest duplicate if both entries are pending * refactor: reuse the same external_id for re-synced pending transactions * chore: replace illogical duplicate collision test with multiple sync test * fix: prevent duplicates when users edit pending lunchflow transactions * chore: add test for preventing duplicates when users edit pending lunchflow transactions * fix: normalise extra hash keys for pending detection
This commit is contained in:
@@ -77,10 +77,14 @@ class Account::ProviderImportAdapter
|
||||
end
|
||||
|
||||
# If still a new entry and this is a POSTED transaction, check for matching pending transactions
|
||||
incoming_pending = extra.is_a?(Hash) && (
|
||||
ActiveModel::Type::Boolean.new.cast(extra.dig("simplefin", "pending")) ||
|
||||
ActiveModel::Type::Boolean.new.cast(extra.dig("plaid", "pending"))
|
||||
)
|
||||
incoming_pending = false
|
||||
if extra.is_a?(Hash)
|
||||
pending_extra = extra.with_indifferent_access
|
||||
incoming_pending =
|
||||
ActiveModel::Type::Boolean.new.cast(pending_extra.dig("simplefin", "pending")) ||
|
||||
ActiveModel::Type::Boolean.new.cast(pending_extra.dig("plaid", "pending")) ||
|
||||
ActiveModel::Type::Boolean.new.cast(pending_extra.dig("lunchflow", "pending"))
|
||||
end
|
||||
|
||||
if entry.new_record? && !incoming_pending
|
||||
pending_match = nil
|
||||
@@ -686,6 +690,7 @@ class Account::ProviderImportAdapter
|
||||
.where(<<~SQL.squish)
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
SQL
|
||||
.order(date: :desc) # Prefer most recent pending transaction
|
||||
|
||||
@@ -731,6 +736,7 @@ class Account::ProviderImportAdapter
|
||||
.where(<<~SQL.squish)
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
SQL
|
||||
|
||||
# If merchant_id is provided, prioritize matching by merchant
|
||||
@@ -799,6 +805,7 @@ class Account::ProviderImportAdapter
|
||||
.where(<<~SQL.squish)
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
SQL
|
||||
|
||||
# For low confidence, require BOTH merchant AND name match (stronger signal needed)
|
||||
@@ -836,6 +843,11 @@ class Account::ProviderImportAdapter
|
||||
# Don't overwrite if already has a suggestion (keep first one found)
|
||||
return if existing_extra["potential_posted_match"].present?
|
||||
|
||||
# Don't suggest if the posted entry is also still pending (pending→pending match)
|
||||
# Suggestions are only for pending→posted reconciliation
|
||||
posted_transaction = posted_entry.entryable
|
||||
return if posted_transaction.is_a?(Transaction) && posted_transaction.pending?
|
||||
|
||||
pending_transaction.update!(
|
||||
extra: existing_extra.merge(
|
||||
"potential_posted_match" => {
|
||||
|
||||
@@ -42,6 +42,7 @@ class Entry < ApplicationRecord
|
||||
.where(<<~SQL.squish)
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
SQL
|
||||
}
|
||||
|
||||
@@ -56,6 +57,7 @@ class Entry < ApplicationRecord
|
||||
AND (
|
||||
(t.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (t.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (t.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
)
|
||||
)
|
||||
SQL
|
||||
@@ -118,6 +120,7 @@ class Entry < ApplicationRecord
|
||||
.where(<<~SQL.squish)
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean IS NOT TRUE
|
||||
AND (transactions.extra -> 'plaid' ->> 'pending')::boolean IS NOT TRUE
|
||||
AND (transactions.extra -> 'lunchflow' ->> 'pending')::boolean IS NOT TRUE
|
||||
SQL
|
||||
.limit(2) # Only need to know if 0, 1, or 2+ candidates
|
||||
.to_a # Load limited records to avoid COUNT(*) on .size
|
||||
@@ -164,6 +167,7 @@ class Entry < ApplicationRecord
|
||||
.where(<<~SQL.squish)
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean IS NOT TRUE
|
||||
AND (transactions.extra -> 'plaid' ->> 'pending')::boolean IS NOT TRUE
|
||||
AND (transactions.extra -> 'lunchflow' ->> 'pending')::boolean IS NOT TRUE
|
||||
SQL
|
||||
|
||||
# Match by name similarity (first 3 words)
|
||||
|
||||
@@ -70,6 +70,7 @@ class EntrySearch
|
||||
AND (
|
||||
(t.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (t.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (t.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
)
|
||||
)
|
||||
SQL
|
||||
@@ -82,6 +83,7 @@ class EntrySearch
|
||||
AND (
|
||||
(t.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (t.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (t.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
)
|
||||
)
|
||||
SQL
|
||||
|
||||
@@ -73,10 +73,20 @@ class LunchflowEntry::Processor
|
||||
base_temp_id = content_hash_for_transaction(data)
|
||||
temp_id_with_prefix = "lunchflow_pending_#{base_temp_id}"
|
||||
|
||||
# Handle collisions: if this external_id already exists for this account,
|
||||
# append a counter to make it unique. This prevents multiple pending transactions
|
||||
# with identical attributes (e.g., two same-day Uber rides) from colliding.
|
||||
# We check both the account's entries and the current raw payload being processed.
|
||||
# Check if entry with this external_id already exists
|
||||
# If it does AND it's still pending, reuse the same ID for re-sync.
|
||||
# The import adapter's skip logic will handle user edits correctly.
|
||||
# We DON'T check if attributes match - user edits should not cause duplicates.
|
||||
if entry_exists_with_external_id?(temp_id_with_prefix)
|
||||
existing_entry = account.entries.find_by(external_id: temp_id_with_prefix, source: "lunchflow")
|
||||
if existing_entry && existing_entry.entryable.is_a?(Transaction) && existing_entry.entryable.pending?
|
||||
Rails.logger.debug "Lunchflow: Reusing ID #{temp_id_with_prefix} for re-synced pending transaction"
|
||||
return temp_id_with_prefix
|
||||
end
|
||||
end
|
||||
|
||||
# Handle true collisions: multiple different transactions with same attributes
|
||||
# (e.g., two Uber rides on the same day for the same amount within the same sync)
|
||||
final_id = temp_id_with_prefix
|
||||
counter = 1
|
||||
|
||||
|
||||
@@ -185,11 +185,13 @@ class Transaction::Search
|
||||
pending_condition = <<~SQL.squish
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'plaid' ->> 'pending')::boolean = true
|
||||
OR (transactions.extra -> 'lunchflow' ->> 'pending')::boolean = true
|
||||
SQL
|
||||
|
||||
confirmed_condition = <<~SQL.squish
|
||||
(transactions.extra -> 'simplefin' ->> 'pending')::boolean IS DISTINCT FROM true
|
||||
AND (transactions.extra -> 'plaid' ->> 'pending')::boolean IS DISTINCT FROM true
|
||||
AND (transactions.extra -> 'lunchflow' ->> 'pending')::boolean IS DISTINCT FROM true
|
||||
SQL
|
||||
|
||||
case statuses.sort
|
||||
|
||||
@@ -151,15 +151,10 @@ class LunchflowEntry::ProcessorTest < ActiveSupport::TestCase
|
||||
# Verify the entry has a generated external_id (since we can't have blank IDs)
|
||||
assert result.external_id.present?
|
||||
assert_match /^lunchflow_pending_[a-f0-9]{32}$/, result.external_id
|
||||
|
||||
# Note: Calling the processor again with identical data will trigger collision
|
||||
# detection and create a SECOND entry (with _1 suffix). In real syncs, the
|
||||
# importer's deduplication prevents this. For true idempotency testing,
|
||||
# use the importer, not the processor directly.
|
||||
end
|
||||
|
||||
test "generates unique IDs for multiple pending transactions with identical attributes" do
|
||||
# Two pending transactions with same merchant, amount, date (e.g., two Uber rides)
|
||||
test "does not duplicate pending transaction when synced multiple times" do
|
||||
# Create a pending transaction
|
||||
transaction_data = {
|
||||
id: "",
|
||||
accountId: 456,
|
||||
@@ -178,9 +173,14 @@ class LunchflowEntry::ProcessorTest < ActiveSupport::TestCase
|
||||
).process
|
||||
|
||||
assert_not_nil result1
|
||||
assert_match /^lunchflow_pending_[a-f0-9]{32}$/, result1.external_id
|
||||
transaction1 = result1.entryable
|
||||
assert transaction1.pending?
|
||||
assert_equal true, transaction1.extra.dig("lunchflow", "pending")
|
||||
|
||||
# Process second transaction with IDENTICAL attributes
|
||||
# Count entries before second sync
|
||||
entries_before = @account.entries.where(source: "lunchflow").count
|
||||
|
||||
# Second sync - same pending transaction (still hasn't posted)
|
||||
result2 = LunchflowEntry::Processor.new(
|
||||
transaction_data,
|
||||
lunchflow_account: @lunchflow_account
|
||||
@@ -188,15 +188,61 @@ class LunchflowEntry::ProcessorTest < ActiveSupport::TestCase
|
||||
|
||||
assert_not_nil result2
|
||||
|
||||
# Should create a DIFFERENT entry (not update the first one)
|
||||
assert_not_equal result1.id, result2.id, "Should create separate entries for distinct pending transactions"
|
||||
# Should return the SAME entry, not create a duplicate
|
||||
assert_equal result1.id, result2.id, "Should update existing pending transaction, not create duplicate"
|
||||
|
||||
# Second should have a counter appended to avoid collision
|
||||
assert_match /^lunchflow_pending_[a-f0-9]{32}_\d+$/, result2.external_id
|
||||
assert_not_equal result1.external_id, result2.external_id, "Should generate different external_ids to avoid collision"
|
||||
# Verify no new entries were created
|
||||
entries_after = @account.entries.where(source: "lunchflow").count
|
||||
assert_equal entries_before, entries_after, "Should not create duplicate entry on re-sync"
|
||||
end
|
||||
|
||||
# Verify both transactions exist
|
||||
entries = @account.entries.where(source: "lunchflow", "entries.date": "2025-01-15")
|
||||
assert_equal 2, entries.count, "Should have created 2 separate entries"
|
||||
test "does not duplicate pending transaction when user has edited it" do
|
||||
# User imports a pending transaction, then edits it (name, amount, date)
|
||||
# Next sync should update the same entry, not create a duplicate
|
||||
transaction_data = {
|
||||
id: "",
|
||||
accountId: 456,
|
||||
amount: -25.50,
|
||||
currency: "USD",
|
||||
date: "2025-01-20",
|
||||
merchant: "Coffee Shop",
|
||||
description: "Morning coffee",
|
||||
isPending: true
|
||||
}
|
||||
|
||||
# First sync - import the pending transaction
|
||||
result1 = LunchflowEntry::Processor.new(
|
||||
transaction_data,
|
||||
lunchflow_account: @lunchflow_account
|
||||
).process
|
||||
|
||||
assert_not_nil result1
|
||||
original_external_id = result1.external_id
|
||||
|
||||
# User edits the transaction (common scenario)
|
||||
result1.update!(name: "Coffee Shop Downtown", amount: 26.00)
|
||||
result1.reload
|
||||
|
||||
# Verify the edits were applied
|
||||
assert_equal "Coffee Shop Downtown", result1.name
|
||||
assert_equal 26.00, result1.amount
|
||||
|
||||
entries_before = @account.entries.where(source: "lunchflow").count
|
||||
|
||||
# Second sync - same pending transaction data from provider (unchanged)
|
||||
result2 = LunchflowEntry::Processor.new(
|
||||
transaction_data,
|
||||
lunchflow_account: @lunchflow_account
|
||||
).process
|
||||
|
||||
assert_not_nil result2
|
||||
|
||||
# Should return the SAME entry (same external_id, not a _1 suffix)
|
||||
assert_equal result1.id, result2.id, "Should reuse existing entry even when user edited it"
|
||||
assert_equal original_external_id, result2.external_id, "Should not create new external_id for user-edited entry"
|
||||
|
||||
# Verify no duplicate was created
|
||||
entries_after = @account.entries.where(source: "lunchflow").count
|
||||
assert_equal entries_before, entries_after, "Should not create duplicate when user has edited pending transaction"
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user