From 34e03c2d6ac09729fa5d24f8cfca7d7e90202435 Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 12 Jul 2024 13:47:39 -0400 Subject: [PATCH] Make balance editing easier (#976) * Make balance editing easier * Translations * Fix money input option * Fix balance sync logic * Rework balance update flow --- app/controllers/accounts_controller.rb | 4 ++- app/helpers/application_form_builder.rb | 2 +- app/models/account.rb | 14 +++++++++ app/models/account/balance/syncer.rb | 10 ++++--- app/views/accounts/edit.html.erb | 3 +- app/views/accounts/show.html.erb | 27 ++++++++++------- config/locales/views/accounts/en.yml | 6 ++++ test/controllers/accounts_controller_test.rb | 31 ++++++++++++++++++++ test/models/account/balance/syncer_test.rb | 17 +++++++---- 9 files changed, 91 insertions(+), 23 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index f93c444b..6371c4db 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -40,7 +40,9 @@ class AccountsController < ApplicationController end def update - @account.update! account_params.except(:accountable_type) + @account.update! account_params.except(:accountable_type, :balance) + @account.update_balance!(account_params[:balance]) if account_params[:balance] + @account.sync_later redirect_back_or_to account_path(@account), notice: t(".success") end diff --git a/app/helpers/application_form_builder.rb b/app/helpers/application_form_builder.rb index 0e33c222..5c0fb8b7 100644 --- a/app/helpers/application_form_builder.rb +++ b/app/helpers/application_form_builder.rb @@ -53,7 +53,7 @@ class ApplicationFormBuilder < ActionView::Helpers::FormBuilder (label(method, *label_args(options)).to_s if options[:label]) + @template.tag.div(class: "flex items-center") do number_field(money_amount_method, merged_options.except(:label)) + - grouped_select(money_currency_method, grouped_options, { selected: selected_currency, disabled: readonly_currency }, class: "ml-auto form-field__input w-fit pr-8", data: { "money-field-target" => "currency", action: "change->money-field#handleCurrencyChange" }) + grouped_select(money_currency_method, grouped_options, { selected: selected_currency }, disabled: readonly_currency, class: "ml-auto form-field__input w-fit pr-8", data: { "money-field-target" => "currency", action: "change->money-field#handleCurrencyChange" }) end end end diff --git a/app/models/account.rb b/app/models/account.rb index d1fcb02f..2fa08845 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -93,4 +93,18 @@ class Account < ApplicationRecord rescue Money::ConversionError TimeSeries.new([]) end + + def update_balance!(balance) + valuation = entries.account_valuations.find_by(date: Date.current) + + if valuation + valuation.update! amount: balance + else + entries.create! \ + date: Date.current, + amount: balance, + currency: currency, + entryable: Account::Valuation.new + end + end end diff --git a/app/models/account/balance/syncer.rb b/app/models/account/balance/syncer.rb index 690c52b2..63bc346e 100644 --- a/app/models/account/balance/syncer.rb +++ b/app/models/account/balance/syncer.rb @@ -14,6 +14,11 @@ class Account::Balance::Syncer Account::Balance.transaction do upsert_balances!(daily_balances) purge_stale_balances! + + if daily_balances.any? + account.reload + account.update! balance: daily_balances.select { |db| db.currency == account.currency }.last&.balance + end end end @@ -53,16 +58,13 @@ class Account::Balance::Syncer entries = account.entries.where("date >= ?", sync_start_date).to_a prior_balance = find_prior_balance - daily_balances = (sync_start_date...Date.current).map do |date| + (sync_start_date..Date.current).map do |date| current_balance = calculate_balance_for_date(date, entries:, prior_balance:) prior_balance = current_balance build_balance(date, current_balance) end - - # Last balance of series is always equal to account balance - daily_balances << build_balance(Date.current, account.balance) end def calculate_converted_balances(balances) diff --git a/app/views/accounts/edit.html.erb b/app/views/accounts/edit.html.erb index 9f593f74..3660aa3f 100644 --- a/app/views/accounts/edit.html.erb +++ b/app/views/accounts/edit.html.erb @@ -6,7 +6,8 @@ <%= form_with model: @account, data: { turbo_frame: "_top" } do |f| %> - <%= f.text_field :name, label: "Name" %> + <%= f.text_field :name, label: t(".name") %> + <%= f.money_field :balance_money, label: t(".balance"), readonly_currency: true %>
<%= f.collection_select :institution_id, Current.family.institutions.alphabetically, :id, :name, { include_blank: t(".ungrouped"), label: t(".institution") } %> diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index 9892794e..e1712d53 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -44,6 +44,7 @@ <% end %>
+ <%= turbo_frame_tag "sync_message" do %> <%= render partial: "accounts/sync_message", locals: { is_syncing: @account.syncing? } %> <% end %> @@ -55,12 +56,18 @@
- <%= render partial: "shared/value_heading", locals: { - label: "Total Value", - period: @period, - value: @account.balance_money, - trend: @balance_series.trend - } %> + <%= tag.p t(".total_value"), class: "text-sm font-medium text-gray-500" %> + <%= tag.p format_money(@account.balance_money, precision: 0), class: "text-gray-900 text-3xl font-medium" %> +
+ <% if @balance_series.trend.direction.flat? %> + <%= tag.span t(".no_change"), class: "text-gray-500" %> + <% else %> + <%= tag.span format_money(@balance_series.trend.value), style: "color: #{@balance_series.trend.color}" %> + <%= tag.span "(#{@balance_series.trend.percent}%)", style: "color: #{@balance_series.trend.color}" %> + <% end %> + + <%= tag.span period_label(@period), class: "text-gray-500" %> +
<%= form_with url: account_path(@account), method: :get, class: "flex items-center gap-4", data: { controller: "auto-submit-form" } do %> <%= render partial: "shared/period_select", locals: { value: @period.name } %> @@ -71,11 +78,11 @@
- <% selected_tab = params[:tab] || "history" %> + <% selected_tab = params[:tab] || "value" %> -
- <%= link_to "History", account_path(tab: "history"), class: ["p-2 rounded-lg", "bg-gray-100": selected_tab == "history"] %> - <%= link_to "Transactions", account_path(tab: "transactions"), class: ["p-2 rounded-lg", "bg-gray-100": selected_tab == "transactions"] %> +
+ <%= link_to t(".value"), account_path(tab: "value"), class: ["px-2 py-1.5 rounded-md border border-transparent", "bg-white shadow-xs border-alpha-black-50": selected_tab == "value"] %> + <%= link_to t(".transactions"), account_path(tab: "transactions"), class: ["px-2 py-1.5 rounded-md border border-transparent", "bg-white shadow-xs border-alpha-black-50": selected_tab == "transactions"] %>
diff --git a/config/locales/views/accounts/en.yml b/config/locales/views/accounts/en.yml index 755b7b8e..686ebd8f 100644 --- a/config/locales/views/accounts/en.yml +++ b/config/locales/views/accounts/en.yml @@ -6,8 +6,10 @@ en: destroy: success: Account deleted successfully edit: + balance: Balance edit: Edit %{account} institution: Financial institution + name: Name ungrouped: "(none)" empty: empty_message: Add an account either via connection, importing or entering manually. @@ -58,9 +60,13 @@ en: confirm_title: Delete account? edit: Edit import: Import transactions + no_change: No change sync_message_missing_rates: Since exchange rates haven't been synced, balance graphs may not reflect accurate values. sync_message_unknown_error: An error has occurred during the sync. + total_value: Total Value + transactions: Transactions + value: Value summary: new: New account sync: diff --git a/test/controllers/accounts_controller_test.rb b/test/controllers/accounts_controller_test.rb index a7b32d09..31fd9b2f 100644 --- a/test/controllers/accounts_controller_test.rb +++ b/test/controllers/accounts_controller_test.rb @@ -46,6 +46,37 @@ class AccountsControllerTest < ActionDispatch::IntegrationTest } assert_redirected_to account_url(@account) + assert_enqueued_with job: AccountSyncJob + assert_equal "Account updated", flash[:notice] + end + + test "updates account balance by creating new valuation" do + assert_difference [ "Account::Entry.count", "Account::Valuation.count" ], 1 do + patch account_url(@account), params: { + account: { + balance: 10000 + } + } + end + + assert_redirected_to account_url(@account) + assert_enqueued_with job: AccountSyncJob + assert_equal "Account updated", flash[:notice] + end + + test "updates account balance by editing existing valuation for today" do + @account.entries.create! date: Date.current, amount: 6000, currency: "USD", entryable: Account::Valuation.new + + assert_no_difference [ "Account::Entry.count", "Account::Valuation.count" ] do + patch account_url(@account), params: { + account: { + balance: 10000 + } + } + end + + assert_redirected_to account_url(@account) + assert_enqueued_with job: AccountSyncJob assert_equal "Account updated", flash[:notice] end diff --git a/test/models/account/balance/syncer_test.rb b/test/models/account/balance/syncer_test.rb index c70f52e9..6d1e3b10 100644 --- a/test/models/account/balance/syncer_test.rb +++ b/test/models/account/balance/syncer_test.rb @@ -22,7 +22,8 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase syncer = Account::Balance::Syncer.new(@account) syncer.run - assert_equal [ 22000, 22000, @account.balance ], @account.balances.chronological.map(&:balance) + assert_equal 22000, @account.balance + assert_equal [ 22000, 22000, 22000 ], @account.balances.chronological.map(&:balance) end test "syncs account with transactions only" do @@ -32,7 +33,8 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase syncer = Account::Balance::Syncer.new(@account) syncer.run - assert_equal [ 19600, 19500, 19500, 20000, 20000, @account.balance ], @account.balances.chronological.map(&:balance) + assert_equal 20000, @account.balance + assert_equal [ 19600, 19500, 19500, 20000, 20000, 20000 ], @account.balances.chronological.map(&:balance) end test "syncs account with valuations and transactions" do @@ -44,7 +46,8 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase syncer = Account::Balance::Syncer.new(@account) syncer.run - assert_equal [ 20000, 20000, 20500, 20400, 25000, @account.balance ], @account.balances.chronological.map(&:balance) + assert_equal 25000, @account.balance + assert_equal [ 20000, 20000, 20500, 20400, 25000, 25000 ], @account.balances.chronological.map(&:balance) end test "syncs account with transactions in multiple currencies" do @@ -57,7 +60,8 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase syncer = Account::Balance::Syncer.new(@account) syncer.run - assert_equal [ 21000, 20900, 20600, 20000, @account.balance ], @account.balances.chronological.map(&:balance) + assert_equal 20000, @account.balance + assert_equal [ 21000, 20900, 20600, 20000, 20000 ], @account.balances.chronological.map(&:balance) end test "converts foreign account balances to family currency" do @@ -75,8 +79,9 @@ class Account::Balance::SyncerTest < ActiveSupport::TestCase usd_balances = @account.balances.where(currency: "USD").chronological.map(&:balance) eur_balances = @account.balances.where(currency: "EUR").chronological.map(&:balance) - assert_equal [ 21000, 20000, @account.balance ], eur_balances # native account balances - assert_equal [ 42000, 40000, @account.balance * 2 ], usd_balances # converted balances at rate of 2:1 + assert_equal 20000, @account.balance + assert_equal [ 21000, 20000, 20000 ], eur_balances # native account balances + assert_equal [ 42000, 40000, 40000 ], usd_balances # converted balances at rate of 2:1 end test "fails with error if exchange rate not available for any entry" do