diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 6628972ac21..b70cf6d243d 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -3,11 +3,55 @@ class AccountsController < ApplicationController include Periodable def index - @manual_accounts = family.accounts.manual.alphabetically + @manual_accounts = family.accounts + .visible_manual + .order(:name) @plaid_items = family.plaid_items.ordered - @simplefin_items = family.simplefin_items.ordered + @simplefin_items = family.simplefin_items.ordered.includes(:syncs) @lunchflow_items = family.lunchflow_items.ordered + # Precompute per-item maps to avoid queries in the view + @simplefin_sync_stats_map = {} + @simplefin_has_unlinked_map = {} + + @simplefin_items.each do |item| + latest_sync = item.syncs.ordered.first + @simplefin_sync_stats_map[item.id] = (latest_sync&.sync_stats || {}) + @simplefin_has_unlinked_map[item.id] = item.family.accounts + .visible_manual + .exists? + end + + # Count of SimpleFin accounts that are not linked (no legacy account and no AccountProvider) + @simplefin_unlinked_count_map = {} + @simplefin_items.each do |item| + count = item.simplefin_accounts + .left_joins(:account, :account_provider) + .where(accounts: { id: nil }, account_providers: { id: nil }) + .count + @simplefin_unlinked_count_map[item.id] = count + end + + # Compute CTA visibility map used by the simplefin_item partial + @simplefin_show_relink_map = {} + @simplefin_items.each do |item| + begin + unlinked_count = @simplefin_unlinked_count_map[item.id] || 0 + manuals_exist = @simplefin_has_unlinked_map[item.id] + sfa_any = if item.simplefin_accounts.loaded? + item.simplefin_accounts.any? + else + item.simplefin_accounts.exists? + end + @simplefin_show_relink_map[item.id] = (unlinked_count.to_i == 0 && manuals_exist && sfa_any) + rescue => e + Rails.logger.warn("SimpleFin card: CTA computation failed for item #{item.id}: #{e.class} - #{e.message}") + @simplefin_show_relink_map[item.id] = false + end + end + + # Prevent Turbo Drive from caching this page to ensure fresh account lists + expires_now render layout: "settings" end diff --git a/app/controllers/concerns/simplefin_items/maps_helper.rb b/app/controllers/concerns/simplefin_items/maps_helper.rb new file mode 100644 index 00000000000..8067cb50461 --- /dev/null +++ b/app/controllers/concerns/simplefin_items/maps_helper.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module SimplefinItems + module MapsHelper + extend ActiveSupport::Concern + + # Build per-item maps consumed by the simplefin_item partial. + # Accepts a single SimplefinItem or a collection. + def build_simplefin_maps_for(items) + items = Array(items).compact + return if items.empty? + + @simplefin_sync_stats_map ||= {} + @simplefin_has_unlinked_map ||= {} + @simplefin_unlinked_count_map ||= {} + @simplefin_duplicate_only_map ||= {} + @simplefin_show_relink_map ||= {} + + # Batch-check if ANY family has manual accounts (same result for all items from same family) + family_ids = items.map { |i| i.family_id }.uniq + families_with_manuals = Account + .visible_manual + .where(family_id: family_ids) + .distinct + .pluck(:family_id) + .to_set + + # Batch-fetch unlinked counts for all items in one query + unlinked_counts = SimplefinAccount + .where(simplefin_item_id: items.map(&:id)) + .left_joins(:account, :account_provider) + .where(accounts: { id: nil }, account_providers: { id: nil }) + .group(:simplefin_item_id) + .count + + items.each do |item| + # Latest sync stats (avoid N+1; rely on includes(:syncs) where appropriate) + latest_sync = if item.syncs.loaded? + item.syncs.max_by(&:created_at) + else + item.syncs.ordered.first + end + stats = (latest_sync&.sync_stats || {}) + @simplefin_sync_stats_map[item.id] = stats + + # Whether the family has any manual accounts available to link (from batch query) + @simplefin_has_unlinked_map[item.id] = families_with_manuals.include?(item.family_id) + + # Count from batch query (defaults to 0 if not found) + @simplefin_unlinked_count_map[item.id] = unlinked_counts[item.id] || 0 + + # Whether all reported errors for this item are duplicate-account warnings + @simplefin_duplicate_only_map[item.id] = compute_duplicate_only_flag(stats) + + # Compute CTA visibility: show relink only when there are zero unlinked SFAs, + # there exist manual accounts to link, and the item has at least one SFA + begin + unlinked_count = @simplefin_unlinked_count_map[item.id] || 0 + manuals_exist = @simplefin_has_unlinked_map[item.id] + sfa_any = if item.simplefin_accounts.loaded? + item.simplefin_accounts.any? + else + item.simplefin_accounts.exists? + end + @simplefin_show_relink_map[item.id] = (unlinked_count.to_i == 0 && manuals_exist && sfa_any) + rescue StandardError => e + Rails.logger.warn("SimpleFin card: CTA computation failed for item #{item.id}: #{e.class} - #{e.message}") + @simplefin_show_relink_map[item.id] = false + end + end + + # Ensure maps are hashes even when items empty + @simplefin_sync_stats_map ||= {} + @simplefin_has_unlinked_map ||= {} + @simplefin_unlinked_count_map ||= {} + @simplefin_duplicate_only_map ||= {} + @simplefin_show_relink_map ||= {} + end + + private + def compute_duplicate_only_flag(stats) + errs = Array(stats && stats["errors"]).map do |e| + if e.is_a?(Hash) + e["message"] || e[:message] + else + e.to_s + end + end + errs.present? && errs.all? { |m| m.to_s.downcase.include?("duplicate upstream account detected") } + rescue + false + end + end +end diff --git a/app/controllers/settings/bank_sync_controller.rb b/app/controllers/settings/bank_sync_controller.rb index 21f3cda31b3..fd0f6e2d8fb 100644 --- a/app/controllers/settings/bank_sync_controller.rb +++ b/app/controllers/settings/bank_sync_controller.rb @@ -18,9 +18,11 @@ def show rel: "noopener noreferrer" }, { - name: "SimpleFin", - description: "US & Canada connections via SimpleFin protocol.", - path: simplefin_items_path + name: "SimpleFIN", + description: "US & Canada connections via SimpleFIN protocol.", + path: "https://beta-bridge.simplefin.org", + target: "_blank", + rel: "noopener noreferrer" } ] end diff --git a/app/controllers/settings/providers_controller.rb b/app/controllers/settings/providers_controller.rb index d31f34a3e56..d4a93ca8731 100644 --- a/app/controllers/settings/providers_controller.rb +++ b/app/controllers/settings/providers_controller.rb @@ -11,9 +11,7 @@ def show [ "Bank Sync Providers", nil ] ] - # Load all provider configurations - Provider::Factory.ensure_adapters_loaded - @provider_configurations = Provider::ConfigurationRegistry.all + prepare_show_context end def update @@ -74,9 +72,7 @@ def update rescue => error Rails.logger.error("Failed to update provider settings: #{error.message}") flash.now[:alert] = "Failed to update provider settings: #{error.message}" - # Set @provider_configurations so the view can render properly - Provider::Factory.ensure_adapters_loaded - @provider_configurations = Provider::ConfigurationRegistry.all + prepare_show_context render :show, status: :unprocessable_entity end @@ -121,4 +117,14 @@ def reload_provider_configs(updated_fields) adapter_class&.reload_configuration end end + + # Prepares instance vars needed by the show view and partials + def prepare_show_context + # Load all provider configurations (exclude SimpleFin, which has its own unified panel below) + Provider::Factory.ensure_adapters_loaded + @provider_configurations = Provider::ConfigurationRegistry.all.reject { |config| config.provider_key.to_s.casecmp("simplefin").zero? } + + # Providers page only needs to know whether any SimpleFin connections exist + @simplefin_items = Current.family.simplefin_items.ordered.select(:id) + end end diff --git a/app/controllers/simplefin_items_controller.rb b/app/controllers/simplefin_items_controller.rb index 2668a60eb2f..77ba9b5bccd 100644 --- a/app/controllers/simplefin_items_controller.rb +++ b/app/controllers/simplefin_items_controller.rb @@ -1,5 +1,6 @@ class SimplefinItemsController < ApplicationController - before_action :set_simplefin_item, only: [ :show, :edit, :update, :destroy, :sync, :setup_accounts, :complete_account_setup ] + include SimplefinItems::MapsHelper + before_action :set_simplefin_item, only: [ :show, :edit, :update, :destroy, :sync, :balances, :setup_accounts, :complete_account_setup, :errors ] def index @simplefin_items = Current.family.simplefin_items.active.ordered @@ -50,7 +51,16 @@ def update # Clear any requires_update status on new item updated_item.update!(status: :good) - redirect_to accounts_path, notice: t(".success") + if turbo_frame_request? + @simplefin_items = Current.family.simplefin_items.ordered + render turbo_stream: turbo_stream.replace( + "simplefin-providers-panel", + partial: "settings/providers/simplefin_panel", + locals: { simplefin_items: @simplefin_items } + ) + else + redirect_to accounts_path, notice: t(".success"), status: :see_other + end rescue ArgumentError, URI::InvalidURIError render_error(t(".errors.invalid_token"), setup_token, context: :edit) rescue Provider::Simplefin::SimplefinError => e @@ -79,10 +89,19 @@ def create begin @simplefin_item = Current.family.create_simplefin_item!( setup_token: setup_token, - item_name: "SimpleFin Connection" + item_name: "SimpleFIN Connection" ) - redirect_to accounts_path, notice: t(".success") + if turbo_frame_request? + @simplefin_items = Current.family.simplefin_items.ordered + render turbo_stream: turbo_stream.replace( + "simplefin-providers-panel", + partial: "settings/providers/simplefin_panel", + locals: { simplefin_items: @simplefin_items } + ) + else + redirect_to accounts_path, notice: t(".success"), status: :see_other + end rescue ArgumentError, URI::InvalidURIError render_error(t(".errors.invalid_token"), setup_token) rescue Provider::Simplefin::SimplefinError => e @@ -100,8 +119,14 @@ def create end def destroy + # Ensure we detach provider links and legacy associations before scheduling deletion + begin + @simplefin_item.unlink_all!(dry_run: false) + rescue => e + Rails.logger.warn("SimpleFin unlink during destroy failed: #{e.class} - #{e.message}") + end @simplefin_item.destroy_later - redirect_to accounts_path, notice: t(".success") + redirect_to accounts_path, notice: t(".success"), status: :see_other end def sync @@ -115,6 +140,17 @@ def sync end end + # Starts a balances-only sync for this SimpleFin item + def balances + sync = @simplefin_item.syncs.create!(status: :pending, sync_stats: { "balances_only" => true }) + SimplefinItem::Syncer.new(@simplefin_item).perform_sync(sync) + + respond_to do |format| + format.html { redirect_back_or_to accounts_path } + format.json { render json: { ok: true, sync_id: sync.id } } + end + end + def setup_accounts @simplefin_accounts = @simplefin_item.simplefin_accounts.includes(:account).where(accounts: { id: nil }) @account_type_options = [ @@ -183,51 +219,286 @@ def complete_account_setup # Trigger a sync to process the imported SimpleFin data (transactions and holdings) @simplefin_item.sync_later - redirect_to accounts_path, notice: t(".success") + flash[:notice] = t(".success") + if turbo_frame_request? + # Recompute data needed by Accounts#index partials + @manual_accounts = Account.uncached { + Current.family.accounts + .visible_manual + .order(:name) + .to_a + } + @simplefin_items = Current.family.simplefin_items.ordered.includes(:syncs) + build_simplefin_maps_for(@simplefin_items) + + manual_accounts_stream = if @manual_accounts.any? + turbo_stream.update( + "manual-accounts", + partial: "accounts/index/manual_accounts", + locals: { accounts: @manual_accounts } + ) + else + turbo_stream.replace("manual-accounts", view_context.tag.div(id: "manual-accounts")) + end + + render turbo_stream: [ + manual_accounts_stream, + turbo_stream.replace( + ActionView::RecordIdentifier.dom_id(@simplefin_item), + partial: "simplefin_items/simplefin_item", + locals: { simplefin_item: @simplefin_item } + ) + ] + Array(flash_notification_stream_items) + else + redirect_to accounts_path, notice: t(".success"), status: :see_other + end end def select_existing_account @account = Current.family.accounts.find(params[:account_id]) - # Get all SimpleFIN accounts from this family's SimpleFIN items - # that are not yet linked to any account + # Filter out SimpleFIN accounts that are already linked to any account + # (either via account_provider or legacy account association) @available_simplefin_accounts = Current.family.simplefin_items .includes(:simplefin_accounts) .flat_map(&:simplefin_accounts) - .select { |sa| sa.account_provider.nil? && sa.account.nil? } # Not linked via new or legacy system + .reject { |sfa| sfa.account_provider.present? || sfa.account.present? } + .sort_by { |sfa| sfa.updated_at || sfa.created_at } + .reverse - if @available_simplefin_accounts.empty? - redirect_to account_path(@account), alert: "No available SimpleFIN accounts to link. Please connect a new SimpleFIN account first." - end + # Always render a modal: either choices or a helpful empty-state + render :select_existing_account, layout: false end def link_existing_account @account = Current.family.accounts.find(params[:account_id]) simplefin_account = SimplefinAccount.find(params[:simplefin_account_id]) + # Guard: only manual accounts can be linked (no existing provider links or legacy IDs) + if @account.account_providers.any? || @account.plaid_account_id.present? || @account.simplefin_account_id.present? + flash[:alert] = "Only manual accounts can be linked" + if turbo_frame_request? + return render turbo_stream: Array(flash_notification_stream_items) + else + return redirect_to account_path(@account), alert: flash[:alert] + end + end + # Verify the SimpleFIN account belongs to this family's SimpleFIN items unless Current.family.simplefin_items.include?(simplefin_account.simplefin_item) - redirect_to account_path(@account), alert: "Invalid SimpleFIN account selected" + flash[:alert] = "Invalid SimpleFIN account selected" + if turbo_frame_request? + render turbo_stream: Array(flash_notification_stream_items) + else + redirect_to account_path(@account), alert: flash[:alert] + end return end - # Verify the SimpleFIN account is not already linked - if simplefin_account.account_provider.present? || simplefin_account.account.present? - redirect_to account_path(@account), alert: "This SimpleFIN account is already linked" - return + # Relink behavior: detach any legacy link and point provider link at the chosen account + Account.transaction do + simplefin_account.lock! + # Clear legacy association if present + if simplefin_account.account_id.present? + simplefin_account.update!(account_id: nil) + end + + # Upsert the AccountProvider mapping deterministically + ap = AccountProvider.find_or_initialize_by(provider: simplefin_account) + previous_account = ap.account + ap.account_id = @account.id + ap.save! + + # If the provider was previously linked to a different account in this family, + # and that account is now orphaned, quietly disable it so it disappears from the + # visible manual list. This mirrors the unified flow expectation that the provider + # follows the chosen account. + if previous_account && previous_account.id != @account.id && previous_account.family_id == @account.family_id + previous_account.disable! rescue nil + end end - # Create the link via AccountProvider - AccountProvider.create!( - account: @account, - provider: simplefin_account - ) + if turbo_frame_request? + # Reload the item to ensure associations are fresh + simplefin_account.reload + item = simplefin_account.simplefin_item + item.reload + + # Recompute data needed by Accounts#index partials + @manual_accounts = Account.uncached { + Current.family.accounts + .visible_manual + .order(:name) + .to_a + } + @simplefin_items = Current.family.simplefin_items.ordered.includes(:syncs) + build_simplefin_maps_for(@simplefin_items) + + flash[:notice] = "Account successfully linked to SimpleFIN" + @account.reload + manual_accounts_stream = if @manual_accounts.any? + turbo_stream.update( + "manual-accounts", + partial: "accounts/index/manual_accounts", + locals: { accounts: @manual_accounts } + ) + else + turbo_stream.replace("manual-accounts", view_context.tag.div(id: "manual-accounts")) + end - redirect_to accounts_path, notice: "Account successfully linked to SimpleFIN" + render turbo_stream: [ + # Optimistic removal of the specific account row if it exists in the DOM + turbo_stream.remove(ActionView::RecordIdentifier.dom_id(@account)), + manual_accounts_stream, + turbo_stream.replace( + ActionView::RecordIdentifier.dom_id(item), + partial: "simplefin_items/simplefin_item", + locals: { simplefin_item: item } + ), + turbo_stream.replace("modal", view_context.turbo_frame_tag("modal")) + ] + Array(flash_notification_stream_items) + else + redirect_to accounts_path(cache_bust: SecureRandom.hex(6)), notice: "Account successfully linked to SimpleFIN", status: :see_other + end + end + + def errors + # Find the latest sync to surface its error messages in a lightweight modal + latest_sync = if @simplefin_item.syncs.loaded? + @simplefin_item.syncs.max_by(&:created_at) + else + @simplefin_item.syncs.ordered.first + end + + stats = (latest_sync&.sync_stats || {}) + raw_errors = Array(stats["errors"]) # may contain strings or hashes with message keys + + @errors = raw_errors.map { |e| + if e.is_a?(Hash) + e["message"] || e[:message] || e.to_s + else + e.to_s + end + }.compact + + # Fall back to simplefin_item.sync_error if present and not already included + if @simplefin_item.respond_to?(:sync_error) && @simplefin_item.sync_error.present? + @errors << @simplefin_item.sync_error + end + + # De-duplicate and keep non-empty messages + @errors = @errors.map(&:to_s).map(&:strip).reject(&:blank?).uniq + + render layout: false end private + NAME_NORM_RE = /\s+/.freeze + + + def normalize_name(str) + s = str.to_s.downcase.strip + return s if s.empty? + s.gsub(NAME_NORM_RE, " ") + end + + def compute_relink_candidates + # Best-effort dedup before building candidates + @simplefin_item.dedup_simplefin_accounts! rescue nil + + family = @simplefin_item.family + manuals = Account.visible_manual.where(family_id: family.id).to_a + + # Evaluate only one SimpleFin account per upstream account_id (prefer linked, else newest) + grouped = @simplefin_item.simplefin_accounts.group_by(&:account_id) + sfas = grouped.values.map { |list| list.find { |s| s.current_account.present? } || list.max_by(&:updated_at) } + + Rails.logger.info("SimpleFin compute_relink_candidates: manuals=#{manuals.size} sfas=#{sfas.size} (item_id=#{@simplefin_item.id})") + + used_manual_ids = Set.new + pairs = [] + + sfas.each do |sfa| + next if sfa.name.blank? + # Heuristics (with ambiguity guards): last4 > balance ±0.01 > name + raw = (sfa.raw_payload || {}).with_indifferent_access + sfa_last4 = raw[:mask] || raw[:last4] || raw[:"last-4"] || raw[:"account_number_last4"] + sfa_last4 = sfa_last4.to_s.strip.presence + sfa_balance = (sfa.current_balance || sfa.available_balance).to_d rescue 0.to_d + + chosen = nil + reason = nil + + # 1) last4 match: compute all candidates not yet used + if sfa_last4.present? + last4_matches = manuals.reject { |a| used_manual_ids.include?(a.id) }.select do |a| + a_last4 = nil + %i[mask last4 number_last4 account_number_last4].each do |k| + if a.respond_to?(k) + val = a.public_send(k) + a_last4 = val.to_s.strip.presence if val.present? + break if a_last4 + end + end + a_last4.present? && a_last4 == sfa_last4 + end + # Ambiguity guard: skip if multiple matches + if last4_matches.size == 1 + cand = last4_matches.first + # Conflict guard: if both have balances and differ wildly, skip + begin + ab = (cand.balance || cand.cash_balance || 0).to_d + if sfa_balance.nonzero? && ab.nonzero? && (ab - sfa_balance).abs > BigDecimal("1.00") + cand = nil + end + rescue + # ignore balance parsing errors + end + if cand + chosen = cand + reason = "last4" + end + end + end + + # 2) balance proximity + if chosen.nil? && sfa_balance.nonzero? + balance_matches = manuals.reject { |a| used_manual_ids.include?(a.id) }.select do |a| + begin + ab = (a.balance || a.cash_balance || 0).to_d + (ab - sfa_balance).abs <= BigDecimal("0.01") + rescue + false + end + end + if balance_matches.size == 1 + chosen = balance_matches.first + reason = "balance" + end + end + + # 3) exact normalized name + if chosen.nil? + name_matches = manuals.reject { |a| used_manual_ids.include?(a.id) }.select { |a| normalize_name(a.name) == normalize_name(sfa.name) } + if name_matches.size == 1 + chosen = name_matches.first + reason = "name" + end + end + + if chosen + used_manual_ids << chosen.id + pairs << { sfa_id: sfa.id, sfa_name: sfa.name, manual_id: chosen.id, manual_name: chosen.name, reason: reason } + end + end + + Rails.logger.info("SimpleFin compute_relink_candidates: built #{pairs.size} pairs (item_id=#{@simplefin_item.id})") + + # Return without the reason field to the view + pairs.map { |p| p.slice(:sfa_id, :sfa_name, :manual_id, :manual_name) } + end + def set_simplefin_item @simplefin_item = Current.family.simplefin_items.find(params[:id]) end diff --git a/app/helpers/settings_helper.rb b/app/helpers/settings_helper.rb index d4fdca786ec..b3c3a75052e 100644 --- a/app/helpers/settings_helper.rb +++ b/app/helpers/settings_helper.rb @@ -74,8 +74,9 @@ def not_self_hosted? !self_hosted? end + # Helper used by SETTINGS_ORDER conditions def admin_user? - Current.user&.admin? == true + Current.user&.admin? end def self_hosted_and_admin? diff --git a/app/helpers/simplefin_items_helper.rb b/app/helpers/simplefin_items_helper.rb new file mode 100644 index 00000000000..9b3c631f168 --- /dev/null +++ b/app/helpers/simplefin_items_helper.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# View helpers for SimpleFin UI rendering +module SimplefinItemsHelper + # Builds a compact tooltip text summarizing sync errors from a stats hash. + # The stats structure comes from SimplefinItem::Importer and Sync records. + # Returns nil when there is nothing meaningful to display. + # + # Example structure: + # { + # "total_errors" => 3, + # "errors" => [ { "name" => "Chase", "message" => "Timeout" }, ... ], + # "error_buckets" => { "auth" => 1, "api" => 2 } + # } + def simplefin_error_tooltip(stats) + return nil unless stats.is_a?(Hash) + + total_errors = stats["total_errors"].to_i + return nil if total_errors.zero? + + sample = Array(stats["errors"]).map do |e| + name = (e[:name] || e["name"]).to_s + msg = (e[:message] || e["message"]).to_s + name.present? ? "#{name}: #{msg}" : msg + end.compact.first(2).join(" • ") + + buckets = stats["error_buckets"] || {} + bucket_text = if buckets.present? + buckets.map { |k, v| "#{k}: #{v}" }.join(", ") + end + + parts = [ "Errors: ", total_errors.to_s ] + parts << " (#{bucket_text})" if bucket_text.present? + parts << " — #{sample}" if sample.present? + parts.join + end +end diff --git a/app/helpers/transactions_helper.rb b/app/helpers/transactions_helper.rb index 173306b9109..024d742be54 100644 --- a/app/helpers/transactions_helper.rb +++ b/app/helpers/transactions_helper.rb @@ -18,4 +18,61 @@ def get_transaction_search_filter_partial_path(filter) def get_default_transaction_search_filter transaction_search_filters[0] end + + # ---- Transaction extra details helpers ---- + # Returns a structured hash describing extra details for a transaction. + # Input can be a Transaction or an Entry (responds_to :transaction). + # Structure: + # { + # kind: :simplefin | :raw, + # simplefin: { payee:, description:, memo: }, + # provider_extras: [ { key:, value:, title: } ], + # raw: String (pretty JSON or string) + # } + def build_transaction_extra_details(obj) + tx = obj.respond_to?(:transaction) ? obj.transaction : obj + return nil unless tx.respond_to?(:extra) && tx.extra.present? + + extra = tx.extra + + if extra.is_a?(Hash) && extra["simplefin"].present? + sf = extra["simplefin"] + simple = { + payee: sf.is_a?(Hash) ? sf["payee"].presence : nil, + description: sf.is_a?(Hash) ? sf["description"].presence : nil, + memo: sf.is_a?(Hash) ? sf["memo"].presence : nil + }.compact + + extras = [] + if sf.is_a?(Hash) && sf["extra"].is_a?(Hash) && sf["extra"].present? + sf["extra"].each do |k, v| + display = (v.is_a?(Hash) || v.is_a?(Array)) ? v.to_json : v + extras << { + key: k.to_s.humanize, + value: display, + title: (v.is_a?(String) ? v : display.to_s) + } + end + end + + { + kind: :simplefin, + simplefin: simple, + provider_extras: extras, + raw: nil + } + else + pretty = begin + JSON.pretty_generate(extra) + rescue StandardError + extra.to_s + end + { + kind: :raw, + simplefin: {}, + provider_extras: [], + raw: pretty + } + end + end end diff --git a/app/jobs/simplefin_item/balances_only_job.rb b/app/jobs/simplefin_item/balances_only_job.rb new file mode 100644 index 00000000000..ec9f2a92f3e --- /dev/null +++ b/app/jobs/simplefin_item/balances_only_job.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class SimplefinItem::BalancesOnlyJob < ApplicationJob + queue_as :default + + # Performs a lightweight, balances-only discovery: + # - import_balances_only + # - update last_synced_at (when column exists) + # Any exceptions are logged and safely swallowed to avoid breaking user flow. + def perform(simplefin_item_id) + item = SimplefinItem.find_by(id: simplefin_item_id) + return unless item + + begin + SimplefinItem::Importer + .new(item, simplefin_provider: item.simplefin_provider) + .import_balances_only + rescue Provider::Simplefin::SimplefinError, ArgumentError, StandardError => e + Rails.logger.warn("SimpleFin BalancesOnlyJob import failed: #{e.class} - #{e.message}") + end + + # Best-effort freshness update + begin + item.update!(last_synced_at: Time.current) if item.has_attribute?(:last_synced_at) + rescue => e + Rails.logger.warn("SimpleFin BalancesOnlyJob last_synced_at update failed: #{e.class} - #{e.message}") + end + + # Refresh the SimpleFin card on Providers/Accounts pages so badges and statuses update without a full reload + begin + card_html = ApplicationController.render( + partial: "simplefin_items/simplefin_item", + formats: [ :html ], + locals: { simplefin_item: item } + ) + target_id = ActionView::RecordIdentifier.dom_id(item) + Turbo::StreamsChannel.broadcast_replace_to(item.family, target: target_id, html: card_html) + + # Also refresh Manual Accounts so the CTA state and duplicates clear without refresh + begin + manual_accounts = item.family.accounts + .visible_manual + .order(:name) + if manual_accounts.any? + manual_html = ApplicationController.render( + partial: "accounts/index/manual_accounts", + formats: [ :html ], + locals: { accounts: manual_accounts } + ) + Turbo::StreamsChannel.broadcast_update_to(item.family, target: "manual-accounts", html: manual_html) + else + manual_html = ApplicationController.render(inline: '
') + Turbo::StreamsChannel.broadcast_replace_to(item.family, target: "manual-accounts", html: manual_html) + end + rescue => inner + Rails.logger.warn("SimpleFin BalancesOnlyJob manual-accounts broadcast failed: #{inner.class} - #{inner.message}") + end + rescue => e + Rails.logger.warn("SimpleFin BalancesOnlyJob broadcast failed: #{e.class} - #{e.message}") + end + end +end diff --git a/app/models/account.rb b/app/models/account.rb index 428cb513d7a..a53c39451bb 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -28,6 +28,10 @@ class Account < ApplicationRecord .where(plaid_account_id: nil, simplefin_account_id: nil) } + scope :visible_manual, -> { + visible.manual + } + has_one_attached :logo delegated_type :accountable, types: Accountable::TYPES, dependent: :destroy @@ -163,14 +167,15 @@ def destroy end def current_holdings - holdings.where(currency: currency) - .where.not(qty: 0) - .where( - id: holdings.select("DISTINCT ON (security_id) id") - .where(currency: currency) - .order(:security_id, date: :desc) - ) - .order(amount: :desc) + holdings + .where(currency: currency) + .where.not(qty: 0) + .where( + id: holdings.select("DISTINCT ON (security_id) id") + .where(currency: currency) + .order(:security_id, date: :desc) + ) + .order(amount: :desc) end def start_date diff --git a/app/models/account/provider_import_adapter.rb b/app/models/account/provider_import_adapter.rb index 0c7fc188327..f1cd3e70bdc 100644 --- a/app/models/account/provider_import_adapter.rb +++ b/app/models/account/provider_import_adapter.rb @@ -15,8 +15,10 @@ def initialize(account) # @param source [String] Provider name (e.g., "plaid", "simplefin") # @param category_id [Integer, nil] Optional category ID # @param merchant [Merchant, nil] Optional merchant object + # @param notes [String, nil] Optional transaction notes/memo + # @param extra [Hash, nil] Optional provider-specific metadata to merge into transaction.extra # @return [Entry] The created or updated entry - def import_transaction(external_id:, amount:, currency:, date:, name:, source:, category_id: nil, merchant: nil) + def import_transaction(external_id:, amount:, currency:, date:, name:, source:, category_id: nil, merchant: nil, notes: nil, extra: nil) raise ArgumentError, "external_id is required" if external_id.blank? raise ArgumentError, "source is required" if source.blank? @@ -64,6 +66,16 @@ def import_transaction(external_id:, amount:, currency:, date:, name:, source:, entry.transaction.enrich_attribute(:merchant_id, merchant.id, source: source) end + if notes.present? && entry.respond_to?(:enrich_attribute) + entry.enrich_attribute(:notes, notes, source: source) + end + + # Persist extra provider metadata on the transaction (non-enriched; always merged) + if extra.present? && entry.entryable.is_a?(Transaction) + existing = entry.transaction.extra || {} + incoming = extra.is_a?(Hash) ? extra.deep_stringify_keys : {} + entry.transaction.extra = existing.deep_merge(incoming) + end entry.save! entry end diff --git a/app/models/family/auto_transfer_matchable.rb b/app/models/family/auto_transfer_matchable.rb index 28d06f43afb..8566cbdcf61 100644 --- a/app/models/family/auto_transfer_matchable.rb +++ b/app/models/family/auto_transfer_matchable.rb @@ -60,10 +60,14 @@ def auto_match_transfers! next if used_transaction_ids.include?(match.inflow_transaction_id) || used_transaction_ids.include?(match.outflow_transaction_id) - Transfer.create!( - inflow_transaction_id: match.inflow_transaction_id, - outflow_transaction_id: match.outflow_transaction_id, - ) + begin + Transfer.find_or_create_by!( + inflow_transaction_id: match.inflow_transaction_id, + outflow_transaction_id: match.outflow_transaction_id, + ) + rescue ActiveRecord::RecordNotUnique + # Another concurrent job created the transfer; safe to ignore + end Transaction.find(match.inflow_transaction_id).update!(kind: "funds_movement") Transaction.find(match.outflow_transaction_id).update!(kind: Transfer.kind_for_account(Transaction.find(match.outflow_transaction_id).entry.account)) diff --git a/app/models/holding.rb b/app/models/holding.rb index ae664a83af7..b6cf379f0b4 100644 --- a/app/models/holding.rb +++ b/app/models/holding.rb @@ -49,6 +49,23 @@ def trend @trend ||= calculate_trend end + # Day change based on previous holding snapshot (same account/security/currency) + # Returns a Trend struct similar to other trend usages or nil if no prior snapshot. + def day_change + # Memoize even when nil to avoid repeated queries during a request lifecycle + return @day_change if instance_variable_defined?(:@day_change) + + return (@day_change = nil) unless amount_money + + prev = account.holdings + .where(security_id: security_id, currency: currency) + .where("date < ?", date) + .order(date: :desc) + .first + + @day_change = prev&.amount_money ? Trend.new(current: amount_money, previous: prev.amount_money) : nil + end + def trades account.entries.where(entryable: account.trades.where(security: security)).reverse_chronological end diff --git a/app/models/recurring_transaction/identifier.rb b/app/models/recurring_transaction/identifier.rb index 33fc0d9bf9f..a7212bb7c5d 100644 --- a/app/models/recurring_transaction/identifier.rb +++ b/app/models/recurring_transaction/identifier.rb @@ -55,7 +55,6 @@ def identify_recurring_patterns entries: entries } - # Set either merchant_id or name based on identifier type if identifier_type == :merchant pattern[:merchant_id] = identifier_value else diff --git a/app/models/simplefin_account.rb b/app/models/simplefin_account.rb index 8e5000dac21..3961bea63e2 100644 --- a/app/models/simplefin_account.rb +++ b/app/models/simplefin_account.rb @@ -9,6 +9,7 @@ class SimplefinAccount < ApplicationRecord has_one :linked_account, through: :account_provider, source: :account validates :name, :account_type, :currency, presence: true + validates :account_id, uniqueness: { scope: :simplefin_item_id, allow_nil: true } validate :has_balance # Helper to get account using new system first, falling back to legacy @@ -16,6 +17,23 @@ def current_account linked_account || account end + # Ensure there is an AccountProvider link for this SimpleFin account and its current Account. + # Safe and idempotent; returns the AccountProvider or nil if no account is associated yet. + def ensure_account_provider! + acct = current_account + return nil unless acct + + AccountProvider + .find_or_initialize_by(provider_type: "SimplefinAccount", provider_id: id) + .tap do |provider| + provider.account = acct + provider.save! + end + rescue => e + Rails.logger.warn("SimplefinAccount##{id}: failed to ensure AccountProvider link: #{e.class} - #{e.message}") + nil + end + def upsert_simplefin_snapshot!(account_snapshot) # Convert to symbol keys or handle both string and symbol keys snapshot = account_snapshot.with_indifferent_access diff --git a/app/models/simplefin_account/investments/holdings_processor.rb b/app/models/simplefin_account/investments/holdings_processor.rb index 27fec583ce2..ee942fac21d 100644 --- a/app/models/simplefin_account/investments/holdings_processor.rb +++ b/app/models/simplefin_account/investments/holdings_processor.rb @@ -5,37 +5,58 @@ def initialize(simplefin_account) def process return if holdings_data.empty? - return unless account&.accountable_type == "Investment" + return unless [ "Investment", "Crypto" ].include?(account&.accountable_type) holdings_data.each do |simplefin_holding| begin symbol = simplefin_holding["symbol"] holding_id = simplefin_holding["id"] - next unless symbol.present? && holding_id.present? + Rails.logger.debug({ event: "simplefin.holding.start", sfa_id: simplefin_account.id, account_id: account&.id, id: holding_id, symbol: symbol, raw: simplefin_holding }.to_json) + + unless symbol.present? && holding_id.present? + Rails.logger.debug({ event: "simplefin.holding.skip", reason: "missing_symbol_or_id", id: holding_id, symbol: symbol }.to_json) + next + end security = resolve_security(symbol, simplefin_holding["description"]) - next unless security.present? + unless security.present? + Rails.logger.debug({ event: "simplefin.holding.skip", reason: "unresolved_security", id: holding_id, symbol: symbol }.to_json) + next + end - # Parse all the data SimpleFin provides - qty = parse_decimal(simplefin_holding["shares"]) - market_value = parse_decimal(simplefin_holding["market_value"]) - cost_basis = parse_decimal(simplefin_holding["cost_basis"]) + # Parse provider data with robust fallbacks across SimpleFin sources + qty = parse_decimal(any_of(simplefin_holding, %w[shares quantity qty units])) + market_value = parse_decimal(any_of(simplefin_holding, %w[market_value value current_value])) + cost_basis = parse_decimal(any_of(simplefin_holding, %w[cost_basis basis total_cost])) - # Calculate price from market_value if we have shares, fallback to purchase_price + # Derive price from market_value when possible; otherwise fall back to any price field + fallback_price = parse_decimal(any_of(simplefin_holding, %w[purchase_price price unit_price average_cost avg_cost])) price = if qty > 0 && market_value > 0 market_value / qty else - parse_decimal(simplefin_holding["purchase_price"]) || 0 + fallback_price || 0 end - # Use the created timestamp as the holding date, fallback to current date - holding_date = parse_holding_date(simplefin_holding["created"]) || Date.current + # Compute an amount we can persist (some providers omit market_value) + computed_amount = if market_value > 0 + market_value + elsif qty > 0 && price > 0 + qty * price + else + 0 + end + + # Use best-known date: created -> updated_at -> as_of -> date -> today + holding_date = parse_holding_date(any_of(simplefin_holding, %w[created updated_at as_of date])) || Date.current + + # Skip zero positions with no value to avoid invisible rows + next if qty.to_d.zero? && computed_amount.to_d.zero? - import_adapter.import_holding( + saved = import_adapter.import_holding( security: security, quantity: qty, - amount: market_value, + amount: computed_amount, currency: simplefin_holding["currency"] || "USD", date: holding_date, price: price, @@ -45,6 +66,8 @@ def process source: "simplefin", delete_future_holdings: false # SimpleFin tracks each holding uniquely ) + + Rails.logger.debug({ event: "simplefin.holding.saved", account_id: account&.id, holding_id: saved.id, security_id: saved.security_id, qty: saved.qty.to_s, amount: saved.amount.to_s, currency: saved.currency, date: saved.date, external_id: saved.external_id }.to_json) rescue => e ctx = (defined?(symbol) && symbol.present?) ? " #{symbol}" : "" Rails.logger.error "Error processing SimpleFin holding#{ctx}: #{e.message}" @@ -69,8 +92,17 @@ def holdings_data end def resolve_security(symbol, description) + # Normalize crypto tickers to a distinct namespace so they don't collide with equities + sym = symbol.to_s.upcase + is_crypto_account = account&.accountable_type == "Crypto" || simplefin_account.name.to_s.downcase.include?("crypto") + is_crypto_symbol = %w[BTC ETH SOL DOGE LTC BCH].include?(sym) + mentions_crypto = description.to_s.downcase.include?("crypto") + + if !sym.include?(":") && (is_crypto_account || is_crypto_symbol || mentions_crypto) + sym = "CRYPTO:#{sym}" + end # Use Security::Resolver to find or create the security - Security::Resolver.new(symbol).resolve + Security::Resolver.new(sym).resolve rescue ArgumentError => e Rails.logger.error "Failed to resolve SimpleFin security #{symbol}: #{e.message}" nil @@ -92,6 +124,19 @@ def parse_holding_date(created_timestamp) nil end + # Returns the first non-empty value for any of the provided keys in the given hash + def any_of(hash, keys) + return nil unless hash.respond_to?(:[]) + Array(keys).each do |k| + # Support symbol or string keys + v = hash[k] + v = hash[k.to_s] if v.nil? + v = hash[k.to_sym] if v.nil? + return v if !v.nil? && v.to_s.strip != "" + end + nil + end + def parse_decimal(value) return 0 unless value.present? diff --git a/app/models/simplefin_account/processor.rb b/app/models/simplefin_account/processor.rb index c97124c6a2c..8429b17afa9 100644 --- a/app/models/simplefin_account/processor.rb +++ b/app/models/simplefin_account/processor.rb @@ -9,11 +9,19 @@ def initialize(simplefin_account) # Processing the account is the first step and if it fails, we halt # Each subsequent step can fail independently, but we continue processing def process + # If the account is missing (e.g., user deleted the connection and re‑linked later), + # do not auto‑link. Relinking is now a manual, user‑confirmed flow via the Relink modal. unless simplefin_account.current_account.present? return end process_account! + # Ensure provider link exists after processing the account/balance + begin + simplefin_account.ensure_account_provider! + rescue => e + Rails.logger.warn("SimpleFin provider link ensure failed for #{simplefin_account.id}: #{e.class} - #{e.message}") + end process_transactions process_investments process_liabilities diff --git a/app/models/simplefin_entry/processor.rb b/app/models/simplefin_entry/processor.rb index c7156f1a22c..d176d48571a 100644 --- a/app/models/simplefin_entry/processor.rb +++ b/app/models/simplefin_entry/processor.rb @@ -16,13 +16,28 @@ def process date: date, name: name, source: "simplefin", - merchant: merchant + merchant: merchant, + notes: notes, + extra: extra_metadata ) end private attr_reader :simplefin_transaction, :simplefin_account + def extra_metadata + sf = {} + # Preserve raw strings from provider so nothing is lost + sf["payee"] = data[:payee] if data.key?(:payee) + sf["memo"] = data[:memo] if data.key?(:memo) + sf["description"] = data[:description] if data.key?(:description) + # Include provider-supplied extra hash if present + sf["extra"] = data[:extra] if data[:extra].is_a?(Hash) + + return nil if sf.empty? + { "simplefin" => sf } + end + def import_adapter @import_adapter ||= Account::ProviderImportAdapter.new(account) end @@ -85,26 +100,37 @@ def log_invalid_currency(currency_value) Rails.logger.warn("Invalid currency code '#{currency_value}' in SimpleFIN transaction #{external_id}, falling back to account currency") end + # UI/entry date selection by account type: + # - Credit cards/loans: prefer transaction date (matches statements), then posted + # - Others: prefer posted date, then transaction date + # Epochs parsed as UTC timestamps via DateUtils def date - case data[:posted] - when String - Date.parse(data[:posted]) - when Integer, Float - # Unix timestamp - Time.at(data[:posted]).to_date - when Time, DateTime - data[:posted].to_date - when Date - data[:posted] + # Prefer transaction date for revolving debt (credit cards/loans); otherwise prefer posted date + acct_type = simplefin_account&.account_type.to_s.strip.downcase.tr(" ", "_") + if %w[credit_card credit loan mortgage].include?(acct_type) + t = transacted_date + return t if t + p = posted_date + return p if p else - Rails.logger.error("SimpleFin transaction has invalid date value: #{data[:posted].inspect}") - raise ArgumentError, "Invalid date format: #{data[:posted].inspect}" + p = posted_date + return p if p + t = transacted_date + return t if t end - rescue ArgumentError, TypeError => e - Rails.logger.error("Failed to parse SimpleFin transaction date '#{data[:posted]}': #{e.message}") - raise ArgumentError, "Unable to parse transaction date: #{data[:posted].inspect}" + Rails.logger.error("SimpleFin transaction missing posted/transacted date: #{data.inspect}") + raise ArgumentError, "Invalid date format: #{data[:posted].inspect} / #{data[:transacted_at].inspect}" + end + + def posted_date + val = data[:posted] + Simplefin::DateUtils.parse_provider_date(val) end + def transacted_date + val = data[:transacted_at] + Simplefin::DateUtils.parse_provider_date(val) + end def merchant # Use SimpleFin's clean payee data for merchant detection @@ -125,4 +151,18 @@ def generate_merchant_id(merchant_name) # Generate a consistent ID for merchants without explicit IDs "simplefin_#{Digest::MD5.hexdigest(merchant_name.downcase)}" end + + def notes + # Prefer memo if present; include payee when it differs from description for richer context + memo = data[:memo].to_s.strip + payee = data[:payee].to_s.strip + description = data[:description].to_s.strip + + parts = [] + parts << memo if memo.present? + if payee.present? && payee != description + parts << "Payee: #{payee}" + end + parts.presence&.join(" | ") + end end diff --git a/app/models/simplefin_item.rb b/app/models/simplefin_item.rb index 8eb44d7724e..e6044b9d878 100644 --- a/app/models/simplefin_item.rb +++ b/app/models/simplefin_item.rb @@ -1,5 +1,6 @@ class SimplefinItem < ApplicationRecord include Syncable, Provided + include SimplefinItem::Unlinking enum :status, { good: "good", requires_update: "requires_update" }, default: :good @@ -10,6 +11,15 @@ class SimplefinItem < ApplicationRecord encrypts :access_url, deterministic: true end + # Helper to detect if ActiveRecord Encryption is configured for this app + def self.encryption_ready? + creds_ready = Rails.application.credentials.active_record_encryption.present? + env_ready = ENV["ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY"].present? && + ENV["ACTIVE_RECORD_ENCRYPTION_DETERMINISTIC_KEY"].present? && + ENV["ACTIVE_RECORD_ENCRYPTION_KEY_DERIVATION_SALT"].present? + creds_ready || env_ready + end + validates :name, :access_url, presence: true before_destroy :remove_simplefin_item @@ -39,8 +49,8 @@ def destroy_later DestroyJob.perform_later(self) end - def import_latest_simplefin_data - SimplefinItem::Importer.new(self, simplefin_provider: simplefin_provider).import + def import_latest_simplefin_data(sync: nil) + SimplefinItem::Importer.new(self, simplefin_provider: simplefin_provider, sync: sync).import end def process_accounts @@ -158,6 +168,8 @@ def institution_summary end end + + # Detect a recent rate-limited sync and return a friendly message, else nil def rate_limited_message latest = latest_sync diff --git a/app/models/simplefin_item/importer.rb b/app/models/simplefin_item/importer.rb index a4b11cccc10..188bd96a023 100644 --- a/app/models/simplefin_item/importer.rb +++ b/app/models/simplefin_item/importer.rb @@ -1,10 +1,11 @@ class SimplefinItem::Importer class RateLimitedError < StandardError; end - attr_reader :simplefin_item, :simplefin_provider + attr_reader :simplefin_item, :simplefin_provider, :sync - def initialize(simplefin_item, simplefin_provider:) + def initialize(simplefin_item, simplefin_provider:, sync: nil) @simplefin_item = simplefin_item @simplefin_provider = simplefin_provider + @sync = sync end def import @@ -12,19 +13,116 @@ def import Rails.logger.info "SimplefinItem::Importer - last_synced_at: #{simplefin_item.last_synced_at.inspect}" Rails.logger.info "SimplefinItem::Importer - sync_start_date: #{simplefin_item.sync_start_date.inspect}" - if simplefin_item.last_synced_at.nil? - # First sync - use chunked approach to get full history - Rails.logger.info "SimplefinItem::Importer - Using chunked history import" - import_with_chunked_history - else - # Regular sync - use single request with buffer - Rails.logger.info "SimplefinItem::Importer - Using regular sync" - import_regular_sync + begin + if simplefin_item.last_synced_at.nil? + # First sync - use chunked approach to get full history + Rails.logger.info "SimplefinItem::Importer - Using chunked history import" + import_with_chunked_history + else + # Regular sync - use single request with buffer + Rails.logger.info "SimplefinItem::Importer - Using regular sync" + import_regular_sync + end + rescue RateLimitedError => e + stats["rate_limited"] = true + stats["rate_limited_at"] = Time.current.iso8601 + persist_stats! + raise e + end + end + + # Balances-only import: discover accounts and update account balances without transactions/holdings + def import_balances_only + Rails.logger.info "SimplefinItem::Importer - Balances-only import for item #{simplefin_item.id}" + stats["balances_only"] = true + + # Fetch accounts without date filters + accounts_data = fetch_accounts_data(start_date: nil) + return if accounts_data.nil? + + # Store snapshot for observability + simplefin_item.upsert_simplefin_snapshot!(accounts_data) + + # Update counts (set to discovered for this run rather than accumulating) + discovered = accounts_data[:accounts]&.size.to_i + stats["total_accounts"] = discovered + persist_stats! + + # Upsert SimpleFin accounts minimal attributes and update linked Account balances + accounts_data[:accounts].to_a.each do |account_data| + begin + import_account_minimal_and_balance(account_data) + rescue => e + stats["accounts_skipped"] = stats.fetch("accounts_skipped", 0) + 1 + stats["errors"] ||= [] + stats["total_errors"] = stats.fetch("total_errors", 0) + 1 + cat = classify_error(e) + buckets = stats["error_buckets"] ||= { "auth" => 0, "api" => 0, "network" => 0, "other" => 0 } + buckets[cat] = buckets.fetch(cat, 0) + 1 + stats["errors"] << { account_id: account_data[:id], name: account_data[:name], message: e.message.to_s, category: cat } + stats["errors"] = stats["errors"].last(5) + ensure + persist_stats! + end end end private + # Minimal upsert and balance update for balances-only mode + def import_account_minimal_and_balance(account_data) + account_id = account_data[:id].to_s + return if account_id.blank? + + sfa = simplefin_item.simplefin_accounts.find_or_initialize_by(account_id: account_id) + sfa.assign_attributes( + name: account_data[:name], + account_type: (account_data["type"].presence || account_data[:type].presence || sfa.account_type.presence || "unknown"), + currency: (account_data[:currency].presence || account_data["currency"].presence || sfa.currency.presence || sfa.current_account&.currency.presence || simplefin_item.family&.currency.presence || "USD"), + current_balance: account_data[:balance], + available_balance: account_data[:"available-balance"], + balance_date: (account_data["balance-date"] || account_data[:"balance-date"]), + raw_payload: account_data, + org_data: account_data[:org] + ) + begin + sfa.save! + rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => e + # Surface a friendly duplicate/validation signal in sync stats and continue + stats["accounts_skipped"] = stats.fetch("accounts_skipped", 0) + 1 + stats["errors"] ||= [] + stats["total_errors"] = stats.fetch("total_errors", 0) + 1 + cat = "other" + msg = e.message.to_s + if msg.downcase.include?("already been taken") || msg.downcase.include?("unique") + msg = "Duplicate upstream account detected for SimpleFin (account_id=#{account_id}). Try relinking to an existing manual account." + end + stats["errors"] << { account_id: account_id, name: account_data[:name], message: msg, category: cat } + stats["errors"] = stats["errors"].last(5) + persist_stats! + return + end + # In pre-prompt balances-only discovery, do NOT auto-create provider-linked accounts. + # Only update balance for already-linked accounts (if any), to avoid creating duplicates in setup. + if (acct = sfa.current_account) + adapter = Account::ProviderImportAdapter.new(acct) + adapter.update_balance( + balance: account_data[:balance], + cash_balance: account_data[:"available-balance"], + source: "simplefin" + ) + end + end + def stats + @stats ||= {} + end + + def persist_stats! + return unless sync && sync.respond_to?(:sync_stats) + merged = (sync.sync_stats || {}).merge(stats) + sync.update_columns(sync_stats: merged) # avoid callbacks/validations during tight loops + end + def import_with_chunked_history # SimpleFin's actual limit is 60 days (not 365 as documented) # Use 60-day chunks to stay within limits @@ -85,11 +183,42 @@ def import_with_chunked_history simplefin_item.upsert_simplefin_snapshot!(accounts_data) end - # Import accounts and transactions for this chunk + # Tally accounts returned for stats + chunk_accounts = accounts_data[:accounts]&.size.to_i + total_accounts_imported += chunk_accounts + # Treat total as max unique accounts seen this run, not per-chunk accumulation + stats["total_accounts"] = [ stats["total_accounts"].to_i, chunk_accounts ].max + + # Import accounts and transactions for this chunk with per-account error skipping accounts_data[:accounts]&.each do |account_data| - import_account(account_data) + begin + import_account(account_data) + rescue => e + stats["accounts_skipped"] = stats.fetch("accounts_skipped", 0) + 1 + # Collect lightweight error info for UI stats + stats["errors"] ||= [] + stats["total_errors"] = stats.fetch("total_errors", 0) + 1 + cat = classify_error(e) + buckets = stats["error_buckets"] ||= { "auth" => 0, "api" => 0, "network" => 0, "other" => 0 } + buckets[cat] = buckets.fetch(cat, 0) + 1 + begin + err_item = { + account_id: account_data[:id], + name: account_data[:name], + message: e.message.to_s, + category: cat + } + stats["errors"] << err_item + # Keep only a small sample for UI (avoid blowing up sync_stats) + stats["errors"] = stats["errors"].last(5) + rescue + # no-op if account_data is missing keys + end + Rails.logger.warn("SimpleFin: Skipping account due to error: #{e.class} - #{e.message}") + ensure + persist_stats! + end end - total_accounts_imported += accounts_data[:accounts]&.size || 0 # Stop if we've reached our target start date if chunk_start_date <= target_start_date @@ -109,15 +238,43 @@ def import_regular_sync # Step 2: Fetch transactions/holdings using the regular window. start_date = determine_sync_start_date - accounts_data = fetch_accounts_data(start_date: start_date) + accounts_data = fetch_accounts_data(start_date: start_date, pending: true) return if accounts_data.nil? # Error already handled # Store raw payload simplefin_item.upsert_simplefin_snapshot!(accounts_data) - # Import accounts (merges transactions/holdings into existing rows) + # Tally accounts for stats + count = accounts_data[:accounts]&.size.to_i + # Treat total as max unique accounts seen this run, not accumulation + stats["total_accounts"] = [ stats["total_accounts"].to_i, count ].max + + # Import accounts (merges transactions/holdings into existing rows), skipping failures per-account accounts_data[:accounts]&.each do |account_data| - import_account(account_data) + begin + import_account(account_data) + rescue => e + stats["accounts_skipped"] = stats.fetch("accounts_skipped", 0) + 1 + stats["errors"] ||= [] + stats["total_errors"] = stats.fetch("total_errors", 0) + 1 + cat = classify_error(e) + buckets = stats["error_buckets"] ||= { "auth" => 0, "api" => 0, "network" => 0, "other" => 0 } + buckets[cat] = buckets.fetch(cat, 0) + 1 + begin + stats["errors"] << { + account_id: account_data[:id], + name: account_data[:name], + message: e.message.to_s, + category: cat + } + stats["errors"] = stats["errors"].last(5) + rescue + # no-op if account_data is missing keys + end + Rails.logger.warn("SimpleFin: Skipping account during regular sync due to error: #{e.class} - #{e.message}") + ensure + persist_stats! + end end end @@ -144,7 +301,34 @@ def perform_account_discovery if discovery_data && discovered_count > 0 simplefin_item.upsert_simplefin_snapshot!(discovery_data) - discovery_data[:accounts]&.each { |account_data| import_account(account_data) } + # Treat total as max unique accounts seen this run, not accumulation + stats["total_accounts"] = [ stats["total_accounts"].to_i, discovered_count ].max + discovery_data[:accounts]&.each do |account_data| + begin + import_account(account_data) + rescue => e + stats["accounts_skipped"] = stats.fetch("accounts_skipped", 0) + 1 + stats["errors"] ||= [] + stats["total_errors"] = stats.fetch("total_errors", 0) + 1 + cat = classify_error(e) + buckets = stats["error_buckets"] ||= { "auth" => 0, "api" => 0, "network" => 0, "other" => 0 } + buckets[cat] = buckets.fetch(cat, 0) + 1 + begin + stats["errors"] << { + account_id: account_data[:id], + name: account_data[:name], + message: e.message.to_s, + category: cat + } + stats["errors"] = stats["errors"].last(5) + rescue + # no-op if account_data is missing keys + end + Rails.logger.warn("SimpleFin discovery: Skipping account due to error: #{e.class} - #{e.message}") + ensure + persist_stats! + end + end end end @@ -169,12 +353,18 @@ def fetch_accounts_data(start_date:, end_date: nil, pending: nil) Rails.logger.info "SimplefinItem::Importer - API Request: #{start_str} to #{end_str} (#{days_requested} days)" begin + # Track API request count for quota awareness + stats["api_requests"] = stats.fetch("api_requests", 0) + 1 accounts_data = simplefin_provider.get_accounts( simplefin_item.access_url, start_date: start_date, end_date: end_date, pending: pending ) + # Soft warning when approaching SimpleFin daily refresh guidance + if stats["api_requests"].to_i >= 20 + stats["rate_limit_warning"] = true + end rescue Provider::Simplefin::SimplefinError => e # Handle authentication errors by marking item as requiring update if e.error_type == :access_forbidden @@ -213,7 +403,7 @@ def determine_sync_start_date end def import_account(account_data) - account_id = account_data[:id] + account_id = account_data[:id].to_s # Validate required account_id to prevent duplicate creation return if account_id.blank? @@ -229,11 +419,11 @@ def import_account(account_data) # Update all attributes; only update transactions if present to avoid wiping prior data attrs = { name: account_data[:name], - account_type: account_data["type"] || account_data[:type] || "unknown", - currency: account_data[:currency] || "USD", + account_type: (account_data["type"].presence || account_data[:type].presence || "unknown"), + currency: (account_data[:currency].presence || account_data["currency"].presence || simplefin_account.currency.presence || simplefin_account.current_account&.currency.presence || simplefin_item.family&.currency.presence || "USD"), current_balance: account_data[:balance], available_balance: account_data[:"available-balance"], - balance_date: account_data[:"balance-date"], + balance_date: (account_data["balance-date"] || account_data[:"balance-date"]), raw_payload: account_data, org_data: account_data[:org] } @@ -259,7 +449,23 @@ def import_account(account_data) simplefin_account.account_id = account_id end - simplefin_account.save! + begin + simplefin_account.save! + rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => e + # Treat duplicates/validation failures as partial success: count and surface friendly error, then continue + stats["accounts_skipped"] = stats.fetch("accounts_skipped", 0) + 1 + stats["errors"] ||= [] + stats["total_errors"] = stats.fetch("total_errors", 0) + 1 + cat = "other" + msg = e.message.to_s + if msg.downcase.include?("already been taken") || msg.downcase.include?("unique") + msg = "Duplicate upstream account detected for SimpleFin (account_id=#{account_id}). Try relinking to an existing manual account." + end + stats["errors"] << { account_id: account_id, name: account_data[:name], message: msg, category: cat } + stats["errors"] = stats["errors"].last(5) + persist_stats! + nil + end end @@ -287,12 +493,31 @@ def handle_errors(errors) raise RateLimitedError, "SimpleFin rate limit: data refreshes at most once every 24 hours. Try again later." end + # Fall back to generic SimpleFin error classified as :api_error raise Provider::Simplefin::SimplefinError.new( "SimpleFin API errors: #{error_messages}", :api_error ) end + # Classify exceptions into simple buckets for UI stats + def classify_error(e) + msg = e.message.to_s.downcase + klass = e.class.name.to_s + # Avoid referencing Net::OpenTimeout/ReadTimeout constants (may not be loaded) + is_timeout = msg.include?("timeout") || msg.include?("timed out") || klass.include?("Timeout") + case + when is_timeout + "network" + when msg.include?("auth") || msg.include?("reauth") || msg.include?("forbidden") || msg.include?("unauthorized") + "auth" + when msg.include?("429") || msg.include?("too many requests") || msg.include?("rate limit") || msg.include?("5xx") || msg.include?("502") || msg.include?("503") || msg.include?("504") + "api" + else + "other" + end + end + def initial_sync_lookback_period # Default to 7 days for initial sync to avoid API limits 7 diff --git a/app/models/simplefin_item/syncer.rb b/app/models/simplefin_item/syncer.rb index da0275d9322..ae250ed0b79 100644 --- a/app/models/simplefin_item/syncer.rb +++ b/app/models/simplefin_item/syncer.rb @@ -6,37 +6,36 @@ def initialize(simplefin_item) end def perform_sync(sync) - # Phase 1: Import data from SimpleFin API + # Balances-only fast path + if sync.respond_to?(:sync_stats) && (sync.sync_stats || {})["balances_only"] + sync.update!(status_text: "Refreshing balances only...") if sync.respond_to?(:status_text) + begin + # Use the Importer to run balances-only path + SimplefinItem::Importer.new(simplefin_item, simplefin_provider: simplefin_item.simplefin_provider, sync: sync).import_balances_only + # Update last_synced_at for UI freshness if the column exists + if simplefin_item.has_attribute?(:last_synced_at) + simplefin_item.update!(last_synced_at: Time.current) + end + finalize_setup_counts(sync) + mark_completed(sync) + rescue => e + mark_failed(sync, e) + end + return + end + + # Full sync path sync.update!(status_text: "Importing accounts from SimpleFin...") if sync.respond_to?(:status_text) - simplefin_item.import_latest_simplefin_data + simplefin_item.import_latest_simplefin_data(sync: sync) - # 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 = simplefin_item.simplefin_accounts.count - linked_accounts = simplefin_item.simplefin_accounts.joins(:account) - unlinked_accounts = simplefin_item.simplefin_accounts.includes(:account).where(accounts: { id: nil }) - - # Store sync statistics for display - sync_stats = { - total_accounts: total_accounts, - linked_accounts: linked_accounts.count, - unlinked_accounts: unlinked_accounts.count - } - - # Set pending_account_setup if there are unlinked accounts - if unlinked_accounts.any? - simplefin_item.update!(pending_account_setup: true) - sync.update!(status_text: "#{unlinked_accounts.count} accounts need setup...") if sync.respond_to?(:status_text) - else - simplefin_item.update!(pending_account_setup: false) - end + finalize_setup_counts(sync) - # Phase 3: Process transactions and holdings for linked accounts only + # Process transactions/holdings only for linked accounts + linked_accounts = simplefin_item.simplefin_accounts.joins(:account) if linked_accounts.any? sync.update!(status_text: "Processing transactions and holdings...") if sync.respond_to?(:status_text) simplefin_item.process_accounts - # Phase 4: Schedule balance calculations for linked accounts sync.update!(status_text: "Calculating balances...") if sync.respond_to?(:status_text) simplefin_item.schedule_account_syncs( parent_sync: sync, @@ -45,13 +44,162 @@ def perform_sync(sync) ) end - # Store sync statistics in the sync record for status display - if sync.respond_to?(:sync_stats) - sync.update!(sync_stats: sync_stats) - end + mark_completed(sync) end + # Public: called by Sync after finalization; keep no-op def perform_post_sync # no-op end + + private + def finalize_setup_counts(sync) + sync.update!(status_text: "Checking account configuration...") if sync.respond_to?(:status_text) + total_accounts = simplefin_item.simplefin_accounts.count + linked_accounts = simplefin_item.simplefin_accounts.joins(:account) + unlinked_accounts = simplefin_item.simplefin_accounts + .left_joins(:account, :account_provider) + .where(accounts: { id: nil }, account_providers: { id: nil }) + + if unlinked_accounts.any? + simplefin_item.update!(pending_account_setup: true) + sync.update!(status_text: "#{unlinked_accounts.count} accounts need setup...") if sync.respond_to?(:status_text) + else + simplefin_item.update!(pending_account_setup: false) + end + + if sync.respond_to?(:sync_stats) + existing = (sync.sync_stats || {}) + setup_stats = { + "total_accounts" => total_accounts, + "linked_accounts" => linked_accounts.count, + "unlinked_accounts" => unlinked_accounts.count + } + sync.update!(sync_stats: existing.merge(setup_stats)) + end + end + + def mark_completed(sync) + if sync.may_start? + sync.start! + end + if sync.may_complete? + sync.complete! + else + # If aasm not used, at least set status text + sync.update!(status: :completed) if sync.status != "completed" + end + + # After completion, compute and persist compact post-run stats for the summary panel + begin + post_stats = compute_post_run_stats(sync) + if post_stats.present? + existing = (sync.sync_stats || {}) + sync.update!(sync_stats: existing.merge(post_stats)) + end + rescue => e + Rails.logger.warn("SimplefinItem::Syncer#mark_completed stats error: #{e.class} - #{e.message}") + end + + # If all recorded errors are duplicate-skips, do not surface a generic failure message + begin + stats = (sync.sync_stats || {}) + errors = Array(stats["errors"]).map { |e| (e.is_a?(Hash) ? e["message"] || e[:message] : e.to_s) } + if errors.present? && errors.all? { |m| m.to_s.downcase.include?("duplicate upstream account detected") } + sync.update_columns(error: nil) if sync.respond_to?(:error) + # Provide a gentle status hint instead + if sync.respond_to?(:status_text) + sync.update_columns(status_text: "Some accounts skipped as duplicates — try Link existing accounts to merge.") + end + end + rescue => e + Rails.logger.warn("SimplefinItem::Syncer duplicate-only error normalization failed: #{e.class} - #{e.message}") + end + + # Bump item freshness timestamp (guard column existence and skip for balances-only) + if simplefin_item.has_attribute?(:last_synced_at) && !(sync.sync_stats || {})["balances_only"].present? + simplefin_item.update!(last_synced_at: Time.current) + end + + # Broadcast UI updates so Providers/Accounts pages refresh without manual reload + begin + # Replace the SimpleFin card + card_html = ApplicationController.render( + partial: "simplefin_items/simplefin_item", + formats: [ :html ], + locals: { simplefin_item: simplefin_item } + ) + target_id = ActionView::RecordIdentifier.dom_id(simplefin_item) + Turbo::StreamsChannel.broadcast_replace_to(simplefin_item.family, target: target_id, html: card_html) + + # Also refresh the Manual Accounts group so duplicates clear without a full page reload + begin + manual_accounts = simplefin_item.family.accounts + .visible_manual + .order(:name) + if manual_accounts.any? + manual_html = ApplicationController.render( + partial: "accounts/index/manual_accounts", + formats: [ :html ], + locals: { accounts: manual_accounts } + ) + Turbo::StreamsChannel.broadcast_update_to(simplefin_item.family, target: "manual-accounts", html: manual_html) + else + manual_html = ApplicationController.render(inline: '') + Turbo::StreamsChannel.broadcast_replace_to(simplefin_item.family, target: "manual-accounts", html: manual_html) + end + rescue => inner + Rails.logger.warn("SimplefinItem::Syncer manual-accounts broadcast failed: #{inner.class} - #{inner.message}") + end + + # Intentionally do not broadcast modal reloads here to avoid unexpected auto-pop after sync. + # Modal opening is controlled explicitly via controller redirects with actionable conditions. + rescue => e + Rails.logger.warn("SimplefinItem::Syncer broadcast failed: #{e.class} - #{e.message}") + end + end + + # Computes transaction/holding counters between sync start and completion + def compute_post_run_stats(sync) + window_start = sync.created_at || 30.minutes.ago + window_end = Time.current + + account_ids = simplefin_item.simplefin_accounts.joins(:account).pluck("accounts.id") + return {} if account_ids.empty? + + tx_scope = Entry.where(account_id: account_ids, source: "simplefin", entryable_type: "Transaction") + tx_imported = tx_scope.where(created_at: window_start..window_end).count + tx_updated = tx_scope.where(updated_at: window_start..window_end).where.not(created_at: window_start..window_end).count + tx_seen = tx_imported + tx_updated + + holdings_scope = Holding.where(account_id: account_ids) + holdings_processed = holdings_scope.where(created_at: window_start..window_end).count + + { + "tx_imported" => tx_imported, + "tx_updated" => tx_updated, + "tx_seen" => tx_seen, + "holdings_processed" => holdings_processed, + "window_start" => window_start, + "window_end" => window_end + } + end + + def mark_failed(sync, error) + # If already completed, do not attempt to fail to avoid AASM InvalidTransition + if sync.respond_to?(:status) && sync.status.to_s == "completed" + Rails.logger.warn("SimplefinItem::Syncer#mark_failed called after completion: #{error.class} - #{error.message}") + return + end + if sync.may_start? + sync.start! + end + if sync.may_fail? + sync.fail! + else + # Avoid forcing failed if transitions are not allowed + sync.update!(status: :failed) if !sync.respond_to?(:aasm) || sync.status.to_s != "failed" + end + sync.update!(error: error.message) if sync.respond_to?(:error) + end end diff --git a/app/models/simplefin_item/unlinking.rb b/app/models/simplefin_item/unlinking.rb new file mode 100644 index 00000000000..a4113cb367c --- /dev/null +++ b/app/models/simplefin_item/unlinking.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module SimplefinItem::Unlinking + # Concern that encapsulates unlinking logic for a SimpleFin item. + # Mirrors the previous SimplefinItem::Unlinker service behavior. + extend ActiveSupport::Concern + + # Idempotently remove all connections between this SimpleFin item and local accounts. + # - Detaches any AccountProvider links for each SimplefinAccount + # - Nullifies legacy Account.simplefin_account_id backrefs + # - Detaches Holdings that point at the AccountProvider links + # Returns a per-SFA result payload for observability + def unlink_all!(dry_run: false) + results = [] + + simplefin_accounts.includes(:account).find_each do |sfa| + links = AccountProvider.where(provider_type: "SimplefinAccount", provider_id: sfa.id).to_a + link_ids = links.map(&:id) + result = { + sfa_id: sfa.id, + name: sfa.name, + account_id: sfa.account_id, + provider_link_ids: link_ids + } + results << result + + next if dry_run + + begin + ActiveRecord::Base.transaction do + # Detach holdings for any provider links found + if link_ids.any? + Holding.where(account_provider_id: link_ids).update_all(account_provider_id: nil) + end + + # Destroy all provider links + links.each do |ap| + ap.destroy! + end + + # Legacy FK fallback: ensure any legacy link is cleared + if sfa.account_id.present? + sfa.update!(account: nil) + end + end + rescue => e + Rails.logger.warn( + "Unlinker: failed to fully unlink SFA ##{sfa.id} (links=#{link_ids.inspect}): #{e.class} - #{e.message}" + ) + # Record error for observability; continue with other SFAs + result[:error] = e.message + end + end + + results + end +end diff --git a/app/services/simplefin_item/unlinker.rb b/app/services/simplefin_item/unlinker.rb new file mode 100644 index 00000000000..d676af99987 --- /dev/null +++ b/app/services/simplefin_item/unlinker.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# DEPRECATED: This thin wrapper remains only for backward compatibility. +# Business logic has moved into `SimplefinItem::Unlinking` (model concern). +# Prefer calling `item.unlink_all!(dry_run: ...)` directly. +class SimplefinItem::Unlinker + attr_reader :item, :dry_run + + def initialize(item, dry_run: false) + @item = item + @dry_run = dry_run + end + + def unlink_all! + item.unlink_all!(dry_run: dry_run) + end +end diff --git a/app/views/accounts/_account.html.erb b/app/views/accounts/_account.html.erb index 584b16da207..b4651c64f54 100644 --- a/app/views/accounts/_account.html.erb +++ b/app/views/accounts/_account.html.erb @@ -33,7 +33,7 @@ <%= icon("pencil-line", size: "sm") %> <% end %> - <% if !account.linked? && (account.accountable_type == "Depository" || account.accountable_type == "CreditCard") %> + <% if !account.linked? && ["Depository", "CreditCard", "Investment"].include?(account.accountable_type) %> <%= link_to select_provider_account_path(account), data: { turbo_frame: :modal }, class: "group-hover/account:flex hidden hover:opacity-80 items-center justify-center gap-1", diff --git a/app/views/accounts/index.html.erb b/app/views/accounts/index.html.erb index d9263143578..c8adc0bf010 100644 --- a/app/views/accounts/index.html.erb +++ b/app/views/accounts/index.html.erb @@ -38,7 +38,12 @@ <% end %> <% if @manual_accounts.any? %> - <%= render "accounts/index/manual_accounts", accounts: @manual_accounts %> +Configured and ready to use
Setup instructions:
+Field descriptions:
+Configured and ready to use. Visit the Accounts tab to manage and set up accounts.
+<%= simplefin_item.institution_summary %>
+ <%# Extra inline badges from latest sync stats %> + <% stats = (@simplefin_sync_stats_map || {})[simplefin_item.id] || {} %> + <% if stats.present? %> +
<% if simplefin_item.last_synced_at %>
@@ -81,6 +126,8 @@
) %>
<% end %>
+
+
<%= render DS::Menu.new do |menu| %>
<% menu.with_item(
variant: "button",
@@ -100,7 +147,85 @@
<%= render "accounts/index/account_groups", accounts: simplefin_item.accounts %>
<% end %>
- <% if simplefin_item.pending_account_setup? %>
+
+ <%# Sync summary (collapsible) %>
+ <% stats = (@simplefin_sync_stats_map || {})[simplefin_item.id] || {} %>
+ <% if stats.present? %>
+
+
+ Accounts
+ Transactions
+ Holdings
+ Health
+
<%= t(".setup_needed") %>
<%= t(".setup_description") %>
@@ -108,7 +233,8 @@ text: t(".setup_action"), icon: "settings", variant: "primary", - href: setup_accounts_simplefin_item_path(simplefin_item) + href: setup_accounts_simplefin_item_path(simplefin_item), + frame: :modal ) %>We found the following errors in the latest sync:
+No errors were recorded for the latest sync.
+Manage your SimpleFin bank account connections
-No SimpleFin connections
-Connect your bank accounts through SimpleFin to automatically sync transactions.
- <%= render DS::Link.new( - text: "Add your first connection", - variant: "primary", - href: new_simplefin_item_path - ) %> -- Get your setup token from - <%= link_to "SimpleFin Bridge", "https://bridge.simplefin.org/simplefin/create", - target: "_blank", - class: "text-link underline" %> -
+<% if @error_message.present? %> +- Note: Setup tokens can only be used once. If the connection fails, you'll need to create a new token. -
-- <%= t(".description") %> -
- -