From ad386c6e27eb12cb394b111bf373bdc8ad40f7bd Mon Sep 17 00:00:00 2001 From: AdamWHY2K Date: Sun, 1 Feb 2026 22:48:54 +0000 Subject: [PATCH] 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 --- app/models/account/provider_import_adapter.rb | 20 ++++- app/models/entry.rb | 4 + app/models/entry_search.rb | 2 + app/models/lunchflow_entry/processor.rb | 18 ++++- app/models/transaction/search.rb | 2 + test/models/lunchflow_entry/processor_test.rb | 80 +++++++++++++++---- 6 files changed, 101 insertions(+), 25 deletions(-) diff --git a/app/models/account/provider_import_adapter.rb b/app/models/account/provider_import_adapter.rb index c66fe69ef..2d814e9e8 100644 --- a/app/models/account/provider_import_adapter.rb +++ b/app/models/account/provider_import_adapter.rb @@ -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" => { diff --git a/app/models/entry.rb b/app/models/entry.rb index 7f83d0d2c..2e4887098 100644 --- a/app/models/entry.rb +++ b/app/models/entry.rb @@ -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) diff --git a/app/models/entry_search.rb b/app/models/entry_search.rb index b082cac34..0c67bd546 100644 --- a/app/models/entry_search.rb +++ b/app/models/entry_search.rb @@ -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 diff --git a/app/models/lunchflow_entry/processor.rb b/app/models/lunchflow_entry/processor.rb index 108581b87..6164752d2 100644 --- a/app/models/lunchflow_entry/processor.rb +++ b/app/models/lunchflow_entry/processor.rb @@ -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 diff --git a/app/models/transaction/search.rb b/app/models/transaction/search.rb index 1bee4ecb6..fa51fb55d 100644 --- a/app/models/transaction/search.rb +++ b/app/models/transaction/search.rb @@ -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 diff --git a/test/models/lunchflow_entry/processor_test.rb b/test/models/lunchflow_entry/processor_test.rb index 2508202d6..a597b2f58 100644 --- a/test/models/lunchflow_entry/processor_test.rb +++ b/test/models/lunchflow_entry/processor_test.rb @@ -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