mbulat / plutus

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

multi tenancy support #27

Closed iffyuva closed 9 years ago

iffyuva commented 10 years ago

hi, im using plutus in multi-tenant app. i have browsed through issues for tenancy and found this: https://github.com/mbulat/plutus/issues/24. the way i understand your reasoning is to not add :tenant_id to plutus models, and let plutus users add :tenant_id to models, and handle themselves. i tried this path, added :tenant_id, and wanted to have default accounts across all tenants. in order to make it work, i have to remove uniqueness constraint on name : https://github.com/mbulat/plutus/blob/master/app/models/plutus/account.rb#L39 and have my own constraint.

  validates :name, uniqueness: { scoped: :tenant_id }

is it possible to remove that constraint: https://github.com/mbulat/plutus/blob/master/app/models/plutus/account.rb#L39 ?

reason being: i think, setting up plutus accounts is a one time process, and this uniqueness validation is of no use once setup is done. thoughts?

yragnos commented 10 years ago

Yuva, that makes great sense to allow more than one account in the table. Many businesses in fact run more than one set of accounts, so multi-tenancy makes sense. The other option would be to fork Plutus and create a multi-tenant compliant version and maintain any changes separate to the main stream. (Actually there already is one - see code-mancers/plutushttps://github.com/code-mancers/plutus ).

On Wed, May 21, 2014 at 2:32 AM, Yuva notifications@github.com wrote:

hi, im using plutus in multi-tenant app. i have browsed through issues for tenancy and found this: #24 https://github.com/mbulat/plutus/issues/24. the way i understand your reasoning is to not add :tenant_id to plutus models, and let plutus users add :tenant_id to models, and handle themselves. i tried this path, added :tenant_id, and wanted to have default accounts across all tenants. in order to make it work, i have to remove uniqueness constraint on name : https://github.com/mbulat/plutus/blob/master/app/models/plutus/account.rb#L39and have my own constraint.

validates :name, uniqueness: { scoped: :tenant_id }

is it possible to remove that constraint: https://github.com/mbulat/plutus/blob/master/app/models/plutus/account.rb#L39?

reason being: i think, setting up plutus accounts is a one time process, and this uniqueness validation is of no use once setup is done. thoughts?

— Reply to this email directly or view it on GitHubhttps://github.com/mbulat/plutus/issues/27 .

Regards

Gary Ferguson 619-991-7020

mbulat commented 10 years ago

The problem with removing the constraint all together is that the Entry.build method assumes that there will be a unique Account name that gets passed in to find the required account. Even if you don't use the build method, generally you'll need some way within your code to pick the appropriate account and ensure some level of uniqueness.

Theoretically you could provide the uniqueness as scoped by tenant_id as you suggested, which would require the whole plugin to support multi-tenancy. Another method might be be to add a unique Account Code as suggested in #10 and use that for lookups, allowing the Account name field to be non-unique.

In general, though, I would say for multi-tenant applications, I would suggest using a fork, or overriding classes as outlined in http://edgeguides.rubyonrails.org/engines.html#improving-engine-functionality

Plutus's main goal is to provide integration for a Rails e-commerce app with your business's internal books. I know certain people are using it to provide full accounting systems for customers, or to provide a full fledged accounting system for their own business, but it was never really intended for that role. (I'm not sure why you wouldn't just use Netsuite, Quickbooks, etc for something like that anyway) Since those are edge cases, though, I'm not really sure what value multi-tenancy would add.

But if there's some use case for multi-tenancy I'm missing, I'd like to know.

iffyuva commented 10 years ago

@yragnos thanks for those kind words. i work for Codemancers, and i did that fork because i wanted something quick.

@mbulat i get it that Entry.build finds accounts by name. And in the current project we rolled something of our own. Reason being Account.find_by_name is not good, as it hits db multiple times. But thats another discussion.

We are using tenant_id approach, and also as suggested, we also have an unique account code, but again they are all scoped under tenant_id. This small change: https://github.com/code-mancers/plutus/compare/master...v0.8.1-multitenant-support helped us a lot.

So, the use-case (with bit of a background): In our multi-tenant app, tenants sell/contract goods to end users. We use invoicing gem to generate invoices, and track payments also. End users can pay invoices upfront, or they can pay in installments. Since invoicing doesnot support accounting out of box, we use plutus. all plutus integrations do is: analyze invoices and payment notes and make entries into plutus system. periodically tenant admins will generate transaction statements, export them and send them to finance team. Tenancy kicks in because we want to have specialized accounts for some tenants in future, so that they can map what invoices can go into what account.

mbulat commented 10 years ago

@iffyuva Thanks for the sharing your use case. That kind of feedback is super helpful. Obviously with something like accounting, businesses are going to vary tremendously in their needs. So a one size fits all Rails engine like Plutus is going to be difficult.

I think a model like that used by Devise might be the most helpful here. Multi-tenancy could be separated into an ActiveSupport::Concern module that's included with Plutus by default, and then configured on installation. So maybe something like a "Multitenantable" or just "Tenantable" module? Another pull request I got from @skyeagle was for adding commodity exchanges to handle accounting in multiple currencies. That could make another module. A config file that lives in the main Rails app initializers folder could then enable or disable the modules.

So then we'd apply the tenant_id in the migrations across all tables, including Accounts, but enabled it on a app by app basis via the concern module. I believe the module could then override the default constraint with your scoped constraint. Thoughts?

iffyuva commented 10 years ago

@mbulat i agree with this kind of approach. make concerns configurable, and include them in plutus models.

:+1:

do let me know if you need any help from me. Thanks again for reconsidering multi-tenant support.

iffyuva commented 10 years ago

people, any updates?

davidkuhta commented 10 years ago

Hey everyone, I think I ran into the same situation as @iffyuva albeit for different reasons. My perception of Accounting was that each account would be matched 1:1 with another Object in the application. Given that there could be an infinite number of models, I tweaked my local Plutus gem to accommodate that.

I modified Accounts.rb to have:

belongs_to :accountable, polymorphic: true

and the Plutus generator:

create_table :plutus_accounts do |t|
  t.string :name
  t.string :type
  t.boolean :contra
  t.references :accountable, :polymorphic => true

  t.timestamps
end
add_index :plutus_accounts, [:name, :type]
add_index :plutus_accounts, [:accountable_id, :accountable_type], :name => "index_accounts_on_accountable"

This provide for some convenient methods whether Account Types Varied:

class User < ActiveRecord::Base
  ...
  has_many :accounts, as: :accountable
  ...
end
@User.account.create(name: "Owner Equity", type: "Plutus::Asset")

or were the same

class User < ActiveRecord::Base
  ...
  has_many :accounts, as: :accountable, class_name: "Plutus::Liability"
  ...
end
@User.account.create(name: "Widget Maker Payable")

I also noticed that a similar polymorphic relationship exists with the entries and commercial_documents (Although maybe entryable would be a more suitable name in terms of abstracting the concept of an entry independent of a document?)

Hence, I wanted to propose a discussion on the possibility of extracting that Polymorphic functionality into a Rails Module?/Concern?, in support of multi-model ownership of accounts and hence support for multi-tenacy directly via tenant account ownership or indirectly via child model ownership of the account and tracing back up the association chain.

As a newer Rails developer and GitHub newbie (I'm still trying to figure out how to push changes to my fork of Plutus), I hope you found this diatribe both constructive and relevant to the discussion. Respectfully, David Thanks @mbulat for Plutus!

FirstGHComment

mbulat commented 10 years ago

@iffyuva I'm currently busy with another project, and personally wouldn't be using multi-tenancy, so if you'd like to work on the feature and submit a pull request, I'd be happy to take a look at merging.

@IceDragon13 Thanks for the comment. I think what your suggesting is different than multi-tenancy, which would really aim at having every database row for Plutus include information on what tenant (ie application owner) it belongs to. In your example, I'm not sure what it would mean to have a Plutus Entry with Credits and Debits from Accounts belonging to different users. Since I think it's a very different concept, it's probably better to take the discussion off the issue tracker and onto the mailing list if you want to discuss it further: https://groups.google.com/forum/#!forum/plutus-gem

iffyuva commented 9 years ago

long story short, finally i've got sometime to add tenancy support, PR here: #29