Improve account sync performance, handle concurrent market data syncing (#2236)

* PlaidConnectable concern

* Remove bad abstraction

* Put sync implementations in own concerns

* Sync strategies

* Move sync orchestration to Sync class

* Clean up sync class, add state machine

* Basic market data sync cron

* Fix price sync

* Improve sync window column names, add timestamps

* 30 day syncs by default

* Clean up market data methods

* Report high duplicate sync counts to Sentry

* Add sync states throughout app

* account tab session

* Persistent account tab selections

* Remove manual sleep

* Add migration to clear stale syncs on self hosted apps

* Tweak sync states

* Sync completion event broadcasts

* Fix timezones in tests

* Cleanup

* More cleanup

* Plaid item UI broadcasts for sync

* Fix account ID namespace conflict

* Sync broadcasters

* Smoother account sync refreshes

* Remove test sync delay

test/controllers/accounts_controller_test.rb

@@ -15,9 +15,4 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest
     post sync_account_path(@account)
     assert_redirected_to account_path(@account)
   end
-
-  test "can sync all accounts" do
-    post sync_all_accounts_path
-    assert_redirected_to accounts_path
-  end
 end

test/controllers/current_sessions_controller_test.rb

@@ -0,0 +1,15 @@
+require "test_helper"
+
+class CurrentSessionsControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    @user = users(:family_admin)
+    sign_in @user
+  end
+
+  test "can update the preferred tab for any namespace" do
+    put current_session_url, params: { current_session: { tab_key: "accounts_sidebar_tab", tab_value: "asset" } }
+    assert_response :success
+    session = Session.order(updated_at: :desc).first
+    assert_equal "asset", session.get_preferred_tab("accounts_sidebar_tab")
+  end
+end

test/controllers/plaid_items_controller_test.rb

@@ -8,7 +8,7 @@ class PlaidItemsControllerTest < ActionDispatch::IntegrationTest
 
   test "create" do
     @plaid_provider = mock
-    PlaidItem.expects(:plaid_provider_for_region).with("us").returns(@plaid_provider)
+    Provider::Registry.expects(:get_provider).with(:plaid_us).returns(@plaid_provider)
 
     public_token = "public-sandbox-1234"
 

test/fixtures/syncs.yml

@@ -1,17 +1,17 @@
 account:
   syncable_type: Account
   syncable: depository
-  last_ran_at: <%= Time.now %>
   status: completed
+  completed_at: <%= Time.now %>
 
 plaid_item:
   syncable_type: PlaidItem
   syncable: one
-  last_ran_at: <%= Time.now %>
   status: completed
+  completed_at: <%= Time.now %>
 
 family:
   syncable_type: Family
   syncable: dylan_family
-  last_ran_at: <%= Time.now %>
   status: completed
+  completed_at: <%= Time.now %>

test/interfaces/syncable_interface_test.rb

@@ -7,18 +7,14 @@ module SyncableInterfaceTest
   test "can sync later" do
     assert_difference "@syncable.syncs.count", 1 do
       assert_enqueued_with job: SyncJob do
-        @syncable.sync_later
+        @syncable.sync_later(window_start_date: 2.days.ago.to_date)
       end
     end
   end
 
-  test "can sync" do
-    assert_difference "@syncable.syncs.count", 1 do
-      @syncable.sync(start_date: 2.days.ago.to_date)
-    end
-  end
-
-  test "implements sync_data" do
-    assert_respond_to @syncable, :sync_data
+  test "can perform sync" do
+    mock_sync = mock
+    @syncable.class.any_instance.expects(:perform_sync).with(mock_sync).once
+    @syncable.perform_sync(mock_sync)
   end
 end

test/jobs/sync_job_test.rb

@@ -4,7 +4,7 @@ class SyncJobTest < ActiveJob::TestCase
   test "sync is performed" do
     syncable = accounts(:depository)
 
-    sync = syncable.syncs.create!(start_date: 2.days.ago.to_date)
+    sync = syncable.syncs.create!(window_start_date: 2.days.ago.to_date)
 
     sync.expects(:perform).once
 

test/models/account/entry_test.rb

@@ -30,7 +30,7 @@ class EntryTest < ActiveSupport::TestCase
     prior_date = @entry.date - 1
     @entry.update! date: prior_date
 
-    @entry.account.expects(:sync_later).with(start_date: prior_date)
+    @entry.account.expects(:sync_later).with(window_start_date: prior_date)
     @entry.sync_account_later
   end
 
@@ -38,14 +38,14 @@ class EntryTest < ActiveSupport::TestCase
     prior_date = @entry.date
     @entry.update! date: @entry.date + 1
 
-    @entry.account.expects(:sync_later).with(start_date: prior_date)
+    @entry.account.expects(:sync_later).with(window_start_date: prior_date)
     @entry.sync_account_later
   end
 
   test "triggers sync with correct start date when transaction deleted" do
     @entry.destroy!
 
-    @entry.account.expects(:sync_later).with(start_date: nil)
+    @entry.account.expects(:sync_later).with(window_start_date: nil)
     @entry.sync_account_later
   end
 

test/models/balance/forward_calculator_test.rb

@@ -15,19 +15,19 @@ class Balance::ForwardCalculatorTest < ActiveSupport::TestCase
 
   test "balance generation respects user timezone and last generated date is current user date" do
     # Simulate user in EST timezone
-    Time.zone = "America/New_York"
+    Time.use_zone("America/New_York") do
+      # Set current time to 1am UTC on Jan 5, 2025
+      # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for)
+      travel_to Time.utc(2025, 01, 05, 1, 0, 0)
 
-    # Set current time to 1am UTC on Jan 5, 2025
-    # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for)
-    travel_to Time.utc(2025, 01, 05, 1, 0, 0)
+      # Create a valuation for Jan 3, 2025
+      create_valuation(account: @account, date: "2025-01-03", amount: 17000)
 
-    # Create a valuation for Jan 3, 2025
-    create_valuation(account: @account, date: "2025-01-03", amount: 17000)
+      expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 17000 ], [ "2025-01-04", 17000 ] ]
+      calculated = Balance::ForwardCalculator.new(@account).calculate
 
-    expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 17000 ], [ "2025-01-04", 17000 ] ]
-    calculated = Balance::ForwardCalculator.new(@account).calculate
-
-    assert_equal expected, calculated.map { |b| [ b.date.to_s, b.balance ] }
+      assert_equal expected, calculated.map { |b| [ b.date.to_s, b.balance ] }
+    end
   end
 
   # When syncing forwards, we don't care about the account balance.  We generate everything based on entries, starting from 0.

test/models/balance/reverse_calculator_test.rb

@@ -25,18 +25,18 @@ class Balance::ReverseCalculatorTest < ActiveSupport::TestCase
 
   test "balance generation respects user timezone and last generated date is current user date" do
     # Simulate user in EST timezone
-    Time.zone = "America/New_York"
+    Time.use_zone("America/New_York") do
+      # Set current time to 1am UTC on Jan 5, 2025
+      # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for)
+      travel_to Time.utc(2025, 01, 05, 1, 0, 0)
 
-    # Set current time to 1am UTC on Jan 5, 2025
-    # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate balances for)
-    travel_to Time.utc(2025, 01, 05, 1, 0, 0)
+      create_valuation(account: @account, date: "2025-01-03", amount: 17000)
 
-    create_valuation(account: @account, date: "2025-01-03", amount: 17000)
+      expected = [ [ "2025-01-02", 17000 ], [ "2025-01-03", 17000 ], [ "2025-01-04", @account.balance ] ]
+      calculated = Balance::ReverseCalculator.new(@account).calculate
 
-    expected = [ [ "2025-01-02", 17000 ], [ "2025-01-03", 17000 ], [ "2025-01-04", @account.balance ] ]
-    calculated = Balance::ReverseCalculator.new(@account).calculate
-
-    assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.balance ] }
+      assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.balance ] }
+    end
   end
 
   test "valuations sync" do

test/models/family/syncer_test.rb

@@ -0,0 +1,30 @@
+require "test_helper"
+
+class Family::SyncerTest < ActiveSupport::TestCase
+  setup do
+    @family = families(:dylan_family)
+  end
+
+  test "syncs plaid items and manual accounts" do
+    family_sync = syncs(:family)
+
+    manual_accounts_count = @family.accounts.manual.count
+    items_count = @family.plaid_items.count
+
+    syncer = Family::Syncer.new(@family)
+
+    Account.any_instance
+           .expects(:sync_later)
+           .with(parent_sync: family_sync, window_start_date: nil, window_end_date: nil)
+           .times(manual_accounts_count)
+
+    PlaidItem.any_instance
+             .expects(:sync_later)
+             .with(parent_sync: family_sync, window_start_date: nil, window_end_date: nil)
+             .times(items_count)
+
+    syncer.perform_sync(family_sync)
+
+    assert_equal "completed", family_sync.reload.status
+  end
+end

test/models/family_test.rb

@@ -1,25 +1,9 @@
 require "test_helper"
-require "csv"
 
 class FamilyTest < ActiveSupport::TestCase
-  include EntriesTestHelper
   include SyncableInterfaceTest
 
   def setup
-    @family = families(:empty)
     @syncable = families(:dylan_family)
   end
-
-  test "syncs plaid items and manual accounts" do
-    family_sync = syncs(:family)
-
-    manual_accounts_count = @syncable.accounts.manual.count
-    items_count = @syncable.plaid_items.count
-
-    Account.any_instance.expects(:sync_later)
-      .with(start_date: nil, parent_sync: family_sync)
-      .times(manual_accounts_count)
-
-    @syncable.sync_data(family_sync, start_date: family_sync.start_date)
-  end
 end

test/models/holding/forward_calculator_test.rb

@@ -20,22 +20,22 @@ class Holding::ForwardCalculatorTest < ActiveSupport::TestCase
 
   test "holding generation respects user timezone and last generated date is current user date" do
     # Simulate user in EST timezone
-    Time.zone = "America/New_York"
+    Time.use_zone("America/New_York") do
+      # Set current time to 1am UTC on Jan 5, 2025
+      # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for)
+      travel_to Time.utc(2025, 01, 05, 1, 0, 0)
 
-    # Set current time to 1am UTC on Jan 5, 2025
-    # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for)
-    travel_to Time.utc(2025, 01, 05, 1, 0, 0)
+      voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF")
+      Security::Price.create!(security: voo, date: "2025-01-02", price: 500)
+      Security::Price.create!(security: voo, date: "2025-01-03", price: 500)
+      Security::Price.create!(security: voo, date: "2025-01-04", price: 500)
+      create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account)
 
-    voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF")
-    Security::Price.create!(security: voo, date: "2025-01-02", price: 500)
-    Security::Price.create!(security: voo, date: "2025-01-03", price: 500)
-    Security::Price.create!(security: voo, date: "2025-01-04", price: 500)
-    create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account)
+      expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ]
+      calculated = Holding::ForwardCalculator.new(@account).calculate
 
-    expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ]
-    calculated = Holding::ForwardCalculator.new(@account).calculate
-
-    assert_equal expected, calculated.map { |b| [ b.date.to_s, b.amount ] }
+      assert_equal expected, calculated.map { |b| [ b.date.to_s, b.amount ] }
+    end
   end
 
   test "forward portfolio calculation" do

test/models/holding/portfolio_cache_test.rb

@@ -28,37 +28,18 @@ class Holding::PortfolioCacheTest < ActiveSupport::TestCase
       price: db_price
     )
 
-    expect_provider_prices([], start_date: @account.start_date)
-
     cache = Holding::PortfolioCache.new(@account)
     assert_equal db_price, cache.get_price(@security.id, Date.current).price
   end
 
-  test "if no price in DB, try fetching from provider" do
-    Security::Price.delete_all
-
-    provider_price = Security::Price.new(
-      security: @security,
-      date: Date.current,
-      price: 220,
-      currency: "USD"
-    )
-
-    expect_provider_prices([ provider_price ], start_date: @account.start_date)
-
-    cache = Holding::PortfolioCache.new(@account)
-    assert_equal provider_price.price, cache.get_price(@security.id, Date.current).price
-  end
-
-  test "if no price from db or provider, try getting the price from trades" do
+  test "if no price from db, try getting the price from trades" do
     Security::Price.destroy_all
-    expect_provider_prices([], start_date: @account.start_date)
 
     cache = Holding::PortfolioCache.new(@account)
     assert_equal @trade.price, cache.get_price(@security.id, @trade.entry.date).price
   end
 
-  test "if no price from db, provider, or trades, search holdings" do
+  test "if no price from db or trades, search holdings" do
     Security::Price.delete_all
     Entry.delete_all
 
@@ -72,16 +53,7 @@ class Holding::PortfolioCacheTest < ActiveSupport::TestCase
       currency: "USD"
     )
 
-    expect_provider_prices([], start_date: @account.start_date)
-
     cache = Holding::PortfolioCache.new(@account, use_holdings: true)
     assert_equal holding.price, cache.get_price(@security.id, holding.date).price
   end
-
-  private
-    def expect_provider_prices(prices, start_date:, end_date: Date.current)
-      @provider.expects(:fetch_security_prices)
-               .with(@security, start_date: start_date, end_date: end_date)
-               .returns(provider_success_response(prices))
-    end
 end

test/models/holding/reverse_calculator_test.rb

@@ -20,26 +20,26 @@ class Holding::ReverseCalculatorTest < ActiveSupport::TestCase
 
   test "holding generation respects user timezone and last generated date is current user date" do
     # Simulate user in EST timezone
-    Time.zone = "America/New_York"
+    Time.use_zone("America/New_York") do
+      # Set current time to 1am UTC on Jan 5, 2025
+      # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for)
+      travel_to Time.utc(2025, 01, 05, 1, 0, 0)
 
-    # Set current time to 1am UTC on Jan 5, 2025
-    # This would be 8pm EST on Jan 4, 2025 (user's time, and the last date we should generate holdings for)
-    travel_to Time.utc(2025, 01, 05, 1, 0, 0)
+      voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF")
+      Security::Price.create!(security: voo, date: "2025-01-02", price: 500)
+      Security::Price.create!(security: voo, date: "2025-01-03", price: 500)
+      Security::Price.create!(security: voo, date: "2025-01-04", price: 500)
 
-    voo = Security.create!(ticker: "VOO", name: "Vanguard S&P 500 ETF")
-    Security::Price.create!(security: voo, date: "2025-01-02", price: 500)
-    Security::Price.create!(security: voo, date: "2025-01-03", price: 500)
-    Security::Price.create!(security: voo, date: "2025-01-04", price: 500)
+      # Today's holdings (provided)
+      @account.holdings.create!(security: voo, date: "2025-01-04", qty: 10, price: 500, amount: 5000, currency: "USD")
 
-    # Today's holdings (provided)
-    @account.holdings.create!(security: voo, date: "2025-01-04", qty: 10, price: 500, amount: 5000, currency: "USD")
+      create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account)
 
-    create_trade(voo, qty: 10, date: "2025-01-03", price: 500, account: @account)
+      expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ]
+      calculated = Holding::ReverseCalculator.new(@account).calculate
 
-    expected = [ [ "2025-01-02", 0 ], [ "2025-01-03", 5000 ], [ "2025-01-04", 5000 ] ]
-    calculated = Holding::ReverseCalculator.new(@account).calculate
-
-    assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.amount ] }
+      assert_equal expected, calculated.sort_by(&:date).map { |b| [ b.date.to_s, b.amount ] }
+    end
   end
 
   # Should be able to handle this case, although we should not be reverse-syncing an account without provided current day holdings

test/models/market_data_syncer_test.rb

@@ -0,0 +1,71 @@
+require "test_helper"
+require "ostruct"
+
+class MarketDataSyncerTest < ActiveSupport::TestCase
+  include EntriesTestHelper, ProviderTestHelper
+
+  test "syncs exchange rates with upsert" do
+    empty_db
+
+    family1 = Family.create!(name: "Family 1", currency: "USD")
+    account1 = family1.accounts.create!(name: "Account 1", currency: "USD", balance: 100, accountable: Depository.new)
+    account2 = family1.accounts.create!(name: "Account 2", currency: "CAD", balance: 100, accountable: Depository.new)
+
+    family2 = Family.create!(name: "Family 2", currency: "EUR")
+    account3 = family2.accounts.create!(name: "Account 3", currency: "EUR", balance: 100, accountable: Depository.new)
+    account4 = family2.accounts.create!(name: "Account 4", currency: "USD", balance: 100, accountable: Depository.new)
+
+    mock_provider = mock
+    Provider::Registry.any_instance.expects(:get_provider).with(:synth).returns(mock_provider).at_least_once
+
+    start_date = 1.month.ago.to_date
+    end_date = Date.current.in_time_zone("America/New_York").to_date
+
+    # Put an existing rate in DB to test upsert
+    ExchangeRate.create!(from_currency: "CAD", to_currency: "USD", date: start_date, rate: 2.0)
+
+    mock_provider.expects(:fetch_exchange_rates)
+                 .with(from: "CAD", to: "USD", start_date: start_date, end_date: end_date)
+                 .returns(provider_success_response([ OpenStruct.new(from: "CAD", to: "USD", date: start_date, rate: 1.0) ]))
+
+    mock_provider.expects(:fetch_exchange_rates)
+                 .with(from: "USD", to: "EUR", start_date: start_date, end_date: end_date)
+                 .returns(provider_success_response([ OpenStruct.new(from: "USD", to: "EUR", date: start_date, rate: 1.0) ]))
+
+    assert_difference "ExchangeRate.count", 1 do
+      MarketDataSyncer.new.sync_exchange_rates
+    end
+
+    assert_equal 1.0, ExchangeRate.where(from_currency: "CAD", to_currency: "USD", date: start_date).first.rate
+  end
+
+  test "syncs security prices with upsert" do
+    empty_db
+
+    aapl = Security.create!(ticker: "AAPL", exchange_operating_mic: "XNAS")
+
+    family = Family.create!(name: "Family 1", currency: "USD")
+    account = family.accounts.create!(name: "Account 1", currency: "USD", balance: 100, accountable: Investment.new)
+
+    mock_provider = mock
+    Provider::Registry.any_instance.expects(:get_provider).with(:synth).returns(mock_provider).at_least_once
+
+    start_date = 1.month.ago.to_date
+    end_date = Date.current.in_time_zone("America/New_York").to_date
+
+    mock_provider.expects(:fetch_security_prices)
+                 .with(aapl, start_date: start_date, end_date: end_date)
+                 .returns(provider_success_response([ OpenStruct.new(security: aapl, date: start_date, price: 100, currency: "USD") ]))
+
+    assert_difference "Security::Price.count", 1 do
+      MarketDataSyncer.new.sync_prices
+    end
+  end
+
+  private
+    def empty_db
+      Invitation.destroy_all
+      Family.destroy_all
+      Security.destroy_all
+    end
+end

test/models/security/price_test.rb

@@ -49,25 +49,6 @@ class Security::PriceTest < ActiveSupport::TestCase
     assert_not @security.find_or_fetch_price(date: Date.current)
   end
 
-  test "upserts historical prices from provider" do
-    Security::Price.delete_all
-
-    # Will be overwritten by upsert
-    Security::Price.create!(security: @security, date: 1.day.ago.to_date, price: 190, currency: "USD")
-
-    expect_provider_prices(security: @security, start_date: 2.days.ago.to_date, end_date: Date.current, prices: [
-      Security::Price.new(security: @security, date: Date.current, price: 215, currency: "USD"),
-      Security::Price.new(security: @security, date: 1.day.ago.to_date, price: 214, currency: "USD"),
-      Security::Price.new(security: @security, date: 2.days.ago.to_date, price: 213, currency: "USD")
-    ])
-
-    @security.sync_provider_prices(start_date: 2.days.ago.to_date)
-
-    assert_equal 215, @security.prices.find_by(date: Date.current).price
-    assert_equal 214, @security.prices.find_by(date: 1.day.ago.to_date).price
-    assert_equal 213, @security.prices.find_by(date: 2.days.ago.to_date).price
-  end
-
   private
     def expect_provider_price(security:, price:, date:)
       @provider.expects(:fetch_security_price)

test/models/sync_test.rb

@@ -1,34 +1,170 @@
 require "test_helper"
 
 class SyncTest < ActiveSupport::TestCase
-  setup do
-    @sync = syncs(:account)
-    @sync.update(status: "pending")
-  end
+  include ActiveJob::TestHelper
 
   test "runs successful sync" do
-    @sync.syncable.expects(:sync_data).with(@sync, start_date: @sync.start_date).once
+    syncable = accounts(:depository)
+    sync = Sync.create!(syncable: syncable)
 
-    assert_equal "pending", @sync.status
+    syncable.expects(:perform_sync).with(sync).once
 
-    previously_ran_at = @sync.last_ran_at
+    assert_equal "pending", sync.status
 
-    @sync.perform
+    sync.perform
 
-    assert @sync.last_ran_at > previously_ran_at
-    assert_equal "completed", @sync.status
+    assert sync.completed_at < Time.now
+    assert_equal "completed", sync.status
   end
 
   test "handles sync errors" do
-    @sync.syncable.expects(:sync_data).with(@sync, start_date: @sync.start_date).raises(StandardError.new("test sync error"))
+    syncable = accounts(:depository)
+    sync = Sync.create!(syncable: syncable)
 
-    assert_equal "pending", @sync.status
-    previously_ran_at = @sync.last_ran_at
+    syncable.expects(:perform_sync).with(sync).raises(StandardError.new("test sync error"))
 
-    @sync.perform
+    assert_equal "pending", sync.status
 
-    assert @sync.last_ran_at > previously_ran_at
-    assert_equal "failed", @sync.status
-    assert_equal "test sync error", @sync.error
+    sync.perform
+
+    assert sync.failed_at < Time.now
+    assert_equal "failed", sync.status
+    assert_equal "test sync error", sync.error
+  end
+
+  test "can run nested syncs that alert the parent when complete" do
+    family = families(:dylan_family)
+    plaid_item = plaid_items(:one)
+    account = accounts(:connected)
+
+    family_sync = Sync.create!(syncable: family)
+    plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync)
+    account_sync = Sync.create!(syncable: account, parent: plaid_item_sync)
+
+    assert_equal "pending", family_sync.status
+    assert_equal "pending", plaid_item_sync.status
+    assert_equal "pending", account_sync.status
+
+    family.expects(:perform_sync).with(family_sync).once
+
+    family_sync.perform
+
+    assert_equal "syncing", family_sync.reload.status
+
+    plaid_item.expects(:perform_sync).with(plaid_item_sync).once
+
+    plaid_item_sync.perform
+
+    assert_equal "syncing", family_sync.reload.status
+    assert_equal "syncing", plaid_item_sync.reload.status
+
+    account.expects(:perform_sync).with(account_sync).once
+
+    # Since these are accessed through `parent`, they won't necessarily be the same
+    # instance we configured above
+    Account.any_instance.expects(:perform_post_sync).once
+    Account.any_instance.expects(:broadcast_sync_complete).once
+    PlaidItem.any_instance.expects(:perform_post_sync).once
+    PlaidItem.any_instance.expects(:broadcast_sync_complete).once
+    Family.any_instance.expects(:perform_post_sync).once
+    Family.any_instance.expects(:broadcast_sync_complete).once
+
+    account_sync.perform
+
+    assert_equal "completed", plaid_item_sync.reload.status
+    assert_equal "completed", account_sync.reload.status
+    assert_equal "completed", family_sync.reload.status
+  end
+
+  test "failures propagate up the chain" do
+    family = families(:dylan_family)
+    plaid_item = plaid_items(:one)
+    account = accounts(:connected)
+
+    family_sync = Sync.create!(syncable: family)
+    plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync)
+    account_sync = Sync.create!(syncable: account, parent: plaid_item_sync)
+
+    assert_equal "pending", family_sync.status
+    assert_equal "pending", plaid_item_sync.status
+    assert_equal "pending", account_sync.status
+
+    family.expects(:perform_sync).with(family_sync).once
+
+    family_sync.perform
+
+    assert_equal "syncing", family_sync.reload.status
+
+    plaid_item.expects(:perform_sync).with(plaid_item_sync).once
+
+    plaid_item_sync.perform
+
+    assert_equal "syncing", family_sync.reload.status
+    assert_equal "syncing", plaid_item_sync.reload.status
+
+    # This error should "bubble up" to the PlaidItem and Family sync results
+    account.expects(:perform_sync).with(account_sync).raises(StandardError.new("test account sync error"))
+
+    # Since these are accessed through `parent`, they won't necessarily be the same
+    # instance we configured above
+    Account.any_instance.expects(:perform_post_sync).once
+    PlaidItem.any_instance.expects(:perform_post_sync).once
+    Family.any_instance.expects(:perform_post_sync).once
+
+    Account.any_instance.expects(:broadcast_sync_complete).once
+    PlaidItem.any_instance.expects(:broadcast_sync_complete).once
+    Family.any_instance.expects(:broadcast_sync_complete).once
+
+    account_sync.perform
+
+    assert_equal "failed", plaid_item_sync.reload.status
+    assert_equal "failed", account_sync.reload.status
+    assert_equal "failed", family_sync.reload.status
+  end
+
+  test "parent failure should not change status if child succeeds" do
+    family = families(:dylan_family)
+    plaid_item = plaid_items(:one)
+    account = accounts(:connected)
+
+    family_sync = Sync.create!(syncable: family)
+    plaid_item_sync = Sync.create!(syncable: plaid_item, parent: family_sync)
+    account_sync = Sync.create!(syncable: account, parent: plaid_item_sync)
+
+    assert_equal "pending", family_sync.status
+    assert_equal "pending", plaid_item_sync.status
+    assert_equal "pending", account_sync.status
+
+    family.expects(:perform_sync).with(family_sync).raises(StandardError.new("test family sync error"))
+
+    family_sync.perform
+
+    assert_equal "failed", family_sync.reload.status
+
+    plaid_item.expects(:perform_sync).with(plaid_item_sync).raises(StandardError.new("test plaid item sync error"))
+
+    plaid_item_sync.perform
+
+    assert_equal "failed", family_sync.reload.status
+    assert_equal "failed", plaid_item_sync.reload.status
+
+    # Leaf level sync succeeds, but shouldn't change the status of the already-failed parent syncs
+    account.expects(:perform_sync).with(account_sync).once
+
+    # Since these are accessed through `parent`, they won't necessarily be the same
+    # instance we configured above
+    Account.any_instance.expects(:perform_post_sync).once
+    PlaidItem.any_instance.expects(:perform_post_sync).once
+    Family.any_instance.expects(:perform_post_sync).once
+
+    Account.any_instance.expects(:broadcast_sync_complete).once
+    PlaidItem.any_instance.expects(:broadcast_sync_complete).once
+    Family.any_instance.expects(:broadcast_sync_complete).once
+
+    account_sync.perform
+
+    assert_equal "failed", plaid_item_sync.reload.status
+    assert_equal "failed", family_sync.reload.status
+    assert_equal "completed", account_sync.reload.status
   end
 end

test/test_helper.rb

@@ -17,6 +17,7 @@ require "rails/test_help"
 require "minitest/mock"
 require "minitest/autorun"
 require "mocha/minitest"
+require "aasm/minitest"
 
 VCR.configure do |config|
   config.cassette_library_dir = "test/vcr_cassettes"