From c6771ebaabd9e3117d6d546eb191b91d69eda04c Mon Sep 17 00:00:00 2001 From: soky srm Date: Mon, 10 Nov 2025 21:32:55 +0100 Subject: [PATCH] Lunchflow fix (#307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix lunch flow pre-loading and UX * Small UX fixes - Proper closing of modal on cancel - Preload on new account already * Review comments * Fix json error * Delete .claude/settings.local.json Signed-off-by: soky srm * Lunch Flow brand (again :-) * FIX process only linked accounts * FIX disable accounts with no name * Fix string normalization --------- Signed-off-by: soky srm Co-authored-by: Juan José Mata --- app/controllers/accounts_controller.rb | 4 + .../concerns/accountable_resource.rb | 25 ------ app/controllers/lunchflow_items_controller.rb | 63 ++++++++++++- .../lunchflow_preload_controller.js | 89 +++++++++++++++++++ app/models/lunchflow_item.rb | 6 +- app/models/lunchflow_item/importer.rb | 41 ++++++--- app/models/lunchflow_item/syncer.rb | 2 +- app/models/provider/lunchflow.rb | 16 ++-- app/models/provider/lunchflow_adapter.rb | 18 ++-- app/views/accounts/new.html.erb | 5 +- .../accounts/new/_method_selector.html.erb | 19 +++- .../lunchflow_items/select_accounts.html.erb | 23 +++-- .../select_existing_account.html.erb | 23 +++-- config/locales/views/lunchflow_items/en.yml | 9 ++ config/routes.rb | 1 + 15 files changed, 271 insertions(+), 73 deletions(-) create mode 100644 app/javascript/controllers/lunchflow_preload_controller.js diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index f7663b49f..c363fdab3 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -11,6 +11,10 @@ class AccountsController < ApplicationController render layout: "settings" end + def new + @show_lunchflow_link = family.can_connect_lunchflow? + end + def sync_all family.sync_later redirect_to accounts_path, notice: "Syncing accounts..." diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index 26d881dad..4d70a225d 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -69,31 +69,6 @@ module AccountableResource @show_us_link = Current.family.can_connect_plaid_us? @show_eu_link = Current.family.can_connect_plaid_eu? @show_lunchflow_link = Current.family.can_connect_lunchflow? - - # Preload Lunchflow accounts if available and cache them - if @show_lunchflow_link - cache_key = "lunchflow_accounts_#{Current.family.id}" - - @lunchflow_accounts = Rails.cache.fetch(cache_key, expires_in: 5.minutes) do - begin - lunchflow_provider = Provider::LunchflowAdapter.build_provider - - if lunchflow_provider.present? - accounts_data = lunchflow_provider.get_accounts - accounts_data[:accounts] || [] - else - [] - end - rescue Provider::Lunchflow::LunchflowError => e - Rails.logger.error("Failed to preload Lunchflow accounts: #{e.message}") - [] - rescue StandardError => e - Rails.logger.error("Unexpected error preloading Lunchflow accounts: #{e.class}: #{e.message}") - Rails.logger.error(e.backtrace.join("\n")) - [] - end - end - end end def accountable_type diff --git a/app/controllers/lunchflow_items_controller.rb b/app/controllers/lunchflow_items_controller.rb index 0cbe7c357..61b266d40 100644 --- a/app/controllers/lunchflow_items_controller.rb +++ b/app/controllers/lunchflow_items_controller.rb @@ -9,6 +9,43 @@ class LunchflowItemsController < ApplicationController def show end + # Preload Lunchflow accounts in background (async, non-blocking) + def preload_accounts + begin + cache_key = "lunchflow_accounts_#{Current.family.id}" + + # Check if already cached + cached_accounts = Rails.cache.read(cache_key) + + if cached_accounts.present? + render json: { success: true, has_accounts: cached_accounts.any?, cached: true } + return + end + + # Fetch from API + lunchflow_provider = Provider::LunchflowAdapter.build_provider + + unless lunchflow_provider.present? + render json: { success: false, error: "no_api_key", has_accounts: false } + 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) + + render json: { success: true, has_accounts: available_accounts.any?, cached: false } + rescue Provider::Lunchflow::LunchflowError => e + Rails.logger.error("Lunchflow preload error: #{e.message}") + render json: { success: false, error: e.message, has_accounts: false } + rescue StandardError => e + Rails.logger.error("Unexpected error preloading Lunchflow accounts: #{e.class}: #{e.message}") + render json: { success: false, error: "unexpected_error", has_accounts: false } + end + end + # Fetch available accounts from Lunchflow API and show selection UI def select_accounts begin @@ -75,12 +112,20 @@ class LunchflowItemsController < ApplicationController created_accounts = [] already_linked_accounts = [] + invalid_accounts = [] selected_account_ids.each do |account_id| # Find the account data from API response account_data = accounts_data[:accounts].find { |acc| acc[:id].to_s == account_id.to_s } next unless account_data + # Validate account name is not blank (required by Account model) + if account_data[:name].blank? + invalid_accounts << account_id + Rails.logger.warn "LunchflowItemsController - Skipping account #{account_id} with blank name" + next + end + # Create or find lunchflow_account lunchflow_account = lunchflow_item.lunchflow_accounts.find_or_initialize_by( account_id: account_id.to_s @@ -117,7 +162,17 @@ class LunchflowItemsController < ApplicationController lunchflow_item.sync_later if created_accounts.any? # Build appropriate flash message - if created_accounts.any? && already_linked_accounts.any? + if invalid_accounts.any? && created_accounts.empty? && already_linked_accounts.empty? + # All selected accounts were invalid (blank names) + redirect_to new_account_path, alert: t(".invalid_account_names", count: invalid_accounts.count) + elsif invalid_accounts.any? && (created_accounts.any? || already_linked_accounts.any?) + # Some accounts were created/already linked, but some had invalid names + redirect_to return_to || accounts_path, + alert: t(".partial_invalid", + created_count: created_accounts.count, + already_linked_count: already_linked_accounts.count, + invalid_count: invalid_accounts.count) + elsif created_accounts.any? && already_linked_accounts.any? redirect_to return_to || accounts_path, notice: t(".partial_success", created_count: created_accounts.count, @@ -243,6 +298,12 @@ class LunchflowItemsController < ApplicationController return end + # Validate account name is not blank (required by Account model) + if account_data[:name].blank? + redirect_to accounts_path, alert: t(".invalid_account_name") + return + end + # Create or find lunchflow_account lunchflow_account = lunchflow_item.lunchflow_accounts.find_or_initialize_by( account_id: lunchflow_account_id.to_s diff --git a/app/javascript/controllers/lunchflow_preload_controller.js b/app/javascript/controllers/lunchflow_preload_controller.js new file mode 100644 index 000000000..6a328d0c9 --- /dev/null +++ b/app/javascript/controllers/lunchflow_preload_controller.js @@ -0,0 +1,89 @@ +import { Controller } from "@hotwired/stimulus"; + +// Connects to data-controller="lunchflow-preload" +export default class extends Controller { + static targets = ["link", "spinner"]; + static values = { + accountableType: String, + returnTo: String, + }; + + connect() { + this.preloadAccounts(); + } + + async preloadAccounts() { + try { + // Show loading state if we have a link target (on method selector page) + if (this.hasLinkTarget) { + this.showLoading(); + } + + // Fetch accounts in background to populate cache + const url = new URL( + "/lunchflow_items/preload_accounts", + window.location.origin + ); + if (this.hasAccountableTypeValue) { + url.searchParams.append("accountable_type", this.accountableTypeValue); + } + if (this.hasReturnToValue) { + url.searchParams.append("return_to", this.returnToValue); + } + + const csrfToken = document.querySelector('[name="csrf-token"]'); + const headers = { + Accept: "application/json", + }; + if (csrfToken) { + headers["X-CSRF-Token"] = csrfToken.content; + } + + const response = await fetch(url, { headers }); + + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + } + + const data = await response.json(); + + if (data.success && data.has_accounts) { + // Accounts loaded successfully, enable the link + if (this.hasLinkTarget) { + this.hideLoading(); + } + } else if (!data.has_accounts) { + // No accounts available, hide the link entirely + if (this.hasLinkTarget) { + this.linkTarget.style.display = "none"; + } + } else { + // Error occurred + if (this.hasLinkTarget) { + this.hideLoading(); + } + console.error("Failed to preload Lunchflow accounts:", data.error); + } + } catch (error) { + // On error, still enable the link so user can try + if (this.hasLinkTarget) { + this.hideLoading(); + } + console.error("Error preloading Lunchflow accounts:", error); + } + } + + showLoading() { + this.linkTarget.classList.add("pointer-events-none", "opacity-50"); + if (this.hasSpinnerTarget) { + this.spinnerTarget.classList.remove("hidden"); + } + } + + hideLoading() { + this.linkTarget.classList.remove("pointer-events-none", "opacity-50"); + if (this.hasSpinnerTarget) { + this.spinnerTarget.classList.add("hidden"); + } + } +} diff --git a/app/models/lunchflow_item.rb b/app/models/lunchflow_item.rb index 409285144..960b380b8 100644 --- a/app/models/lunchflow_item.rb +++ b/app/models/lunchflow_item.rb @@ -37,7 +37,8 @@ class LunchflowItem < ApplicationRecord return [] if lunchflow_accounts.empty? results = [] - lunchflow_accounts.joins(:account).each do |lunchflow_account| + # Only process accounts that are linked and have active status + lunchflow_accounts.joins(:account).merge(Account.visible).each do |lunchflow_account| begin result = LunchflowAccount::Processor.new(lunchflow_account).process results << { lunchflow_account_id: lunchflow_account.id, success: true, result: result } @@ -55,7 +56,8 @@ class LunchflowItem < ApplicationRecord return [] if accounts.empty? results = [] - accounts.each do |account| + # Only schedule syncs for active accounts + accounts.visible.each do |account| begin account.sync_later( parent_sync: parent_sync, diff --git a/app/models/lunchflow_item/importer.rb b/app/models/lunchflow_item/importer.rb index e2b6136ba..c0fbe94d9 100644 --- a/app/models/lunchflow_item/importer.rb +++ b/app/models/lunchflow_item/importer.rb @@ -24,31 +24,39 @@ class LunchflowItem::Importer # Continue with import even if snapshot storage fails end - # Step 2: Import accounts - accounts_imported = 0 + # Step 2: Update only previously selected accounts (don't create new ones) + accounts_updated = 0 accounts_failed = 0 if accounts_data[:accounts].present? + # Get all existing lunchflow account IDs for this item (normalize to strings for comparison) + existing_account_ids = lunchflow_item.lunchflow_accounts.pluck(:account_id).map(&:to_s) + accounts_data[:accounts].each do |account_data| + account_id = account_data[:id]&.to_s + next unless account_id.present? + + # Only update if this account was previously selected (exists in our DB) + next unless existing_account_ids.include?(account_id) + begin import_account(account_data) - accounts_imported += 1 + accounts_updated += 1 rescue => e accounts_failed += 1 - account_id = account_data[:id] || "unknown" - Rails.logger.error "LunchflowItem::Importer - Failed to import account #{account_id}: #{e.message}" - # Continue importing other accounts even if one fails + Rails.logger.error "LunchflowItem::Importer - Failed to update account #{account_id}: #{e.message}" + # Continue updating other accounts even if one fails end end end - Rails.logger.info "LunchflowItem::Importer - Imported #{accounts_imported} accounts (#{accounts_failed} failed)" + Rails.logger.info "LunchflowItem::Importer - Updated #{accounts_updated} accounts (#{accounts_failed} failed)" - # Step 3: Fetch transactions for each account + # Step 3: Fetch transactions only for linked accounts with active status transactions_imported = 0 transactions_failed = 0 - lunchflow_item.lunchflow_accounts.each do |lunchflow_account| + lunchflow_item.lunchflow_accounts.joins(:account).merge(Account.visible).each do |lunchflow_account| begin result = fetch_and_store_transactions(lunchflow_account) if result[:success] @@ -63,11 +71,11 @@ class LunchflowItem::Importer end end - Rails.logger.info "LunchflowItem::Importer - Completed import for item #{lunchflow_item.id}: #{accounts_imported} accounts, #{transactions_imported} transactions" + Rails.logger.info "LunchflowItem::Importer - Completed import for item #{lunchflow_item.id}: #{accounts_updated} accounts updated, #{transactions_imported} transactions" { success: accounts_failed == 0 && transactions_failed == 0, - accounts_imported: accounts_imported, + accounts_updated: accounts_updated, accounts_failed: accounts_failed, transactions_imported: transactions_imported, transactions_failed: transactions_failed @@ -123,16 +131,23 @@ class LunchflowItem::Importer account_id = account_data[:id] - # Validate required account_id to prevent duplicate creation + # Validate required account_id if account_id.blank? Rails.logger.warn "LunchflowItem::Importer - Skipping account with missing ID" raise ArgumentError, "Account ID is required" end - lunchflow_account = lunchflow_item.lunchflow_accounts.find_or_initialize_by( + # Only find existing accounts, don't create new ones during sync + lunchflow_account = lunchflow_item.lunchflow_accounts.find_by( account_id: account_id.to_s ) + # Skip if account wasn't previously selected + unless lunchflow_account + Rails.logger.debug "LunchflowItem::Importer - Skipping unselected account #{account_id}" + return + end + begin lunchflow_account.upsert_lunchflow_snapshot!(account_data) lunchflow_account.save! diff --git a/app/models/lunchflow_item/syncer.rb b/app/models/lunchflow_item/syncer.rb index 5618e63a9..4d10b2578 100644 --- a/app/models/lunchflow_item/syncer.rb +++ b/app/models/lunchflow_item/syncer.rb @@ -13,7 +13,7 @@ class LunchflowItem::Syncer # Phase 2: Check account setup status and collect sync statistics sync.update!(status_text: "Checking account configuration...") if sync.respond_to?(:status_text) total_accounts = lunchflow_item.lunchflow_accounts.count - linked_accounts = lunchflow_item.lunchflow_accounts.joins(:account) + linked_accounts = lunchflow_item.lunchflow_accounts.joins(:account).merge(Account.visible) unlinked_accounts = lunchflow_item.lunchflow_accounts.includes(:account).where(accounts: { id: nil }) # Store sync statistics for display diff --git a/app/models/provider/lunchflow.rb b/app/models/provider/lunchflow.rb index 069987690..17668507c 100644 --- a/app/models/provider/lunchflow.rb +++ b/app/models/provider/lunchflow.rb @@ -21,10 +21,10 @@ class Provider::Lunchflow handle_response(response) rescue SocketError, Net::OpenTimeout, Net::ReadTimeout => e - Rails.logger.error "Lunchflow API: GET /accounts failed: #{e.class}: #{e.message}" + Rails.logger.error "Lunch Flow API: GET /accounts failed: #{e.class}: #{e.message}" raise LunchflowError.new("Exception during GET request: #{e.message}", :request_failed) rescue => e - Rails.logger.error "Lunchflow API: Unexpected error during GET /accounts: #{e.class}: #{e.message}" + Rails.logger.error "Lunch Flow API: Unexpected error during GET /accounts: #{e.class}: #{e.message}" raise LunchflowError.new("Exception during GET request: #{e.message}", :request_failed) end @@ -52,10 +52,10 @@ class Provider::Lunchflow handle_response(response) rescue SocketError, Net::OpenTimeout, Net::ReadTimeout => e - Rails.logger.error "Lunchflow API: GET #{path} failed: #{e.class}: #{e.message}" + Rails.logger.error "Lunch Flow API: GET #{path} failed: #{e.class}: #{e.message}" raise LunchflowError.new("Exception during GET request: #{e.message}", :request_failed) rescue => e - Rails.logger.error "Lunchflow API: Unexpected error during GET #{path}: #{e.class}: #{e.message}" + Rails.logger.error "Lunch Flow API: Unexpected error during GET #{path}: #{e.class}: #{e.message}" raise LunchflowError.new("Exception during GET request: #{e.message}", :request_failed) end @@ -71,10 +71,10 @@ class Provider::Lunchflow handle_response(response) rescue SocketError, Net::OpenTimeout, Net::ReadTimeout => e - Rails.logger.error "Lunchflow API: GET #{path} failed: #{e.class}: #{e.message}" + Rails.logger.error "Lunch Flow API: GET #{path} failed: #{e.class}: #{e.message}" raise LunchflowError.new("Exception during GET request: #{e.message}", :request_failed) rescue => e - Rails.logger.error "Lunchflow API: Unexpected error during GET #{path}: #{e.class}: #{e.message}" + Rails.logger.error "Lunch Flow API: Unexpected error during GET #{path}: #{e.class}: #{e.message}" raise LunchflowError.new("Exception during GET request: #{e.message}", :request_failed) end @@ -93,7 +93,7 @@ class Provider::Lunchflow when 200 JSON.parse(response.body, symbolize_names: true) when 400 - Rails.logger.error "Lunchflow API: Bad request - #{response.body}" + Rails.logger.error "Lunch Flow API: Bad request - #{response.body}" raise LunchflowError.new("Bad request to Lunchflow API: #{response.body}", :bad_request) when 401 raise LunchflowError.new("Invalid API key", :unauthorized) @@ -104,7 +104,7 @@ class Provider::Lunchflow when 429 raise LunchflowError.new("Rate limit exceeded. Please try again later.", :rate_limited) else - Rails.logger.error "Lunchflow API: Unexpected response - Code: #{response.code}, Body: #{response.body}" + Rails.logger.error "Lunch Flow API: Unexpected response - Code: #{response.code}, Body: #{response.body}" raise LunchflowError.new("Failed to fetch data: #{response.code} #{response.message} - #{response.body}", :fetch_failed) end end diff --git a/app/models/provider/lunchflow_adapter.rb b/app/models/provider/lunchflow_adapter.rb index 8eb2688e0..17b342a12 100644 --- a/app/models/provider/lunchflow_adapter.rb +++ b/app/models/provider/lunchflow_adapter.rb @@ -6,12 +6,12 @@ class Provider::LunchflowAdapter < Provider::Base # Register this adapter with the factory Provider::Factory.register("LunchflowAccount", self) - # Configuration for Lunchflow + # Configuration for Lunch Flow configure do description <<~DESC Setup instructions: - 1. Visit [Lunchflow](https://www.lunchflow.app) to get your API key - 2. Enter your API key below to enable Lunchflow bank data sync + 1. Visit [Lunch Flow](https://www.lunchflow.app) to get your API key + 2. Enter your API key below to enable Lunch Flow bank data sync 3. Choose the appropriate environment (production or staging) DESC @@ -20,21 +20,21 @@ class Provider::LunchflowAdapter < Provider::Base required: true, secret: true, env_key: "LUNCHFLOW_API_KEY", - description: "Your Lunchflow API key for authentication" + description: "Your Lunch Flow API key for authentication" field :base_url, label: "Base URL", required: false, env_key: "LUNCHFLOW_BASE_URL", default: "https://lunchflow.app/api/v1", - description: "Base URL for Lunchflow API" + description: "Base URL for Lunch Flow API" end def provider_name "lunchflow" end - # Build a Lunchflow provider instance with configured credentials + # Build a Lunch Flow provider instance with configured credentials # @return [Provider::Lunchflow, nil] Returns nil if API key is not configured def self.build_provider api_key = config_value(:api_key) @@ -46,7 +46,7 @@ class Provider::LunchflowAdapter < Provider::Base # Reload Lunchflow configuration when settings are updated def self.reload_configuration - # Lunchflow doesn't need to configure Rails.application.config like Plaid does + # Lunch Flow doesn't need to configure Rails.application.config like Plaid does # The configuration is read dynamically via config_value(:api_key) and config_value(:base_url) # This method exists to be called by the settings controller after updates # No action needed here since values are fetched on-demand @@ -65,7 +65,7 @@ class Provider::LunchflowAdapter < Provider::Base end def institution_domain - # Lunchflow may provide institution metadata in account data + # Lunch Flow may provide institution metadata in account data metadata = provider_account.institution_metadata return nil unless metadata.present? @@ -77,7 +77,7 @@ class Provider::LunchflowAdapter < Provider::Base begin domain = URI.parse(url).host&.gsub(/^www\./, "") rescue URI::InvalidURIError - Rails.logger.warn("Invalid institution URL for Lunchflow account #{provider_account.id}: #{url}") + Rails.logger.warn("Invalid institution URL for Lunch Flow account #{provider_account.id}: #{url}") end end diff --git a/app/views/accounts/new.html.erb b/app/views/accounts/new.html.erb index 354075611..34b338c4f 100644 --- a/app/views/accounts/new.html.erb +++ b/app/views/accounts/new.html.erb @@ -1,5 +1,8 @@ <%= render layout: "accounts/new/container", locals: { title: t(".title") } do %> -
+
+ data-controller="lunchflow-preload" + <% end %>> <% unless params[:classification] == "liability" %> <%= render "account_type", accountable: Depository.new %> <%= render "account_type", accountable: Investment.new %> diff --git a/app/views/accounts/new/_method_selector.html.erb b/app/views/accounts/new/_method_selector.html.erb index 8a3456744..6c2419236 100644 --- a/app/views/accounts/new/_method_selector.html.erb +++ b/app/views/accounts/new/_method_selector.html.erb @@ -1,7 +1,14 @@ <%# locals: (path:, accountable_type:, show_us_link: true, show_eu_link: true, show_lunchflow_link: false) %> <%= render layout: "accounts/new/container", locals: { title: t(".title"), back_path: new_account_path } do %> -
+
+ data-controller="lunchflow-preload" + data-lunchflow-preload-accountable-type-value="<%= h(accountable_type) %>" + <% if params[:return_to] %> + data-lunchflow-preload-return-to-value="<%= h(params[:return_to]) %>" + <% end %> + <% end %>> <%= link_to path, class: "flex items-center gap-4 w-full text-center text-primary focus:outline-hidden focus:bg-surface border border-transparent focus:border focus:border-gray-200 px-2 hover:bg-surface rounded-lg p-2" do %> <%= icon("keyboard") %> @@ -39,12 +46,18 @@ class: "text-primary flex items-center gap-4 w-full text-center focus:outline-hidden focus:bg-surface border border-transparent focus:border focus:border-primary px-2 hover:bg-surface rounded-lg p-2", data: { turbo_frame: "modal", - turbo_action: "advance" + turbo_action: "advance", + lunchflow_preload_target: "link" } do %> <%= icon("link-2") %> - <%= t("accounts.new.method_selector.lunchflow_entry") %> + + <%= t("accounts.new.method_selector.lunchflow_entry") %> + + <% end %> <% end %> diff --git a/app/views/lunchflow_items/select_accounts.html.erb b/app/views/lunchflow_items/select_accounts.html.erb index c92cbaf02..9642de59e 100644 --- a/app/views/lunchflow_items/select_accounts.html.erb +++ b/app/views/lunchflow_items/select_accounts.html.erb @@ -15,15 +15,28 @@
<% @available_accounts.each do |account| %> -