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

@@ -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
 

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

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

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)

app/views/accounts/edit.html.erb

@@ -6,7 +6,8 @@
     </header>
 
     <%= 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 %>
 
       <div class="relative">
         <%= f.collection_select :institution_id, Current.family.institutions.alphabetically, :id, :name, { include_blank: t(".ungrouped"), label: t(".institution") } %>

app/views/accounts/show.html.erb

@@ -44,6 +44,7 @@
       <% end %>
     </div>
   </div>
+
   <%= turbo_frame_tag "sync_message" do %>
     <%= render partial: "accounts/sync_message", locals: { is_syncing: @account.syncing? } %>
   <% end %>
@@ -55,12 +56,18 @@
   <div class="bg-white shadow-xs rounded-xl border border-alpha-black-25 rounded-lg">
     <div class="p-4 flex justify-between">
       <div class="space-y-2">
-        <%= render partial: "shared/value_heading", locals: {
+        <%= tag.p t(".total_value"), class: "text-sm font-medium text-gray-500" %>
-          label: "Total Value",
+        <%= tag.p format_money(@account.balance_money, precision: 0), class: "text-gray-900 text-3xl font-medium" %>
-          period: @period,
+        <div>
-          value: @account.balance_money,
+          <% if @balance_series.trend.direction.flat? %>
-          trend: @balance_series.trend
+            <%= 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" %>
+        </div>
       </div>
       <%= 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 @@
     </div>
   </div>
 
-  <% selected_tab = params[:tab] || "history" %>
+  <% selected_tab = params[:tab] || "value" %>
 
-  <div class="flex gap-1 text-sm text-gray-900 font-medium mb-4">
+  <div class="flex gap-2 text-sm text-gray-900 font-medium mb-4">
-    <%= link_to "History", account_path(tab: "history"), class: ["p-2 rounded-lg", "bg-gray-100": selected_tab == "history"] %>
+    <%= 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 "Transactions", account_path(tab: "transactions"), class: ["p-2 rounded-lg", "bg-gray-100": selected_tab == "transactions"] %>
+    <%= 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"] %>
   </div>
 
   <div class="min-h-[800px]">

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:

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
 

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 20000, @account.balance
-    assert_equal [ 42000, 40000, @account.balance * 2 ], usd_balances # converted balances at rate of 2:1
+    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