Basic Account Balance Sync Algorithm (#501)

* Sketch out sync interface

* Add basic account sync algorithm

* Update logic for final balance in series

* Remove start_date concept

* Clean up tests

* Improve clarity of test

* Update app/models/account.rb

Co-authored-by: Rob Zolkos <rob@zolkos.com>
Signed-off-by: Zach Gollwitzer <zach.gollwitzer@gmail.com>

* Update app/models/transaction.rb

Co-authored-by: Rob Zolkos <rob@zolkos.com>
Signed-off-by: Zach Gollwitzer <zach.gollwitzer@gmail.com>

* Update app/models/valuation.rb

Co-authored-by: Rob Zolkos <rob@zolkos.com>
Signed-off-by: Zach Gollwitzer <zach.gollwitzer@gmail.com>

* Re-organize code, simplify job interface

* Consolidate balance calculations

* More cleanup

---------

Signed-off-by: Zach Gollwitzer <zach.gollwitzer@gmail.com>
Co-authored-by: Rob Zolkos <rob@zolkos.com>

app/jobs/account_balance_sync_job.rb

@@ -1,74 +0,0 @@
-class AccountBalanceSyncJob < ApplicationJob
-  queue_as :default
-
-  # Naive implementation of the perform method (will be refactored to handle transactions later)
-  def perform(account_id:, valuation_date:, sync_type:, sync_action:)
-    account = Account.find(account_id)
-
-    account.status = "SYNCING"
-    account.save!
-
-    case sync_type
-    when "valuation"
-      case sync_action
-      when "update"
-        handle_valuation_update(account: account, valuation_date: valuation_date)
-      when "destroy"
-        handle_valuation_destroy(account: account, valuation_date: valuation_date)
-      else
-        logger.error "Unsupported sync_action: #{sync_action} for sync_type: #{sync_type}"
-      end
-    else
-      logger.error "Unsupported sync_type: #{sync_type}"
-    end
-
-    sync_current_account_balance(account)
-
-    account.status = "OK"
-    account.save!
-  end
-
-  private
-
-    def sync_current_account_balance(account)
-      today_balance = account.balances.find_or_initialize_by(date: Date.current)
-      today_balance.update(balance: account.converted_balance)
-    end
-
-    def handle_valuation_update(account:, valuation_date:)
-      updated_valuation = account.valuations.find_by(date: valuation_date)
-
-      return unless updated_valuation
-
-      update_period_start = valuation_date
-      update_period_end = (account.valuations.where("date > ?", valuation_date).order(:date).first&.date || Date.current) - 1.day
-
-      balances_to_upsert = (update_period_start..update_period_end).map do |date|
-        { date: date, balance: updated_valuation.value, created_at: Time.current, updated_at: Time.current }
-      end
-
-      account.balances.upsert_all(balances_to_upsert, unique_by: :index_account_balances_on_account_id_and_date)
-
-      logger.info "Upserted balances for account #{account.id} from #{update_period_start} to #{update_period_end}"
-    end
-
-    def handle_valuation_destroy(account:, valuation_date:)
-      prior_valuation = account.valuations.where("date < ?", valuation_date).order(:date).last
-      period_start = prior_valuation&.date
-      period_end = (account.valuations.where("date > ?", valuation_date).order(:date).first&.date || Date.current) - 1.day
-
-      if prior_valuation
-        balances_to_upsert = (period_start..period_end).map do |date|
-          { date: date, balance: prior_valuation.value, created_at: Time.current, updated_at: Time.current }
-        end
-
-        account.balances.upsert_all(balances_to_upsert, unique_by: :index_account_balances_on_account_id_and_date)
-        logger.info "Upserted balances for account #{account.id} from #{period_start} to #{period_end}"
-      else
-        delete_count = account.balances.where(date: period_start..period_end).delete_all
-        logger.info "Deleted #{delete_count} balances for account #{account.id} from #{period_start} to #{period_end}"
-      end
-    rescue => e
-      logger.error "Sync failed after valuation destroy operation on account #{account.id} with message: #{e.message}"
-    end
-end

app/jobs/account_sync_job.rb

@@ -0,0 +1,7 @@
+class AccountSyncJob < ApplicationJob
+  queue_as :default
+
+  def perform(account)
+    account.sync
+  end
+end

app/models/account.rb

@@ -1,4 +1,6 @@
 class Account < ApplicationRecord
+  include Syncable
+
   broadcasts_refreshes
   belongs_to :family
   has_many :balances, class_name: "AccountBalance"

app/models/account/balance_calculator.rb

@@ -0,0 +1,38 @@
+class Account::BalanceCalculator
+    def initialize(account)
+      @account = account
+    end
+
+    def daily_balances(start_date = nil)
+      calc_start_date = [ start_date, @account.effective_start_date ].compact.max
+
+      valuations = @account.valuations.where("date >= ?", calc_start_date).order(:date).select(:date, :value)
+      transactions = @account.transactions.where("date > ?", calc_start_date).order(:date).select(:date, :amount)
+      oldest_entry = [ valuations.first, transactions.first ].compact.min_by(&:date)
+
+      net_transaction_flows = transactions.sum(&:amount)
+      implied_start_balance = oldest_entry.is_a?(Valuation) ? oldest_entry.value : @account.balance + net_transaction_flows
+
+      prior_balance = implied_start_balance
+      calculated_balances = ((calc_start_date + 1.day)...Date.current).map do |date|
+        valuation = valuations.find { |v| v.date == date }
+
+        if valuation
+            current_balance = valuation.value
+        else
+            current_day_net_transaction_flows = transactions.select { |t| t.date == date }.sum(&:amount)
+            current_balance = prior_balance - current_day_net_transaction_flows
+        end
+
+        prior_balance = current_balance
+
+        { date: date, balance: current_balance, updated_at: Time.current }
+      end
+
+      [
+        { date: calc_start_date, balance: implied_start_balance, updated_at: Time.current },
+        *calculated_balances,
+        { date: Date.current, balance: @account.balance, updated_at: Time.current } # Last balance must always match "source of truth"
+      ]
+    end
+end

app/models/account/syncable.rb

@@ -0,0 +1,26 @@
+module Account::Syncable
+    extend ActiveSupport::Concern
+
+    def sync_later
+        AccountSyncJob.perform_later self
+    end
+
+    def sync
+        update!(status: "SYNCING")
+        synced_daily_balances = Account::BalanceCalculator.new(self).daily_balances
+        self.balances.upsert_all(synced_daily_balances, unique_by: :index_account_balances_on_account_id_and_date)
+        self.balances.where("date < ?", effective_start_date).delete_all
+        update!(status: "OK")
+    rescue => e
+        update!(status: "ERROR")
+        Rails.logger.error("Failed to sync account #{id}: #{e.message}")
+    end
+
+    # The earliest date we can calculate a balance for
+    def effective_start_date
+        first_valuation_date = self.valuations.order(:date).pluck(:date).first
+        first_transaction_date = self.transactions.order(:date).pluck(:date).first
+
+        [ first_valuation_date, first_transaction_date&.prev_day ].compact.min || Date.current
+    end
+end

app/models/transaction.rb

@@ -1,3 +1,11 @@
 class Transaction < ApplicationRecord
   belongs_to :account
+
+  after_commit :sync_account
+
+  private
+
+    def sync_account
+      self.account.sync_later
+    end
 end

app/models/valuation.rb

@@ -1,20 +1,14 @@
 class Valuation < ApplicationRecord
   belongs_to :account
 
-  after_commit :sync_account_balances, on: [ :create, :update ]
-  after_destroy :sync_account_balances_after_destroy
+  after_commit :sync_account
 
   def trend(previous)
     Trend.new(value, previous&.value)
   end
 
   private
-
-    def sync_account_balances_after_destroy
-      AccountBalanceSyncJob.perform_later(account_id: account_id, valuation_date: date, sync_type: "valuation", sync_action: "destroy")
-    end
-
-    def sync_account_balances
-      AccountBalanceSyncJob.perform_later(account_id: account_id, valuation_date: date, sync_type: "valuation", sync_action: "update")
+    def sync_account
+      self.account.sync_later
     end
 end

db/seeds.rb

@@ -26,92 +26,35 @@ puts "User created: #{user.email} for family: #{family.name}"
 # Create default currency
 Currency.find_or_create_by(iso_code: "USD", name: "United States Dollar")
 
-current_balance = 350000
-
-account = Account.create_or_find_by(name: "Seed Property Account", accountable: Account::Property.new, family: family, balance: current_balance, currency: "USD")
+checking_account = Account::Depository.new
+account = Account.create_or_find_by(
+            name: "Seed Checking Account",
+            accountable: checking_account,
+            family: family,
+            balance: 5000
+          )
 puts "Account created: #{account.name}"
 
-# Represent user-defined "Valuations" at various dates
 valuations = [
-  { date: Date.today - 30, value: 300000 },
-  { date: Date.today - 22, value: 300700 },
-  { date: Date.today - 17, value: 301400 },
-  { date: Date.today - 10, value: 300000 },
-  { date: Date.today - 3, value: 301900 }
+  { date: 1.year.ago.to_date, value: 4200 },
+  { date: 250.days.ago.to_date, value: 4500 },
+  { date: 200.days.ago.to_date, value: 4444.96 }
 ]
 
+account.valuations.upsert_all(valuations, unique_by: :index_valuations_on_account_id_and_date)
+
+puts "Valuations created: #{valuations.count}"
+
 transactions = [
-  { date: Date.today - 27, amount: 7.56, currency: "USD", name: "Starbucks" },
-  { date: Date.today - 18, amount: -2000, currency: "USD", name: "Paycheck" },
-  { date: Date.today - 18, amount: 18.20, currency: "USD", name: "Walgreens" },
-  { date: Date.today - 13, amount: 34.20, currency: "USD", name: "Chipotle" },
-  { date: Date.today - 9, amount: -200, currency: "USD", name: "Birthday check" },
-  { date: Date.today - 5, amount: 85.00, currency: "USD", name: "Amazon stuff" }
+  { date: Date.today - 27, amount: 7.56, name: "Starbucks" },
+  { date: Date.today - 18, amount: -500, name: "Paycheck" },
+  { date: Date.today - 18, amount: 18.20, name: "Walgreens" },
+  { date: Date.today - 13, amount: 34.20, name: "Chipotle" },
+  { date: Date.today - 9, amount: -200, name: "Birthday check" },
+  { date: Date.today - 5, amount: 85.00, name: "Amazon stuff" }
 ]
-
-# Represent system-generated "Balances" at various dates, based on valuations
-balances = [
-  { date: Date.today - 30, balance: 300000 },
-  { date: Date.today - 29, balance: 300000 },
-  { date: Date.today - 28, balance: 300000 },
-  { date: Date.today - 27, balance: 300000 },
-  { date: Date.today - 26, balance: 300000 },
-  { date: Date.today - 25, balance: 300000 },
-  { date: Date.today - 24, balance: 300000 },
-  { date: Date.today - 23, balance: 300000 },
-  { date: Date.today - 22, balance: 300700 },
-  { date: Date.today - 21, balance: 300700 },
-  { date: Date.today - 20, balance: 300700 },
-  { date: Date.today - 19, balance: 300700 },
-  { date: Date.today - 18, balance: 300700 },
-  { date: Date.today - 17, balance: 301400 },
-  { date: Date.today - 16, balance: 301400 },
-  { date: Date.today - 15, balance: 301400 },
-  { date: Date.today - 14, balance: 301400 },
-  { date: Date.today - 13, balance: 301400 },
-  { date: Date.today - 12, balance: 301400 },
-  { date: Date.today - 11, balance: 301400 },
-  { date: Date.today - 10, balance: 300000 },
-  { date: Date.today - 9, balance: 300000 },
-  { date: Date.today - 8, balance: 300000 },
-  { date: Date.today - 7, balance: 300000 },
-  { date: Date.today - 6, balance: 300000 },
-  { date: Date.today - 5, balance: 300000 },
-  { date: Date.today - 4, balance: 300000 },
-  { date: Date.today - 3, balance: 301900 },
-  { date: Date.today - 2, balance: 301900 },
-  { date: Date.today - 1, balance: 301900 },
-  { date: Date.today, balance: current_balance }
-]
-
-
-
-valuations.each do |valuation|
-  Valuation.find_or_create_by(
-    account_id: account.id,
-    date: valuation[:date]
-  ) do |valuation_record|
-    valuation_record.value = valuation[:value]
-    valuation_record.currency = "USD"
-  end
+transactions.each do |t|
+  account.transactions.find_or_create_by(t)
 end
 
-balances.each do |balance|
-  AccountBalance.find_or_create_by(
-    account_id: account.id,
-    date: balance[:date]
-  ) do |balance_record|
-    balance_record.balance = balance[:balance]
-  end
-end
-
-transactions.each do |transaction|
-  Transaction.find_or_create_by(
-    account_id: account.id,
-    date: transaction[:date],
-    amount: transaction[:amount]
-  ) do |transaction_record|
-    transaction_record.currency = transaction[:currency]
-    transaction_record.name = transaction[:name]
-  end
-end
+puts "Transactions created: #{transactions.count}"

test/fixtures/account/depositories.yml

@@ -1 +1,2 @@
 checking: {}
+savings: {}

test/fixtures/accounts.yml

@@ -5,10 +5,10 @@ generic:
   balance: 1200
 
 # Account with only valuations
-collectible:
+collectable:
   family: dylan_family
-  name: Collectible Account
-  balance: 500
+  name: Collectable Account
+  balance: 550
 
 # Account with only transactions
 checking:

test/fixtures/transactions.yml

@@ -25,7 +25,7 @@ checking_four:
 
 checking_five:
   name: Netflix
-  date: <%= 30.days.ago.to_date %>
+  date: <%= 29.days.ago.to_date %>
   amount: 15
   account: checking
 
@@ -50,6 +50,6 @@ savings_three:
 
 savings_four:
   name: Check Deposit
-  date: <%= 30.days.ago.to_date %>
+  date: <%= 29.days.ago.to_date %>
   amount: -500
   account: savings_with_valuation_overrides

test/fixtures/valuations.yml

@@ -1,18 +1,18 @@
-# For collectible account that only has valuations (no transactions)
-collectible_one:
+# For collectable account that only has valuations (no transactions)
+collectable_one:
   value: 550
   date: <%= 4.days.ago.to_date %>
-  account: collectible
+  account: collectable
 
-collectible_two:
+collectable_two:
   value: 700
   date: <%= 12.days.ago.to_date %>
-  account: collectible
+  account: collectable
 
-collectible_three:
+collectable_three:
   value: 400
   date: <%= 30.days.ago.to_date %>
-  account: collectible
+  account: collectable
 
 # For checking account that has valuations and transactions
 savings_one:
@@ -24,3 +24,8 @@ savings_two:
   value: 19500
   date: <%= 12.days.ago.to_date %>
   account: savings_with_valuation_overrides
+
+savings_three:
+  value: 21000
+  date: <%= 25.days.ago.to_date %>
+  account: savings_with_valuation_overrides

test/models/account/balance_calculator_test.rb

@@ -0,0 +1,50 @@
+require "test_helper"
+
+class Account::BalanceCalculatorTest < ActiveSupport::TestCase
+    test "syncs account with only valuations" do
+        account = accounts(:collectable)
+        account.accountable = account_other_assets(:one)
+
+        daily_balances = Account::BalanceCalculator.new(account).daily_balances
+
+        expected_balances = [
+            400, 400, 400, 400, 400, 400, 400, 400, 400, 400,
+            400, 400, 400, 400, 400, 400, 400, 400, 700, 700,
+            700, 700, 700, 700, 700, 700, 550, 550, 550, 550,
+            550
+        ].map(&:to_d)
+
+        assert_equal expected_balances, daily_balances.map { |b| b[:balance] }
+    end
+
+    test "syncs account with only transactions" do
+        account = accounts(:checking)
+        account.accountable = account_depositories(:checking)
+
+        daily_balances = Account::BalanceCalculator.new(account).daily_balances
+
+        expected_balances = [
+            4000, 3985, 3985, 3985, 3985, 3985, 3985, 3985, 5060, 5060,
+            5060, 5060, 5060, 5060, 5060, 5040, 5040, 5040, 5010, 5010,
+            5010, 5010, 5010, 5010, 5010, 5000, 5000, 5000, 5000, 5000,
+            5000
+        ].map(&:to_d)
+
+        assert_equal expected_balances, daily_balances.map { |b| b[:balance] }
+    end
+
+    test "syncs account with both valuations and transactions" do
+        account = accounts(:savings_with_valuation_overrides)
+        account.accountable = account_depositories(:savings)
+        daily_balances = Account::BalanceCalculator.new(account).daily_balances
+
+        expected_balances = [
+            21250, 21750, 21750, 21750, 21750, 21000, 21000, 21000, 21000, 21000,
+            21000, 21000, 19000, 19000, 19000, 19000, 19000, 19000, 19500, 19500,
+            19500, 19500, 19500, 19500, 19500, 19700, 19700, 20500, 20500, 20500,
+            20000
+        ].map(&:to_d)
+
+        assert_equal expected_balances, daily_balances.map { |b| b[:balance] }
+    end
+end

test/models/account/syncable_test.rb

@@ -0,0 +1,32 @@
+require "test_helper"
+
+class Account::SyncableTest < ActiveSupport::TestCase
+    test "account has no balances until synced" do
+        account = accounts(:savings_with_valuation_overrides)
+        account.accountable = account_depositories(:savings)
+
+        assert_equal 0, account.balances.count
+    end
+
+    test "account has balances after syncing" do
+        account = accounts(:savings_with_valuation_overrides)
+        account.accountable = account_depositories(:savings)
+        account.sync
+
+        assert_equal 31, account.balances.count
+    end
+
+    test "stale balances are purged after syncing" do
+        account = accounts(:savings_with_valuation_overrides)
+        account.accountable = account_depositories(:savings)
+
+        # Create old, stale balances that should be purged (since they are before account start date)
+        account.balances.create!(date: 1.year.ago, balance: 1000)
+        account.balances.create!(date: 2.years.ago, balance: 2000)
+        account.balances.create!(date: 3.years.ago, balance: 3000)
+
+        account.sync
+
+        assert_equal 31, account.balances.count
+    end
+end