When using :through assocations validations firing on join model multiple times #8854

Closed
gnufied opened this Issue Jan 9, 2013 · 10 comments

Projects

None yet

6 participants

@gnufied
Contributor
gnufied commented Jan 9, 2013

This is related to #7661 and I am opening this ticket to demonstrate the problem,

class Article < ActiveRecord::Base
  attr_accessible :content, :title
  has_many :article_tags
  has_many :tags, :through => :article_tags

  before_validation :add_to_default_tag, :on => :create

  def add_to_default_tag
    if self.tags.empty?
      self.tags << Tag.find_or_create_by_title("default")
    end
  end
end

class Tag < ActiveRecord::Base
  attr_accessible :title

  has_many :article_tags
  has_many :articles, :through => :article_tags
end

class ArticleTag < ActiveRecord::Base
  attr_accessible :article_id, :name, :tag_id

  belongs_to :article
  belongs_to :tag

  validate :check_stuff

  private
  def check_stuff
    puts "whoa"
  end

end

article = Article.new(:title => "hemant", :content => "kumar")
article.save
=> Whoa
=> Whoa
=> Whoa

For the lazy, there is a repo https://github.com/gnufied/try_through which reproduces the same problem. I can perhaps add some ActiveRecord tests as well quickly.

@gnufied
Contributor
gnufied commented Jan 9, 2013
@tenderlove
Member

@gnufied if you can add a failing test to AR, I'd really appreciate it. Otherwise, I need to translate your example to a failing AR test (I can do that, it will just take me longer).

Thanks for filing this issue!

@gnufied
Contributor
gnufied commented Jan 9, 2013
@gnufied
Contributor
gnufied commented Jan 10, 2013

does that work, @tenderlove

@adamcooke

I have confirmed this behaviour too and concluded that it is related to #7661. I reverted the changes introduced in #7661 using the following however it's not ideal.

class ActiveRecord::Associations::HasManyThroughAssociation
  remove_method :concat_records
end

I would encourage whoever cherry picked #7661 into 3-2-stable to remove it, if necessary, introduce it in 4.0 with documentation about the changed behaviour when saving these associations.

@gnufied
Contributor
gnufied commented Jan 11, 2013

Because #7661 was reverted this is fixed. But I believe there should be some tests for existing behaviour.

@tenderlove
Member

@gnufied I agree. I will make add a test before closing this issue.

@smasry
Contributor
smasry commented Apr 29, 2013

@tenderlove ... #7661 was reverted on 3.2 but is still on the master branch. Is rails 4 supposed to go out with the change?

@gnufied gnufied added the stale label Apr 23, 2014
@rafaelfranca
Member

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@rails-bot rails-bot closed this May 27, 2014
@rails-bot

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-1-stable, 4-0-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment