Handle nested child syncs (#2220)

app/models/plaid_item.rb

@@ -47,7 +47,7 @@ class PlaidItem < ApplicationRecord
 
       # Schedule account syncs
       accounts.each do |account|
-        account.sync_later(start_date: start_date)
+        account.sync_later(start_date: start_date, parent_sync: sync)
       end
 
       Rails.logger.info("Plaid data fetched and loaded")

app/models/sync.rb

@@ -22,9 +22,8 @@ class Sync < ApplicationRecord
         data = syncable.sync_data(self, start_date: start_date)
         update!(data: data) if data
 
-        complete! unless has_pending_child_syncs?
-
         unless has_pending_child_syncs?
+          complete!
           Rails.logger.info("Sync completed, starting post-sync")
           syncable.post_sync(self)
           Rails.logger.info("Post-sync completed")
@@ -33,10 +32,7 @@ class Sync < ApplicationRecord
         fail! error
         raise error if Rails.env.development?
       ensure
-        if has_parent?
+        notify_parent_of_completion! if has_parent?
-          Rails.logger.info("notifying parent sync id=#{parent_id} of completion")
-          notify_parent_of_completion!
-        end
       end
     end
   end
@@ -49,6 +45,10 @@ class Sync < ApplicationRecord
 
       unless has_pending_child_syncs?
         complete!
+
+        # 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?
+
         syncable.post_sync(self)
       end
     end

test/models/sync_test.rb

@@ -32,29 +32,43 @@ class SyncTest < ActiveSupport::TestCase
     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
     family = families(:dylan_family)
 
     parent = Sync.create!(syncable: family)
     child1 = Sync.create!(syncable: family.accounts.first, parent: parent)
-    child2 = Sync.create!(syncable: family.accounts.last, parent: parent)
+    child2 = Sync.create!(syncable: family.accounts.second, parent: parent)
+    grandchild = Sync.create!(syncable: family.accounts.last, parent: child2)
 
     parent.syncable.expects(:sync_data).returns([]).once
     child1.syncable.expects(:sync_data).returns([]).once
     child2.syncable.expects(:sync_data).returns([]).once
+    grandchild.syncable.expects(:sync_data).returns([]).once
 
-    parent.perform # no-op
+    assert_equal "pending", parent.status
-
-    assert_equal "syncing", parent.status
     assert_equal "pending", child1.status
     assert_equal "pending", child2.status
+    assert_equal "pending", grandchild.status
+
+    parent.perform
+    assert_equal "syncing", parent.reload.status
 
     child1.perform
-    assert_equal "completed", child1.status
+    assert_equal "completed", child1.reload.status
-    assert_equal "syncing", parent.status
+    assert_equal "syncing", parent.reload.status
 
     child2.perform
-    assert_equal "completed", child2.status
+    assert_equal "syncing", child2.reload.status
-    assert_equal "completed", parent.status
+    assert_equal "completed", child1.reload.status
+    assert_equal "syncing", parent.reload.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
   end
 end