From 89cc64418e5202d8121018955b80b6965c00a1fb Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Tue, 15 Jul 2025 18:58:40 -0400 Subject: [PATCH] Add confirmation dialog for balance reconciliation creates and updates (#2457) --- app/controllers/valuations_controller.rb | 40 ++++++++++----- app/models/account.rb | 4 +- app/models/account/balance_updater.rb | 14 ++++-- app/models/investment.rb | 15 ++++++ app/views/entries/_selection_bar.html.erb | 2 +- .../_confirmation_contents.html.erb | 49 +++++++++++++++++++ app/views/valuations/_form.html.erb | 17 ------- app/views/valuations/confirm_create.html.erb | 20 ++++++++ app/views/valuations/confirm_update.html.erb | 19 +++++++ app/views/valuations/new.html.erb | 15 +++++- app/views/valuations/show.html.erb | 27 +++++++--- config/routes.rb | 5 +- .../controllers/valuations_controller_test.rb | 2 - 13 files changed, 180 insertions(+), 49 deletions(-) create mode 100644 app/views/valuations/_confirmation_contents.html.erb delete mode 100644 app/views/valuations/_form.html.erb create mode 100644 app/views/valuations/confirm_create.html.erb create mode 100644 app/views/valuations/confirm_update.html.erb diff --git a/app/controllers/valuations_controller.rb b/app/controllers/valuations_controller.rb index 90aa1da0..85f3413a 100644 --- a/app/controllers/valuations_controller.rb +++ b/app/controllers/valuations_controller.rb @@ -1,15 +1,24 @@ class ValuationsController < ApplicationController include EntryableResource, StreamExtensions + def confirm_create + @account = Current.family.accounts.find(params.dig(:entry, :account_id)) + @entry = @account.entries.build(entry_params.merge(currency: @account.currency)) + + render :confirm_create + end + + def confirm_update + @entry = Current.family.entries.find(params[:id]) + @account = @entry.account + @entry.assign_attributes(entry_params.merge(currency: @account.currency)) + + render :confirm_update + end + 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] - ) + 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." @@ -25,12 +34,7 @@ class ValuationsController < ApplicationController end def update - result = @entry.account.update_balance( - date: @entry.date, - balance: entry_params[:amount], - currency: entry_params[:currency], - notes: entry_params[:notes] - ) + result = perform_balance_update(@entry.account, entry_params.merge(currency: @entry.currency, existing_valuation_id: @entry.id)) if result.success? @entry.reload @@ -59,4 +63,14 @@ class ValuationsController < ApplicationController 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] + ) + end end diff --git a/app/models/account.rb b/app/models/account.rb index 1217c5fe..837d6e57 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -115,8 +115,8 @@ class Account < ApplicationRecord end - def update_balance(balance:, date: Date.current, currency: nil, notes: nil) - Account::BalanceUpdater.new(self, balance:, currency:, date:, notes:).update + def update_balance(balance:, date: Date.current, currency: nil, notes: nil, existing_valuation_id: nil) + Account::BalanceUpdater.new(self, balance:, currency:, date:, notes:, existing_valuation_id:).update end def start_date diff --git a/app/models/account/balance_updater.rb b/app/models/account/balance_updater.rb index 0eb9a789..1c1b1556 100644 --- a/app/models/account/balance_updater.rb +++ b/app/models/account/balance_updater.rb @@ -1,10 +1,11 @@ class Account::BalanceUpdater - def initialize(account, balance:, currency: nil, date: Date.current, notes: nil) + def initialize(account, balance:, currency: nil, date: Date.current, notes: nil, existing_valuation_id: nil) @account = account @balance = balance.to_d @currency = currency @date = date.to_date @notes = notes + @existing_valuation_id = existing_valuation_id end def update @@ -17,10 +18,15 @@ class Account::BalanceUpdater account.save! end - valuation_entry = account.entries.valuations.find_or_initialize_by(date: date) do |entry| - entry.entryable = Valuation.new(kind: "reconciliation") + valuation_entry = if existing_valuation_id + account.entries.find(existing_valuation_id) + else + account.entries.valuations.find_or_initialize_by(date: date) do |entry| + entry.entryable = Valuation.new(kind: "reconciliation") + end end + valuation_entry.date = date valuation_entry.amount = balance valuation_entry.currency = currency if currency.present? valuation_entry.name = Valuation.build_reconciliation_name(account.accountable_type) @@ -37,7 +43,7 @@ class Account::BalanceUpdater end private - attr_reader :account, :balance, :currency, :date, :notes + attr_reader :account, :balance, :currency, :date, :notes, :existing_valuation_id Result = Struct.new(:success?, :updated?, :error_message) diff --git a/app/models/investment.rb b/app/models/investment.rb index 4e4c25c8..10d879e4 100644 --- a/app/models/investment.rb +++ b/app/models/investment.rb @@ -28,4 +28,19 @@ 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 diff --git a/app/views/entries/_selection_bar.html.erb b/app/views/entries/_selection_bar.html.erb index 1c633bde..a1faca09 100644 --- a/app/views/entries/_selection_bar.html.erb +++ b/app/views/entries/_selection_bar.html.erb @@ -6,7 +6,7 @@
- <%= form_with url: transactions_bulk_deletion_path, data: { turbo_confirm: true, turbo_frame: "_top" } do %> + <%= form_with url: transactions_bulk_deletion_path, data: { turbo_confirm: CustomConfirm.for_resource_deletion("entry").to_data_attribute, turbo_frame: "_top" } do %> diff --git a/app/views/valuations/_confirmation_contents.html.erb b/app/views/valuations/_confirmation_contents.html.erb new file mode 100644 index 00000000..c3dc3724 --- /dev/null +++ b/app/views/valuations/_confirmation_contents.html.erb @@ -0,0 +1,49 @@ +
+ <% if account.investment? %> + <% holdings_value = account.investment.holdings_value_for_date(entry.date) %> + <% brokerage_cash = entry.amount - holdings_value %> + +

This will <%= action_verb %> the account value on <%= entry.date.strftime("%B %d, %Y") %> to:

+ +
+
+ Total account value + <%= entry.amount_money.format %> +
+
+ Holdings value + <%= Money.new(holdings_value, account.currency).format %> +
+
+ Brokerage cash + "><%= Money.new(brokerage_cash, account.currency).format %> +
+
+ <% else %> +

<%= action_verb.capitalize %> + <% if account.depository? %> + account balance + <% elsif account.credit_card? %> + credit card balance + <% elsif account.loan? %> + loan balance + <% elsif account.property? %> + property value + <% elsif account.vehicle? %> + vehicle value + <% elsif account.crypto? %> + crypto balance + <% elsif account.other_asset? %> + asset value + <% elsif account.other_liability? %> + liability balance + <% else %> + balance + <% end %> + on <%= entry.date.strftime("%B %d, %Y") %> to + <%= entry.amount_money.format %>. +

+ <% end %> + +

All future transactions and balances will be recalculated based on this <%= is_update ? "change" : "update" %>.

+
\ No newline at end of file diff --git a/app/views/valuations/_form.html.erb b/app/views/valuations/_form.html.erb deleted file mode 100644 index 5429f3a7..00000000 --- a/app/views/valuations/_form.html.erb +++ /dev/null @@ -1,17 +0,0 @@ -<%# locals: (entry:, error_message:) %> - -<%= styled_form_with model: entry, url: valuations_path, class: "space-y-4" do |form| %> - <%= form.hidden_field :account_id %> - - <% if error_message.present? %> - <%= render AlertComponent.new(message: error_message, variant: :error) %> - <% end %> - -
- <%= 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.submit t(".submit") %> -<% end %> diff --git a/app/views/valuations/confirm_create.html.erb b/app/views/valuations/confirm_create.html.erb new file mode 100644 index 00000000..75646450 --- /dev/null +++ b/app/views/valuations/confirm_create.html.erb @@ -0,0 +1,20 @@ +<%= 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| %> + <%= 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", + is_update: false %> + + <%= form.submit "Confirm" %> + <% end %> + <% end %> +<% end %> diff --git a/app/views/valuations/confirm_update.html.erb b/app/views/valuations/confirm_update.html.erb new file mode 100644 index 00000000..43c67c37 --- /dev/null +++ b/app/views/valuations/confirm_update.html.erb @@ -0,0 +1,19 @@ +<%= 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| %> + <%= 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", + is_update: true %> + + <%= form.submit "Update" %> + <% end %> + <% end %> +<% end %> \ No newline at end of file diff --git a/app/views/valuations/new.html.erb b/app/views/valuations/new.html.erb index 82102f16..bfa8d5e2 100644 --- a/app/views/valuations/new.html.erb +++ b/app/views/valuations/new.html.erb @@ -1,6 +1,19 @@ <%= render DialogComponent.new do |dialog| %> <% dialog.with_header(title: t(".title")) %> <% dialog.with_body do %> - <%= render "form", entry: @entry, error_message: @error_message %> + <%= styled_form_with model: @entry, url: confirm_create_valuations_path, class: "space-y-4" do |form| %> + <%= form.hidden_field :account_id %> + + <% if @error_message.present? %> + <%= render AlertComponent.new(message: @error_message, variant: :error) %> + <% end %> + +
+ <%= 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, disable_currency: true %> +
+ + <%= form.submit t(".submit") %> + <% end %> <% end %> <% end %> diff --git a/app/views/valuations/show.html.erb b/app/views/valuations/show.html.erb index 6e96dcaa..b4156e39 100644 --- a/app/views/valuations/show.html.erb +++ b/app/views/valuations/show.html.erb @@ -15,18 +15,25 @@ <% dialog.with_section(title: t(".overview"), open: true) do %>
<%= styled_form_with model: entry, - url: entry_path(entry), - class: "space-y-2", - data: { controller: "auto-submit-form" } do |f| %> + url: confirm_update_valuation_path(entry), + method: :post, + data: { turbo_frame: :modal }, + class: "space-y-4" do |f| %> <%= f.date_field :date, label: t(".date_label"), - max: Date.current, - "data-auto-submit-form-target": "auto" %> + max: Date.current %> <%= f.money_field :amount, label: t(".amount"), - auto_submit: true, disable_currency: true %> + +
+ <%= render ButtonComponent.new( + text: "Update value", + variant: :primary, + type: "submit" + ) %> +
<% end %>
<% end %> @@ -34,9 +41,13 @@ <% dialog.with_section(title: t(".details")) do %>
<%= styled_form_with model: entry, - url: entry_path(entry), + 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 %> <%= f.text_area :notes, label: t(".note_label"), placeholder: t(".note_placeholder"), @@ -59,7 +70,7 @@ entry_path(entry), method: :delete, class: "rounded-lg px-3 py-2 text-red-500 text-sm font-medium border border-secondary", - data: { turbo_confirm: true, turbo_frame: "_top" } %> + data: { turbo_confirm: CustomConfirm.for_resource_deletion("value update").to_data_attribute, turbo_frame: "_top" } %>
<% end %> diff --git a/config/routes.rb b/config/routes.rb index f0414287..3dd1d7f1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -110,7 +110,10 @@ Rails.application.routes.draw do resources :holdings, only: %i[index new show destroy] resources :trades, only: %i[show new create update destroy] - resources :valuations, only: %i[show new create update destroy] + resources :valuations, only: %i[show new create update destroy] do + post :confirm_create, on: :collection + post :confirm_update, on: :member + end namespace :transactions do resource :bulk_deletion, only: :create diff --git a/test/controllers/valuations_controller_test.rb b/test/controllers/valuations_controller_test.rb index 52c62ad4..4746eb32 100644 --- a/test/controllers/valuations_controller_test.rb +++ b/test/controllers/valuations_controller_test.rb @@ -15,7 +15,6 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest post valuations_url, params: { entry: { amount: account.balance + 100, - currency: "USD", date: Date.current.to_s, account_id: account.id } @@ -37,7 +36,6 @@ class ValuationsControllerTest < ActionDispatch::IntegrationTest patch valuation_url(@entry), params: { entry: { amount: 20000, - currency: "USD", date: Date.current } }