views:

328

answers:

2

I have an Entry model which has many Tags. Tags are added to an entry by typing them into a textbox on my form, via a tag_names virtual attribute. Before validation on the Entry model, the tag_names string is converted into actual Tag objects using find_or_create_by_name. The Tag model also has validation to make sure the tag name matches a regex, which is run via the association.

My Entry model looks like this:

class Entry < ActiveRecord::Base
  has_many :entry_tags
  has_many :tags, :through => :entry_tags

  before_validation :update_tags

  attr_writer :tag_names

private
  def update_tags
    if @tag_names
      self.tags = @tag_names.split(",").uniq.map do |name|
        Tag.find_or_create_by_name(name.strip)
      end
    end
  end
end

When I create a new Entry object and assign it tags, everything works correctly -- the tags are not saved if there is a validation error on one of the Tags, and an error message is passed back. However, if I try to update an existing Entry object with an invalid tag, instead of passing back a message, my self.tags= call (in update_tags above) is throwing an exception with the validation error message. Even if I overwrite find_or_create_by_name to actually just return a new object instead of calling create, I get the same outcome.

It seems to me (and the docs seem to corroborate) that the tags= call is actually saving my Tag objects before the main record gets saved when the Entry object already exists. Is there anything I can do to make this save not happen, or to stop it from raising an exception and just causing my save to return false?

+1  A: 

You could catch the exception raised and return false, in that case, from update_tags which will halt the save on the Entry.

Alternatively, if you want to avoid handling that exception, you could build a new Tag instance where one doesn't already exist and check whether it is valid before proceeding (new_tag.valid?) and if it is not then return false from update_tags.

Shadwell
+1  A: 

I would try something like this:

class Entry < ActiveRecord::Base
  has_many :entry_tags
  has_many :tags, :through => :entry_tags

  before_validation :update_tags

  attr_writer :tag_names
  validates_associated :tags

private
  def update_tags
    return unless @tag_names
    current_tag_names = tags.map(&:name)
    user_tag_names = @tag_names.split(",").uniq
    #add in new tags
    user_tag_names.each do |name|
      next if current_tag_names.include?(name)
      tags.build :name => name
    end
    #remove dropped tags
    ( current_tag_names - user_tag_names ).each do |name|
      removed_tag = tags.find_by_name(name)
      tags.delete(removed_tag)
    end
  end
end

This way you're only initializing the related models in your update_tags action and so won't throw validation errors. I also added in the validates_associated :tags so that errors on these related models can be reported back via the standard input form using error_messages_for :entry.

Update included code for removing dropped tags.

austinfromboston
Wouldn't that just append to tags instead of overwrite? I guess I'd need to do something like `tags.clear` in my `update_tags` method? Also, I tried using validates_associated before, but I was ending up with duplicate error messages for tags.
Daniel Vandersluis
hmm, i wouldn't do a `tags.clear` -- i'll edit my code to handle removing tags the user has dropped. You may be happier writing your own custom validation function for the tags array, might be worth posting another question on that topic.
austinfromboston