Review fixes

app/controllers/accounts_controller.rb

@@ -56,10 +56,10 @@ def create
  @account = Current.family.accounts.build(account_params.except(:accountable_type))
  @account.accountable = Accountable.from_type(account_params[:accountable_type])&.new
 
- # TODO: Make sure `start_balance` is nil for connected accounts
- @account.start_balance = @account.balance
-
  if @account.save
+ @valuation = @account.valuations.new(date: account_params[:start_date] || Date.today, value: @account.balance, currency: @account.currency)
+ @valuation.save!
+
  redirect_to accounts_path, notice: t(".success")
  else
  render "new", status: :unprocessable_entity
@@ -89,6 +89,6 @@ def set_account
  end
 
  def account_params
- params.require(:account).permit(:name, :accountable_type, :balance, :currency, :subtype, :is_active)
+ params.require(:account).permit(:name, :accountable_type, :balance, :start_date, :currency, :subtype, :is_active)
  end
 end

app/models/account.rb

@@ -10,7 +10,7 @@ class Account < ApplicationRecord
  has_many :valuations, dependent: :destroy
  has_many :transactions, dependent: :destroy
 
- monetize :balance, :start_balance
+ monetize :balance
 
  enum :status, { ok: "ok", syncing: "syncing", error: "error" }, validate: true
 
@@ -40,11 +40,6 @@ def foreign_currency?
  currency != family.currency
  end
 
- # Whether this is a manual or connected (via a 3rd party provider) account
- def manual?
- start_balance.present?
- end
-
  def self.by_provider
  # TODO: When 3rd party providers are supported, dynamically load all providers and their accounts
  [ { name: "Manual accounts", accounts: all.order(balance: :desc).group_by(&:accountable_type) } ]

app/models/account/balance/calculator.rb

@@ -13,7 +13,7 @@ def initialize(account, options = {})
  def calculate
  prior_balance = implied_start_balance
 
- calculated_balances = ((@calc_start_date + 1.day)...Date.current).map do |date|
+ calculated_balances = ((@calc_start_date + 1.day)..Date.current).map do |date|
  valuation = normalized_valuations.find { |v| v["date"] == date }
 
  if valuation
@@ -30,8 +30,7 @@ def calculate
 
  @daily_balances = [
  { date: @calc_start_date, balance: implied_start_balance, currency: @account.currency, updated_at: Time.current },
- *calculated_balances,
- { date: Date.current, balance: @account.balance, currency: @account.currency, updated_at: Time.current } # Last balance must always match "source of truth"
+ *calculated_balances
  ]
 
  if @account.foreign_currency?
@@ -93,17 +92,16 @@ def implied_start_balance
  return @account.balance_on(@calc_start_date)
  end
 
- oldest_valuation_date = normalized_valuations.first&.dig("date")
- oldest_transaction_date = normalized_transactions.first&.dig("date")
- oldest_entry_date = [ oldest_valuation_date, oldest_transaction_date ].compact.min
+ net_transaction_multiplier = @account.classification == "liability" ? -1 : 1
 
- if oldest_entry_date.present? && oldest_entry_date == oldest_valuation_date
+ oldest_valuation_date = normalized_valuations.first&.dig("date")
+ if oldest_valuation_date.present?
+ net_transaction_flows = normalized_transactions.select { |t| t["date"] <= oldest_valuation_date }.sum { |t| t["amount"].to_d }
  oldest_valuation = normalized_valuations.find { |v| v["date"] == oldest_valuation_date }
- oldest_valuation["value"].to_d
+ oldest_valuation["value"].to_d + net_transaction_flows * net_transaction_multiplier
  else
  net_transaction_flows = normalized_transactions.sum { |t| t["amount"].to_d }
- net_transaction_flows *= -1 if @account.classification == "liability"
- @account.balance.to_d + net_transaction_flows
+ @account.balance.to_d + net_transaction_flows * net_transaction_multiplier
  end
  end
 end

app/models/account/syncable.rb

@@ -10,18 +10,16 @@ def sync(start_date = nil)
 
  sync_exchange_rates
 
- if self.manual?
- calculator = Account::Balance::ManualAccountBalanceCalculator.new(self)
- self.balance = calculator.calculate_current_balance
- self.save!
- end
-
  calc_start_date = start_date - 1.day if start_date.present? && self.balance_on(start_date - 1.day).present?
 
  calculator = Account::Balance::Calculator.new(self, { calc_start_date: })
  calculator.calculate
  self.balances.upsert_all(calculator.daily_balances, unique_by: :index_account_balances_on_account_id_date_currency_unique)
  self.balances.where("date < ?", effective_start_date).delete_all
+ new_balance = calculator.daily_balances.last[:balance]
+ self.balance = new_balance
+ self.save!
+
  update!(status: "ok", last_sync_date: Date.today)
  rescue => e
  update!(status: "error")

app/views/accounts/new.html.erb

@@ -67,9 +67,10 @@
  <%= form_with model: @account, url: accounts_path, scope: :account, html: { class: "m-5 mt-1 flex flex-col justify-between grow", data: { turbo: false } } do |f| %>
  <div class="space-y-4 grow">
  <%= f.hidden_field :accountable_type %>
- <%= f.text_field :name, placeholder: t("accounts.new.name.placeholder"), required: "required", label: t("accounts.new.name.label"), autofocus: true %>
+ <%= f.text_field :name, placeholder: t(".name.placeholder"), required: "required", label: t(".name.label"), autofocus: true %>
  <%= render "accounts/#{permitted_accountable_partial(@account.accountable_type)}", f: f %>
- <%= f.money_field :balance_money, label: "Balance", required: "required" %>
+ <%= f.money_field :balance_money, label: t(".balance.label"), required: "required" %>
+ <%= f.date_field :date, label: t(".start_date.label"), required: true, max: Date.today, value: Date.today %>
  </div>
  <%= f.submit "Add #{@account.accountable.model_name.human.downcase}" %>
  <% end %>

app/views/transactions/_form.html.erb

@@ -12,7 +12,7 @@
  <%= f.collection_select :account_id, Current.family.accounts.alphabetically, :id, :name, { prompt: t(".account_prompt"), label: t(".account") }, required: true %>
  <%= f.money_field :amount_money, label: t(".amount"), required: true %>
  <%= f.collection_select :category_id, Current.family.transaction_categories.alphabetically, :id, :name, { prompt: t(".category_prompt"), label: t(".category") }, required: true %>
- <%= f.date_field :date, label: t(".date"), required: true %>
+ <%= f.date_field :date, label: t(".date"), required: true, max: Date.today %>
  </section>
 
  <section>

app/views/transactions/show.html.erb

@@ -13,7 +13,7 @@
  <%= lucide_icon("chevron-right", class: "group-open:hidden text-gray-500 w-5 h-5") %>
  </div>
  </summary>
- <%= f.date_field :date, label: "Date", "data-auto-submit-form-target": "auto" %>
+ <%= f.date_field :date, label: "Date", max: Date.today, "data-auto-submit-form-target": "auto" %>
  <div class="h-2"></div>
  <%= f.collection_select :account_id, Current.family.accounts, :id, :name, { prompt: "Select an Account", label: "Account", class: "text-gray-400" }, {class: "form-field__input cursor-not-allowed text-gray-400", disabled: "disabled"} %>
  </details>

app/views/valuations/_form_row.html.erb

@@ -6,7 +6,7 @@
  </div>
  </div>
  <div class="flex items-center justify-between grow">
- <%= f.date_field :date, required: "required", class: "border border-alpha-black-200 bg-white rounded-lg shadow-xs min-w-[200px] px-3 py-1.5 text-gray-900 text-sm" %>
+ <%= f.date_field :date, required: "required", max: Date.today, class: "border border-alpha-black-200 bg-white rounded-lg shadow-xs min-w-[200px] px-3 py-1.5 text-gray-900 text-sm" %>
  <%= f.number_field :value, required: "required", placeholder: "0.00", step: "0.01", class: "bg-white border border-alpha-black-200 rounded-lg shadow-xs text-gray-900 text-sm px-3 py-1.5 text-right" %>
  </div>
  <div class="w-[296px] flex gap-2 justify-end items-center">

config/locales/views/account/en.yml

@@ -21,6 +21,10 @@ en:
  name:
  label: Account name
  placeholder: Example account name
+ start_date:
+ label: Start date
+ balance:
+ label: Balance
  select_accountable_type: What would you like to add?
  title: Add an account
  show:

lib/tasks/demo_data.rake

@@ -60,7 +60,7 @@ namespace :demo_data do
  crypto = Account.create(name: "Bitcoin Account", family: family, accountable: Account::Crypto.new, balance: 0.1, currency: "BTC")
  mortgage = Account.create(name: "Demo Mortgage", family: family, accountable: Account::Loan.new, balance: 450000, currency: "USD")
  main_car = Account.create(name: "Demo Main Car", family: family, accountable: Account::Vehicle.new, balance: 25000, currency: "USD")
- cash = Account.create(name: "Demo Physical Cash", family: family, accountable: Account::OtherAsset.new, start_balance: 500, balance: 500, currency: "USD")
+ cash = Account.create(name: "Demo Physical Cash", family: family, accountable: Account::OtherAsset.new, balance: 500, currency: "USD")
  car_loan = Account.create(name: "Demo Car Loan", family: family, accountable: Account::Loan.new, balance: 10000, currency: "USD")
  house = Account.create(name: "Demo Primary Residence", family: family, accountable: Account::Property.new, balance: 2500000, currency: "USD")
  personal_iou = Account.create(name: "Demo Personal IOU", family: family, accountable: Account::OtherLiability.new, balance: 1000, currency: "USD")

test/fixtures/account/depositories.yml

@@ -6,7 +6,3 @@ eur_checking:
  id: "123e4567-e89b-12d3-a456-426614174004"
 multi_currency:
  id: "123e4567-e89b-12d3-a456-426614174005"
-manual:
- id: "123e4567-e89b-12d3-a456-426614174006"
-manual_with_valuation_overrides:
- id: "123e4567-e89b-12d3-a456-426614174007"

test/fixtures/accounts.yml

@@ -47,19 +47,3 @@ multi_currency:
  accountable_type: Account::Depository
  accountable_id: "123e4567-e89b-12d3-a456-426614174005"
 
-# Manual account (not connected to a 3rd party provider)
-manual:
- family: dylan_family
- name: Physical Cash
- balance: 5000
- start_balance: 200
- accountable_type: Account::OtherAsset
- accountable_id: "123e4567-e89b-12d3-a456-426614174006"
-
-manual_with_valuation_overrides:
- family: dylan_family
- name: Physical Cash with valuation overrides
- balance: 1000
- start_balance: 1000
- accountable_type: Account::OtherAsset
- accountable_id: "123e4567-e89b-12d3-a456-426614174007"

test/fixtures/transactions.yml

@@ -153,33 +153,3 @@ multi_currency_four:
  amount: -200
  currency: USD
  account: multi_currency
-
-# manual account transactions
-manual_one:
- name: Bus ticket
- date: <%= 5.days.ago.to_date %>
- amount: 5
- account: manual
- currency: USD
-
-manual_two:
- name: Pocket money
- date: <%= 10.days.ago.to_date %>
- amount: -50
- account: manual
- currency: USD
-
-# manual account with valuation overrides transactions
-manual_with_valuation_one:
- name: Bus ticket
- date: <%= 5.days.ago.to_date %>
- amount: 5
- account: manual_with_valuation_overrides
- currency: USD
-
-manual_with_valuation_two:
- name: Pocket money
- date: <%= 10.days.ago.to_date %>
- amount: -50
- account: manual_with_valuation_overrides
- currency: USD

test/fixtures/valuations.yml

@@ -29,15 +29,3 @@ savings_three:
  value: 21000
  date: <%= 25.days.ago.to_date %>
  account: savings_with_valuation_overrides
-
-# For manual account that has valuations and transactions
-manual_one:
- value: 500
- date: <%= 7.days.ago.to_date %>
- account: manual_with_valuation_overrides
-
-manual_two:
- value: 5000
- date: <%= 12.days.ago.to_date %>
- account: manual_with_valuation_overrides
-

test/models/account/syncable_test.rb

@@ -78,18 +78,4 @@ class Account::SyncableTest < ActiveSupport::TestCase
 
  assert_equal 31, account.balances.count
  end
-
- test "current balance of a connected account is unchanged after syncing" do
- account = accounts(:checking)
- assert_no_difference "account.balance" do
- account.sync
- end
- end
-
- test "current balance of a manual account is updated after syncing" do
- account = accounts(:manual)
- assert_changes "account.balance", to: 245 do
- account.sync
- end
- end
 end

test/models/account_test.rb

@@ -87,7 +87,7 @@ def setup
  assert_equal 0, properties.children.count
  assert_equal 0, vehicles.children.count
  assert_equal 0, investments.children.count
- assert_equal 3, other_assets.children.count
+ assert_equal 1, other_assets.children.count
 
  assert_equal 1, credits.children.count
  assert_equal 0, loans.children.count