diff --git a/app/controllers/lunchflow_items_controller.rb b/app/controllers/lunchflow_items_controller.rb index 6b1fd308b..0cbe7c357 100644 --- a/app/controllers/lunchflow_items_controller.rb +++ b/app/controllers/lunchflow_items_controller.rb @@ -138,6 +138,139 @@ class LunchflowItemsController < ApplicationController redirect_to new_account_path, alert: t(".api_error", message: e.message) end + # Fetch available Lunchflow accounts to link with an existing account + def select_existing_account + account_id = params[:account_id] + + unless account_id.present? + redirect_to accounts_path, alert: t(".no_account_specified") + return + end + + @account = Current.family.accounts.find(account_id) + + # Check if account is already linked + if @account.account_providers.exists? + redirect_to accounts_path, alert: t(".account_already_linked") + return + end + + begin + cache_key = "lunchflow_accounts_#{Current.family.id}" + + # Try to get cached accounts first + @available_accounts = Rails.cache.read(cache_key) + + # If not cached, fetch from API + if @available_accounts.nil? + lunchflow_provider = Provider::LunchflowAdapter.build_provider + + unless lunchflow_provider.present? + redirect_to accounts_path, alert: t(".no_api_key") + return + end + + accounts_data = lunchflow_provider.get_accounts + + @available_accounts = accounts_data[:accounts] || [] + + # Cache the accounts for 5 minutes + Rails.cache.write(cache_key, @available_accounts, expires_in: 5.minutes) + end + + if @available_accounts.empty? + redirect_to accounts_path, alert: t(".no_accounts_found") + return + end + + # Filter out already linked accounts + lunchflow_item = Current.family.lunchflow_items.first + if lunchflow_item + linked_account_ids = lunchflow_item.lunchflow_accounts.joins(:account_provider).pluck(:account_id) + @available_accounts = @available_accounts.reject { |acc| linked_account_ids.include?(acc[:id].to_s) } + end + + if @available_accounts.empty? + redirect_to accounts_path, alert: t(".all_accounts_already_linked") + return + end + + @return_to = safe_return_to_path + + render layout: false + rescue Provider::Lunchflow::LunchflowError => e + redirect_to accounts_path, alert: t(".api_error", message: e.message) + end + end + + # Link a selected Lunchflow account to an existing account + def link_existing_account + account_id = params[:account_id] + lunchflow_account_id = params[:lunchflow_account_id] + return_to = safe_return_to_path + + unless account_id.present? && lunchflow_account_id.present? + redirect_to accounts_path, alert: t(".missing_parameters") + return + end + + @account = Current.family.accounts.find(account_id) + + # Check if account is already linked + if @account.account_providers.exists? + redirect_to accounts_path, alert: t(".account_already_linked") + return + end + + # Create or find lunchflow_item for this family + lunchflow_item = Current.family.lunchflow_items.first_or_create!( + name: "Lunchflow Connection" + ) + + # Fetch account details from API + lunchflow_provider = Provider::LunchflowAdapter.build_provider + unless lunchflow_provider.present? + redirect_to accounts_path, alert: t(".no_api_key") + return + end + + accounts_data = lunchflow_provider.get_accounts + + # Find the selected Lunchflow account data + account_data = accounts_data[:accounts].find { |acc| acc[:id].to_s == lunchflow_account_id.to_s } + unless account_data + redirect_to accounts_path, alert: t(".lunchflow_account_not_found") + return + end + + # Create or find lunchflow_account + lunchflow_account = lunchflow_item.lunchflow_accounts.find_or_initialize_by( + account_id: lunchflow_account_id.to_s + ) + lunchflow_account.upsert_lunchflow_snapshot!(account_data) + lunchflow_account.save! + + # Check if this lunchflow_account is already linked to another account + if lunchflow_account.account_provider.present? + redirect_to accounts_path, alert: t(".lunchflow_account_already_linked") + return + end + + # Link account to lunchflow_account via account_providers join table + AccountProvider.create!( + account: @account, + provider: lunchflow_account + ) + + # Trigger sync to fetch transactions + lunchflow_item.sync_later + + redirect_to return_to || accounts_path, + notice: t(".success", account_name: @account.name) + rescue Provider::Lunchflow::LunchflowError => e + redirect_to accounts_path, alert: t(".api_error", message: e.message) + end + def new @lunchflow_item = Current.family.lunchflow_items.build end diff --git a/app/models/account/provider_import_adapter.rb b/app/models/account/provider_import_adapter.rb index ba3deaece..5304b59f8 100644 --- a/app/models/account/provider_import_adapter.rb +++ b/app/models/account/provider_import_adapter.rb @@ -27,6 +27,19 @@ class Account::ProviderImportAdapter e.entryable = Transaction.new end + # If this is a new entry, check for potential duplicates from manual/CSV imports + # This handles the case where a user manually created or CSV imported a transaction + # before linking their account to a provider + if entry.new_record? + duplicate = find_duplicate_transaction(date: date, amount: amount, currency: currency) + if duplicate + # "Claim" the duplicate by updating its external_id and source + # This prevents future duplicate checks from matching it again + entry = duplicate + entry.assign_attributes(external_id: external_id, source: source) + end + end + # Validate entryable type matches to prevent external_id collisions if entry.persisted? && !entry.entryable.is_a?(Transaction) raise ArgumentError, "Entry with external_id '#{external_id}' already exists with different entryable type: #{entry.entryable_type}" @@ -252,4 +265,34 @@ class Account::ProviderImportAdapter Rails.logger.error("Failed to update #{account.accountable_type} attributes from #{source}: #{e.message}") false end + + private + + # Finds a potential duplicate transaction from manual entry or CSV import + # Matches on date, amount, and currency + # Only matches transactions without external_id (manual/CSV imported) + # + # @param date [Date, String] Transaction date + # @param amount [BigDecimal, Numeric] Transaction amount + # @param currency [String] Currency code + # @return [Entry, nil] The duplicate entry or nil if not found + def find_duplicate_transaction(date:, amount:, currency:) + # Convert date to Date object if it's a string + date = Date.parse(date.to_s) unless date.is_a?(Date) + + # Look for entries on the same account with: + # 1. Same date + # 2. Same amount (exact match) + # 3. Same currency + # 4. No external_id (manual/CSV imported transactions) + # 5. Entry type is Transaction (not Trade or Valuation) + account.entries + .where(entryable_type: "Transaction") + .where(date: date) + .where(amount: amount) + .where(currency: currency) + .where(external_id: nil) + .order(created_at: :asc) + .first + end end diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb index abd11beb3..70c57438f 100644 --- a/app/models/plaid_item.rb +++ b/app/models/plaid_item.rb @@ -104,12 +104,19 @@ class PlaidItem < ApplicationRecord plaid_provider.remove_item(access_token) rescue Plaid::ApiError => e json_response = JSON.parse(e.response_body) + error_code = json_response["error_code"] - # If the item is not found, that means it was already deleted by the user on their - # Plaid portal OR by Plaid support. Either way, we're not being billed, so continue - # with the deletion of our internal record. - unless json_response["error_code"] == "ITEM_NOT_FOUND" - raise e + # Continue with deletion if: + # - ITEM_NOT_FOUND: Item was already deleted by the user on their Plaid portal OR by Plaid support + # - INVALID_API_KEYS: API credentials are invalid/missing, so we can't communicate with Plaid anyway + # - Other credential errors: We're deleting our record, so no need to fail if we can't reach Plaid + ignorable_errors = %w[ITEM_NOT_FOUND INVALID_API_KEYS INVALID_CLIENT_ID INVALID_SECRET] + + unless ignorable_errors.include?(error_code) + # Log the error but don't prevent deletion - we're removing the item from our database + # If we can't tell Plaid, we'll at least stop using it on our end + Rails.logger.warn("Failed to remove Plaid item: #{error_code} - #{json_response['error_message']}") + Sentry.capture_exception(e) if defined?(Sentry) end end diff --git a/app/views/accounts/_account.html.erb b/app/views/accounts/_account.html.erb index b4b88c1ee..b332a294b 100644 --- a/app/views/accounts/_account.html.erb +++ b/app/views/accounts/_account.html.erb @@ -32,6 +32,15 @@ <%= link_to edit_account_path(account, return_to: return_to), data: { turbo_frame: :modal }, class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center" do %> <%= icon("pencil-line", size: "sm") %> <% end %> + + <% if !account.account_providers.exists? && (account.accountable_type == "Depository" || account.accountable_type == "CreditCard") %> + <%= link_to select_existing_account_lunchflow_items_path(account_id: account.id, return_to: return_to), + data: { turbo_frame: :modal }, + class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center gap-1", + title: t("accounts.account.link_lunchflow") do %> + <%= icon("link", size: "sm") %> + <% end %> + <% end %> <% end %>
diff --git a/app/views/lunchflow_items/select_existing_account.html.erb b/app/views/lunchflow_items/select_existing_account.html.erb new file mode 100644 index 000000000..1db3779af --- /dev/null +++ b/app/views/lunchflow_items/select_existing_account.html.erb @@ -0,0 +1,43 @@ +<%= turbo_frame_tag "modal" do %> + <%= render DS::Dialog.new do |dialog| %> + <% dialog.with_header(title: t(".title", account_name: @account.name)) %> + + <% dialog.with_body do %> +
+

+ <%= t(".description") %> +

+ +
+ <%= hidden_field_tag :authenticity_token, form_authenticity_token %> + <%= hidden_field_tag :account_id, @account.id %> + <%= hidden_field_tag :return_to, @return_to %> + +
+ <% @available_accounts.each do |account| %> + + <% end %> +
+ +
+ <%= link_to t(".cancel"), @return_to || accounts_path, + class: "inline-flex items-center gap-1 px-3 py-2 text-sm font-medium rounded-lg text-primary button-bg-secondary hover:button-bg-secondary-hover", + data: { turbo_frame: "_top" } %> + <%= submit_tag t(".link_account"), + class: "inline-flex items-center gap-1 px-3 py-2 text-sm font-medium rounded-lg text-inverse bg-inverse hover:bg-inverse-hover disabled:button-bg-disabled" %> +
+
+
+ <% end %> + <% end %> +<% end %> diff --git a/config/locales/views/accounts/en.yml b/config/locales/views/accounts/en.yml index 9e055af26..63b0de4a5 100644 --- a/config/locales/views/accounts/en.yml +++ b/config/locales/views/accounts/en.yml @@ -2,6 +2,7 @@ en: accounts: account: + link_lunchflow: Link with Lunch Flow troubleshoot: Troubleshoot chart: data_not_available: Data not available for the selected period diff --git a/config/locales/views/lunchflow_items/en.yml b/config/locales/views/lunchflow_items/en.yml index 5555903e7..eb5b98c6e 100644 --- a/config/locales/views/lunchflow_items/en.yml +++ b/config/locales/views/lunchflow_items/en.yml @@ -39,6 +39,24 @@ en: no_accounts_found: No accounts found. Please check your API key configuration. no_api_key: Lunch Flow API key is not configured. Please configure it in Settings. title: Select Lunch Flow Accounts + select_existing_account: + account_already_linked: This account is already linked to a provider + all_accounts_already_linked: All Lunch Flow accounts are already linked + api_error: "API error: %{message}" + cancel: Cancel + description: Select a Lunch Flow account to link with this account. Transactions will be synced and deduplicated automatically. + link_account: Link account + no_account_specified: No account specified + no_accounts_found: No Lunch Flow accounts found. Please check your API key configuration. + no_api_key: Lunch Flow API key is not configured. Please configure it in Settings. + title: "Link %{account_name} with Lunch Flow" + link_existing_account: + account_already_linked: This account is already linked to a provider + api_error: "API error: %{message}" + lunchflow_account_already_linked: This Lunch Flow account is already linked to another account + lunchflow_account_not_found: Lunch Flow account not found + missing_parameters: Missing required parameters + success: "Successfully linked %{account_name} with Lunch Flow" sync: success: Sync started update: diff --git a/config/routes.rb b/config/routes.rb index 72a0abdb7..ba8cf8ad8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -277,6 +277,8 @@ Rails.application.routes.draw do collection do get :select_accounts post :link_accounts + get :select_existing_account + post :link_existing_account end member do diff --git a/test/jobs/family_reset_job_test.rb b/test/jobs/family_reset_job_test.rb new file mode 100644 index 000000000..d5979f3f7 --- /dev/null +++ b/test/jobs/family_reset_job_test.rb @@ -0,0 +1,53 @@ +require "test_helper" + +class FamilyResetJobTest < ActiveJob::TestCase + setup do + @family = families(:dylan_family) + @plaid_provider = mock + Provider::Registry.stubs(:plaid_provider_for_region).returns(@plaid_provider) + end + + test "resets family data successfully" do + initial_account_count = @family.accounts.count + initial_category_count = @family.categories.count + + # Family should have existing data + assert initial_account_count > 0 + assert initial_category_count > 0 + + # Don't expect Plaid removal calls since we're using fixtures without setup + @plaid_provider.stubs(:remove_item) + + FamilyResetJob.perform_now(@family) + + # All data should be removed + assert_equal 0, @family.accounts.reload.count + assert_equal 0, @family.categories.reload.count + end + + test "resets family data even when Plaid credentials are invalid" do + # Use existing plaid item from fixtures + plaid_item = plaid_items(:one) + assert_equal @family, plaid_item.family + + initial_plaid_count = @family.plaid_items.count + assert initial_plaid_count > 0 + + # Simulate invalid Plaid credentials error + error_response = { + "error_code" => "INVALID_API_KEYS", + "error_message" => "invalid client_id or secret provided" + }.to_json + + plaid_error = Plaid::ApiError.new(code: 400, response_body: error_response) + @plaid_provider.expects(:remove_item).raises(plaid_error) + + # Job should complete successfully despite the Plaid error + assert_nothing_raised do + FamilyResetJob.perform_now(@family) + end + + # PlaidItem should be deleted + assert_equal 0, @family.plaid_items.reload.count + end +end diff --git a/test/models/account/provider_import_adapter_test.rb b/test/models/account/provider_import_adapter_test.rb index 290c6db21..e13c93a20 100644 --- a/test/models/account/provider_import_adapter_test.rb +++ b/test/models/account/provider_import_adapter_test.rb @@ -566,4 +566,223 @@ class Account::ProviderImportAdapterTest < ActiveSupport::TestCase assert_match(/Entry with external_id.*already exists with different entryable type/i, exception.message) end + + test "claims manual transaction when provider syncs matching transaction" do + # Create a manual transaction (no external_id or source) + manual_entry = @account.entries.create!( + date: Date.today, + amount: 42.50, + currency: "USD", + name: "Coffee Shop", + entryable: Transaction.new + ) + + assert_nil manual_entry.external_id + assert_nil manual_entry.source + + # Provider syncs a matching transaction - should claim the manual entry, not create new + assert_no_difference "@account.entries.count" do + entry = @adapter.import_transaction( + external_id: "lunchflow_12345", + amount: 42.50, + currency: "USD", + date: Date.today, + name: "Coffee Shop - Lunchflow", + source: "lunchflow" + ) + + # Should be the same entry, now claimed by the provider + assert_equal manual_entry.id, entry.id + assert_equal "lunchflow_12345", entry.external_id + assert_equal "lunchflow", entry.source + assert_equal "Coffee Shop - Lunchflow", entry.name + end + end + + test "claims CSV imported transaction when provider syncs matching transaction" do + # Create a CSV imported transaction (has import_id but no external_id) + import = Import.create!( + family: @family, + type: "TransactionImport", + status: :complete + ) + + csv_entry = @account.entries.create!( + date: Date.today - 1.day, + amount: 125.00, + currency: "USD", + name: "Grocery Store", + import: import, + entryable: Transaction.new + ) + + assert_nil csv_entry.external_id + assert_nil csv_entry.source + assert_equal import.id, csv_entry.import_id + + # Provider syncs a matching transaction - should claim the CSV entry + assert_no_difference "@account.entries.count" do + entry = @adapter.import_transaction( + external_id: "plaid_csv_match", + amount: 125.00, + currency: "USD", + date: Date.today - 1.day, + name: "Grocery Store - Plaid", + source: "plaid" + ) + + # Should be the same entry, now claimed by the provider + assert_equal csv_entry.id, entry.id + assert_equal "plaid_csv_match", entry.external_id + assert_equal "plaid", entry.source + assert_equal import.id, entry.import_id # Should preserve the import_id + end + end + + test "does not claim transaction when date does not match" do + # Create a manual transaction + manual_entry = @account.entries.create!( + date: Date.today - 5.days, + amount: 50.00, + currency: "USD", + name: "Restaurant", + entryable: Transaction.new + ) + + # Provider syncs similar transaction but different date - should create new entry + assert_difference "@account.entries.count", 1 do + entry = @adapter.import_transaction( + external_id: "lunchflow_different_date", + amount: 50.00, + currency: "USD", + date: Date.today, + name: "Restaurant", + source: "lunchflow" + ) + + # Should be a different entry + assert_not_equal manual_entry.id, entry.id + end + end + + test "does not claim transaction when amount does not match" do + # Create a manual transaction + manual_entry = @account.entries.create!( + date: Date.today, + amount: 50.00, + currency: "USD", + name: "Restaurant", + entryable: Transaction.new + ) + + # Provider syncs similar transaction but different amount - should create new entry + assert_difference "@account.entries.count", 1 do + entry = @adapter.import_transaction( + external_id: "lunchflow_different_amount", + amount: 51.00, + currency: "USD", + date: Date.today, + name: "Restaurant", + source: "lunchflow" + ) + + # Should be a different entry + assert_not_equal manual_entry.id, entry.id + end + end + + test "does not claim transaction when currency does not match" do + # Create a manual transaction + manual_entry = @account.entries.create!( + date: Date.today, + amount: 50.00, + currency: "EUR", + name: "Restaurant", + entryable: Transaction.new + ) + + # Provider syncs similar transaction but different currency - should create new entry + assert_difference "@account.entries.count", 1 do + entry = @adapter.import_transaction( + external_id: "lunchflow_different_currency", + amount: 50.00, + currency: "USD", + date: Date.today, + name: "Restaurant", + source: "lunchflow" + ) + + # Should be a different entry + assert_not_equal manual_entry.id, entry.id + end + end + + test "does not claim transaction that already has external_id from different provider" do + # Create a transaction already synced from SimpleFin + simplefin_entry = @adapter.import_transaction( + external_id: "simplefin_123", + amount: 30.00, + currency: "USD", + date: Date.today, + name: "Gas Station", + source: "simplefin" + ) + + # Provider (Lunchflow) syncs matching transaction - should create new entry, not claim SimpleFin's + assert_difference "@account.entries.count", 1 do + entry = @adapter.import_transaction( + external_id: "lunchflow_gas", + amount: 30.00, + currency: "USD", + date: Date.today, + name: "Gas Station", + source: "lunchflow" + ) + + # Should be a different entry because SimpleFin already claimed it + assert_not_equal simplefin_entry.id, entry.id + assert_equal "lunchflow", entry.source + assert_equal "simplefin", simplefin_entry.reload.source + end + end + + test "claims oldest matching manual transaction when multiple exist" do + # Create multiple manual transactions with same date, amount, currency + older_entry = @account.entries.create!( + date: Date.today, + amount: 20.00, + currency: "USD", + name: "Parking - Old", + entryable: Transaction.new, + created_at: 2.hours.ago + ) + + newer_entry = @account.entries.create!( + date: Date.today, + amount: 20.00, + currency: "USD", + name: "Parking - New", + entryable: Transaction.new, + created_at: 1.hour.ago + ) + + # Provider syncs matching transaction - should claim the oldest one + assert_no_difference "@account.entries.count" do + entry = @adapter.import_transaction( + external_id: "lunchflow_parking", + amount: 20.00, + currency: "USD", + date: Date.today, + name: "Parking - Provider", + source: "lunchflow" + ) + + # Should claim the older entry + assert_equal older_entry.id, entry.id + assert_equal "lunchflow_parking", entry.external_id + + # Newer entry should remain unclaimed + assert_nil newer_entry.reload.external_id + end + end end diff --git a/test/models/plaid_item_test.rb b/test/models/plaid_item_test.rb index b4f46bcda..b0d55bb5d 100644 --- a/test/models/plaid_item_test.rb +++ b/test/models/plaid_item_test.rb @@ -16,4 +16,32 @@ class PlaidItemTest < ActiveSupport::TestCase @plaid_item.destroy end end + + test "destroys item even when Plaid credentials are invalid" do + error_response = { + "error_code" => "INVALID_API_KEYS", + "error_message" => "invalid client_id or secret provided" + }.to_json + + plaid_error = Plaid::ApiError.new(code: 400, response_body: error_response) + @plaid_provider.expects(:remove_item).raises(plaid_error) + + assert_difference "PlaidItem.count", -1 do + @plaid_item.destroy + end + end + + test "destroys item even when Plaid item not found" do + error_response = { + "error_code" => "ITEM_NOT_FOUND", + "error_message" => "item not found" + }.to_json + + plaid_error = Plaid::ApiError.new(code: 400, response_body: error_response) + @plaid_provider.expects(:remove_item).raises(plaid_error) + + assert_difference "PlaidItem.count", -1 do + @plaid_item.destroy + end + end end