Proposal: Account model namespace changes to support investment transactions

[zachgoll]

As I was implementing #888 which introduces a "delegated type" for an InvestmentTransaction, I noticed some model namespacing that will soon become confusing as we add these new transaction types.

A slight re-org of these model namespaces would significantly speed up future development and allow for a better understanding of the codebase for new contributors.

This issue is a preview of the namespace changes (and additions) I am considering, which will hopefully serve 2 purposes:

1. Get some early feedback / ideas on my proposed changes
2. Serve as documentation for future contributors to understand the domain model better

None of these changes should affect existing functionality, and I will be adding tests where needed prior to refactoring.

Below is a rough draft of the domain model changes:

Account Namespace

Since we originally introduced delegated Account types, we've introduced several new domain models that also relate to an Account and belong in the Account namespace. For example, "account valuations", "account balances", "account transactions", etc.

After examining the existing accountable types, I believe that adding these to the global namespace could make the relationships a bit clearer.

For example, moving Account::Depository => Depository, Account::Loan => Loan, etc.

This aligns with the delegated types docs and generally thins out DB tables, class names, etc. Furthermore, since we'll have another delegated type (event.rb) that is only valid in the context of an Account, this helps avoid a hierarchy of Account::Event::Transaction / Account::Event::Valuation, which is a bit verbose in my opinion.

account.rb   # Delegated "supertype"

# Delegated "subtypes"
depository.rb
investment.rb
crypto.rb
property.rb
vehicle.rb
credit_card.rb
loan.rb
other_asset.rb
other_liability.rb

# Account namespace
account/
  balance.rb
  entryable.rb
  entry.rb  # More on this below
  valuation.rb
  transaction.rb
  trade.rb
  crypto_trade.rb # future

Account "Entries"

As I was thinking through the delegated "Transaction" type for #888, I came up with the following:

class Account::Entry < ApplicationRecord
  belongs_to :account

  delegated_type :entryable, types: %w[ Valuation Transaction Trade CryptoTrade ]
end

module Account::Entryable
  extend ActiveSupport::Concern

  included do
    has_one :entry, as: :entryable, touch: true
  end
end

# Point-in-time valuation of an account
class Valuation < ApplicationRecord
  include Account::Entryable
end

class Transaction < ApplicationRecord
  include Account::Entryable
end

# i.e. buy/sell of AAPL
class Trade < ApplicationRecord
  include Account::Entryable
end

class CryptoTrade < ApplicationRecord
  include Account::Entryable
end

For more details about each of these entry types, see #888

Benefits

• Will make it easier to display a "history" for the account (see Make account "History" and "Transactions" sections more clear #838)
• Will make it easier to "sync" an account and avoid logic like this that has to check both transactions and valuations to determine how to sync the account

Categories, Tags, Merchants

Currently, we have both categories and tags under the Transaction namespace—Transaction::Category and Transaction::Merchant

Given the changes proposed above, this hierarchy gets pretty messy—do we put them under Account::Event? Account::Transaction?

These 3 concepts make sense on their own and I don't see an issue moving them up the hierarchy and letting the domain models like Account::Transaction decide whether a belongs_to relationship is valid rather than trying to communicate this idea via namespacing.

Proposed namespace

In app/models:

tag.rb
institution.rb
category.rb
merchant.rb

depository.rb
investment.rb
crypto.rb
property.rb
vehicle.rb
credit_card.rb
loan.rb
other_asset.rb
other_liability.rb

account.rb

account/
  transfer.rb
  balance.rb
  entryable.rb
  entry.rb
  valuation.rb
  transaction.rb
  trade.rb