From 6c84fc760e7f1a8b56e57f094a37617bab152c0c Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Sun, 3 May 2026 02:56:31 -0600 Subject: [PATCH] fix(mercury): support named multiple API connections (#1627) * fix(mercury): support named multiple connections * fix(mercury): address multi-connection review feedback * fix(mercury): localize connection labels * fix(mercury): strip API tokens before provider calls * test(mercury): localize provider config assertions * fix(mercury): address multi-connection review * refactor(mercury): simplify connection selection failure --- app/controllers/mercury_items_controller.rb | 168 +++++++---- .../settings/providers_controller.rb | 2 +- app/models/family/mercury_connectable.rb | 2 +- app/models/mercury_item.rb | 2 +- app/models/provider/mercury_adapter.rb | 65 +++-- .../mercury_items/select_accounts.html.erb | 1 + .../select_existing_account.html.erb | 1 + .../providers/_mercury_panel.html.erb | 159 ++++++++--- config/locales/views/mercury_items/en.yml | 34 +++ .../mercury_items_controller_test.rb | 268 ++++++++++++++++++ test/models/mercury_account_test.rb | 28 ++ test/models/mercury_item_test.rb | 41 +++ test/models/provider/mercury_adapter_test.rb | 105 ++++++- 13 files changed, 747 insertions(+), 129 deletions(-) create mode 100644 test/controllers/mercury_items_controller_test.rb diff --git a/app/controllers/mercury_items_controller.rb b/app/controllers/mercury_items_controller.rb index 14e34ca0f..f971caec5 100644 --- a/app/controllers/mercury_items_controller.rb +++ b/app/controllers/mercury_items_controller.rb @@ -13,13 +13,19 @@ class MercuryItemsController < ApplicationController # Preload Mercury accounts in background (async, non-blocking) def preload_accounts begin - # Check if family has credentials - unless Current.family.has_mercury_credentials? + account_flow = mercury_item_account_flow_context + mercury_item = account_flow[:mercury_item] + unless mercury_item + render json: mercury_item_selection_error_payload(account_flow[:credentialed_items]) + return + end + + unless mercury_item.credentials_configured? render json: { success: false, error: "no_credentials", has_accounts: false } return end - cache_key = "mercury_accounts_#{Current.family.id}" + cache_key = mercury_accounts_cache_key(mercury_item) # Check if already cached cached_accounts = Rails.cache.read(cache_key) @@ -29,8 +35,7 @@ class MercuryItemsController < ApplicationController return end - # Fetch from API - mercury_provider = Provider::MercuryAdapter.build_provider(family: Current.family) + mercury_provider = mercury_item.mercury_provider unless mercury_provider.present? render json: { success: false, error: "no_api_token", has_accounts: false } @@ -58,28 +63,21 @@ class MercuryItemsController < ApplicationController # Fetch available accounts from Mercury API and show selection UI def select_accounts begin - # Check if family has Mercury credentials configured - unless Current.family.has_mercury_credentials? - if turbo_frame_request? - # Render setup modal for turbo frame requests - render partial: "mercury_items/setup_required", layout: false - else - # Redirect for regular requests - redirect_to settings_providers_path, - alert: t(".no_credentials_configured", - default: "Please configure your Mercury API token first in Provider Settings.") - end + account_flow = mercury_item_account_flow_context + @mercury_item = account_flow[:mercury_item] + unless @mercury_item + render_mercury_item_selection_failure(credentialed_items: account_flow[:credentialed_items]) return end - cache_key = "mercury_accounts_#{Current.family.id}" + cache_key = mercury_accounts_cache_key(@mercury_item) # Try to get cached accounts first @available_accounts = Rails.cache.read(cache_key) # If not cached, fetch from API if @available_accounts.nil? - mercury_provider = Provider::MercuryAdapter.build_provider(family: Current.family) + mercury_provider = @mercury_item.mercury_provider unless mercury_provider.present? redirect_to settings_providers_path, alert: t(".no_api_token", @@ -95,12 +93,8 @@ class MercuryItemsController < ApplicationController Rails.cache.write(cache_key, @available_accounts, expires_in: 5.minutes) end - # Filter out already linked accounts - mercury_item = Current.family.mercury_items.first - if mercury_item - linked_account_ids = mercury_item.mercury_accounts.joins(:account_provider).pluck(:account_id) - @available_accounts = @available_accounts.reject { |acc| linked_account_ids.include?(acc[:id].to_s) } - end + linked_account_ids = @mercury_item.mercury_accounts.joins(:account_provider).pluck(:account_id) + @available_accounts = @available_accounts.reject { |acc| linked_account_ids.include?(acc[:id].to_s) } @accountable_type = params[:accountable_type] || "Depository" @return_to = safe_return_to_path @@ -139,13 +133,16 @@ class MercuryItemsController < ApplicationController return end - # Create or find mercury_item for this family - mercury_item = Current.family.mercury_items.first_or_create!( - name: "Mercury Connection" - ) + account_flow = mercury_item_account_flow_context + mercury_item = account_flow[:mercury_item] + + unless mercury_item + redirect_to settings_providers_path, alert: t(".select_connection", default: "Choose a Mercury connection before linking accounts.") + return + end # Fetch account details from API - mercury_provider = Provider::MercuryAdapter.build_provider(family: Current.family) + mercury_provider = mercury_item.mercury_provider unless mercury_provider.present? redirect_to new_account_path, alert: t(".no_api_token") return @@ -259,29 +256,22 @@ class MercuryItemsController < ApplicationController return end - # Check if family has Mercury credentials configured - unless Current.family.has_mercury_credentials? - if turbo_frame_request? - # Render setup modal for turbo frame requests - render partial: "mercury_items/setup_required", layout: false - else - # Redirect for regular requests - redirect_to settings_providers_path, - alert: t(".no_credentials_configured", - default: "Please configure your Mercury API token first in Provider Settings.") - end + account_flow = mercury_item_account_flow_context + @mercury_item = account_flow[:mercury_item] + unless @mercury_item + render_mercury_item_selection_failure(credentialed_items: account_flow[:credentialed_items]) return end begin - cache_key = "mercury_accounts_#{Current.family.id}" + cache_key = mercury_accounts_cache_key(@mercury_item) # Try to get cached accounts first @available_accounts = Rails.cache.read(cache_key) # If not cached, fetch from API if @available_accounts.nil? - mercury_provider = Provider::MercuryAdapter.build_provider(family: Current.family) + mercury_provider = @mercury_item.mercury_provider unless mercury_provider.present? redirect_to settings_providers_path, alert: t(".no_api_token", @@ -302,12 +292,8 @@ class MercuryItemsController < ApplicationController return end - # Filter out already linked accounts - mercury_item = Current.family.mercury_items.first - if mercury_item - linked_account_ids = mercury_item.mercury_accounts.joins(:account_provider).pluck(:account_id) - @available_accounts = @available_accounts.reject { |acc| linked_account_ids.include?(acc[:id].to_s) } - end + linked_account_ids = @mercury_item.mercury_accounts.joins(:account_provider).pluck(:account_id) + @available_accounts = @available_accounts.reject { |acc| linked_account_ids.include?(acc[:id].to_s) } if @available_accounts.empty? redirect_to accounts_path, alert: t(".all_accounts_already_linked") @@ -343,6 +329,9 @@ class MercuryItemsController < ApplicationController return end + account_flow = mercury_item_account_flow_context + mercury_item = account_flow[:mercury_item] + @account = Current.family.accounts.find(account_id) # Check if account is already linked @@ -351,13 +340,13 @@ class MercuryItemsController < ApplicationController return end - # Create or find mercury_item for this family - mercury_item = Current.family.mercury_items.first_or_create!( - name: "Mercury Connection" - ) + unless mercury_item + redirect_to settings_providers_path, alert: t(".select_connection", default: "Choose a Mercury connection before linking accounts.") + return + end # Fetch account details from API - mercury_provider = Provider::MercuryAdapter.build_provider(family: Current.family) + mercury_provider = mercury_item.mercury_provider unless mercury_provider.present? redirect_to accounts_path, alert: t(".no_api_token") return @@ -423,7 +412,7 @@ class MercuryItemsController < ApplicationController if turbo_frame_request? flash.now[:notice] = t(".success") - @mercury_items = Current.family.mercury_items.ordered + @mercury_items = Current.family.mercury_items.active.ordered.includes(:syncs, :mercury_accounts) render turbo_stream: [ turbo_stream.replace( "mercury-providers-panel", @@ -454,10 +443,15 @@ class MercuryItemsController < ApplicationController end def update - if @mercury_item.update(mercury_item_params) + permitted_params = mercury_item_params + expire_accounts_cache = mercury_accounts_cache_sensitive_update?(permitted_params) + + if @mercury_item.update(permitted_params) + Rails.cache.delete(mercury_accounts_cache_key(@mercury_item)) if expire_accounts_cache + if turbo_frame_request? flash.now[:notice] = t(".success") - @mercury_items = Current.family.mercury_items.ordered + @mercury_items = Current.family.mercury_items.active.ordered.includes(:syncs, :mercury_accounts) render turbo_stream: [ turbo_stream.replace( "mercury-providers-panel", @@ -750,7 +744,67 @@ class MercuryItemsController < ApplicationController end def mercury_item_params - params.require(:mercury_item).permit(:name, :sync_start_date, :token, :base_url) + permitted = params.require(:mercury_item).permit(:name, :sync_start_date, :token, :base_url) + permitted.delete(:token) if @mercury_item&.persisted? && permitted[:token].blank? + permitted + end + + def mercury_items_with_credentials + Current.family.mercury_items.active.ordered.select(&:credentials_configured?) + end + + def mercury_item_account_flow_context + credentialed_items = mercury_items_with_credentials + mercury_item = nil + + if params[:mercury_item_id].present? + mercury_item = credentialed_items.find { |item| item.id.to_s == params[:mercury_item_id].to_s } + elsif credentialed_items.one? + mercury_item = credentialed_items.first + end + + { + mercury_item: mercury_item, + credentialed_items: credentialed_items + } + end + + def mercury_accounts_cache_key(mercury_item) + "mercury_accounts_#{Current.family.id}_#{mercury_item.id}" + end + + def mercury_accounts_cache_sensitive_update?(permitted_params) + permitted_params.key?(:token) || permitted_params.key?(:base_url) + end + + def mercury_item_selection_error_payload(credentialed_items) + if mercury_item_selection_required?(credentialed_items) + { + success: false, + error: "select_connection", + error_message: t(".select_connection", default: "Choose a Mercury connection before loading accounts."), + has_accounts: nil + } + else + { success: false, error: "no_credentials", has_accounts: false } + end + end + + def render_mercury_item_selection_failure(credentialed_items:) + if mercury_item_selection_required?(credentialed_items) + redirect_to settings_providers_path, + alert: t(".select_connection", default: "Choose a Mercury connection in Provider Settings.") + elsif turbo_frame_request? + render partial: "mercury_items/setup_required", layout: false + else + redirect_to settings_providers_path, + alert: t(".no_credentials_configured", + default: "Please configure your Mercury API token first in Provider Settings.") + end + end + + def mercury_item_selection_required?(credentialed_items) + credentialed_items.count > 1 && params[:mercury_item_id].blank? end # Sanitize return_to parameter to prevent XSS attacks diff --git a/app/controllers/settings/providers_controller.rb b/app/controllers/settings/providers_controller.rb index f95e94a2b..7c6428376 100644 --- a/app/controllers/settings/providers_controller.rb +++ b/app/controllers/settings/providers_controller.rb @@ -141,7 +141,7 @@ class Settings::ProvidersController < ApplicationController # Providers page only needs to know whether any Sophtron connections exist with valid credentials @sophtron_items = Current.family.sophtron_items.where.not(user_id: [ nil, "" ], access_key: [ nil, "" ]).ordered.select(:id) @coinstats_items = Current.family.coinstats_items.ordered # CoinStats panel needs account info for status display - @mercury_items = Current.family.mercury_items.ordered.select(:id) + @mercury_items = Current.family.mercury_items.active.ordered.includes(:syncs, :mercury_accounts) @coinbase_items = Current.family.coinbase_items.ordered # Coinbase panel needs name and sync info for status display @snaptrade_items = Current.family.snaptrade_items.includes(:snaptrade_accounts).ordered @indexa_capital_items = Current.family.indexa_capital_items.ordered.select(:id) diff --git a/app/models/family/mercury_connectable.rb b/app/models/family/mercury_connectable.rb index d0bf9fe27..31f3998e8 100644 --- a/app/models/family/mercury_connectable.rb +++ b/app/models/family/mercury_connectable.rb @@ -23,6 +23,6 @@ module Family::MercuryConnectable end def has_mercury_credentials? - mercury_items.where.not(token: nil).exists? + mercury_items.active.any?(&:credentials_configured?) end end diff --git a/app/models/mercury_item.rb b/app/models/mercury_item.rb index eb062b2b9..5869156fb 100644 --- a/app/models/mercury_item.rb +++ b/app/models/mercury_item.rb @@ -168,7 +168,7 @@ class MercuryItem < ApplicationRecord end def credentials_configured? - token.present? + token.to_s.strip.present? end def effective_base_url diff --git a/app/models/provider/mercury_adapter.rb b/app/models/provider/mercury_adapter.rb index e1a70b839..96f6666ba 100644 --- a/app/models/provider/mercury_adapter.rb +++ b/app/models/provider/mercury_adapter.rb @@ -15,23 +15,11 @@ class Provider::MercuryAdapter < Provider::Base def self.connection_configs(family:) return [] unless family.can_connect_mercury? - [ { - key: "mercury", - name: "Mercury", - description: "Connect to your bank via Mercury", - can_connect: true, - new_account_path: ->(accountable_type, return_to) { - Rails.application.routes.url_helpers.select_accounts_mercury_items_path( - accountable_type: accountable_type, - return_to: return_to - ) - }, - existing_account_path: ->(account_id) { - Rails.application.routes.url_helpers.select_existing_account_mercury_items_path( - account_id: account_id - ) - } - } ] + mercury_items = family.mercury_items.active.ordered.select(&:credentials_configured?) + + return [ connection_config_for(nil) ] if mercury_items.empty? + + mercury_items.map { |mercury_item| connection_config_for(mercury_item) } end def provider_name @@ -41,19 +29,54 @@ class Provider::MercuryAdapter < Provider::Base # Build a Mercury provider instance with family-specific credentials # @param family [Family] The family to get credentials for (required) # @return [Provider::Mercury, nil] Returns nil if credentials are not configured - def self.build_provider(family: nil) + def self.build_provider(family: nil, mercury_item_id: nil) return nil unless family.present? - # Get family-specific credentials - mercury_item = family.mercury_items.where.not(token: nil).first + mercury_item = resolve_mercury_item(family, mercury_item_id) return nil unless mercury_item&.credentials_configured? Provider::Mercury.new( - mercury_item.token, + mercury_item.token.to_s.strip, base_url: mercury_item.effective_base_url ) end + def self.connection_config_for(mercury_item) + path_params = ->(extra = {}) do + mercury_item.present? ? extra.merge(mercury_item_id: mercury_item.id) : extra + end + + { + key: mercury_item.present? ? "mercury_#{mercury_item.id}" : "mercury", + name: mercury_item.present? ? I18n.t("mercury_items.provider_connection.name", name: mercury_item.name) : I18n.t("mercury_items.provider_connection.default_name"), + description: mercury_item.present? ? I18n.t("mercury_items.provider_connection.description", name: mercury_item.name) : I18n.t("mercury_items.provider_connection.default_description"), + can_connect: true, + new_account_path: ->(accountable_type, return_to) { + Rails.application.routes.url_helpers.select_accounts_mercury_items_path( + path_params.call(accountable_type: accountable_type, return_to: return_to) + ) + }, + existing_account_path: ->(account_id) { + Rails.application.routes.url_helpers.select_existing_account_mercury_items_path( + path_params.call(account_id: account_id) + ) + } + } + end + private_class_method :connection_config_for + + def self.resolve_mercury_item(family, mercury_item_id) + if mercury_item_id.present? + item = family.mercury_items.active.find_by(id: mercury_item_id) + return item if item&.credentials_configured? + + return nil + end + + family.mercury_items.active.ordered.find(&:credentials_configured?) + end + private_class_method :resolve_mercury_item + def sync_path Rails.application.routes.url_helpers.sync_mercury_item_path(item) end diff --git a/app/views/mercury_items/select_accounts.html.erb b/app/views/mercury_items/select_accounts.html.erb index 7b9c15857..5e80783c6 100644 --- a/app/views/mercury_items/select_accounts.html.erb +++ b/app/views/mercury_items/select_accounts.html.erb @@ -10,6 +10,7 @@