Add reconciliation manager (#2459)

zachgoll · web-flow · commit 52333e3fa69d · 2025-07-16T11:31:47.000-04:00
* Add reconciliation manager

* Fix notes editing
diff --git a/app/controllers/valuations_controller.rb b/app/controllers/valuations_controller.rb
@@ -5,6 +5,12 @@ def confirm_create
     @account = Current.family.accounts.find(params.dig(:entry, :account_id))
     @entry = @account.entries.build(entry_params.merge(currency: @account.currency))
 
+    @reconciliation_dry_run = @entry.account.create_reconciliation(
+      balance: entry_params[:amount],
+      date: entry_params[:date],
+      dry_run: true
+    )
+
     render :confirm_create
   end
 
@@ -13,19 +19,28 @@ def confirm_update
     @account = @entry.account
     @entry.assign_attributes(entry_params.merge(currency: @account.currency))
 
+    @reconciliation_dry_run = @entry.account.update_reconciliation(
+      @entry,
+      balance: entry_params[:amount],
+      date: entry_params[:date],
+      dry_run: true
+    )
+
     render :confirm_update
   end
 
   def create
     account = Current.family.accounts.find(params.dig(:entry, :account_id))
-    result = perform_balance_update(account, entry_params.merge(currency: account.currency))
 
-    if result.success?
-      @success_message = result.updated? ? "Balance updated" : "No changes made. Account is already up to date."
+    result = account.create_reconciliation(
+      balance: entry_params[:amount],
+      date: entry_params[:date],
+    )
 
+    if result.success?
       respond_to do |format|
-        format.html { redirect_back_or_to account_path(account), notice: @success_message }
-        format.turbo_stream { stream_redirect_back_or_to(account_path(account), notice: @success_message) }
+        format.html { redirect_back_or_to account_path(account), notice: "Account updated" }
+        format.turbo_stream { stream_redirect_back_or_to(account_path(account), notice: "Account updated") }
       end
     else
       @error_message = result.error_message
@@ -34,13 +49,22 @@ def create
   end
 
   def update
-    result = perform_balance_update(@entry.account, entry_params.merge(currency: @entry.currency, existing_valuation_id: @entry.id))
+    # Notes updating is independent of reconciliation, just a simple CRUD operation
+    @entry.update!(notes: entry_params[:notes]) if entry_params[:notes].present?
 
-    if result.success?
+    if entry_params[:date].present? && entry_params[:amount].present?
+      result = @entry.account.update_reconciliation(
+        @entry,
+        balance: entry_params[:amount],
+        date: entry_params[:date],
+      )
+    end
+
+    if result.nil? || result.success?
       @entry.reload
 
       respond_to do |format|
-        format.html { redirect_back_or_to account_path(@entry.account), notice: result.updated? ? "Balance updated" : "No changes made. Account is already up to date." }
+        format.html { redirect_back_or_to account_path(@entry.account), notice: "Entry updated" }
         format.turbo_stream do
           render turbo_stream: [
             turbo_stream.replace(
@@ -60,17 +84,6 @@ def update
 
   private
     def entry_params
-      params.require(:entry)
-            .permit(:date, :amount, :currency, :notes)
-    end
-
-    def perform_balance_update(account, params)
-      account.update_balance(
-        balance: params[:amount],
-        date: params[:date],
-        currency: params[:currency],
-        notes: params[:notes],
-        existing_valuation_id: params[:existing_valuation_id]
-      )
+      params.require(:entry).permit(:date, :amount, :notes)
     end
 end
diff --git a/app/models/account.rb b/app/models/account.rb
@@ -1,5 +1,5 @@
 class Account < ApplicationRecord
-  include AASM, Syncable, Monetizable, Chartable, Linkable, Enrichable, Anchorable
+  include AASM, Syncable, Monetizable, Chartable, Linkable, Enrichable, Anchorable, Reconcileable
 
   validates :name, :balance, :currency, presence: true
 
diff --git a/app/models/account/reconcileable.rb b/app/models/account/reconcileable.rb
@@ -0,0 +1,16 @@
+module Account::Reconcileable
+  extend ActiveSupport::Concern
+
+  def create_reconciliation(balance:, date:, dry_run: false)
+    reconciliation_manager.reconcile_balance(balance: balance, date: date, dry_run: dry_run)
+  end
+
+  def update_reconciliation(existing_valuation_entry, balance:, date:, dry_run: false)
+    reconciliation_manager.reconcile_balance(balance: balance, date: date, existing_valuation_entry: existing_valuation_entry, dry_run: dry_run)
+  end
+
+  private
+    def reconciliation_manager
+      @reconciliation_manager ||= Account::ReconciliationManager.new(self)
+    end
+end
diff --git a/app/models/account/reconciliation_manager.rb b/app/models/account/reconciliation_manager.rb
@@ -0,0 +1,90 @@
+class Account::ReconciliationManager
+  attr_reader :account
+
+  def initialize(account)
+    @account = account
+  end
+
+  # Reconciles balance by creating a Valuation entry. If existing valuation is provided, it will be updated instead of creating a new one.
+  def reconcile_balance(balance:, date: Date.current, dry_run: false, existing_valuation_entry: nil)
+    old_balance_components = old_balance_components(reconciliation_date: date, existing_valuation_entry: existing_valuation_entry)
+    prepared_valuation = prepare_reconciliation(balance, date, existing_valuation_entry)
+
+    unless dry_run
+      prepared_valuation.save!
+      account.sync_later
+    end
+
+    ReconciliationResult.new(
+      success?: true,
+      old_cash_balance: old_balance_components[:cash_balance],
+      old_balance: old_balance_components[:balance],
+      new_cash_balance: derived_cash_balance(date: date, total_balance: prepared_valuation.amount),
+      new_balance: prepared_valuation.amount,
+      error_message: nil
+    )
+  rescue => e
+    ReconciliationResult.new(
+      success?: false,
+      error_message: e.message
+    )
+  end
+
+  private
+    # Returns before -> after OR error message
+    ReconciliationResult = Struct.new(
+      :success?,
+      :old_cash_balance,
+      :old_balance,
+      :new_cash_balance,
+      :new_balance,
+      :error_message,
+      keyword_init: true
+    )
+
+    def prepare_reconciliation(balance, date, existing_valuation)
+      valuation_record = existing_valuation ||
+                         account.entries.valuations.find_by(date: date) || # In case of conflict, where existing valuation is not passed as arg, but one exists
+                         account.entries.build(
+                                  name: Valuation.build_reconciliation_name(account.accountable_type),
+                                  entryable: Valuation.new(kind: "reconciliation")
+                                )
+
+      valuation_record.assign_attributes(
+        date: date,
+        amount: balance,
+        currency: account.currency
+      )
+
+      valuation_record
+    end
+
+    def derived_cash_balance(date:, total_balance:)
+      balance_components_for_reconciliation_date = get_balance_components_for_date(date)
+
+      return nil unless balance_components_for_reconciliation_date[:balance] && balance_components_for_reconciliation_date[:cash_balance]
+
+      # We calculate the existing non-cash balance, which for investments would represents "holdings" for the date of reconciliation
+      # Since the user is setting "total balance", we have to subtract the existing non-cash balance from the total balance to get the new cash balance
+      existing_non_cash_balance = balance_components_for_reconciliation_date[:balance] - balance_components_for_reconciliation_date[:cash_balance]
+
+      total_balance - existing_non_cash_balance
+    end
+
+    def old_balance_components(reconciliation_date:, existing_valuation_entry: nil)
+      if existing_valuation_entry
+        get_balance_components_for_date(existing_valuation_entry.date)
+      else
+        get_balance_components_for_date(reconciliation_date)
+      end
+    end
+
+    def get_balance_components_for_date(date)
+      balance_record = account.balances.find_by(date: date, currency: account.currency)
+
+      {
+        cash_balance: balance_record&.cash_balance,
+        balance: balance_record&.balance
+      }
+    end
+end
diff --git a/app/models/investment.rb b/app/models/investment.rb
@@ -28,19 +28,4 @@ def icon
       "line-chart"
     end
   end
-
-  def holdings_value_for_date(date)
-    # Find the most recent holding for each security on or before the given date
-    # Using a subquery to get the max date for each security
-    account.holdings
-      .where(currency: account.currency)
-      .where("date <= ?", date)
-      .where("(security_id, date) IN (
-        SELECT security_id, MAX(date) as max_date
-        FROM holdings
-        WHERE account_id = ? AND date <= ?
-        GROUP BY security_id
-      )", account.id, date)
-      .sum(:amount)
-  end
 end
diff --git a/app/views/valuations/_confirmation_contents.html.erb b/app/views/valuations/_confirmation_contents.html.erb
@@ -1,14 +1,16 @@
+<%# locals: (account:, entry:, reconciliation_dry_run:, is_update:, action_verb:) %>
+
 <div class="space-y-4 text-sm text-secondary">
   <% if account.investment? %>
-    <% holdings_value = account.investment.holdings_value_for_date(entry.date) %>
-    <% brokerage_cash = entry.amount - holdings_value %>
+    <% holdings_value = reconciliation_dry_run.new_balance - reconciliation_dry_run.new_cash_balance %>
+    <% brokerage_cash = reconciliation_dry_run.new_cash_balance %>
 
     <p>This will <%= action_verb %> the account value on <span class="font-medium text-primary"><%= entry.date.strftime("%B %d, %Y") %></span> to:</p>
 
     <div class="bg-container rounded-lg p-4 space-y-2 border border-primary">
       <div class="flex justify-between">
         <span>Total account value</span>
-        <span class="font-medium text-primary"><%= entry.amount_money.format %></span>
+        <span class="font-medium text-primary"><%= Money.new(reconciliation_dry_run.new_balance, account.currency).format %></span>
       </div>
       <div class="flex justify-between text-xs">
         <span>Holdings value</span>
@@ -20,7 +22,7 @@
       </div>
     </div>
   <% else %>
-    <p><%= action_verb.capitalize %> 
+    <p><%= action_verb.capitalize %>
       <% if account.depository? %>
         account balance
       <% elsif account.credit_card? %>
@@ -40,10 +42,10 @@
       <% else %>
         balance
       <% end %>
-      on <span class="font-medium text-primary"><%= entry.date.strftime("%B %d, %Y") %></span> to 
+      on <span class="font-medium text-primary"><%= entry.date.strftime("%B %d, %Y") %></span> to
       <span class="font-medium text-primary"><%= entry.amount_money.format %></span>.
     </p>
   <% end %>
 
   <p>All future transactions and balances will be recalculated based on this <%= is_update ? "change" : "update" %>.</p>
-</div>
+</div>
diff --git a/app/views/valuations/confirm_create.html.erb b/app/views/valuations/confirm_create.html.erb
@@ -1,17 +1,16 @@
 <%= render DialogComponent.new do |dialog| %>
   <% dialog.with_header(title: "Confirm new balance") %>
   <% dialog.with_body do %>
-    <%= styled_form_with model: @entry, url: valuations_path, class: "space-y-4", data: { turbo: false } do |form| %>
+    <%= styled_form_with model: @entry, url: valuations_path, class: "space-y-4" do |form| %>
       <%= form.hidden_field :account_id %>
       <%= form.hidden_field :date %>
       <%= form.hidden_field :amount %>
-      <%= form.hidden_field :currency %>
-      <%= form.hidden_field :notes %>
 
-      <%= render "confirmation_contents", 
-          account: @account, 
-          entry: @entry, 
-          action_verb: "set", 
+      <%= render "confirmation_contents",
+          reconciliation_dry_run: @reconciliation_dry_run,
+          account: @account,
+          entry: @entry,
+          action_verb: "set",
           is_update: false %>
 
       <%= form.submit "Confirm" %>
diff --git a/app/views/valuations/confirm_update.html.erb b/app/views/valuations/confirm_update.html.erb
@@ -1,19 +1,18 @@
 <%= render DialogComponent.new do |dialog| %>
   <% dialog.with_header(title: "Update balance") %>
   <% dialog.with_body do %>
-    <%= styled_form_with model: @entry, url: valuation_path(@entry), method: :patch, class: "space-y-4", data: { turbo: false } do |form| %>
+    <%= styled_form_with model: @entry, url: valuation_path(@entry), method: :patch, class: "space-y-4", data: { turbo_frame: :_top } do |form| %>
       <%= form.hidden_field :date %>
       <%= form.hidden_field :amount %>
-      <%= form.hidden_field :currency %>
-      <%= form.hidden_field :notes %>
 
-      <%= render "confirmation_contents", 
-          account: @account, 
-          entry: @entry, 
-          action_verb: "update", 
+      <%= render "confirmation_contents",
+          reconciliation_dry_run: @reconciliation_dry_run,
+          account: @account,
+          entry: @entry,
+          action_verb: "update",
           is_update: true %>
 
       <%= form.submit "Update" %>
     <% end %>
   <% end %>
-<% end %>
+<% end %>
diff --git a/app/views/valuations/show.html.erb b/app/views/valuations/show.html.erb
@@ -44,10 +44,7 @@
               url: valuation_path(entry),
               method: :patch,
               class: "space-y-2",
-              data: { controller: "auto-submit-form" } do |f| %>
-          <%= f.hidden_field :date, value: entry.date %>
-          <%= f.hidden_field :amount, value: entry.amount %>
-          <%= f.hidden_field :currency, value: entry.currency %>
+              data: { controller: "auto-submit-form", auto_submit_form_trigger_event_value: "blur" } do |f| %>
           <%= f.text_area :notes,
                 label: t(".note_label"),
                 placeholder: t(".note_placeholder"),
diff --git a/test/controllers/valuations_controller_test.rb b/test/controllers/valuations_controller_test.rb
@@ -8,7 +8,7 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest
     @entry = entries(:valuation)
   end
 
-  test "creates entry with basic attributes" do
+  test "can create reconciliation" do
     account = accounts(:investment)
 
     assert_difference [ "Entry.count", "Valuation.count" ], 1 do
@@ -35,14 +35,19 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest
     assert_no_difference [ "Entry.count", "Valuation.count" ] do
       patch valuation_url(@entry), params: {
         entry: {
-          amount: 20000,
-          date: Date.current
+          amount: 22000,
+          date: Date.current,
+          notes: "Test notes"
         }
       }
     end
 
     assert_enqueued_with job: SyncJob
 
     assert_redirected_to account_url(@entry.account)
+
+    @entry.reload
+    assert_equal 22000, @entry.amount
+    assert_equal "Test notes", @entry.notes
   end
 end
diff --git a/test/models/account/reconciliation_manager_test.rb b/test/models/account/reconciliation_manager_test.rb
