Add back price and exchange rate syncs to account syncs

app/models/account.rb

@@ -1,5 +1,5 @@
 class Account < ApplicationRecord
-  include Syncable, Monetizable, Chartable, Linkable, Convertible, Enrichable
+  include Syncable, Monetizable, Chartable, Linkable, Enrichable
 
   validates :name, :balance, :currency, presence: true
 

app/models/account/convertible.rb

@@ -1,28 +0,0 @@
-module Account::Convertible
-  extend ActiveSupport::Concern
-
-  def sync_required_exchange_rates
-    unless requires_exchange_rates?
-      Rails.logger.info("No exchange rate sync needed for account #{id}")
-      return
-    end
-
-    affected_row_count = ExchangeRate.sync_provider_rates(
-      from: currency,
-      to: target_currency,
-      start_date: start_date,
-      end_date: Date.current
-    )
-
-    Rails.logger.info("Synced #{affected_row_count} exchange rates for account #{id}")
-  end
-
-  private
-    def target_currency
-      family.currency
-    end
-
-    def requires_exchange_rates?
-      currency != target_currency
-    end
-end

app/models/account/market_data_syncer.rb

@@ -0,0 +1,82 @@
+class Account::MarketDataSyncer
+  attr_reader :account
+
+  def initialize(account)
+    @account = account
+  end
+
+  def sync_market_data
+    sync_exchange_rates
+    sync_security_prices
+  end
+
+  private
+    def sync_exchange_rates
+      return unless needs_exchange_rates?
+      return unless ExchangeRate.provider
+
+      pair_dates = {}
+
+      # 1. ENTRY-BASED PAIRS – currencies that differ from the account currency
+      account.entries
+             .where.not(currency: account.currency)
+             .group(:currency)
+             .minimum(:date)
+             .each do |source_currency, date|
+        key = [ source_currency, account.currency ]
+        pair_dates[key] = [ pair_dates[key], date ].compact.min
+      end
+
+      # 2. ACCOUNT-BASED PAIR – convert the account currency to the family currency (if different)
+      if foreign_account?
+        key = [ account.currency, account.family.currency ]
+        pair_dates[key] = [ pair_dates[key], account.start_date ].compact.min
+      end
+
+      pair_dates.each do |(source, target), start_date|
+        ExchangeRate.sync_provider_rates(
+          from: source,
+          to: target,
+          start_date: start_date,
+          end_date: Date.current
+        )
+      end
+    end
+
+    def sync_security_prices
+      return unless Security.provider
+
+      account_securities = account.trades.map(&:security).uniq
+
+      return if account_securities.empty?
+
+      account_securities.each do |security|
+        security.sync_provider_prices(
+          start_date: first_required_price_date(security),
+          end_date: Date.current
+        )
+
+        security.sync_provider_details
+      end
+    end
+
+    # Calculates the first date we require a price for the given security scoped to this account
+    def first_required_price_date(security)
+      account.trades.with_entry
+                    .where(security: security)
+                    .where(entries: { account_id: account.id })
+                    .minimum("entries.date")
+    end
+
+    def needs_exchange_rates?
+      has_multi_currency_entries? || foreign_account?
+    end
+
+    def has_multi_currency_entries?
+      account.entries.where.not(currency: account.currency).exists?
+    end
+
+    def foreign_account?
+      account.currency != account.family.currency
+    end
+end

app/models/account/syncer.rb

@@ -7,6 +7,7 @@ class Account::Syncer
 
   def perform_sync(sync)
     Rails.logger.info("Processing balances (#{account.linked? ? 'reverse' : 'forward'})")
+    sync_market_data
     sync_balances
   end
 
@@ -19,4 +20,18 @@ class Account::Syncer
       strategy = account.linked? ? :reverse : :forward
       Balance::Syncer.new(account, strategy: strategy).sync_balances
     end
+
+    # Syncs all the exchange rates + security prices this account needs to display historical chart data
+    #
+    # This is a *supplemental* sync.  The daily market data sync should have already populated
+    # a majority or all of this data, so this is often a no-op.
+    #
+    # We rescue errors here because if this operation fails, we don't want to fail the entire sync since
+    # we have reasonable fallbacks for missing market data.
+    def sync_market_data
+      Account::MarketDataSyncer.new(account).sync_market_data
+    rescue => e
+      Rails.logger.error("Error syncing market data for account #{account.id}: #{e.message}")
+      Sentry.capture_exception(e)
+    end
 end

app/models/balance/syncer.rb

@@ -19,8 +19,6 @@ class Balance::Syncer
       if strategy == :forward
         update_account_info
       end
-
-      account.sync_required_exchange_rates
     end
   end
 

test/models/account/convertible_test.rb

@@ -1,52 +0,0 @@
-require "test_helper"
-require "ostruct"
-
-class Account::ConvertibleTest < ActiveSupport::TestCase
-  include EntriesTestHelper, ProviderTestHelper
-
-  setup do
-    @family = families(:empty)
-    @family.update!(currency: "USD")
-
-    # Foreign account (currency is not in the family's primary currency, so it will require exchange rates for net worth rollups)
-    @account = @family.accounts.create!(name: "Test Account", currency: "EUR", balance: 10000, accountable: Depository.new)
-
-    @provider = mock
-    ExchangeRate.stubs(:provider).returns(@provider)
-  end
-
-  test "syncs required exchange rates for an account" do
-    create_valuation(account: @account, date: 1.day.ago.to_date, amount: 9500, currency: "EUR")
-
-    # Since we had a valuation 1 day ago, this account starts 2 days ago and needs daily exchange rates looking forward
-    assert_equal 2.days.ago.to_date, @account.start_date
-
-    ExchangeRate.delete_all
-
-    provider_response = provider_success_response(
-      [
-        OpenStruct.new(from: "EUR", to: "USD", date: 2.days.ago.to_date, rate: 1.1),
-        OpenStruct.new(from: "EUR", to: "USD", date: 1.day.ago.to_date, rate: 1.2),
-        OpenStruct.new(from: "EUR", to: "USD", date: Date.current, rate: 1.3)
-      ]
-    )
-
-    @provider.expects(:fetch_exchange_rates)
-             .with(from: "EUR", to: "USD", start_date: 2.days.ago.to_date - 5.days, end_date: Date.current)
-             .returns(provider_response)
-
-    assert_difference "ExchangeRate.count", 3 do
-      @account.sync_required_exchange_rates
-    end
-  end
-
-  test "does not sync rates for a domestic account" do
-    @account.update!(currency: "USD")
-
-    @provider.expects(:fetch_exchange_rates).never
-
-    assert_no_difference "ExchangeRate.count" do
-      @account.sync_required_exchange_rates
-    end
-  end
-end

test/models/account/market_data_syncer_test.rb

@@ -0,0 +1,107 @@
+require "test_helper"
+require "ostruct"
+
+class Account::MarketDataSyncerTest < ActiveSupport::TestCase
+  include ProviderTestHelper
+
+  PROVIDER_BUFFER = 5.days
+
+  setup do
+    # Ensure a clean slate for deterministic assertions
+    Security::Price.delete_all
+    ExchangeRate.delete_all
+    Trade.delete_all
+    Holding.delete_all
+    Security.delete_all
+    Entry.delete_all
+
+    @provider = mock("provider")
+    Provider::Registry.any_instance
+                      .stubs(:get_provider)
+                      .with(:synth)
+                      .returns(@provider)
+  end
+
+  test "syncs required exchange rates for a foreign-currency account" do
+    family = Family.create!(name: "Smith", currency: "USD")
+
+    account = family.accounts.create!(
+      name: "Chequing",
+      currency: "CAD",
+      balance: 100,
+      accountable: Depository.new
+    )
+
+    # Seed a rate for the first required day so that the syncer only needs the next day forward
+    existing_date = account.start_date
+    ExchangeRate.create!(from_currency: "CAD", to_currency: "USD", date: existing_date, rate: 2.0)
+
+    expected_start_date = (existing_date + 1.day) - PROVIDER_BUFFER
+    end_date            = Date.current.in_time_zone("America/New_York").to_date
+
+    @provider.expects(:fetch_exchange_rates)
+             .with(from: "CAD",
+                   to: "USD",
+                   start_date: expected_start_date,
+                   end_date: end_date)
+             .returns(provider_success_response([
+               OpenStruct.new(from: "CAD", to: "USD", date: existing_date, rate: 1.5)
+             ]))
+
+    before = ExchangeRate.count
+    Account::MarketDataSyncer.new(account).sync_market_data
+    after  = ExchangeRate.count
+
+    assert_operator after, :>, before, "Should insert at least one new exchange-rate row"
+  end
+
+  test "syncs security prices for securities traded by the account" do
+    family = Family.create!(name: "Smith", currency: "USD")
+
+    account = family.accounts.create!(
+      name: "Brokerage",
+      currency: "USD",
+      balance: 0,
+      accountable: Investment.new
+    )
+
+    security = Security.create!(ticker: "AAPL", exchange_operating_mic: "XNAS")
+
+    trade_date = 10.days.ago.to_date
+    trade      = Trade.new(security: security, qty: 1, price: 100, currency: "USD")
+
+    account.entries.create!(
+      name: "Buy AAPL",
+      date: trade_date,
+      amount: 100,
+      currency: "USD",
+      entryable: trade
+    )
+
+    expected_start_date = trade_date - PROVIDER_BUFFER
+    end_date            = Date.current.in_time_zone("America/New_York").to_date
+
+    @provider.expects(:fetch_security_prices)
+             .with(symbol: security.ticker,
+                   exchange_operating_mic: security.exchange_operating_mic,
+                   start_date: expected_start_date,
+                   end_date: end_date)
+             .returns(provider_success_response([
+               OpenStruct.new(security: security,
+                              date: trade_date,
+                              price: 100,
+                              currency: "USD")
+             ]))
+
+    @provider.stubs(:fetch_security_info)
+             .with(symbol: security.ticker, exchange_operating_mic: security.exchange_operating_mic)
+             .returns(provider_success_response(OpenStruct.new(name: "Apple", logo_url: "logo")))
+
+    # Ignore exchange-rate calls for this test
+    @provider.stubs(:fetch_exchange_rates).returns(provider_success_response([]))
+
+    Account::MarketDataSyncer.new(account).sync_market_data
+
+    assert_equal 1, Security::Price.where(security: security, date: trade_date).count
+  end
+end