Move sync orchestration to Sync class

app/models/account/syncer.rb

@@ -5,19 +5,23 @@ class Account::Syncer
     @account = account
   end
 
+  def child_syncables
+    []
+  end
+
   def perform_sync(start_date: nil)
     Rails.logger.info("Processing balances (#{account.linked? ? 'reverse' : 'forward'})")
     sync_balances
   end
 
-  def perform_post_sync(sync)
+  def perform_post_sync
     account.family.remove_syncing_notice!
 
-    account.accountable.post_sync(sync)
+    # account.accountable.post_sync(sync)
 
-    unless sync.child?
-      account.family.auto_match_transfers!
-    end
+    # unless sync.child?
+    account.family.auto_match_transfers!
+    # end
   end
 
   private

app/models/family/syncer.rb

@@ -5,27 +5,21 @@ class Family::Syncer
     @family = family
   end
 
-  def perform_sync(sync, start_date: nil)
+  def child_syncables
+    family.plaid_items + family.accounts.manual
+  end
+
+  def perform_sync(start_date: nil)
     # We don't rely on this value to guard the app, but keep it eventually consistent
     family.sync_trial_status!
 
-    Rails.logger.info("Syncing accounts for family #{family.id}")
-    family.accounts.manual.each do |account|
-      account.sync_later(start_date: start_date, parent_sync: sync)
-    end
-
-    Rails.logger.info("Syncing plaid items for family #{family.id}")
-    family.plaid_items.each do |plaid_item|
-      plaid_item.sync_later(start_date: start_date, parent_sync: sync)
-    end
-
     Rails.logger.info("Applying rules for family #{family.id}")
     family.rules.each do |rule|
       rule.apply_later
     end
   end
 
-  def perform_post_sync(sync)
+  def perform_post_sync
     family.auto_match_transfers!
     family.broadcast_refresh
   end

app/models/plaid_item/syncer.rb

@@ -5,17 +5,16 @@ class PlaidItem::Syncer
     @plaid_item = plaid_item
   end
 
-  def perform_sync(sync, start_date: nil)
+  def child_syncables
+    plaid_item.accounts
+  end
+
+  def perform_sync(start_date: nil)
     begin
       Rails.logger.info("Fetching and loading Plaid data")
       fetch_and_load_plaid_data
       plaid_item.update!(status: :good) if plaid_item.requires_update?
 
-      # Schedule account syncs
-      plaid_item.accounts.each do |account|
-        account.sync_later(start_date: start_date, parent_sync: sync)
-      end
-
       Rails.logger.info("Plaid data fetched and loaded")
     rescue Plaid::ApiError => e
       handle_plaid_error(e)
@@ -23,7 +22,7 @@ class PlaidItem::Syncer
     end
   end
 
-  def perform_post_sync(sync)
+  def perform_post_sync
     plaid_item.auto_match_categories!
     plaid_item.family.broadcast_refresh
   end

app/models/sync.rb

@@ -19,12 +19,17 @@ class Sync < ApplicationRecord
       start!
 
       begin
-        syncer.perform_sync(self, start_date: start_date)
+        syncer.perform_sync(start_date: start_date)
+
+        # Schedule child syncables to sync later
+        syncer.child_syncables.each do |child_syncable|
+          child_syncable.sync_later(start_date: start_date, parent_sync: self)
+        end
 
         unless has_pending_child_syncs?
           complete!
           Rails.logger.info("Sync completed, starting post-sync")
-          syncer.perform_post_sync(self)
+          syncer.perform_post_sync
           Rails.logger.info("Post-sync completed")
         end
       rescue StandardError => error
@@ -51,7 +56,7 @@ class Sync < ApplicationRecord
         # If this sync is both a child and a parent, we need to notify the parent of completion
         notify_parent_of_completion! if has_parent?
 
-        syncer.perform_post_sync(self)
+        syncer.perform_post_sync
       end
     end
   end

test/models/family/syncer_test.rb

@@ -11,14 +11,9 @@ class Family::SyncerTest < ActiveSupport::TestCase
     manual_accounts_count = @family.accounts.manual.count
     items_count = @family.plaid_items.count
 
-    Account.any_instance.expects(:sync_later)
-      .with(start_date: nil, parent_sync: family_sync)
-      .times(manual_accounts_count)
+    syncer = Family::Syncer.new(@family)
+    syncer.perform_sync(start_date: family_sync.start_date)
 
-    PlaidItem.any_instance.expects(:sync_later)
-      .with(start_date: nil, parent_sync: family_sync)
-      .times(items_count)
-
-    Family::Syncer.new(@family).perform_sync(family_sync, start_date: family_sync.start_date)
+    assert_equal manual_accounts_count + items_count, syncer.child_syncables.count
   end
 end

test/models/sync_test.rb

@@ -1,74 +1,86 @@
 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
-    Account::Syncer.any_instance.expects(:perform_sync).with(@sync, start_date: @sync.start_date).once
+    sync = Sync.create!(syncable: accounts(:depository), last_ran_at: 1.day.ago)
 
-    assert_equal "pending", @sync.status
+    Account::Syncer.any_instance.expects(:perform_sync).with(start_date: sync.start_date).once
 
-    previously_ran_at = @sync.last_ran_at
+    assert_equal "pending", sync.status
 
-    @sync.perform
+    previously_ran_at = sync.last_ran_at
 
-    assert @sync.last_ran_at > previously_ran_at
-    assert_equal "completed", @sync.status
+    sync.perform
+
+    assert sync.last_ran_at > previously_ran_at
+    assert_equal "completed", sync.status
   end
 
   test "handles sync errors" do
-    Account::Syncer.any_instance.expects(:perform_sync).with(@sync, start_date: @sync.start_date).raises(StandardError.new("test sync error"))
+    sync = Sync.create!(syncable: accounts(:depository), last_ran_at: 1.day.ago)
+    Account::Syncer.any_instance.expects(:perform_sync).with(start_date: sync.start_date).raises(StandardError.new("test sync error"))
 
-    assert_equal "pending", @sync.status
-    previously_ran_at = @sync.last_ran_at
+    assert_equal "pending", sync.status
+    previously_ran_at = sync.last_ran_at
 
-    @sync.perform
+    sync.perform
 
-    assert @sync.last_ran_at > previously_ran_at
-    assert_equal "failed", @sync.status
-    assert_equal "test sync error", @sync.error
+    assert sync.last_ran_at > previously_ran_at
+    assert_equal "failed", sync.status
+    assert_equal "test sync error", sync.error
   end
 
-  # Order is important here.  Parent syncs must implement sync_data so that their own work
-  # is 100% complete *prior* to queueing up child syncs.
-  test "runs sync with child syncs" do
+  test "can run nested syncs that alert the parent when complete" do
+    # Clear out fixture syncs
+    Sync.destroy_all
+
+    # These fixtures represent a Parent -> Child -> Grandchild sync hierarchy
+    # Family -> PlaidItem -> Account
     family = families(:dylan_family)
+    plaid_item = plaid_items(:one)
+    account = accounts(:connected)
 
-    parent = Sync.create!(syncable: family)
-    child1 = Sync.create!(syncable: family.accounts.first, parent: parent)
-    child2 = Sync.create!(syncable: family.accounts.second, parent: parent)
-    grandchild = Sync.create!(syncable: family.accounts.last, parent: child2)
+    sync = Sync.create!(syncable: family)
 
-    Family::Syncer.any_instance.expects(:perform_sync).with(parent, start_date: parent.start_date).once
-    Account::Syncer.any_instance.expects(:perform_sync).with(child1, start_date: parent.start_date).once
-    Account::Syncer.any_instance.expects(:perform_sync).with(child2, start_date: parent.start_date).once
-    Account::Syncer.any_instance.expects(:perform_sync).with(grandchild, start_date: parent.start_date).once
+    Family::Syncer.any_instance.expects(:perform_sync).with(start_date: sync.start_date).once
+    Family::Syncer.any_instance.expects(:perform_post_sync).once
+    Family::Syncer.any_instance.expects(:child_syncables).returns([ plaid_item ])
 
-    assert_equal "pending", parent.status
-    assert_equal "pending", child1.status
-    assert_equal "pending", child2.status
-    assert_equal "pending", grandchild.status
+    PlaidItem::Syncer.any_instance.expects(:perform_sync).with(start_date: sync.start_date).once
+    PlaidItem::Syncer.any_instance.expects(:perform_post_sync).once
+    PlaidItem::Syncer.any_instance.expects(:child_syncables).returns([ account ])
 
-    parent.perform
-    assert_equal "syncing", parent.reload.status
+    Account::Syncer.any_instance.expects(:perform_sync).with(start_date: sync.start_date).once
+    Account::Syncer.any_instance.expects(:perform_post_sync).once
+    Account::Syncer.any_instance.expects(:child_syncables).returns([])
 
-    child1.perform
-    assert_equal "completed", child1.reload.status
-    assert_equal "syncing", parent.reload.status
+    sync.perform
 
-    child2.perform
-    assert_equal "syncing", child2.reload.status
-    assert_equal "completed", child1.reload.status
-    assert_equal "syncing", parent.reload.status
+    assert_equal 1, family.syncs.count
+    assert_equal "syncing", family.syncs.first.status
+    assert_equal 1, plaid_item.syncs.count
+    assert_equal "pending", plaid_item.syncs.first.status
 
-    # Will complete the parent and grandparent syncs
-    grandchild.perform
-    assert_equal "completed", grandchild.reload.status
-    assert_equal "completed", child1.reload.status
-    assert_equal "completed", child2.reload.status
-    assert_equal "completed", parent.reload.status
+    # We have to perform jobs 2x because the child sync will schedule the grandchild sync,
+    # which then needs to be run.
+    perform_enqueued_jobs
+
+    assert_equal 1, family.syncs.count
+    assert_equal "syncing", family.syncs.first.status
+    assert_equal 1, plaid_item.syncs.count
+    assert_equal "syncing", plaid_item.syncs.first.status
+    assert_equal 1, account.syncs.count
+    assert_equal "pending", account.syncs.first.status
+
+    perform_enqueued_jobs
+
+    assert_equal 1, family.syncs.count
+    assert_equal "completed", family.syncs.first.status
+    assert_equal 1, plaid_item.syncs.count
+    assert_equal "completed", plaid_item.syncs.first.status
+    assert_equal 1, account.syncs.count
+    assert_equal "completed", account.syncs.first.status
   end
 end