From 8648f11413e6cd3c50d80a6ffb8747aad61bed46 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 11 Apr 2025 12:13:46 -0400 Subject: [PATCH] Sync hierarchy updates (#2087) * Add parent sync orchestration * Pass sync object to dependents --- app/jobs/sync_job.rb | 1 - app/models/account.rb | 18 ++++---- app/models/concerns/accountable.rb | 2 +- app/models/concerns/syncable.rb | 8 ++-- app/models/family.rb | 17 ++++--- app/models/plaid_item.rb | 4 +- app/models/sync.rb | 45 +++++++++++++++++-- db/migrate/20250411140604_add_parent_syncs.rb | 5 +++ db/schema.rb | 5 ++- test/models/family_test.rb | 6 +-- test/models/sync_test.rb | 30 ++++++++++++- 11 files changed, 111 insertions(+), 30 deletions(-) create mode 100644 db/migrate/20250411140604_add_parent_syncs.rb diff --git a/app/jobs/sync_job.rb b/app/jobs/sync_job.rb index 3c7497df..0d9b848c 100644 --- a/app/jobs/sync_job.rb +++ b/app/jobs/sync_job.rb @@ -2,7 +2,6 @@ class SyncJob < ApplicationJob queue_as :high_priority def perform(sync) - sleep 1 # simulate work for faster jobs sync.perform end end diff --git a/app/models/account.rb b/app/models/account.rb index 5e1383e4..cd1bd8aa 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -70,24 +70,26 @@ class Account < ApplicationRecord DestroyJob.perform_later(self) end - def sync_data(start_date: nil) + def sync_data(sync, start_date: nil) update!(last_synced_at: Time.current) - Rails.logger.info("Auto-matching transfers") - family.auto_match_transfers! - Rails.logger.info("Processing balances (#{linked? ? 'reverse' : 'forward'})") sync_balances + end + + def post_sync(sync) + family.remove_syncing_notice! + + accountable.post_sync(sync) if enrichable? Rails.logger.info("Enriching transaction data") enrich_data end - end - def post_sync - broadcast_remove_to(family, target: "syncing-notice") - accountable.post_sync + unless sync.child? + family.auto_match_transfers! + end end def original_balance diff --git a/app/models/concerns/accountable.rb b/app/models/concerns/accountable.rb index c3e2b783..2d545ec9 100644 --- a/app/models/concerns/accountable.rb +++ b/app/models/concerns/accountable.rb @@ -45,7 +45,7 @@ module Accountable end end - def post_sync + def post_sync(sync) broadcast_replace_to( account, target: "chart_account_#{account.id}", diff --git a/app/models/concerns/syncable.rb b/app/models/concerns/syncable.rb index 042eb6b1..0ae45122 100644 --- a/app/models/concerns/syncable.rb +++ b/app/models/concerns/syncable.rb @@ -9,8 +9,8 @@ module Syncable syncs.where(status: [ :syncing, :pending ]).any? end - def sync_later(start_date: nil) - new_sync = syncs.create!(start_date: start_date) + def sync_later(start_date: nil, parent_sync: nil) + new_sync = syncs.create!(start_date: start_date, parent: parent_sync) SyncJob.perform_later(new_sync) end @@ -18,11 +18,11 @@ module Syncable syncs.create!(start_date: start_date).perform end - def sync_data(start_date: nil) + def sync_data(sync, start_date: nil) raise NotImplementedError, "Subclasses must implement the `sync_data` method" end - def post_sync + def post_sync(sync) # no-op, syncable can optionally provide implementation end diff --git a/app/models/family.rb b/app/models/family.rb index 0b4405e8..413c969b 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -43,29 +43,36 @@ class Family < ApplicationRecord @income_statement ||= IncomeStatement.new(self) end - def sync_data(start_date: nil) + def sync_data(sync, start_date: nil) update!(last_synced_at: Time.current) accounts.manual.each do |account| - account.sync_later(start_date: start_date) + account.sync_later(start_date: start_date, parent_sync: sync) end plaid_items.each do |plaid_item| - plaid_item.sync_later(start_date: start_date) + plaid_item.sync_later(start_date: start_date, parent_sync: sync) end end - def post_sync + def remove_syncing_notice! + broadcast_remove target: "syncing-notice" + end + + def post_sync(sync) + auto_match_transfers! broadcast_refresh end + # If family has any syncs pending/syncing within the last hour, we show a persistent "syncing" notice. + # Ignore syncs older than 1 hour as they are considered "stale" def syncing? Sync.where( "(syncable_type = 'Family' AND syncable_id = ?) OR (syncable_type = 'Account' AND syncable_id IN (SELECT id FROM accounts WHERE family_id = ? AND plaid_account_id IS NULL)) OR (syncable_type = 'PlaidItem' AND syncable_id IN (SELECT id FROM plaid_items WHERE family_id = ?))", id, id, id - ).where(status: [ "pending", "syncing" ]).exists? + ).where(status: [ "pending", "syncing" ], created_at: 1.hour.ago..).exists? end def eu? diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb index b990729a..93a3a13e 100644 --- a/app/models/plaid_item.rb +++ b/app/models/plaid_item.rb @@ -37,7 +37,7 @@ class PlaidItem < ApplicationRecord end end - def sync_data(start_date: nil) + def sync_data(sync, start_date: nil) update!(last_synced_at: Time.current) begin @@ -79,7 +79,7 @@ class PlaidItem < ApplicationRecord end end - def post_sync + def post_sync(sync) family.broadcast_refresh end diff --git a/app/models/sync.rb b/app/models/sync.rb index 64446cdd..cf11c313 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -1,32 +1,71 @@ class Sync < ApplicationRecord belongs_to :syncable, polymorphic: true + belongs_to :parent, class_name: "Sync", optional: true + has_many :children, class_name: "Sync", foreign_key: :parent_id, dependent: :destroy + enum :status, { pending: "pending", syncing: "syncing", completed: "completed", failed: "failed" } scope :ordered, -> { order(created_at: :desc) } + def child? + parent_id.present? + end + def perform Rails.logger.tagged("Sync", id, syncable_type, syncable_id) do start! begin - data = syncable.sync_data(start_date: start_date) + data = syncable.sync_data(self, start_date: start_date) update!(data: data) if data - complete! + + complete! unless has_pending_child_syncs? rescue StandardError => error fail! error raise error if Rails.env.development? ensure Rails.logger.info("Sync completed, starting post-sync") - syncable.post_sync + if has_parent? + notify_parent_of_completion! + else + syncable.post_sync(self) + end Rails.logger.info("Post-sync completed") end end end + def handle_child_completion_event + unless has_pending_child_syncs? + if has_failed_child_syncs? + fail!("One or more child syncs failed") + else + complete! + syncable.post_sync(self) + end + end + end + private + def has_pending_child_syncs? + children.where(status: [ :pending, :syncing ]).any? + end + + def has_failed_child_syncs? + children.where(status: :failed).any? + end + + def has_parent? + parent_id.present? + end + + def notify_parent_of_completion! + parent.handle_child_completion_event + end + def start! Rails.logger.info("Starting sync") update! status: :syncing diff --git a/db/migrate/20250411140604_add_parent_syncs.rb b/db/migrate/20250411140604_add_parent_syncs.rb new file mode 100644 index 00000000..113d82ef --- /dev/null +++ b/db/migrate/20250411140604_add_parent_syncs.rb @@ -0,0 +1,5 @@ +class AddParentSyncs < ActiveRecord::Migration[7.2] + def change + add_reference :syncs, :parent, foreign_key: { to_table: :syncs }, type: :uuid + end +end diff --git a/db/schema.rb b/db/schema.rb index 5e2a0c09..5bd3bb37 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2025_04_10_144939) do +ActiveRecord::Schema[7.2].define(version: 2025_04_11_140604) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -547,6 +547,8 @@ ActiveRecord::Schema[7.2].define(version: 2025_04_10_144939) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.text "error_backtrace", array: true + t.uuid "parent_id" + t.index ["parent_id"], name: "index_syncs_on_parent_id" t.index ["syncable_type", "syncable_id"], name: "index_syncs_on_syncable" end @@ -665,6 +667,7 @@ ActiveRecord::Schema[7.2].define(version: 2025_04_10_144939) do add_foreign_key "security_prices", "securities" add_foreign_key "sessions", "impersonation_sessions", column: "active_impersonator_session_id" add_foreign_key "sessions", "users" + add_foreign_key "syncs", "syncs", column: "parent_id" add_foreign_key "taggings", "tags" add_foreign_key "tags", "families" add_foreign_key "tool_calls", "messages" diff --git a/test/models/family_test.rb b/test/models/family_test.rb index da50fb02..1488234e 100644 --- a/test/models/family_test.rb +++ b/test/models/family_test.rb @@ -17,13 +17,13 @@ class FamilyTest < ActiveSupport::TestCase items_count = @syncable.plaid_items.count Account.any_instance.expects(:sync_later) - .with(start_date: nil) + .with(start_date: nil, parent_sync: family_sync) .times(manual_accounts_count) PlaidItem.any_instance.expects(:sync_later) - .with(start_date: nil) + .with(start_date: nil, parent_sync: family_sync) .times(items_count) - @syncable.sync_data(start_date: family_sync.start_date) + @syncable.sync_data(family_sync, start_date: family_sync.start_date) end end diff --git a/test/models/sync_test.rb b/test/models/sync_test.rb index 5fdf5898..ec4482b6 100644 --- a/test/models/sync_test.rb +++ b/test/models/sync_test.rb @@ -7,7 +7,7 @@ class SyncTest < ActiveSupport::TestCase end test "runs successful sync" do - @sync.syncable.expects(:sync_data).with(start_date: @sync.start_date).once + @sync.syncable.expects(:sync_data).with(@sync, start_date: @sync.start_date).once assert_equal "pending", @sync.status @@ -20,7 +20,7 @@ class SyncTest < ActiveSupport::TestCase end test "handles sync errors" do - @sync.syncable.expects(:sync_data).with(start_date: @sync.start_date).raises(StandardError.new("test sync error")) + @sync.syncable.expects(:sync_data).with(@sync, start_date: @sync.start_date).raises(StandardError.new("test sync error")) assert_equal "pending", @sync.status previously_ran_at = @sync.last_ran_at @@ -31,4 +31,30 @@ class SyncTest < ActiveSupport::TestCase assert_equal "failed", @sync.status assert_equal "test sync error", @sync.error end + + 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) + + parent.syncable.expects(:sync_data).returns([]).once + child1.syncable.expects(:sync_data).returns([]).once + child2.syncable.expects(:sync_data).returns([]).once + + parent.perform # no-op + + assert_equal "syncing", parent.status + assert_equal "pending", child1.status + assert_equal "pending", child2.status + + child1.perform + assert_equal "completed", child1.status + assert_equal "syncing", parent.status + + child2.perform + assert_equal "completed", child2.status + assert_equal "completed", parent.status + end end