Migrate valuations controller to new reconciliation methods

app/components/button_component.html.erb

@@ -1,6 +1,6 @@
 <%= container do %>
   <% if icon && (icon_position != :right) %>
-    <%= helpers.icon(icon, size: size, color: icon_color) %>
+    <%= helpers.icon(icon, size: size, color: icon_color, class: icon_classes) %>
   <% end %>
 
   <% unless icon_only? %>

app/components/buttonish_component.rb

@@ -5,7 +5,7 @@ class ButtonishComponent < ViewComponent::Base
       icon_classes: "fg-inverse"
     },
     secondary: {
-      container_classes: "text-secondary bg-gray-50 theme-dark:bg-gray-700 hover:bg-gray-100 theme-dark:hover:bg-gray-600 disabled:bg-gray-200 theme-dark:disabled:bg-gray-600",
+      container_classes: "text-primary bg-gray-50 theme-dark:bg-gray-700 hover:bg-gray-100 theme-dark:hover:bg-gray-600 disabled:bg-gray-200 theme-dark:disabled:bg-gray-600",
       icon_classes: "fg-primary"
     },
     destructive: {

app/controllers/concerns/accountable_resource.rb

@@ -46,29 +46,24 @@ module AccountableResource
   end
 
   def update
-    # Handle balance update if provided
-    if account_params[:balance].present?
-      result = @account.update_balance(balance: account_params[:balance], currency: account_params[:currency])
-      unless result.success?
-        @error_message = result.error_message
-        render :edit, status: :unprocessable_entity
-        return
+    form = Account::OverviewForm.new(
+      account: @account,
+      name: account_params[:name],
+      currency: account_params[:currency],
+      current_balance: account_params[:balance],
+      current_cash_balance: @account.depository? ? account_params[:balance] : "0"
+    )
+
+    result = form.save
+
+    if result.success?
+      respond_to do |format|
+        format.html { redirect_back_or_to account_path(@account), notice: accountable_type.name.underscore.humanize + " account updated" }
+        format.turbo_stream { stream_redirect_to account_path(@account), notice: accountable_type.name.underscore.humanize + " account updated" }
       end
-    end
-
-    # Update remaining account attributes
-    update_params = account_params.except(:return_to, :balance, :currency, :tracking_start_date)
-    unless @account.update(update_params)
-      @error_message = @account.errors.full_messages.join(", ")
+    else
+      @error_message = result.error || "Unable to update account details."
       render :edit, status: :unprocessable_entity
-      return
-    end
-
-    @account.lock_saved_attributes!
-
-    respond_to do |format|
-      format.html { redirect_back_or_to account_path(@account), notice: accountable_type.name.underscore.humanize + " account updated" }
-      format.turbo_stream { stream_redirect_to account_path(@account), notice: accountable_type.name.underscore.humanize + " account updated" }
     end
   end
 

app/controllers/credit_cards_controller.rb

@@ -9,4 +9,31 @@ class CreditCardsController < ApplicationController
     :annual_fee,
     :expiration_date
   )
+
+  def update
+    form = Account::OverviewForm.new(
+      account: @account,
+      name: account_params[:name],
+      currency: account_params[:currency],
+      current_balance: account_params[:balance],
+      current_cash_balance: @account.depository? ? account_params[:balance] : "0"
+    )
+
+    result = form.save
+
+    if result.success?
+      # Update credit card-specific attributes
+      if account_params[:accountable_attributes].present?
+        @account.credit_card.update!(account_params[:accountable_attributes])
+      end
+
+      respond_to do |format|
+        format.html { redirect_back_or_to account_path(@account), notice: "Credit card account updated" }
+        format.turbo_stream { stream_redirect_to account_path(@account), notice: "Credit card account updated" }
+      end
+    else
+      @error_message = result.error || "Unable to update account details."
+      render :edit, status: :unprocessable_entity
+    end
+  end
 end

app/controllers/loans_controller.rb

@@ -4,4 +4,31 @@ class LoansController < ApplicationController
   permitted_accountable_attributes(
     :id, :rate_type, :interest_rate, :term_months, :initial_balance
   )
+
+  def update
+    form = Account::OverviewForm.new(
+      account: @account,
+      name: account_params[:name],
+      currency: account_params[:currency],
+      current_balance: account_params[:balance],
+      current_cash_balance: @account.depository? ? account_params[:balance] : "0"
+    )
+
+    result = form.save
+
+    if result.success?
+      # Update loan-specific attributes
+      if account_params[:accountable_attributes].present?
+        @account.loan.update!(account_params[:accountable_attributes])
+      end
+
+      respond_to do |format|
+        format.html { redirect_back_or_to account_path(@account), notice: "Loan account updated" }
+        format.turbo_stream { stream_redirect_to account_path(@account), notice: "Loan account updated" }
+      end
+    else
+      @error_message = result.error || "Unable to update account details."
+      render :edit, status: :unprocessable_entity
+    end
+  end
 end

app/controllers/valuations_controller.rb

@@ -4,59 +4,55 @@ class ValuationsController < ApplicationController
   def create
     account = Current.family.accounts.find(params.dig(:entry, :account_id))
 
-    result = account.update_balance(
-      balance: entry_params[:amount],
-      date: entry_params[:date],
-      currency: entry_params[:currency],
-      notes: entry_params[:notes]
-    )
-
-    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.turbo_stream { stream_redirect_back_or_to(account_path(account), notice: @success_message) }
-      end
+    if entry_params[:date].to_date == Date.current
+      account.update_current_balance!(balance: entry_params[:amount].to_d)
     else
-      @error_message = result.error_message
-      render :new, status: :unprocessable_entity
+      account.reconcile_balance!(
+        balance: entry_params[:amount].to_d,
+        date: entry_params[:date].to_date
+      )
+    end
+
+    account.sync_later
+
+    respond_to do |format|
+      format.html { redirect_back_or_to account_path(account), notice: "Account value updated" }
+      format.turbo_stream { stream_redirect_back_or_to(account_path(account), notice: "Account value updated") }
     end
   end
 
   def update
-    result = @entry.account.update_balance(
-      date: @entry.date,
-      balance: entry_params[:amount],
-      currency: entry_params[:currency],
-      notes: entry_params[:notes]
+    # ActiveRecord::Base.transaction do
+    @entry.account.reconcile_balance!(
+      balance: entry_params[:amount].to_d,
+      date: entry_params[:date].to_date
     )
 
-    if result.success?
-      @entry.reload
+    if entry_params[:notes].present?
+      @entry.update!(notes: entry_params[:notes])
+    end
 
-      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.turbo_stream do
-          render turbo_stream: [
-            turbo_stream.replace(
-              dom_id(@entry, :header),
-              partial: "valuations/header",
-              locals: { entry: @entry }
-            ),
-            turbo_stream.replace(@entry)
-          ]
-        end
+    @entry.account.sync_later
+
+    @entry.reload
+
+    respond_to do |format|
+      format.html { redirect_back_or_to account_path(@entry.account), notice: "Account value updated" }
+      format.turbo_stream do
+        render turbo_stream: [
+          turbo_stream.replace(
+            dom_id(@entry, :header),
+            partial: "valuations/header",
+            locals: { entry: @entry }
+          ),
+          turbo_stream.replace(@entry)
+        ]
       end
-    else
-      @error_message = result.error_message
-      render :show, status: :unprocessable_entity
     end
   end
 
   private
     def entry_params
-      params.require(:entry)
-            .permit(:date, :amount, :currency, :notes)
+      params.require(:entry).permit(:date, :amount, :notes)
     end
 end

app/models/account.rb

@@ -14,8 +14,6 @@ class Account < ApplicationRecord
   has_many :holdings, dependent: :destroy
   has_many :balances, dependent: :destroy
 
-
-
   enum :classification, { asset: "asset", liability: "liability" }, validate: { allow_nil: true }
 
   scope :visible, -> { where(status: [ "draft", "active" ]) }
@@ -120,11 +118,6 @@ class Account < ApplicationRecord
             .order(amount: :desc)
   end
 
-
-  def update_balance(balance:, date: Date.current, currency: nil, notes: nil)
-    Account::BalanceUpdater.new(self, balance:, currency:, date:, notes:).update
-  end
-
   def update_currency!(new_currency)
     raise "Currency cannot be changed" if linked?
 
@@ -134,7 +127,6 @@ class Account < ApplicationRecord
     end
   end
 
-
   def start_date
     first_entry_date = entries.minimum(:date) || Date.current
     first_entry_date - 1.day

app/models/account/balance_updater.rb

@@ -1,55 +0,0 @@
-class Account::BalanceUpdater
-  def initialize(account, balance:, currency: nil, date: Date.current, notes: nil)
-    @account = account
-    @balance = balance.to_d
-    @currency = currency
-    @date = date.to_date
-    @notes = notes
-  end
-
-  def update
-    return Result.new(success?: true, updated?: false) unless requires_update?
-
-    Account.transaction do
-      if date == Date.current
-        account.balance = balance
-        account.currency = currency if currency.present?
-        account.save!
-      end
-
-      valuation_entry = account.entries.valuations.find_or_initialize_by(date: date) do |entry|
-        entry.entryable = Valuation.new(
-          kind: "recon",
-          balance: balance,
-          cash_balance: balance
-        )
-      end
-
-      valuation_entry.amount = balance
-      valuation_entry.currency = currency if currency.present?
-      valuation_entry.name = valuation_name(valuation_entry, account)
-      valuation_entry.notes = notes if notes.present?
-      valuation_entry.save!
-    end
-
-    account.sync_later
-
-    Result.new(success?: true, updated?: true)
-  rescue => e
-    message = Rails.env.development? ? e.message : "Unable to update account values. Please try again."
-    Result.new(success?: false, updated?: false, error_message: message)
-  end
-
-  private
-    attr_reader :account, :balance, :currency, :date, :notes
-
-    Result = Struct.new(:success?, :updated?, :error_message)
-
-    def requires_update?
-      date != Date.current || account.balance != balance || account.currency != currency
-    end
-
-    def valuation_name(valuation_entry, account)
-      Valuation::Name.new(valuation_entry.entryable.kind, account.accountable_type).to_s
-    end
-end

app/models/account/overview_form.rb

@@ -42,6 +42,7 @@ class Account::OverviewForm
       # Update name if provided
       if name.present? && name != account.name
         account.update!(name: name)
+        account.lock_attr!(:name)
         updated = true
       end
 

app/models/account/reconcileable.rb

@@ -29,18 +29,26 @@ module Account::Reconcileable
     @opening_date ||= opening_anchor_valuation&.entry&.date
   end
 
-  def reconcile_balance!(balance:, cash_balance:, date:)
-    raise InvalidBalanceError, "Cash balance cannot exceed balance" if cash_balance > balance
+  def reconcile_balance!(balance:, cash_balance: nil, date: nil)
+    raise InvalidBalanceError, "Cash balance cannot exceed balance" if cash_balance.present? && cash_balance > balance
     raise InvalidBalanceError, "Linked accounts cannot be reconciled" if linked?
 
+    derived_cash_balance = cash_balance.present? ? cash_balance : choose_cash_balance_from_balance(balance)
+
+    if date.nil?
+      update_current_balance!(balance:, cash_balance: derived_cash_balance)
+      return
+    end
+
     existing_valuation = valuations.joins(:entry).where(kind: "recon", entry: { date: date }).first
 
     transaction do
       if existing_valuation.present?
         existing_valuation.update!(
           balance: balance,
-          cash_balance: cash_balance
+          cash_balance: derived_cash_balance
         )
+        existing_valuation.entry.update!(amount: balance)
       else
         entries.create!(
           date: date,
@@ -50,43 +58,36 @@ module Account::Reconcileable
           entryable: Valuation.new(
             kind: "recon",
             balance: balance,
-            cash_balance: cash_balance
+            cash_balance: derived_cash_balance
           )
         )
       end
 
       # Update cached balance fields on account when reconciling for current date
       if date == Date.current
-        update!(balance: balance, cash_balance: cash_balance)
+        update!(balance: balance, cash_balance: derived_cash_balance)
       end
     end
   end
 
-  def update_current_balance!(balance:, cash_balance:)
-    raise InvalidBalanceError, "Cash balance cannot exceed balance" if cash_balance > balance
+  def update_current_balance!(balance:, cash_balance: nil)
+    raise InvalidBalanceError, "Cash balance cannot exceed balance" if cash_balance.present? && cash_balance > balance
+
+    derived_cash_balance = cash_balance.present? ? cash_balance : choose_cash_balance_from_balance(balance)
 
     transaction do
-      if opening_anchor_valuation.present? && valuations.where(kind: "recon").empty?
-        adjust_opening_balance_with_delta(balance:, cash_balance:)
+      # See test for explanation - Depository accounts are handled as a special case for current balance updates
+      if opening_anchor_valuation.present? && valuations.where(kind: "recon").empty? && self.depository?
+        adjust_opening_balance_with_delta!(balance:, cash_balance: derived_cash_balance)
       else
-        reconcile_balance!(balance:, cash_balance:, date: Date.current)
+        reconcile_balance!(balance:, cash_balance: derived_cash_balance, date: Date.current)
       end
 
       # Always update cached balance fields when updating current balance
-      update!(balance: balance, cash_balance: cash_balance)
+      update!(balance: balance, cash_balance: derived_cash_balance)
     end
   end
 
-  def adjust_opening_balance_with_delta(balance:, cash_balance:)
-    delta = self.balance - balance
-    cash_delta = self.cash_balance - cash_balance
-
-    set_or_update_opening_balance!(
-      balance: balance - delta,
-      cash_balance: cash_balance - cash_delta
-    )
-  end
-
   def set_or_update_opening_balance!(balance:, cash_balance:, date: nil)
     # A reasonable start date for most accounts to fill up adequate history for graphs
     fallback_opening_date = 2.years.ago.to_date
@@ -130,4 +131,24 @@ module Account::Reconcileable
     def current_anchor_valuation
       valuations.current_anchor.first
     end
+
+    def adjust_opening_balance_with_delta!(balance:, cash_balance:)
+      delta = self.balance - balance
+      cash_delta = self.cash_balance - cash_balance
+
+      set_or_update_opening_balance!(
+        balance: balance - delta,
+        cash_balance: cash_balance - cash_delta
+      )
+    end
+
+    # For depository accounts, the cash balance is the same as the balance always
+    # Otherwise, if not specified, we assume cash balance is 0
+    def choose_cash_balance_from_balance(balance)
+      if self.depository?
+        balance
+      else
+        0
+      end
+    end
 end

app/views/accounts/show/_activity.html.erb

@@ -10,7 +10,7 @@
 
           <% menu.with_item(
               variant: "link",
-              text: "New balance",
+              text: "Account #{account.asset? ? "value" : "balance"} update",
               icon: "circle-dollar-sign",
               href: new_valuation_path(account_id: @account.id),
               data: { turbo_frame: :modal }) %>

app/views/valuations/_form.html.erb

@@ -10,8 +10,8 @@
   <div class="space-y-3">
     <%= form.hidden_field :name, value: "Balance update" %>
     <%= form.date_field :date, label: true, required: true, value: Date.current, min: Entry.min_supported_date, max: Date.current %>
-    <%= form.money_field :amount, label: t(".amount"), required: true %>
+    <%= form.money_field :amount, label: t(".amount"), required: true, disable_currency: true %>
   </div>
 
-  <%= form.submit t(".submit") %>
+  <%= form.submit "Update account #{entry.account.asset? ? "value" : "balance"}" %>
 <% end %>

app/views/valuations/new.html.erb

@@ -1,6 +1,12 @@
+<% term = @entry.account.asset? ? "value" : "balance" %>
+
 <%= render DialogComponent.new do |dialog| %>
-  <% dialog.with_header(title: t(".title")) %>
+  <% dialog.with_header(title: "Account #{term} update") %>
   <% dialog.with_body do %>
     <%= render "form", entry: @entry, error_message: @error_message %>
+
+    <p class="text-sm text-secondary mt-4">
+      This action "resets" the account's <%= term %> to the new value, on the date. Subsequent entries after this date will reference the new value.
+    </p>
   <% end %>
 <% end %>

test/controllers/valuations_controller_test.rb

@@ -38,7 +38,7 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest
         entry: {
           amount: 20000,
           currency: "USD",
-          date: Date.current
+          date: @entry.date
         }
       }
     end

test/interfaces/entryable_resource_interface_test.rb

@@ -4,7 +4,7 @@ module EntryableResourceInterfaceTest
   extend ActiveSupport::Testing::Declarative
 
   test "shows new form" do
-    get new_polymorphic_url(@entry.entryable)
+    get new_polymorphic_url(@entry.entryable, account_id: @entry.account_id)
     assert_response :success
   end
 

test/models/account/overview_form_test.rb

@@ -30,6 +30,7 @@ class Account::OverviewFormTest < ActiveSupport::TestCase
     )
 
     @account.expects(:update!).with(name: "New Property Name").once
+    @account.expects(:lock_attr!).with(:name).once
     @account.expects(:sync_later).never  # Name change should not trigger sync
 
     result = form.save
@@ -84,6 +85,7 @@ class Account::OverviewFormTest < ActiveSupport::TestCase
 
     # Simulate a validation error on opening balance update
     @account.expects(:update!).with(name: "New Name").once
+    @account.expects(:lock_attr!).with(:name).once
     @account.expects(:set_or_update_opening_balance!).raises(Account::Reconcileable::InvalidBalanceError.new("Cash balance cannot exceed balance"))
     @account.expects(:sync_later).never  # Should NOT sync if any update fails
 

test/models/account/reconcileable_test.rb

@@ -6,16 +6,9 @@ class Account::ReconcileableTest < ActiveSupport::TestCase
     @family = families(:dylan_family)
   end
 
-  # Currency updates earn their own method because updating an account currency incurs
-  # side effects like recalculating balances, etc.
-  test "can update the account currency" do
-    @account.update_currency!("EUR")
-
-    assert_equal "EUR", @account.currency
-    assert_equal "EUR", @account.entries.valuations.first.currency
-  end
-
-  # If a user has an opening balance (valuation) for their manual account and has 1+ transactions, the intent of
+  # Scope: Depository Only
+  #
+  # If a user has an opening balance (valuation) for their manual *Depository* account and has 1+ transactions, the intent of
   # "updating current balance" typically means that their start balance is incorrect. We follow that user intent
   # by default and find the delta required, and update the opening balance so that the timeline reflects this current balance
   #

test/models/account_test.rb

@@ -31,4 +31,13 @@ class AccountTest < ActiveSupport::TestCase
     assert_equal "Investments", account.short_subtype_label
     assert_equal "Investments", account.long_subtype_label
   end
+
+  # Currency updates earn their own method because updating an account currency incurs
+  # side effects like recalculating balances, etc.
+  test "can update the account currency" do
+    @account.update_currency!("EUR")
+
+    assert_equal "EUR", @account.currency
+    assert_equal "EUR", @account.entries.valuations.first.currency
+  end
 end

test/system/accounts_test.rb

@@ -23,20 +23,22 @@ class AccountsTest < ApplicationSystemTestCase
   end
 
   test "can create property account" do
-    # Step 1: Select property type and enter basic details
+    # Step 1: Enter basic property details
     click_link "Property"
 
     account_name = "[system test] Property Account"
-    fill_in "Name*", with: account_name
-    select "Single Family Home", from: "Property type*"
-    fill_in "Year Built (optional)", with: 2005
-    fill_in "Area (optional)", with: 2250
+    fill_in "Name", with: account_name
+    fill_in "account[current_estimated_value]", with: 500000
+    fill_in "account[purchase_price]", with: 450000
+    fill_in "account[purchase_date]", with: "01/15/2020"
 
     click_button "Next"
 
-    # Step 2: Enter balance information
-    assert_text "Value"
-    fill_in "account[balance]", with: 500000
+    # Step 2: Enter property details
+    assert_text "Property type"
+    select "Single Family Home", from: "Property type"
+    fill_in "Year built", with: 2005
+    fill_in "Area", with: 2250
     click_button "Next"
 
     # Step 3: Enter address information