views:

1306

answers:

1

Hi,

I am using multiple child models in a complex form (a la http://railsforum.com/viewtopic.php?id=28447). The form works great, but I need to validate a property of the set of child models before accepting the form data into the database. I've come up with one mostly-working, very-kludgy way of doing this. Seems like there has to be a better way, so I'm asking for suggestions...

Basically a Person has_many Distributions. The Distribution has (among other things) a percentage attribute. For a given person, their distributions must total 100% to be valid. This screams "transaction" to me, but I figured I should give the validators a shot first.

I tried writing this as a custom validator, but the validator only works on data already saved to the database. It did not check the parameters submitted by the form. In other words I was able to enter invalid percentages via the form, which were saved, and then subsequent edits all failed because of the bad data already in the model.

Next I extended update_attributes in my Person model, adding a transaction:

def update_attributes(params)
  retval = true
  self.transaction do
    retval = super

    unless distributions_exactly_100?
      retval = false
      errors.add_to_base("Distribution must add up to exactly 100%")
      raise ActiveRecord::Rollback
    end
  end
  retval
end

The retval business is ugly but this more or less works (sometimes some of the pending distributions are missing from the form, when it finds an error and re-renders). There is an additional nuance that convinces me this is a poor approach: if my distirbutions association is defined with helper methods, as below, I can't use the helper methods in my update_attributes() (or in distributions_exactly_100?), because they go to the database rather than operating on the set of just-assigned-but-not-yet-committed distributions.

 has_many :distributions do
   def for_month_and_year(month, year)
     find :all, :conditions => ['month = ? and year = ?', month, year]
   end

   def total_for_month_and_year(month, year)
     sum :percentage, :conditions => ['month = ? and year = ?', month, year]
   end

   ...

   def years_and_months
     ds = find(:all, :order => 'year DESC, month DESC')
     (ds.collect {|d| [d.year, d.month]}).uniq
   end

 end

The only other thing I can think of is to process the params themselves, as text, on the way into update_attributes. But that is just Wrong. :)

Anyone else doing validation on a whole collection of children? What's the right way to go about it?

+2  A: 

I don't recommend setting errors in update_attributes. Keep validations in their normal place.

Back to your problem, can you change the validation check to work on the distributions in memory rather than performing the calculation on the database?

# in Person model
validate :ensure_distributions_equal_100

def ensure_distributions_equal_100
  percent = distributions.map(&:percent).sum
  if percent != 100
    errors.add_to_base("Distribution must add up to exactly 100%, they are #{percent}")
  end
end
ryanb
Yes, thank you, I did write a function that gets called as a validation, once I figured out the memory vs. DB angle. :)I still have problems with how the edit view get rendered when the validation fails (some pending changes are lost, but not all of them), but that's a different bug!
korinthe