assigning collection values and exceptions

rubyonrails-talk
1

hi everyone,

I've this code in my Report model:

  has_many :report_reasons, :validate => true   has_many :reasons, :through => :report_reasons #, :uniq => true # :accessible => true

  def reason_attributes=(reason_attributes)     reasons.clear       reason_attributes.uniq.each do |reason|         reasons << Reason.find_or_create_by_content(reason)       end   end

What I'm trying to accomplish (and what works) is to have a form which can submit to a Report some Reason via fields_for (Reason :content field). So I just swipe existing associations with .clear and proceed to rebuild new ones. I like to keep things that way so I have my validation errors back, this is true especially when saving a new Report.

What happens here is that, according to rails' code, when the owner object, in this case Report, doesn't exists yet, I can assign to it every kind of Reason I can think of, most importantly even those that doesn't pass validation. Because validations happens at .save I've my errors and I'm quite happy.

However this is different when using update_attributes, for updating. The owner object (Report) already exists, and every invalid Reason assigned through << will simply result in a raised ActiveRecord::InvalidRecord exception. This is best described directly into rails' code in association_collection.rb (<<) and has_many_through_association.rb (insert_record).

Now I may trap this exception in controller and do something with it, however I lose in some way my validation error messages on my form, which is dynamic (add/remove reasons via js).

The reason why I'm not using :accessible or some other plugin to ease multimodel forms is that they wants to create a record in Reason, which is good, but they don't try to reuse it if it already exists. Instead they, of course, will duplicate rows, defeating my purpose.

I can post some other code if anybody have some suggestions on how to proceed.

Thanks.

2

FWIW and for posterity's sake I workarounded with the not-yet-dry code:

  def reason_attributes=(reason_attributes)     reasons.clear     reason_attributes.uniq.each do |attributes|       reason = Reason.find_or_initialize_by_content(attributes)       if self.new_record?         # Entering create.         reasons << reason # no matter if it's invalid       else         # Entering update.         if reason.valid?           if Reason.exists?(reason)             # Reason is valid and already exists, instead of duplicating             # a record just assing it.             reasons << reason           else             # Reason is valid but doesn't exists, build it.             reasons.build(attributes)           end         else           reasons.build(attributes) # workarounded to show error messages         end       end     end   end

3

i could be wrong but that seems functionally equivalent to http://pastie.org/263949, right?

RSL

4

hi Russel, no, they are different. I must differentiate the fact that a owner object, self (which is a Report), is a new record.

Because if it's the case, then rails permits to push objects to a collection even if they are not valid (via validates_associated :reasons), and triggering validation only on save, called from the create action of the controller.

if self already exists, and therefore self.new_record? returns false, every assignment made through the use of push (<<) will bombs and return instantly an ActiveRecord::InvalidRecord.

As I said this behaviour is best described directly into rails' code in association_collection.rb (<< method) and has_many_through_association.rb (insert_record method).

So, to avoid the need to rescue those errors in controller, probably losing my validation messages and due to the fact that this is an ajax form with a dynamic number of fields on update I must check a couple of things.

If the reason that I'm trying to pass passes validations (with .valid?) then I must check if it already exists in database, since I don't want duplicates in Reason table, and if yes assign it to the collection of self, which will not raise any exception since if it's in database it's already valid, if not present just do a build, which will not trigger any exception but kist making update_attributes in controller do its job.

if the reason is not valid, always on update, then I fake out a build in any case, since the record will not pass, but at least I have now my error messages right near the fields on a multimodel form, reusing data from the collection.

5

you could still use your new_record? checks etc without using reasons.build to recreate the same object you already have from the find_or_initialize. that was my point.

RSL

6

also, that in every single case you show here... you add the reason to reasons. there's no case [unless i'm missing it] that you do something besides add the new reason to the reasons collection.

RSL

7

you're right, as I wrote this code wasn't DRYed up yet, I wrote it in
a hurry.

besides, thanks to your advice I've shortened this up to

   def reason_attributes=(reason_attributes)      reasons.clear      reason_attributes.uniq.each do |attributes|        reason = Reason.find_or_initialize_by_content(attributes)        if self.new_record?          # Entering create.          reasons << reason # no matter if it's invalid        else          # Entering update.          if reason.valid?            reasons << reason          else            reasons.build(attributes) # workarounded to show error
messages          end        end      end    end

do you have further optimisation in mind or I'm missing something
obvious?

8

another attempt

def reason_attributes=(reason_attributes)    reasons.clear    reason_attributes.uniq.each do |attributes|      reason = Reason.find_or_initialize_by_content(attributes)      reasons << reason rescue reasons.build(attributes)    end end

9

    reason = Reason.find_or_initialize_by_content(attributes)     reasons << reason rescue reasons.build(attributes)

doesn't make sense to me. the first line already finds or creates a reason. if it raises an exception adding it to the reasons collection, i don't see why building a new one with the exact same information would solve anything. you've tried this?

RSL

10

I'll add some view code. It's a kind of workaround to keep reasons collection with something
filled in. the new and edit view shares a partial which is from a collection:

# _report.erb <div class="reason">   <dl class="form">    <% fields_for("report[reason_attributes]", reason) do |r| %>     <dt class="required">Reason <%= r.error_message_on :content %></dt>      <dd>       <%= r.text_field :content, { :index => nil, :autocomplete =>
'off' } %>      <%= link_to_function "remove", "$(this).up('.reason').remove()" %>     </dd>    <% end %>   </dl> </div>

# edit.erb <% form_for [:administration, @document, @report] do |f| -%> ... <div id="reasons">   <%= render :partial => 'reason', :collection => @report.reasons %> </div> <p><%= add_reason_link "Add a reason" %></p> <%= f.submit %> <% end -%>

# controller update action uses only if
@report.update_attributes(params[:report])

when you first access the edit page, _report is cycled through every
reason it finds, and so it's ok. the moment I edit a reason field, putting in there an invalid
attribute that doesn't pass the validation I need to rescue the
exception thrown by << somewhere. I decided to rescue this exception with a reasons.build(attributes),
which will instantiate a new reason on the invalid field, thus showing
the error message.