From 47aeaf8cea97cee50fb2c8dd09b92452a582d614 Mon Sep 17 00:00:00 2001 From: Alex Hatzenbuhler Date: Tue, 22 Apr 2025 13:10:50 -0500 Subject: [PATCH] Add nice formatting for subtypes on account list (#2138) * Add nice formatting for subtypes on account list * Fix rubocop linting errors * Implement better mapping * Fix rubocop linting * Add short and long versions of subtypes * Simplify subtype reference Co-authored-by: Zach Gollwitzer Signed-off-by: Alex Hatzenbuhler * Simplify reference logic, add a small test * Fix test * Fix tests --------- Signed-off-by: Alex Hatzenbuhler Co-authored-by: Zach Gollwitzer --- app/models/account.rb | 10 +++++++ app/models/balance_sheet.rb | 1 + app/models/concerns/accountable.rb | 21 ++++++++++++++ app/models/depository.rb | 8 +++--- app/models/investment.rb | 28 +++++++++---------- app/models/property.rb | 16 +++++------ app/views/accounts/_account.html.erb | 3 ++ .../accounts/_accountable_group.html.erb | 2 +- app/views/depositories/_form.html.erb | 2 +- app/views/investments/_form.html.erb | 2 +- app/views/properties/_form.html.erb | 2 +- test/models/account_test.rb | 18 ++++++++++++ 12 files changed, 83 insertions(+), 30 deletions(-) diff --git a/app/models/account.rb b/app/models/account.rb index 6317fb94..f07b442f 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -155,6 +155,16 @@ class Account < ApplicationRecord first_valuation&.amount_money || balance_money end + # Get short version of the subtype label + def short_subtype_label + accountable_class.short_subtype_label_for(subtype) || accountable_class.display_name + end + + # Get long version of the subtype label + def long_subtype_label + accountable_class.long_subtype_label_for(subtype) || accountable_class.display_name + end + private def sync_balances strategy = linked? ? :reverse : :forward diff --git a/app/models/balance_sheet.rb b/app/models/balance_sheet.rb index f6b07f16..21b4aeca 100644 --- a/app/models/balance_sheet.rb +++ b/app/models/balance_sheet.rb @@ -59,6 +59,7 @@ class BalanceSheet account.define_singleton_method(:weight) do classification_total.zero? ? 0 : account.converted_balance / classification_total.to_d * 100 end + account end.sort_by(&:weight).reverse ) diff --git a/app/models/concerns/accountable.rb b/app/models/concerns/accountable.rb index beb3cf23..9d0ecb34 100644 --- a/app/models/concerns/accountable.rb +++ b/app/models/concerns/accountable.rb @@ -3,6 +3,9 @@ module Accountable TYPES = %w[Depository Investment Crypto Property Vehicle OtherAsset CreditCard Loan OtherLiability] + # Define empty hash to ensure all accountables have this defined + SUBTYPES = {}.freeze + def self.from_type(type) return nil unless TYPES.include?(type) type.constantize @@ -27,6 +30,24 @@ module Accountable raise NotImplementedError, "Accountable must implement #color" end + # Given a subtype, look up the label for this accountable type + def subtype_label_for(subtype, format: :short) + return nil if subtype.nil? + + label_type = format == :long ? :long : :short + self::SUBTYPES[subtype]&.fetch(label_type, nil) + end + + # Convenience method for getting the short label + def short_subtype_label_for(subtype) + subtype_label_for(subtype, format: :short) + end + + # Convenience method for getting the long label + def long_subtype_label_for(subtype) + subtype_label_for(subtype, format: :long) + end + def favorable_direction classification == "asset" ? "up" : "down" end diff --git a/app/models/depository.rb b/app/models/depository.rb index 0e03547c..577a061c 100644 --- a/app/models/depository.rb +++ b/app/models/depository.rb @@ -1,10 +1,10 @@ class Depository < ApplicationRecord include Accountable - SUBTYPES = [ - [ "Checking", "checking" ], - [ "Savings", "savings" ] - ].freeze + SUBTYPES = { + "checking" => { short: "Checking", long: "Checking" }, + "savings" => { short: "Savings", long: "Savings" } + }.freeze class << self def display_name diff --git a/app/models/investment.rb b/app/models/investment.rb index 1dabae3e..6b2518ce 100644 --- a/app/models/investment.rb +++ b/app/models/investment.rb @@ -1,20 +1,20 @@ class Investment < ApplicationRecord include Accountable - SUBTYPES = [ - [ "Brokerage", "brokerage" ], - [ "Pension", "pension" ], - [ "Retirement", "retirement" ], - [ "401(k)", "401k" ], - [ "Traditional 401(k)", "traditional_401k" ], - [ "Roth 401(k)", "roth_401k" ], - [ "529 Plan", "529_plan" ], - [ "Health Savings Account", "hsa" ], - [ "Mutual Fund", "mutual_fund" ], - [ "Traditional IRA", "traditional_ira" ], - [ "Roth IRA", "roth_ira" ], - [ "Angel", "angel" ] - ].freeze + SUBTYPES = { + "brokerage" => { short: "Brokerage", long: "Brokerage" }, + "pension" => { short: "Pension", long: "Pension" }, + "retirement" => { short: "Retirement", long: "Retirement" }, + "401k" => { short: "401(k)", long: "401(k)" }, + "traditional_401k" => { short: "Traditional 401(k)", long: "Traditional 401(k)" }, + "roth_401k" => { short: "Roth 401(k)", long: "Roth 401(k)" }, + "529_plan" => { short: "529 Plan", long: "529 Plan" }, + "hsa" => { short: "HSA", long: "Health Savings Account" }, + "mutual_fund" => { short: "Mutual Fund", long: "Mutual Fund" }, + "traditional_ira" => { short: "Traditional IRA", long: "Traditional IRA" }, + "roth_ira" => { short: "Roth IRA", long: "Roth IRA" }, + "angel" => { short: "Angel", long: "Angel" } + }.freeze class << self def color diff --git a/app/models/property.rb b/app/models/property.rb index 42d2979e..a530f8b1 100644 --- a/app/models/property.rb +++ b/app/models/property.rb @@ -1,14 +1,14 @@ class Property < ApplicationRecord include Accountable - SUBTYPES = [ - [ "Single Family Home", "single_family_home" ], - [ "Multi-Family Home", "multi_family_home" ], - [ "Condominium", "condominium" ], - [ "Townhouse", "townhouse" ], - [ "Investment Property", "investment_property" ], - [ "Second Home", "second_home" ] - ] + SUBTYPES = { + "single_family_home" => { short: "Single Family Home", long: "Single Family Home" }, + "multi_family_home" => { short: "Multi-Family Home", long: "Multi-Family Home" }, + "condominium" => { short: "Condo", long: "Condominium" }, + "townhouse" => { short: "Townhouse", long: "Townhouse" }, + "investment_property" => { short: "Investment Property", long: "Investment Property" }, + "second_home" => { short: "Second Home", long: "Second Home" } + }.freeze has_one :address, as: :addressable, dependent: :destroy diff --git a/app/views/accounts/_account.html.erb b/app/views/accounts/_account.html.erb index afbd6fb3..07236d6c 100644 --- a/app/views/accounts/_account.html.erb +++ b/app/views/accounts/_account.html.erb @@ -17,6 +17,9 @@

<% else %> <%= link_to account.name, account, class: [(account.is_active ? "text-primary" : "text-subdued"), "text-sm font-medium hover:underline"], data: { turbo_frame: "_top" } %> + <% if account.long_subtype_label %> +

<%= account.long_subtype_label %>

+ <% end %> <% end %> diff --git a/app/views/accounts/_accountable_group.html.erb b/app/views/accounts/_accountable_group.html.erb index b9676a9a..3e2a26ee 100644 --- a/app/views/accounts/_accountable_group.html.erb +++ b/app/views/accounts/_accountable_group.html.erb @@ -24,7 +24,7 @@
<%= tag.p account.name, class: "text-sm text-primary font-medium mb-0.5 truncate" %> - <%= tag.p account.subtype&.humanize.presence || account_group.name, class: "text-sm text-secondary truncate" %> + <%= tag.p account.short_subtype_label, class: "text-sm text-secondary truncate" %>
diff --git a/app/views/depositories/_form.html.erb b/app/views/depositories/_form.html.erb index 1cb9d1aa..df853e8d 100644 --- a/app/views/depositories/_form.html.erb +++ b/app/views/depositories/_form.html.erb @@ -2,6 +2,6 @@ <%= render "accounts/form", account: account, url: url do |form| %> <%= form.select :subtype, - Depository::SUBTYPES, + Depository::SUBTYPES.map { |k, v| [v[:long], k] }, { label: true, prompt: t("depositories.form.subtype_prompt"), include_blank: t("depositories.form.none") } %> <% end %> diff --git a/app/views/investments/_form.html.erb b/app/views/investments/_form.html.erb index e6b2dc46..a9387af1 100644 --- a/app/views/investments/_form.html.erb +++ b/app/views/investments/_form.html.erb @@ -2,6 +2,6 @@ <%= render "accounts/form", account: account, url: url do |form| %> <%= form.select :subtype, - Investment::SUBTYPES, + Investment::SUBTYPES.map { |k, v| [v[:long], k] }, { label: true, prompt: t("investments.form.subtype_prompt"), include_blank: t("investments.form.none") } %> <% end %> diff --git a/app/views/properties/_form.html.erb b/app/views/properties/_form.html.erb index c26c71ba..61b6cb59 100644 --- a/app/views/properties/_form.html.erb +++ b/app/views/properties/_form.html.erb @@ -2,7 +2,7 @@ <%= render "accounts/form", account: account, url: url do |form| %> <%= form.select :subtype, - Property::SUBTYPES, + Property::SUBTYPES.map { |k, v| [v[:long], k] }, { label: true, prompt: t("properties.form.subtype_prompt"), include_blank: t("properties.form.none") } %>
diff --git a/test/models/account_test.rb b/test/models/account_test.rb index 4d10da21..c8eb9749 100644 --- a/test/models/account_test.rb +++ b/test/models/account_test.rb @@ -13,4 +13,22 @@ class AccountTest < ActiveSupport::TestCase @account.destroy end end + + test "gets short/long subtype label" do + account = @family.accounts.create!( + name: "Test Investment", + balance: 1000, + currency: "USD", + subtype: "hsa", + accountable: Investment.new + ) + + assert_equal "HSA", account.short_subtype_label + assert_equal "Health Savings Account", account.long_subtype_label + + # Test with nil subtype + account.update!(subtype: nil) + assert_equal "Investments", account.short_subtype_label + assert_equal "Investments", account.long_subtype_label + end end