diff --git a/app/jobs/simplefin_item/balances_only_job.rb b/app/jobs/simplefin_item/balances_only_job.rb index ec9f2a92f..a34602bcd 100644 --- a/app/jobs/simplefin_item/balances_only_job.rb +++ b/app/jobs/simplefin_item/balances_only_job.rb @@ -19,12 +19,9 @@ class SimplefinItem::BalancesOnlyJob < ApplicationJob 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 + # IMPORTANT: Do NOT update last_synced_at during balances-only discovery. + # Leaving last_synced_at nil ensures the next full sync uses the + # chunked-history path to fetch historical transactions. # Refresh the SimpleFin card on Providers/Accounts pages so badges and statuses update without a full reload begin diff --git a/app/models/simplefin_account/processor.rb b/app/models/simplefin_account/processor.rb index c59020f8d..4c82db4b4 100644 --- a/app/models/simplefin_account/processor.rb +++ b/app/models/simplefin_account/processor.rb @@ -41,10 +41,14 @@ class SimplefinAccount::Processor account = simplefin_account.current_account balance = simplefin_account.current_balance || simplefin_account.available_balance || 0 - # SimpleFIN balance convention matches our app convention: - # - Positive balance = debt (you owe money) - # - Negative balance = credit balance (bank owes you, e.g., overpayment) - # No sign conversion needed - pass through as-is (same as Plaid) + # Normalize balances for liabilities (SimpleFIN typically uses opposite sign) + # App convention: + # - Liabilities: positive => you owe; negative => provider owes you (overpayment/credit) + # Since providers often send the opposite sign, ALWAYS invert for liabilities so + # that both debt and overpayment cases are represented correctly. + if [ "CreditCard", "Loan" ].include?(account.accountable_type) + balance = -balance + end # Calculate cash balance correctly for investment accounts cash_balance = if account.accountable_type == "Investment" diff --git a/app/models/simplefin_item/importer.rb b/app/models/simplefin_item/importer.rb index 0e36af726..93ca14ec0 100644 --- a/app/models/simplefin_item/importer.rb +++ b/app/models/simplefin_item/importer.rb @@ -16,8 +16,14 @@ class SimplefinItem::Importer Rails.logger.info "SimplefinItem::Importer - sync_start_date: #{simplefin_item.sync_start_date.inspect}" begin - if simplefin_item.last_synced_at.nil? - # First sync - use chunked approach to get full history + # Defensive guard: If last_synced_at is set but there are linked accounts + # with no transactions captured yet (typical after a balances-only run), + # force the first full run to use chunked history to backfill. + linked_accounts = simplefin_item.simplefin_accounts.joins(:account) + no_txns_yet = linked_accounts.any? && linked_accounts.all? { |sfa| sfa.raw_transactions_payload.blank? } + + if simplefin_item.last_synced_at.nil? || no_txns_yet + # First sync (or balances-only pre-run) — use chunked approach to get full history Rails.logger.info "SimplefinItem::Importer - Using chunked history import" import_with_chunked_history else @@ -211,9 +217,15 @@ class SimplefinItem::Importer max_requests = 22 current_end_date = Time.current - # Use user-selected sync_start_date if available, otherwise use default lookback + # Decide how far back to walk: + # - If the user set a custom sync_start_date, honor it + # - Else, for first-time chunked history, walk back up to the provider-safe + # limit implied by chunking so we actually import meaningful history. + # We do NOT use the small initial lookback (7 days) here, because that + # would clip the very first chunk to ~1 week and prevent further history. user_start_date = simplefin_item.sync_start_date - default_start_date = initial_sync_lookback_period.days.ago + implied_max_lookback_days = chunk_size_days * max_requests + default_start_date = implied_max_lookback_days.days.ago target_start_date = user_start_date ? user_start_date.beginning_of_day : default_start_date # Enforce maximum 3-year lookback to respect SimpleFin's actual 60-day limit per request @@ -698,7 +710,9 @@ class SimplefinItem::Importer end def initial_sync_lookback_period - # Default to 7 days for initial sync to avoid API limits + # Default to 7 days for initial sync. Providers that support deeper + # history will supply it via chunked fetches, and users can optionally + # set a custom `sync_start_date` to go further back. 7 end diff --git a/app/models/simplefin_item/syncer.rb b/app/models/simplefin_item/syncer.rb index ae250ed0b..997d684ab 100644 --- a/app/models/simplefin_item/syncer.rb +++ b/app/models/simplefin_item/syncer.rb @@ -6,16 +6,38 @@ class SimplefinItem::Syncer end def perform_sync(sync) + # If no accounts are linked yet, run a balances-only discovery pass so the user + # can review and manually link accounts first. This mirrors the historical flow + # users expect: initial 7-day balances snapshot, then full chunked history after linking. + begin + if simplefin_item.simplefin_accounts.joins(:account).count == 0 + sync.update!(status_text: "Discovering accounts (balances only)...") if sync.respond_to?(:status_text) + # Pre-mark the sync as balances_only so downstream completion code does not + # bump last_synced_at. The importer also sets this flag, but setting it here + # guarantees the guard is present even if the importer exits early. + if sync.respond_to?(:sync_stats) + existing = (sync.sync_stats || {}) + sync.update_columns(sync_stats: existing.merge("balances_only" => true)) + end + SimplefinItem::Importer.new(simplefin_item, simplefin_provider: simplefin_item.simplefin_provider, sync: sync).import_balances_only + finalize_setup_counts(sync) + mark_completed(sync) + return + end + rescue => e + # If discovery-only path errors, fall back to regular logic below so we don't block syncs entirely + Rails.logger.warn("SimplefinItem::Syncer auto balances-only path failed: #{e.class} - #{e.message}") + end + # 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 + # IMPORTANT: Do NOT update last_synced_at during balances-only runs. + # Leaving last_synced_at nil ensures the next full sync uses the + # chunked-history path to fetch full historical transactions. finalize_setup_counts(sync) mark_completed(sync) rescue => e diff --git a/test/models/simplefin_account_processor_test.rb b/test/models/simplefin_account_processor_test.rb new file mode 100644 index 000000000..9025c959e --- /dev/null +++ b/test/models/simplefin_account_processor_test.rb @@ -0,0 +1,84 @@ +require "test_helper" + +class SimplefinAccountProcessorTest < ActiveSupport::TestCase + setup do + @family = families(:dylan_family) + @item = SimplefinItem.create!( + family: @family, + name: "SimpleFIN", + access_url: "https://example.com/token" + ) + end + + test "inverts negative balance for credit card liabilities" do + sfin_acct = SimplefinAccount.create!( + simplefin_item: @item, + name: "Chase Credit", + account_id: "cc_1", + currency: "USD", + account_type: "credit", + current_balance: BigDecimal("-123.45") + ) + + acct = accounts(:credit_card) + acct.update!(simplefin_account: sfin_acct) + + SimplefinAccount::Processor.new(sfin_acct).send(:process_account!) + + assert_equal BigDecimal("123.45"), acct.reload.balance + end + + test "does not invert balance for asset accounts (depository)" do + sfin_acct = SimplefinAccount.create!( + simplefin_item: @item, + name: "Checking", + account_id: "dep_1", + currency: "USD", + account_type: "checking", + current_balance: BigDecimal("1000.00") + ) + + acct = accounts(:depository) + acct.update!(simplefin_account: sfin_acct) + + SimplefinAccount::Processor.new(sfin_acct).send(:process_account!) + + assert_equal BigDecimal("1000.00"), acct.reload.balance + end + + test "inverts negative balance for loan liabilities" do + sfin_acct = SimplefinAccount.create!( + simplefin_item: @item, + name: "Mortgage", + account_id: "loan_1", + currency: "USD", + account_type: "mortgage", + current_balance: BigDecimal("-50000") + ) + + acct = accounts(:loan) + acct.update!(simplefin_account: sfin_acct) + + SimplefinAccount::Processor.new(sfin_acct).send(:process_account!) + + assert_equal BigDecimal("50000"), acct.reload.balance + end + + test "positive provider balance (overpayment) becomes negative for credit card liabilities" do + sfin_acct = SimplefinAccount.create!( + simplefin_item: @item, + name: "Chase Credit", + account_id: "cc_overpay", + currency: "USD", + account_type: "credit", + current_balance: BigDecimal("75.00") # provider sends positive for overpayment + ) + + acct = accounts(:credit_card) + acct.update!(simplefin_account: sfin_acct) + + SimplefinAccount::Processor.new(sfin_acct).send(:process_account!) + + assert_equal BigDecimal("-75.00"), acct.reload.balance + end +end