From c60ddaec1d68b4267a5eb80ce876a0fab7d145fa Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Mon, 11 Mar 2024 16:32:13 -0400 Subject: [PATCH] Multi-currency part 1 (#542) * Add family snapshots table * Add snapshot method, clean up family expected results * Remove old sync trigger --- app/controllers/pages_controller.rb | 7 +- app/models/account.rb | 84 +++++++++---------- app/models/account/syncable.rb | 6 +- app/models/family.rb | 80 +++++++----------- app/views/accounts/show.html.erb | 2 +- ...80636_add_sync_status_fields_to_account.rb | 13 +++ db/schema.rb | 10 ++- test/fixtures/family/expected_snapshots.csv | 32 +++++++ test/models/family_test.rb | 76 +++++++++-------- 9 files changed, 169 insertions(+), 141 deletions(-) create mode 100644 db/migrate/20240309180636_add_sync_status_fields_to_account.rb create mode 100644 test/fixtures/family/expected_snapshots.csv diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 25cee640..b2a3bb98 100644 --- a/app/controllers/pages_controller.rb +++ b/app/controllers/pages_controller.rb @@ -3,9 +3,10 @@ class PagesController < ApplicationController before_action :authenticate_user! def dashboard - @asset_series = Current.family.asset_series(@period) - @liability_series = Current.family.liability_series(@period) - @net_worth_series = Current.family.net_worth_series(@period) + snapshot = Current.family.snapshot(@period) + @net_worth_series = snapshot[:net_worth_series] + @asset_series = snapshot[:asset_series] + @liability_series = snapshot[:liability_series] @account_groups = Current.family.accounts.by_group(@period) end end diff --git a/app/models/account.rb b/app/models/account.rb index ae1655c7..a882cf53 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -7,7 +7,11 @@ class Account < ApplicationRecord has_many :valuations has_many :transactions + enum :status, { ok: "ok", syncing: "syncing", error: "error" }, validate: true + scope :active, -> { where(is_active: true) } + scope :assets, -> { where(classification: "asset") } + scope :liabilities, -> { where(classification: "liability") } delegated_type :accountable, types: Accountable::TYPES, dependent: :destroy @@ -28,6 +32,10 @@ class Account < ApplicationRecord [ { name: "Manual accounts", accounts: all.order(balance: :desc).group_by(&:accountable_type) } ] end + def self.some_syncing? + exists?(status: "syncing") + end + # TODO: We will need a better way to encapsulate large queries & transformation logic, but leaving all in one spot until # we have a better understanding of the requirements def self.by_group(period = Period.all) @@ -70,50 +78,8 @@ class Account < ApplicationRecord total_liabilities = liabilities.sum(&:end_balance) { - asset: { - total: total_assets, - groups: assets.group_by(&:accountable_type).transform_values do |rows| - end_balance = rows.sum(&:end_balance) - start_balance = rows.sum(&:start_balance) - { - start_balance: start_balance, - end_balance: end_balance, - allocation: (end_balance / total_assets * 100).round(2), - trend: Trend.new(current: end_balance, previous: start_balance, type: "asset"), - accounts: rows.map do |account| - { - name: account.name, - start_balance: account.start_balance, - end_balance: account.end_balance, - allocation: (account.end_balance / total_assets * 100).round(2), - trend: Trend.new(current: account.end_balance, previous: account.start_balance, type: "asset") - } - end - } - end - }, - liability: { - total: total_liabilities, - groups: liabilities.group_by(&:accountable_type).transform_values do |rows| - end_balance = rows.sum(&:end_balance) - start_balance = rows.sum(&:start_balance) - { - start_balance: start_balance, - end_balance: end_balance, - allocation: (end_balance / total_liabilities * 100).round(2), - trend: Trend.new(current: end_balance, previous: start_balance, type: "liability"), - accounts: rows.map do |account| - { - name: account.name, - start_balance: account.start_balance, - end_balance: account.end_balance, - allocation: (account.end_balance / total_liabilities * 100).round(2), - trend: Trend.new(current: account.end_balance, previous: account.start_balance, type: "liability") - } - end - } - end - } + asset: build_group_summary(assets, "asset"), + liability: build_group_summary(liabilities, "liability") } end @@ -128,4 +94,34 @@ class Account < ApplicationRecord self.converted_currency = self.family.currency end end + + def self.build_group_summary(accounts, classification) + total_balance = accounts.sum(&:end_balance) + { + total: total_balance, + groups: accounts.group_by(&:accountable_type).transform_values do |rows| + build_account_summary(rows, total_balance, classification) + end + } + end + + def self.build_account_summary(accounts, total_balance, classification) + end_balance = accounts.sum(&:end_balance) + start_balance = accounts.sum(&:start_balance) + { + start_balance: start_balance, + end_balance: end_balance, + allocation: (end_balance / total_balance * 100).round(2), + trend: Trend.new(current: end_balance, previous: start_balance, type: classification), + accounts: accounts.map do |account| + { + name: account.name, + start_balance: account.start_balance, + end_balance: account.end_balance, + allocation: (account.end_balance / total_balance * 100).round(2), + trend: Trend.new(current: account.end_balance, previous: account.start_balance, type: classification) + } + end + } + end end diff --git a/app/models/account/syncable.rb b/app/models/account/syncable.rb index 45efe6d8..7e0b0d8b 100644 --- a/app/models/account/syncable.rb +++ b/app/models/account/syncable.rb @@ -6,13 +6,13 @@ module Account::Syncable end def sync - update!(status: "SYNCING") + update!(status: "syncing") synced_daily_balances = Account::BalanceCalculator.new(self).daily_balances self.balances.upsert_all(synced_daily_balances, unique_by: :index_account_balances_on_account_id_and_date) self.balances.where("date < ?", effective_start_date).delete_all - update!(status: "OK") + update!(status: "ok") rescue => e - update!(status: "ERROR") + update!(status: "error") Rails.logger.error("Failed to sync account #{id}: #{e.message}") end diff --git a/app/models/family.rb b/app/models/family.rb index e69620fc..3f81b93b 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -4,65 +4,41 @@ class Family < ApplicationRecord has_many :transactions, through: :accounts has_many :transaction_categories, dependent: :destroy, class_name: "Transaction::Category" + def snapshot(period = Period.all) + query = accounts.active.joins(:balances) + .where("account_balances.currency = ?", self.currency) + .select( + "account_balances.currency", + "account_balances.date", + "SUM(CASE WHEN accounts.classification = 'liability' THEN account_balances.balance ELSE 0 END) AS liabilities", + "SUM(CASE WHEN accounts.classification = 'asset' THEN account_balances.balance ELSE 0 END) AS assets", + "SUM(CASE WHEN accounts.classification = 'asset' THEN account_balances.balance WHEN accounts.classification = 'liability' THEN -account_balances.balance ELSE 0 END) AS net_worth", + ) + .group("account_balances.date, account_balances.currency") + .order("account_balances.date") + + query = query.where("account_balances.date BETWEEN ? AND ?", period.date_range.begin, period.date_range.end) if period.date_range + + { + asset_series: MoneySeries.new(query, { trend_type: :asset, amount_accessor: "assets" }), + liability_series: MoneySeries.new(query, { trend_type: :liability, amount_accessor: "liabilities" }), + net_worth_series: MoneySeries.new(query, { trend_type: :asset, amount_accessor: "net_worth" }) + } + end + + def effective_start_date + accounts.active.joins(:balances).minimum("account_balances.date") || Date.current + end + def net_worth accounts.active.sum("CASE WHEN classification = 'asset' THEN balance ELSE -balance END") end def assets - accounts.active.where(classification: "asset").sum(:balance) + accounts.active.assets.sum(:balance) end def liabilities - accounts.active.where(classification: "liability").sum(:balance) - end - - def net_worth_series(period = nil) - query = accounts.joins(:balances) - .select("account_balances.date, SUM(CASE WHEN accounts.classification = 'asset' THEN account_balances.balance ELSE -account_balances.balance END) AS balance, 'USD' as currency") - .group("account_balances.date") - .order("account_balances.date ASC") - - if period && period.date_range - query = query.where("account_balances.date BETWEEN ? AND ?", period.date_range.begin, period.date_range.end) - end - - MoneySeries.new( - query, - { trend_type: "asset" } - ) - end - - def asset_series(period = nil) - query = accounts.joins(:balances) - .select("account_balances.date, SUM(account_balances.balance) AS balance, 'asset' AS classification, 'USD' AS currency") - .group("account_balances.date") - .order("account_balances.date ASC") - .where(classification: "asset") - - if period && period.date_range - query = query.where("account_balances.date BETWEEN ? AND ?", period.date_range.begin, period.date_range.end) - end - - MoneySeries.new( - query, - { trend_type: "asset" } - ) - end - - def liability_series(period = nil) - query = accounts.joins(:balances) - .select("account_balances.date, SUM(account_balances.balance) AS balance, 'liability' AS classification, 'USD' AS currency") - .group("account_balances.date") - .order("account_balances.date ASC") - .where(classification: "liability") - - if period && period.date_range - query = query.where("account_balances.date BETWEEN ? AND ?", period.date_range.begin, period.date_range.end) - end - - MoneySeries.new( - query, - { trend_type: "liability" } - ) + accounts.active.liabilities.sum(:balance) end end diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index c7780643..11123b4c 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -21,7 +21,7 @@ <%= turbo_frame_tag "sync_message" do %> - <%= render partial: "accounts/sync_message", locals: { is_syncing: @account.status == "SYNCING" } %> + <%= render partial: "accounts/sync_message", locals: { is_syncing: @account.syncing? } %> <% end %>
diff --git a/db/migrate/20240309180636_add_sync_status_fields_to_account.rb b/db/migrate/20240309180636_add_sync_status_fields_to_account.rb new file mode 100644 index 00000000..cdeb8437 --- /dev/null +++ b/db/migrate/20240309180636_add_sync_status_fields_to_account.rb @@ -0,0 +1,13 @@ +class AddSyncStatusFieldsToAccount < ActiveRecord::Migration[7.2] + def change + create_enum :account_status, %w[ok syncing error] + + remove_column :accounts, :status, :string + + change_table :accounts do |t| + t.enum :status, enum_type: :account_status, default: "ok", null: false + t.jsonb :sync_warnings, default: '[]', null: false + t.jsonb :sync_errors, default: '[]', null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 21376fb1..aed174db 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,11 +10,15 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.2].define(version: 2024_03_08_214956) do +ActiveRecord::Schema[7.2].define(version: 2024_03_09_180636) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" + # Custom types defined in this database. + # Note that some types may not work with other database engines. Be careful if changing database. + create_enum "account_status", ["ok", "syncing", "error"] + create_table "account_balances", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "account_id", null: false t.date "date", null: false @@ -78,9 +82,11 @@ ActiveRecord::Schema[7.2].define(version: 2024_03_08_214956) do t.string "currency", default: "USD" t.decimal "converted_balance", precision: 19, scale: 4, default: "0.0" t.string "converted_currency", default: "USD" - t.string "status", default: "OK" t.virtual "classification", type: :string, as: "\nCASE\n WHEN ((accountable_type)::text = ANY (ARRAY[('Account::Loan'::character varying)::text, ('Account::Credit'::character varying)::text, ('Account::OtherLiability'::character varying)::text])) THEN 'liability'::text\n ELSE 'asset'::text\nEND", stored: true t.boolean "is_active", default: true, null: false + t.enum "status", default: "ok", null: false, enum_type: "account_status" + t.jsonb "sync_warnings", default: "[]", null: false + t.jsonb "sync_errors", default: "[]", null: false t.index ["accountable_type"], name: "index_accounts_on_accountable_type" t.index ["family_id"], name: "index_accounts_on_family_id" end diff --git a/test/fixtures/family/expected_snapshots.csv b/test/fixtures/family/expected_snapshots.csv new file mode 100644 index 00000000..2bb2bfb2 --- /dev/null +++ b/test/fixtures/family/expected_snapshots.csv @@ -0,0 +1,32 @@ +date_offset,net_worth,assets,liabilities,depositories,investments,loans,credits,properties,vehicles,other_assets,other_liabilities +-30,24610,25650,1040,25250,0,0,1040,0,0,400,0 +-29,25195,26135,940,25735,0,0,940,0,0,400,0 +-28,25195,26135,940,25735,0,0,940,0,0,400,0 +-27,25195,26135,940,25735,0,0,940,0,0,400,0 +-26,25195,26135,940,25735,0,0,940,0,0,400,0 +-25,24445,25385,940,24985,0,0,940,0,0,400,0 +-24,24445,25385,940,24985,0,0,940,0,0,400,0 +-23,24445,25385,940,24985,0,0,940,0,0,400,0 +-22,25520,26460,940,26060,0,0,940,0,0,400,0 +-21,25520,26460,940,26060,0,0,940,0,0,400,0 +-20,25520,26460,940,26060,0,0,940,0,0,400,0 +-19,25520,26460,940,26060,0,0,940,0,0,400,0 +-18,23520,24460,940,24060,0,0,940,0,0,400,0 +-17,23520,24460,940,24060,0,0,940,0,0,400,0 +-16,23520,24460,940,24060,0,0,940,0,0,400,0 +-15,23480,24440,960,24040,0,0,960,0,0,400,0 +-14,23480,24440,960,24040,0,0,960,0,0,400,0 +-13,23480,24440,960,24040,0,0,960,0,0,400,0 +-12,24220,25210,990,24510,0,0,990,0,0,700,0 +-11,24220,25210,990,24510,0,0,990,0,0,700,0 +-10,24220,25210,990,24510,0,0,990,0,0,700,0 +-9,24220,25210,990,24510,0,0,990,0,0,700,0 +-8,24220,25210,990,24510,0,0,990,0,0,700,0 +-7,24220,25210,990,24510,0,0,990,0,0,700,0 +-6,24220,25210,990,24510,0,0,990,0,0,700,0 +-5,24400,25400,1000,24700,0,0,1000,0,0,700,0 +-4,24250,25250,1000,24700,0,0,1000,0,0,550,0 +-3,25050,26050,1000,25500,0,0,1000,0,0,550,0 +-2,25050,26050,1000,25500,0,0,1000,0,0,550,0 +-1,25050,26050,1000,25500,0,0,1000,0,0,550,0 +0,24550,25550,1000,25000,0,0,1000,0,0,550,0 \ No newline at end of file diff --git a/test/models/family_test.rb b/test/models/family_test.rb index b0e132d4..88ac1086 100644 --- a/test/models/family_test.rb +++ b/test/models/family_test.rb @@ -1,4 +1,5 @@ require "test_helper" +require "csv" class FamilyTest < ActiveSupport::TestCase def setup @@ -48,6 +49,45 @@ class FamilyTest < ActiveSupport::TestCase assert_equal BigDecimal("24550"), @family.net_worth end + test "should calculate snapshot correctly" do + # See this Google Sheet for calculations and expected results for dylan_family: + # https://docs.google.com/spreadsheets/d/18LN5N-VLq4b49Mq1fNwF7_eBiHSQB46qQduRtdAEN98/edit?usp=sharing + expected_snapshots = CSV.read("test/fixtures/family/expected_snapshots.csv", headers: true).map do |row| + { + "date" => (Date.current + row["date_offset"].to_i.days).to_date, + "net_worth" => row["net_worth"], + "assets" => row["assets"], + "liabilities" => row["liabilities"] + } + end + + asset_series = @family.snapshot[:asset_series] + liability_series = @family.snapshot[:liability_series] + net_worth_series = @family.snapshot[:net_worth_series] + + assert_equal expected_snapshots.count, asset_series.data.count + assert_equal expected_snapshots.count, liability_series.data.count + assert_equal expected_snapshots.count, net_worth_series.data.count + + expected_snapshots.each_with_index do |row, index| + expected = { + date: row["date"], + assets: row["assets"].to_d, + liabilities: row["liabilities"].to_d, + net_worth: row["net_worth"].to_d + } + + actual = { + date: asset_series.data[index][:date], + assets: asset_series.data[index][:value].amount, + liabilities: liability_series.data[index][:value].amount, + net_worth: net_worth_series.data[index][:value].amount + } + + assert_equal expected, actual + end + end + test "should exclude disabled accounts from calculations" do assets_before = @family.assets liabilities_before = @family.liabilities @@ -64,42 +104,6 @@ class FamilyTest < ActiveSupport::TestCase assert_equal net_worth_before - disabled_checking.balance + disabled_cc.balance, @family.net_worth end - test "calculates asset series" do - # Sum of expected balances for all asset accounts in balance_calculator_test.rb - expected_balances = [ - 25650, 26135, 26135, 26135, 26135, 25385, 25385, 25385, 26460, 26460, - 26460, 26460, 24460, 24460, 24460, 24440, 24440, 24440, 25210, 25210, - 25210, 25210, 25210, 25210, 25210, 25400, 25250, 26050, 26050, 26050, - 25550 - ].map(&:to_d) - - assert_equal expected_balances, @family.asset_series.data.map { |b| b[:value].amount } - end - - test "calculates liability series" do - # Sum of expected balances for all liability accounts in balance_calculator_test.rb - expected_balances = [ - 1040, 940, 940, 940, 940, 940, 940, 940, 940, 940, - 940, 940, 940, 940, 940, 960, 960, 960, 990, 990, - 990, 990, 990, 990, 990, 1000, 1000, 1000, 1000, 1000, - 1000 - ].map(&:to_d) - - assert_equal expected_balances, @family.liability_series.data.map { |b| b[:value].amount } - end - - test "calculates net worth" do - # Net difference between asset and liability series above - expected_balances = [ - 24610, 25195, 25195, 25195, 25195, 24445, 24445, 24445, 25520, 25520, - 25520, 25520, 23520, 23520, 23520, 23480, 23480, 23480, 24220, 24220, - 24220, 24220, 24220, 24220, 24220, 24400, 24250, 25050, 25050, 25050, - 24550 - ].map(&:to_d) - - assert_equal expected_balances, @family.net_worth_series.data.map { |b| b[:value].amount } - end - test "calculates balances by type" do verify_balances_by_type( period: Period.all,