Add partial account sync support (#653)

* Add partial account sync support

* Fix indentation

* Use historical balance as base when doing partial sync

* Simplify controller crud tests

* Fix linter errors

app/controllers/transactions_controller.rb

@@ -75,7 +75,7 @@ class TransactionsController < ApplicationController
 
     respond_to do |format|
       if @transaction.save
-        @transaction.account.sync_later
+        @transaction.account.sync_later(@transaction.date)
         format.html { redirect_to transactions_url, notice: t(".success") }
       else
         format.html { render :new, status: :unprocessable_entity }
@@ -85,8 +85,9 @@ class TransactionsController < ApplicationController
 
   def update
     respond_to do |format|
+      sync_start_date = [ @transaction.date, Date.parse(transaction_params[:date]) ].compact.min
       if @transaction.update(transaction_params)
-        @transaction.account.sync_later
+        @transaction.account.sync_later(sync_start_date)
 
         format.html { redirect_to transaction_url(@transaction), notice: t(".success") }
         format.turbo_stream do
@@ -102,8 +103,10 @@ class TransactionsController < ApplicationController
   end
 
   def destroy
+    @account = @transaction.account
+    sync_start_date = @account.transactions.where("date < ?", @transaction.date).order(date: :desc).first&.date
     @transaction.destroy!
-    @transaction.account.sync_later
+    @account.sync_later(sync_start_date)
 
     respond_to do |format|
       format.html { redirect_to transactions_url, notice: t(".success") }

app/controllers/valuations_controller.rb

@@ -1,11 +1,12 @@
 class ValuationsController < ApplicationController
+  before_action :set_valuation, only: %i[ edit update destroy ]
   def create
     @account = Current.family.accounts.find(params[:account_id])
 
     # TODO: placeholder logic until we have a better abstraction for trends
     @valuation = @account.valuations.new(valuation_params.merge(currency: Current.family.currency))
     if @valuation.save
-      @valuation.account.sync_later
+      @valuation.account.sync_later(@valuation.date)
 
       respond_to do |format|
         format.html { redirect_to account_path(@account), notice: "Valuation created" }
@@ -24,13 +25,12 @@ class ValuationsController < ApplicationController
   end
 
   def edit
-    @valuation = Valuation.find(params[:id])
   end
 
   def update
-    @valuation = Valuation.find(params[:id])
+    sync_start_date = [ @valuation.date, Date.parse(valuation_params[:date]) ].compact.min
     if @valuation.update(valuation_params)
-      @valuation.account.sync_later
+      @valuation.account.sync_later(sync_start_date)
 
       redirect_to account_path(@valuation.account), notice: "Valuation updated"
     else
@@ -42,10 +42,10 @@ class ValuationsController < ApplicationController
   end
 
   def destroy
-    @valuation = Valuation.find(params[:id])
     @account = @valuation.account
+    sync_start_date = @account.valuations.where("date < ?", @valuation.date).order(date: :desc).first&.date
     @valuation.destroy!
-    @account.sync_later
+    @account.sync_later(sync_start_date)
 
     respond_to do |format|
       format.html { redirect_to account_path(@account), notice: "Valuation deleted" }
@@ -59,6 +59,11 @@ class ValuationsController < ApplicationController
   end
 
   private
+    # Use callbacks to share common setup or constraints between actions.
+    def set_valuation
+      @valuation = Valuation.find(params[:id])
+    end
+
     def valuation_params
       params.require(:valuation).permit(:date, :value)
     end

app/jobs/account_sync_job.rb

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

app/models/account/balance/calculator.rb

@@ -25,7 +25,7 @@ class Account::Balance::Calculator
 
         prior_balance = current_balance
 
-        { date: date, balance: current_balance, currency: @account.currency, updated_at: Time.current }
+        { date:, balance: current_balance, currency: @account.currency, updated_at: Time.current }
       end
 
       @daily_balances = [
@@ -92,11 +92,15 @@ class Account::Balance::Calculator
       end
 
       def implied_start_balance
+        if @calc_start_date > @account.effective_start_date
+          return @account.balance_on(@calc_start_date)
+        end
+
         oldest_valuation_date = normalized_valuations.first&.dig("date")
         oldest_transaction_date = normalized_transactions.first&.dig("date")
         oldest_entry_date = [ oldest_valuation_date, oldest_transaction_date ].compact.min
 
-        if oldest_entry_date == oldest_valuation_date
+        if oldest_entry_date.present? && oldest_entry_date == oldest_valuation_date
           oldest_valuation = normalized_valuations.find { |v| v["date"] == oldest_valuation_date }
           oldest_valuation["value"].to_d
         else

app/models/account/syncable.rb

@@ -1,16 +1,18 @@
 module Account::Syncable
     extend ActiveSupport::Concern
 
-    def sync_later
-        AccountSyncJob.perform_later self
+    def sync_later(start_date = nil)
+        AccountSyncJob.perform_later(self, start_date)
     end
 
-    def sync
+    def sync(start_date = nil)
         update!(status: "syncing")
 
         sync_exchange_rates
 
-        calculator = Account::Balance::Calculator.new(self)
+        calc_start_date = start_date - 1.day if start_date.present? && self.balance_on(start_date - 1.day).present?
+
+        calculator = Account::Balance::Calculator.new(self, { calc_start_date: })
         calculator.calculate
         self.balances.upsert_all(calculator.daily_balances, unique_by: :index_account_balances_on_account_id_date_currency_unique)
         self.balances.where("date < ?", effective_start_date).delete_all

test/controllers/transactions_controller_test.rb

@@ -4,6 +4,7 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
     @transaction = transactions(:checking_one)
+    @account = @transaction.account
   end
 
   test "should get index" do
@@ -31,6 +32,12 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to transactions_url
   end
 
+  test "create should sync account with correct start date" do
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, @transaction.date ]) do
+      post transactions_url, params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: @transaction.date, name: @transaction.name } }
+    end
+  end
+
   test "creation preserves decimals" do
     assert_difference("Transaction.count") do
       post transactions_url, params: { transaction: {
@@ -91,6 +98,18 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to transaction_url(@transaction)
   end
 
+  test "update should sync account with correct start date" do
+    new_date = @transaction.date - 1.day
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, new_date ]) do
+      patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: new_date, name: @transaction.name } }
+    end
+
+    new_date = @transaction.reload.date + 1.day
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, @transaction.date ]) do
+      patch transaction_url(@transaction), params: { transaction: { account_id: @transaction.account_id, amount: @transaction.amount, currency: @transaction.currency, date: new_date, name: @transaction.name } }
+    end
+  end
+
   test "should destroy transaction" do
     assert_difference("Transaction.count", -1) do
       delete transaction_url(@transaction)
@@ -98,4 +117,16 @@ class TransactionsControllerTest < ActionDispatch::IntegrationTest
 
     assert_redirected_to transactions_url
   end
+
+  test "destroy should sync account with correct start date" do
+    first, second = @transaction.account.transactions.order(:date).all
+
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, first.date ]) do
+      delete transaction_url(second)
+    end
+
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, nil ]) do
+      delete transaction_url(first)
+    end
+  end
 end

test/controllers/valuations_controller_test.rb

@@ -3,7 +3,8 @@ require "test_helper"
 class ValuationsControllerTest < ActionDispatch::IntegrationTest
   setup do
     sign_in @user = users(:family_admin)
-    @account = accounts(:checking)
+    @valuation = valuations(:savings_one)
+    @account = @valuation.account
   end
 
   test "new" do
@@ -11,9 +12,55 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
   end
 
-  test "create" do
+  test "should create valuation" do
     assert_difference("Valuation.count") do
       post account_valuations_url(@account), params: { valuation: { value: 1, date: Date.current, type: "Appraisal" } }
     end
   end
+
+  test "create should sync account with correct start date" do
+    date = Date.current - 1.day
+
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, date ]) do
+      post account_valuations_url(@account), params: { valuation: { value: 2, date:, type: "Appraisal" } }
+    end
+  end
+
+  test "should update valuation" do
+    date = @valuation.date
+    patch valuation_url(@valuation), params: { valuation: { account_id: @valuation.account_id, value: 1, date:, type: "Appraisal" } }
+    assert_redirected_to account_path(@valuation.account)
+  end
+
+  test "update should sync account with correct start date" do
+    new_date = @valuation.date - 1.day
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, new_date ]) do
+      patch valuation_url(@valuation), params: { valuation: { account_id: @valuation.account_id, value: @valuation.value, date: new_date, type: "Appraisal" } }
+    end
+
+    new_date = @valuation.reload.date + 1.day
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, @valuation.date ]) do
+      patch valuation_url(@valuation), params: { valuation: { account_id: @valuation.account_id, value: @valuation.value, date: new_date, type: "Appraisal" } }
+    end
+  end
+
+  test "should destroy valuation" do
+    assert_difference("Valuation.count", -1) do
+      delete valuation_url(@valuation)
+    end
+
+    assert_redirected_to account_path(@account)
+  end
+
+  test "destroy should sync account with correct start date" do
+    first, second = @account.valuations.order(:date).all
+
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, first.date ]) do
+      delete valuation_url(second)
+    end
+
+    assert_enqueued_with(job: AccountSyncJob, args: [ @account, nil ]) do
+      delete valuation_url(first)
+    end
+  end
 end

test/models/account/balance/calculator_test.rb

@@ -2,97 +2,111 @@ require "test_helper"
 require "csv"
 
 class Account::Balance::CalculatorTest < ActiveSupport::TestCase
-    # See: https://docs.google.com/spreadsheets/d/18LN5N-VLq4b49Mq1fNwF7_eBiHSQB46qQduRtdAEN98/edit?usp=sharing
-    setup do
-        @expected_balances = CSV.read("test/fixtures/account/expected_balances.csv", headers: true).map do |row|
-            {
-                "date" => (Date.current + row["date_offset"].to_i.days).to_date,
-                "collectable" => row["collectable"],
-                "checking" => row["checking"],
-                "savings_with_valuation_overrides" => row["savings_with_valuation_overrides"],
-                "credit_card" => row["credit_card"],
-                "multi_currency" => row["multi_currency"],
+  # See: https://docs.google.com/spreadsheets/d/18LN5N-VLq4b49Mq1fNwF7_eBiHSQB46qQduRtdAEN98/edit?usp=sharing
+  setup do
+    @expected_balances = CSV.read("test/fixtures/account/expected_balances.csv", headers: true).map do |row|
+      {
+        "date" => (Date.current + row["date_offset"].to_i.days).to_date,
+        "collectable" => row["collectable"],
+        "checking" => row["checking"],
+        "savings_with_valuation_overrides" => row["savings_with_valuation_overrides"],
+        "credit_card" => row["credit_card"],
+        "multi_currency" => row["multi_currency"],
 
-                # Balances should be calculated for all currencies of an account
-                "eur_checking_eur" => row["eur_checking_eur"],
-                "eur_checking_usd" => row["eur_checking_usd"]
-            }
-        end
+        # Balances should be calculated for all currencies of an account
+        "eur_checking_eur" => row["eur_checking_eur"],
+        "eur_checking_usd" => row["eur_checking_usd"]
+      }
     end
+  end
 
-    test "syncs account with only valuations" do
-        account = accounts(:collectable)
+  test "syncs account with only valuations" do
+    account = accounts(:collectable)
 
-        calculator = Account::Balance::Calculator.new(account)
-        calculator.calculate
+    calculator = Account::Balance::Calculator.new(account)
+    calculator.calculate
 
-        expected = @expected_balances.map { |row| row["collectable"].to_d }
-        actual = calculator.daily_balances.map { |b| b[:balance] }
+    expected = @expected_balances.map { |row| row["collectable"].to_d }
+    actual = calculator.daily_balances.map { |b| b[:balance] }
 
-        assert_equal expected, actual
-    end
+    assert_equal expected, actual
+  end
 
-    test "syncs account with only transactions" do
-        account = accounts(:checking)
+  test "syncs account with only transactions" do
+    account = accounts(:checking)
 
-        calculator = Account::Balance::Calculator.new(account)
-        calculator.calculate
+    calculator = Account::Balance::Calculator.new(account)
+    calculator.calculate
 
-        expected = @expected_balances.map { |row| row["checking"].to_d }
-        actual = calculator.daily_balances.map { |b| b[:balance] }
+    expected = @expected_balances.map { |row| row["checking"].to_d }
+    actual = calculator.daily_balances.map { |b| b[:balance] }
 
-        assert_equal expected, actual
-    end
+    assert_equal expected, actual
+  end
 
-    test "syncs account with both valuations and transactions" do
-        account = accounts(:savings_with_valuation_overrides)
+  test "syncs account with both valuations and transactions" do
+    account = accounts(:savings_with_valuation_overrides)
 
-        calculator = Account::Balance::Calculator.new(account)
-        calculator.calculate
+    calculator = Account::Balance::Calculator.new(account)
+    calculator.calculate
 
-        expected = @expected_balances.map { |row| row["savings_with_valuation_overrides"].to_d }
-        actual = calculator.daily_balances.map { |b| b[:balance] }
+    expected = @expected_balances.map { |row| row["savings_with_valuation_overrides"].to_d }
+    actual = calculator.daily_balances.map { |b| b[:balance] }
 
-        assert_equal expected, actual
-    end
+    assert_equal expected, actual
+  end
 
-    test "syncs liability account" do
-        account = accounts(:credit_card)
+  test "syncs liability account" do
+    account = accounts(:credit_card)
 
-        calculator = Account::Balance::Calculator.new(account)
-        calculator.calculate
+    calculator = Account::Balance::Calculator.new(account)
+    calculator.calculate
 
-        expected = @expected_balances.map { |row| row["credit_card"].to_d }
-        actual = calculator.daily_balances.map { |b| b[:balance] }
+    expected = @expected_balances.map { |row| row["credit_card"].to_d }
+    actual = calculator.daily_balances.map { |b| b[:balance] }
 
-        assert_equal expected, actual
-    end
+    assert_equal expected, actual
+  end
 
-    test "syncs foreign currency account" do
-        account = accounts(:eur_checking)
-        calculator = Account::Balance::Calculator.new(account)
-        calculator.calculate
+  test "syncs foreign currency account" do
+    account = accounts(:eur_checking)
+    calculator = Account::Balance::Calculator.new(account)
+    calculator.calculate
 
-        # Calculator should calculate balances in both account and family currency
-        expected_eur_balances = @expected_balances.map { |row| row["eur_checking_eur"].to_d }
-        expected_usd_balances = @expected_balances.map { |row| row["eur_checking_usd"].to_d }
+    # Calculator should calculate balances in both account and family currency
+    expected_eur_balances = @expected_balances.map { |row| row["eur_checking_eur"].to_d }
+    expected_usd_balances = @expected_balances.map { |row| row["eur_checking_usd"].to_d }
 
-        actual_eur_balances = calculator.daily_balances.select { |b| b[:currency] == "EUR" }.sort_by { |b| b[:date] }.map { |b| b[:balance] }
-        actual_usd_balances = calculator.daily_balances.select { |b| b[:currency] == "USD" }.sort_by { |b| b[:date] }.map { |b| b[:balance] }
+    actual_eur_balances = calculator.daily_balances.select { |b| b[:currency] == "EUR" }.sort_by { |b| b[:date] }.map { |b| b[:balance] }
+    actual_usd_balances = calculator.daily_balances.select { |b| b[:currency] == "USD" }.sort_by { |b| b[:date] }.map { |b| b[:balance] }
 
-        assert_equal expected_eur_balances, actual_eur_balances
-        assert_equal expected_usd_balances, actual_usd_balances
-    end
+    assert_equal expected_eur_balances, actual_eur_balances
+    assert_equal expected_usd_balances, actual_usd_balances
+  end
 
-    test "syncs multi currency account" do
-        account = accounts(:multi_currency)
-        calculator = Account::Balance::Calculator.new(account)
-        calculator.calculate
+  test "syncs multi currency account" do
+    account = accounts(:multi_currency)
+    calculator = Account::Balance::Calculator.new(account)
+    calculator.calculate
 
-        expected_balances = @expected_balances.map { |row| row["multi_currency"].to_d }
+    expected_balances = @expected_balances.map { |row| row["multi_currency"].to_d }
 
-        actual_balances = calculator.daily_balances.map { |b| b[:balance] }
+    actual_balances = calculator.daily_balances.map { |b| b[:balance] }
 
-        assert_equal expected_balances, actual_balances
-    end
+    assert_equal expected_balances, actual_balances
+  end
+
+  test "syncs with overridden start date" do
+    account = accounts(:multi_currency)
+    account.sync
+    calc_start_date = 10.days.ago.to_date
+    calculator = Account::Balance::Calculator.new(account, { calc_start_date: })
+    calculator.calculate
+
+    expected_balances = @expected_balances.filter { |row| row["date"] >= calc_start_date }.map { |row| row["multi_currency"].to_d }
+
+    actual_balances = calculator.daily_balances.map { |b| b[:balance] }
+
+    assert_equal expected_balances, actual_balances
+  end
 end

test/models/account/syncable_test.rb

@@ -1,38 +1,81 @@
 require "test_helper"
 
 class Account::SyncableTest < ActiveSupport::TestCase
-    test "account has no balances until synced" do
-        account = accounts(:savings_with_valuation_overrides)
+  test "account has no balances until synced" do
+    account = accounts(:savings_with_valuation_overrides)
 
-        assert_equal 0, account.balances.count
-    end
+    assert_equal 0, account.balances.count
+  end
 
-    test "account has balances after syncing" do
-        account = accounts(:savings_with_valuation_overrides)
-        account.sync
+  test "account has balances after syncing" do
+    account = accounts(:savings_with_valuation_overrides)
+    account.sync
 
-        assert_equal 31, account.balances.count
-    end
+    assert_equal 31, account.balances.count
+  end
 
-    test "foreign currency account has balances in each currency after syncing" do
-        account = accounts(:eur_checking)
-        account.sync
+  test "partial sync with missing historical balances performs a full sync" do
+    account = accounts(:savings_with_valuation_overrides)
+    account.sync 10.days.ago.to_date
 
-        assert_equal 62, account.balances.count
-        assert_equal 31, account.balances.where(currency: "EUR").count
-        assert_equal 31, account.balances.where(currency: "USD").count
-    end
+    assert_equal 31, account.balances.count
+  end
 
-    test "stale balances are purged after syncing" do
-        account = accounts(:savings_with_valuation_overrides)
+  test "balances are updated after syncing" do
+    account = accounts(:savings_with_valuation_overrides)
+    balance_date = 10.days.ago
+    account.balances.create!(date: balance_date, balance: 1000)
+    account.sync
 
-        # 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)
+    assert_equal 19500, account.balances.find_by(date: balance_date)[:balance]
+  end
 
-        account.sync
+  test "balances before sync start date are not updated after syncing" do
+    account = accounts(:savings_with_valuation_overrides)
+    balance_date = 10.days.ago
+    account.balances.create!(date: balance_date, balance: 1000)
+    account.sync 5.days.ago.to_date
 
-        assert_equal 31, account.balances.count
-    end
+    assert_equal 1000, account.balances.find_by(date: balance_date)[:balance]
+  end
+
+  test "balances after sync start date are updated after syncing" do
+    account = accounts(:savings_with_valuation_overrides)
+    balance_date = 10.days.ago
+    account.balances.create!(date: balance_date, balance: 1000)
+    account.sync 20.days.ago.to_date
+
+    assert_equal 19500, account.balances.find_by(date: balance_date)[:balance]
+  end
+
+  test "balance on the sync date is updated after syncing" do
+    account = accounts(:savings_with_valuation_overrides)
+    balance_date = 5.days.ago
+    account.balances.create!(date: balance_date, balance: 1000)
+    account.sync balance_date.to_date
+
+    assert_equal 19700, account.balances.find_by(date: balance_date)[:balance]
+  end
+
+  test "foreign currency account has balances in each currency after syncing" do
+    account = accounts(:eur_checking)
+    account.sync
+
+    assert_equal 62, account.balances.count
+    assert_equal 31, account.balances.where(currency: "EUR").count
+    assert_equal 31, account.balances.where(currency: "USD").count
+  end
+
+  test "stale balances are purged after syncing" do
+    account = accounts(:savings_with_valuation_overrides)
+
+    # 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