Should rails models be concerned with other models for the sake of skinny controllers?

Question

I read everywhere that business logic belongs in the models and not in controller but where is the limit? I am toying with a personnal accounting application.

Account
Entry
Operation

When creating an operation it is only valid if the corresponding entries are created and linked to accounts so that the operation is balanced for exemple buy a 6-pack :

o=Operation.new({:description=>"b33r", :user=>current_user, :date=>"2008/09/15"})
o.entries.build({:account_id=>1, :amount=>15})
o.valid? #=>false
o.entries.build({:account_id=>2, :amount=>-15})
o.valid? #=>true

Now the form shown to the user in the case of basic operations is simplified to hide away the entries details, the accounts are selected among 5 default by the kind of operation requested by the user (intialise account -> equity to accout, spend assets->expenses, earn revenues->assets, borrow liabilities->assets, pay debt assets->liabilities ...) I want the entries created from default values.

I also want to be able to create more complex operations (more than 2 entries). For this second use case I will have a different form where the additional complexity is exposed.This second use case prevents me from including a debit and credit field on the Operation and getting rid of the Entry link.

Which is the best form ? Using the above code in a SimpleOperationController as I do for the moment, or defining a new method on the Operation class so I can call Operation.new_simple_operation(params[:operation])

Isn't it breaking the separation of concerns to actually create and manipulate Entry objects from the Operation class ?

I am not looking for advice on my twisted accounting principles :)

edit -- It seems I didn't express myself too clearly. I am not so concerned about the validation. I am more concerned about where the creation logic code should go :

assuming the operation on the controller is called spend, when using spend, the params hash would contain : amount, date, description. Debit and credit accounts would be derived from the action which is called, but then I have to create all the objects. Would it be better to have

#error and transaction handling is left out for the sake of clarity
def spend
  amount=params[:operation].delete(:amount)#remove non existent Operation attribute
  op=Operation.new(params[:operation])
  #select accounts in some way
  ...
  #build entries
  op.entries.build(...)
  op.entries.build(...)
  op.save
end

or to create a method on Operation that would make the above look like

def spend
  op=Operation.new_simple_operation(params)
  op.save
end

this definitely give a much thinner controller and a fatter model, but then the model will create and store instances of other models which is where my problem is.

Answer 1

It's easier to think in terms of each entity validating itself, and entities which depend on one another delegating their state to the state of their associated entries. In your case, for instance:

class Operation < ActiveRecord::Base
  has_many :entries
  validates_associated :entries
end

validates_associated will check whether each associated entity is valid (in this case, all entries should if the operation is to be valid).

It is very tempting to try to validate entire hierarchies of models as a whole, but as you said, the place where that would be most easily done is the controller, which should act more as a router of requests and responses than in dealing with business logic.

Answer 2

The way I look at it is that the controller should reflect the end-user view and translate requests into model operations and reponses while also doing formatting. In your case there are 2 kinds of operations that represent simple operations with a default account/entry, and more complex operations that have user selected entries and accounts. The forms should reflect the user view (2 forms with different fields), and there should be 2 actions in the controller to match. The controller however should have no logic relating to how the data is manipulated, only how to receive and respond. I would have class methods on the Operation class that take in the proper data from the forms and creates one or more object as needed, or place those class methods on a support class that is not an AR model, but has business logic that crosses model boundaries. The advantage of the separate utility class is that it keeps each model focused on one purpose, the down side is that the utility classes have no defined place to live. I put them in lib/ but Rails does not specify a place for model helpers as such.

Answer 3

Virtual Attributes (more info here and here) will help with this greatly. Passing the whole params back to the model keeps things simple in the controller. This will allow you to dynamically build your form and easily build the entries objects.

class Operation
  has_many :entries

  def entry_attributes=(entry_attributes)
    entry_attributes.each do |entry|
      entries.build(entry)
    end
  end

end

class OperationController < ApplicationController
  def create
    @operation = Operation.new(params[:opertaion])
    if @operation.save
      flash[:notice] = "Successfully saved operation."
      redirect_to operations_path
    else
      render :action => 'new'
    end
  end
end

The save will fail if everything isn't valid. Which brings us to validation. Because each Entry stands alone and you need to check all entries at "creation" you should probably override validate in Operation:

class Operation
  # methods from above
  protected
    def validate
      total = 0
      entries.each { |e| t += e.amount }
      errors.add("entries", "unbalanced transfers") unless total == 0
    end
end

Now you will get an error message telling the user that the amounts are off and they should fix the problem. You can get really fancy here and add a lot of value by being specific about the problem, like tell them how much they are off.

Answer 4

but then the model will create and store instances of other models which is where my problem is.

What is wrong with this?

If your 'business logic' states that an Operation must have a valid set of Entries, then surely there is nothing wrong for the Operation class to know about, and deal with your Entry objects.

You'll only get problems if you take this too far, and have your models manipulating things they don't need to know about, like an EntryHtmlFormBuilder or whatever :-)

Answer 5

If you are concerned about embedding this logic into any particular model, why not put them into an observer class, that will keep the logic for your creation of the associated items separate from the classes being observed.