From 2707a40a2a94be79dc2d0c2902f10409d49297a3 Mon Sep 17 00:00:00 2001
From: Zach Gollwitzer <zach@maybe.co>
Date: Wed, 7 May 2025 18:12:08 -0400
Subject: [PATCH] Handle nested child syncs (#2220)

---
 app/models/plaid_item.rb |  2 +-
 app/models/sync.rb       | 12 ++++++------
 test/models/sync_test.rb | 30 ++++++++++++++++++++++--------
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb
index baab5104..0d0a6b1b 100644
--- a/app/models/plaid_item.rb
+++ b/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")
diff --git a/app/models/sync.rb b/app/models/sync.rb
index afd768ae..0be26e5a 100644
--- a/app/models/sync.rb
+++ b/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?
-          Rails.logger.info("notifying parent sync id=#{parent_id} of completion")
-          notify_parent_of_completion!
-        end
+        notify_parent_of_completion! if has_parent?
       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
diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb
index ec4482b6..6b246a1f 100644
--- a/test/models/sync_test.rb
+++ b/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 "syncing", parent.status
+    assert_equal "pending", 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 "syncing", parent.status
+    assert_equal "completed", child1.reload.status
+    assert_equal "syncing", parent.reload.status
 
     child2.perform
-    assert_equal "completed", child2.status
-    assert_equal "completed", parent.status
+    assert_equal "syncing", child2.reload.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