mbulat / plutus

A Ruby on Rails Engine which provides a double entry accounting system for your application
Other
733 stars 233 forks source link

New Account fields #10

Open mbulat opened 12 years ago

mbulat commented 12 years ago

ezmend on Google Groups inquired about a couple of fields he felt were missing on the Account model, specifically an Account Code field for numbering and organizing like accounts, and valid dates to allow the book of Accounts to change over time. I agree that they should really be included, so I've opened this issue to keep track of adding those fields. At the moment, I think we can simply add the fields to the migration, and developers can choose to implement them or not.

Here's what I'd add to the migration

t.integer :account_code
t.datetime :valid_from
t.datetime :valid_to

Comments and other suggestions welcome

enrico commented 12 years ago

Michael, I'm not quite clear on why you want to do this, unless you plan to implement added model functionality related to those added columns.

Are you planning to allow for organization of accounts in a tree/list based on account_code, and changing the models to be aware of valid/invalid account dates in the gem itself?

My point is that unless you plan to implement those features, you should let gem users decide how to augment the schema for their needs... what if I like to have 2 string account codes instead of a single integer one?

I'm not saying those 2 features are not needed, I'm saying either I would like for you to implement the functionality related in the gem itself or focus on giving the gem users an easy way to augment/customize their models... Either one of these would be OK, but the hybrid of simply adding unused fields in the model smells fishy to me...

just my $0.02 Enrico.

mbulat commented 12 years ago

Point taken, and I agree with you generally. The last thing we want is to add a whole bunch of custom fields that only some people use, and then end up with a spagetti mess of fields. However, regarding the valid_to/from dates, I think we need at least a valid_to, or single "inactive" field. In accounting obviously it's a no go to delete anything, so we need a way to hide old accounts, and prevent inactive accounts from being used in transactions. Every accounting system I have seen has this functionality. I would add also propose to add a validation to the amounts that checks against inactive accounts. The sample views and reporting features of plutus still need a lot of work, but I think the views and reporting will need to implement the field as well.

As for the account code, it's a fairly universal practice to have a human readable code for each account (see http://en.wikipedia.org/wiki/Chart_of_accounts). I think there's a good argument though that this should be a string field instead of an integer. (it's for display and human consumption, and I can't imagine anyone doing calculations with the field) I'd also add the field to the sample chart of accounts view. This field would be primary for human consumption, so the business logic in the model might be fairly limited or non existant. I think that's okay since it seems to me plutus is in many ways about human data consumption. Obviously computers don't need double entry for accuracy's sake, so much of plutus's value (at least it seems to me) is in the point were it integrates with accounting people in the business, or outside accounting systems, where the account codes would be critical.

mbulat commented 12 years ago

Oh and @enrico thanks for the 2 cents, it's much appreciated! Exactly the kind if feedback I was looking for.

kjbkjb commented 10 years ago

Thank you for your work on this gem. I am about to start using it for a project and immediately I wished to extend this account class and add fields. Perhaps using your established model of using the business_object polymorphic association would be adequate to accomplish this goal.

In the attempt to create my own model "Account" and make an association belongs_to :plutus_account, I painfully discovered that ActiveRecord does not like to have multiple models with the same class name, regardless of their module namespace. I wouldn't have attempted creating my own Account class if you provided business object support for Plutus::Account

mbulat commented 10 years ago

@kjbkjb Rails engines are pretty flexible and are easily extended. Instead of creating your own class, I'd suggest using a decorator pattern with ActiveSupport::Concern.

You can read more about extending rails engines here: http://guides.rubyonrails.org/engines.html#improving-engine-functionality

kjbkjb commented 10 years ago

@mbulat I can't believe this actually works I have a AR Model called "AccountProperty" and based on your suggestion, I did the following:

module PlutusAccountExtension
  extend ActiveSupport::Concern

  module ClassMethods
    Plutus::Account::has_many :properties, class_name: 'AccountProperty', inverse_of: :account
  end
end
Plutus::Account.send :include, PlutusAccountExtension

Which when run:

[4] pry(main)> Plutus::Equity.create(name: 'test equity account').properties
   (0.2ms)  BEGIN
  Plutus::Account Exists (0.3ms)  SELECT 1 AS one FROM "plutus_accounts" WHERE "plutus_accounts"."name" = 'test equity account' LIMIT 1
  SQL (0.4ms)  INSERT INTO "plutus_accounts" ("created_at", "name", "type", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id"  [["created_at", Sat, 19 Apr 2014 17:28:13 UTC +00:00], ["name", "test equity account"], ["type", "Plutus::Equity"], ["updated_at", Sat, 19 Apr 2014 17:28:13 UTC +00:00]]
   (1.1ms)  COMMIT
  AccountProperty Load (0.3ms)  SELECT "account_properties".* FROM "account_properties" WHERE "account_properties"."account_id" = $1  [["account_id", 7]]
=> []
freerobby commented 9 years ago

@mbulat Thanks for linking me here from #45.

Adding account codes makes sense to me. I think when we do that we should fully implement it not just as an identifier, but also a statement of hierarchy, where the number of trailing zeroes defines the level of the account. E.g. Current assets might be 1000; Cash equivalents, 1100; Checking account, 1110; Petty cash held, 1120. This impacts not only how these accounts are presented on financial statements, but also how they may be used by the application. For instance, Current assets in this example is a roll-up account that appears on a balance sheet, but should never appear directly in a journal entry.

I am nervous about having valid_from and valid_to attributes on an account but that might be because I'm not sure what that really means, in an accounting sense. Does it merely mark an account as not accepting new credits and debits? Or does it mean you don't show that account on a financial statement outside of those dates? If the latter, do you obey that if it has a non-zero balance that impacts the accounting equation? What happens if somebody re-enables the account, and now we can no longer rely on those fields for determining when it should be shown?

Barring easy resolution to these questions, I think a safer, and simpler approach would be to have an active field that, when false, raises an exception when one tries to credit or debit it. Setting that flag to true raises an exception if its balance is non-zero. Financial statement generators would disregard this flag completely and, like any other account, show it on statements where it has relevance (i.e. statements for periods over which it has transactions or on dates where it has a non-zero balance). If I've misunderstood the use case here, please take my opinion with a heaping tablespoon of salt.

kjbkjb commented 9 years ago

We ended up overloading the account object with code and rollup_code which provides hierarchy. From there we do not allow entries against accounts where code == rollup_code and instead we perform sum operations of all accounts that have the same rollup_code.

# == Schema Information
#
# Table name: plutus_accounts
#
#  id            :integer          not null, primary key
#  name          :string(255)
#  type          :string(255)
#  contra        :boolean          default(FALSE), not null
#  created_at    :datetime
#  updated_at    :datetime
#  display_name  :string(255)
#  external_name :string(255)
#  code          :integer
#  rollup_code   :integer
#  ledger_id     :integer
#

require Plutus::Engine.root.join('app', 'models', 'plutus', 'account')
class Plutus::Account < ActiveRecord::Base
  has_paper_trail
  scope_accessible :ledger_id, :omit_rollups

  belongs_to :ledger, inverse_of: :accounts
  scope :by_name, ->(name) { where(name: name) }
  scope :ledger_id, ->(ledger_id) { where(ledger_id: ledger_id) }
  scope :omit_rollups, ->(omit) { where('plutus_accounts.code != plutus_accounts.rollup_code')}

  validates :name, presence: true
  validates :display_name, presence: true, length: { maximum: 128 }
  validates :external_name, length: { maximum: 128 }
  validates :rollup_code, presence: true, numericality: { greater_than_or_equal_to: 100 }
  validates :code, presence: true, numericality: { greater_than_or_equal_to: 100 }
  validates :code, numericality: { greater_than_or_equal_to: :rollup_code , message: "must be greater than or equal to Rollup Code."}
  validates :ledger, presence: true
mbulat commented 9 years ago

@freerobby, @kjbkjb Thanks for the input on the codes. I agree that a hierarchy with rollup codes would be the way to go, and I'm sure it would be helpful from a reporting standpoint. I think it would make sense to implement in a way that allows people to continue to use plutus without using the codes.

@freerobby I agree completely with your assessment of the valid_from and valid_to, and with having an active field instead. From an auditing standpoint, though, it might be nice to have a date for the field, so we have a record of when it was marked no longer active. So maybe instead it's inactive as a single timestamp, or inactive boolean and inactive_at timestamp separately. Also, instead of throwing an exception, we could add a validation to the Amount class that checks the inactive attribute of the referenced account, and also a validation on the Account class that checks the balance of the account if setting the value of inactive.

ramontayag commented 8 years ago

I'm thinking of having a native model in my app called Account. A Plutus::Account would belong to this Account (via tenancy), and the Account would have one Plutus::Account. On Account, I can put a balance cache, a currency field, user foreign key, etc -- things that my app needs. Would this be a reasonable approach?

kendocode commented 6 years ago

Is Plutus currently supporting codes, or do we need to add this ourselves?

ramontayag commented 6 years ago

It seems like the approach that is most flexible to all would be to create local models that own a plutus account. The plutus account would be used just for double entry accounting, while the local model would have everything your app needs.