QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-04-13 17:14:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

Lunch flow improvements (#268)

Browse Source 
- Add support to link existing account with lunch-flow
The account will be promoted to a lunch flow connection now
( TBD if we want to allow un-linking? )
- Add support for proper de-dup at provider import level. This will handle de-dups for Lunch Flow, Plaid and SimpleFIN
- Fix plaid account removal on invalid credentials
This commit is contained in:
soky srm
2025-10-31 13:29:44 +01:00
committed by GitHub
parent da114b5b3d
commit 106fcd06e4
11 changed files with 561 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

133
app/controllers/lunchflow_items_controller.rb
View File

@@ -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

43
app/models/account/provider_import_adapter.rb
View File

@@ -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

17
app/models/plaid_item.rb
View File

@@ -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

9
app/views/accounts/_account.html.erb
View File

@@ -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 %>
</div>
<div class="flex items-center gap-8">

43
app/views/lunchflow_items/select_existing_account.html.erb Normal file
View File

@@ -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 %>
<div class="space-y-4">
<p class="text-sm text-secondary">
<%= t(".description") %>
</p>
<form action="<%= link_existing_account_lunchflow_items_path %>" method="post" class="space-y-4" data-turbo-frame="_top">
<%= hidden_field_tag :authenticity_token, form_authenticity_token %>
<%= hidden_field_tag :account_id, @account.id %>
<%= hidden_field_tag :return_to, @return_to %>
<div class="space-y-2">
<% @available_accounts.each do |account| %>
<label class="flex items-start gap-3 p-3 border border-primary rounded-lg hover:bg-subtle cursor-pointer transition-colors">
<%= radio_button_tag "lunchflow_account_id", account[:id], false, class: "mt-1" %>
<div class="flex-1">
<div class="font-medium text-sm text-primary">
<%= account[:name] %>
</div>
<div class="text-xs text-secondary mt-1">
<%= account[:institution_name] %> • <%= account[:currency] %> • <%= account[:status] %>
</div>
</div>
</label>
<% end %>
</div>
<div class="flex gap-2 justify-end pt-4">
<%= 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" %>
</div>
</form>
</div>
<% end %>
<% end %>
<% end %>

1
config/locales/views/accounts/en.yml
View File

@@ -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

18
config/locales/views/lunchflow_items/en.yml
View File

@@ -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:

2
config/routes.rb
View File

@@ -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

53
test/jobs/family_reset_job_test.rb Normal file
View File

@@ -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

219
test/models/account/provider_import_adapter_test.rb
View File

@@ -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

28
test/models/plaid_item_test.rb
View File

@@ -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