Add reconciliation manager (#2459)

* Add reconciliation manager

* Fix notes editing

app/controllers/valuations_controller.rb

@@ -5,6 +5,12 @@ class ValuationsController < ApplicationController
     @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 @@ class ValuationsController < ApplicationController
     @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))
+
+    result = account.create_reconciliation(
+      balance: entry_params[:amount],
+      date: entry_params[:date],
+    )
 
     if result.success?
-      @success_message = result.updated? ? "Balance updated" : "No changes made. Account is already up to date."
-
       respond_to do |format|
-        format.html { 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: @success_message) }
+        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 @@ class ValuationsController < ApplicationController
   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 @@ class ValuationsController < ApplicationController
 
   private
     def entry_params
-      params.require(:entry)
+      params.require(:entry).permit(:date, :amount, :notes)
-            .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]
-      )
     end
 end

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
 

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

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

app/models/investment.rb

@@ -28,19 +28,4 @@ class Investment < ApplicationRecord
       "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

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) %>
+    <% holdings_value = reconciliation_dry_run.new_balance - reconciliation_dry_run.new_cash_balance %>
-    <% brokerage_cash = entry.amount - holdings_value %>
+    <% 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>

app/views/valuations/confirm_create.html.erb

@@ -1,14 +1,13 @@
 <%= 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",
+          reconciliation_dry_run: @reconciliation_dry_run,
           account: @account,
           entry: @entry,
           action_verb: "set",

app/views/valuations/confirm_update.html.erb

@@ -1,13 +1,12 @@
 <%= 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",
+          reconciliation_dry_run: @reconciliation_dry_run,
           account: @account,
           entry: @entry,
           action_verb: "update",

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| %>
+              data: { controller: "auto-submit-form", auto_submit_form_trigger_event_value: "blur" } do |f| %>
-          <%= f.hidden_field :date, value: entry.date %>
-          <%= f.hidden_field :amount, value: entry.amount %>
-          <%= f.hidden_field :currency, value: entry.currency %>
           <%= f.text_area :notes,
                 label: t(".note_label"),
                 placeholder: t(".note_placeholder"),

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,8 +35,9 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest
     assert_no_difference [ "Entry.count", "Valuation.count" ] do
       patch valuation_url(@entry), params: {
         entry: {
-          amount: 20000,
+          amount: 22000,
-          date: Date.current
+          date: Date.current,
+          notes: "Test notes"
         }
       }
     end
@@ -44,5 +45,9 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest
     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

test/models/account/reconciliation_manager_test.rb

@@ -0,0 +1,103 @@
+require "test_helper"
+
+class Account::ReconciliationManagerTest < ActiveSupport::TestCase
+  setup do
+    @account = accounts(:investment)
+    @manager = Account::ReconciliationManager.new(@account)
+  end
+
+  test "new reconciliation" do
+    @account.balances.create!(
+      date: Date.current,
+      balance: 1000,
+      cash_balance: 500,
+      currency: @account.currency
+    )
+
+    result = @manager.reconcile_balance(balance: 1200, date: Date.current)
+
+    assert_equal 1200, result.new_balance
+    assert_equal 700, result.new_cash_balance # Non cash stays the same since user is valuing the entire account balance
+    assert_equal 1000, result.old_balance
+    assert_equal 500, result.old_cash_balance
+    assert_equal true, result.success?
+  end
+
+  test "updates existing reconciliation without date change" do
+    @account.balances.create!(date: Date.current, balance: 1000, cash_balance: 500, currency: @account.currency)
+
+    # Existing reconciliation entry
+    existing_entry = @account.entries.create!(name: "Test", amount: 1000, date: Date.current, entryable: Valuation.new(kind: "reconciliation"), currency: @account.currency)
+
+    result = @manager.reconcile_balance(balance: 1200, date: Date.current, existing_valuation_entry: existing_entry)
+
+    assert_equal 1200, result.new_balance
+    assert_equal 700, result.new_cash_balance # Non cash stays the same since user is valuing the entire account balance
+    assert_equal 1000, result.old_balance
+    assert_equal 500, result.old_cash_balance
+    assert_equal true, result.success?
+  end
+
+  test "updates existing reconciliation with date and amount change" do
+    @account.balances.create!(date: 5.days.ago, balance: 1000, cash_balance: 500, currency: @account.currency)
+    @account.balances.create!(date: Date.current, balance: 1200, cash_balance: 700, currency: @account.currency)
+
+    # Existing reconciliation entry (5 days ago)
+    existing_entry = @account.entries.create!(name: "Test", amount: 1000, date: 5.days.ago, entryable: Valuation.new(kind: "reconciliation"), currency: @account.currency)
+
+    # Should update and change date for existing entry; not create a new one
+    assert_no_difference "Valuation.count" do
+      # "Update valuation from 5 days ago to today, set balance from 1000 to 1500"
+      result = @manager.reconcile_balance(balance: 1500, date: Date.current, existing_valuation_entry: existing_entry)
+
+      assert_equal true, result.success?
+
+      # Reconciliation
+      assert_equal 1500, result.new_balance # Equal to new valuation amount
+      assert_equal 1000, result.new_cash_balance # Get non-cash balance today (1200 - 700 = 500). Then subtract this from new valuation (1500 - 500 = 1000)
+
+      # Prior valuation
+      assert_equal 1000, result.old_balance # This is the balance from the old valuation, NOT the date we're reconciling to
+      assert_equal 500, result.old_cash_balance
+    end
+  end
+
+  test "handles date conflicts" do
+    @account.balances.create!(
+      date: Date.current,
+      balance: 1000,
+      cash_balance: 1000,
+      currency: @account.currency
+    )
+
+    # Existing reconciliation entry
+    @account.entries.create!(
+      name: "Test",
+      amount: 1000,
+      date: Date.current,
+      entryable: Valuation.new(kind: "reconciliation"),
+      currency: @account.currency
+    )
+
+    # Doesn't pass existing_valuation_entry, but reconciliation manager should recognize its the same date and update the existing entry
+    assert_no_difference "Valuation.count" do
+      result = @manager.reconcile_balance(balance: 1200, date: Date.current)
+
+      assert result.success?
+      assert_equal 1200, result.new_balance
+    end
+  end
+
+  test "dry run does not persist or sync account" do
+    @account.balances.create!(date: Date.current, balance: 1000, cash_balance: 500, currency: @account.currency)
+
+    assert_no_difference "Valuation.count" do
+      @manager.reconcile_balance(balance: 1200, date: Date.current, dry_run: true)
+    end
+
+    assert_difference "Valuation.count", 1 do
+      @account.expects(:sync_later).once
+      @manager.reconcile_balance(balance: 1200, date: Date.current)
+    end
+  end
+end