diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb index c35975e9..a1576aea 100644 --- a/app/models/account/syncer.rb +++ b/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 diff --git a/app/models/family/syncer.rb b/app/models/family/syncer.rb index fe35f42f..3134a65c 100644 --- a/app/models/family/syncer.rb +++ b/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 diff --git a/app/models/plaid_item/syncer.rb b/app/models/plaid_item/syncer.rb index 4474c992..031b586c 100644 --- a/app/models/plaid_item/syncer.rb +++ b/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 diff --git a/app/models/sync.rb b/app/models/sync.rb index b335eb14..d3d9f225 100644 --- a/app/models/sync.rb +++ b/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 diff --git a/test/models/family/syncer_test.rb b/test/models/family/syncer_test.rb index 6fe17e1d..bc35e678 100644 --- a/test/models/family/syncer_test.rb +++ b/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 diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index 621cd47d..2919c2f2 100644 --- a/test/models/sync_test.rb +++ b/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