Refactor HasManyThroughAssociation to inherit from HasManyAssociation. Association callbacks and <association>_ids= now work with hm:t. Closes #11516 [rubyruy]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9230 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 232 insertions, 8 deletions

diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb
index 2b929d4480..952ea63706 100644
--- a/activerecord/test/cases/associations/join_model_test.rb
+++ b/activerecord/test/cases/associations/join_model_test.rb
@@ -443,11 +443,33 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
     assert_nil posts(:thinking).tags.find_by_name("General").attributes["tag_id"]
   end
 
-  def test_raise_error_when_adding_new_record_to_has_many_through
-    assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags << tags(:general).clone }
-    assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).clone.tags << tags(:general) }
-    assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags.build }
-    assert_raise(ActiveRecord::HasManyThroughCantAssociateNewRecords) { posts(:thinking).tags.new }
+  def test_associating_unsaved_records_with_has_many_through
+    saved_post = posts(:thinking)
+    new_tag = Tag.new(:name => "new")
+
+    saved_post.tags << new_tag
+    assert !new_tag.new_record? #consistent with habtm!
+    assert !saved_post.new_record?
+    assert saved_post.tags.include?(new_tag)
+
+    assert !new_tag.new_record?
+    assert saved_post.reload.tags(true).include?(new_tag)
+
+
+    new_post = Post.new(:title => "Association replacmenet works!", :body => "You best believe it.")
+    saved_tag = tags(:general)
+
+    new_post.tags << saved_tag
+    assert new_post.new_record?
+    assert !saved_tag.new_record?
+    assert new_post.tags.include?(saved_tag)
+
+    new_post.save!
+    assert !new_post.new_record?
+    assert new_post.reload.tags(true).include?(saved_tag)
+
+    assert posts(:thinking).tags.build.new_record?
+    assert posts(:thinking).tags.new.new_record?
   end
 
   def test_create_associate_when_adding_to_has_many_through
diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb
index c302eda131..22a6bfc3a5 100755
--- a/activerecord/test/cases/associations_test.rb
+++ b/activerecord/test/cases/associations_test.rb
@@ -550,8 +550,181 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
 
 end
 
+class HasManyThroughAssociationsTest < ActiveRecord::TestCase
+  fixtures :posts, :readers, :people
+
+  def test_associate_existing
+    assert_queries(2) { posts(:thinking);people(:david) }
+    
+    assert_queries(1) do
+       posts(:thinking).people << people(:david)
+    end
+    
+    assert_queries(1) do
+      assert posts(:thinking).people.include?(people(:david))
+    end
+    
+    assert posts(:thinking).reload.people(true).include?(people(:david))
+  end
+
+  def test_associating_new
+    assert_queries(1) { posts(:thinking) }
+    new_person = nil # so block binding catches it
+    
+    assert_queries(0) do
+      new_person = Person.new
+    end
+    
+    # Associating new records always saves them
+    # Thus, 1 query for the new person record, 1 query for the new join table record
+    assert_queries(2) do
+      posts(:thinking).people << new_person
+    end
+    
+    assert_queries(1) do
+      assert posts(:thinking).people.include?(new_person)
+    end
+    
+    assert posts(:thinking).reload.people(true).include?(new_person)
+  end
+
+  def test_associate_new_by_building
+    assert_queries(1) { posts(:thinking) }
+    
+    assert_queries(0) do
+      posts(:thinking).people.build(:first_name=>"Bob")
+      posts(:thinking).people.new(:first_name=>"Ted")
+    end
+    
+    # Should only need to load the association once
+    assert_queries(1) do
+      assert posts(:thinking).people.collect(&:first_name).include?("Bob")
+      assert posts(:thinking).people.collect(&:first_name).include?("Ted")
+    end
+    
+    # 2 queries for each new record (1 to save the record itself, 1 for the join model)
+    #    * 2 new records = 4
+    # + 1 query to save the actual post = 5
+    assert_queries(5) do
+      posts(:thinking).save
+    end
+    
+    assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Bob")
+    assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Ted")
+  end
+
+  def test_delete_association
+    assert_queries(2){posts(:welcome);people(:michael); }
+    
+    assert_queries(1) do
+      posts(:welcome).people.delete(people(:michael))
+    end
+    
+    assert_queries(1) do
+      assert posts(:welcome).people.empty?
+    end
+    
+    assert posts(:welcome).reload.people(true).empty?
+  end
+
+  def test_replace_association
+    assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)}
+    
+    # 1 query to delete the existing reader (michael)
+    # 1 query to associate the new reader (david)
+    assert_queries(2) do
+      posts(:welcome).people = [people(:david)]
+    end
+    
+    assert_queries(0){
+      assert posts(:welcome).people.include?(people(:david))
+      assert !posts(:welcome).people.include?(people(:michael))
+    }
+    
+    assert posts(:welcome).reload.people(true).include?(people(:david))
+    assert !posts(:welcome).reload.people(true).include?(people(:michael))
+  end
+
+  def test_associate_with_create
+    assert_queries(1) { posts(:thinking) }
+    
+    # 1 query for the new record, 1 for the join table record
+    # No need to update the actual collection yet!
+    assert_queries(2) do
+      posts(:thinking).people.create(:first_name=>"Jeb")
+    end
+    
+    # *Now* we actually need the collection so it's loaded
+    assert_queries(1) do
+      assert posts(:thinking).people.collect(&:first_name).include?("Jeb")
+    end
+    
+    assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Jeb")
+  end
+
+  def test_clear_associations
+    assert_queries(2) { posts(:welcome);posts(:welcome).people(true) }
+    
+    assert_queries(1) do
+      posts(:welcome).people.clear
+    end
+    
+    assert_queries(0) do
+      assert posts(:welcome).people.empty?
+    end
+    
+    assert posts(:welcome).reload.people(true).empty?
+  end
+
+  def test_association_callback_ordering
+    Post.reset_log
+    log = Post.log
+    post = posts(:thinking)
+
+    post.people_with_callbacks << people(:michael)
+    assert_equal [
+      [:added, :before, "Michael"],
+      [:added, :after, "Michael"]
+    ], log.last(2)
+
+    post.people_with_callbacks.push(people(:david), Person.create!(:first_name => "Bob"), Person.new(:first_name => "Lary"))
+    assert_equal [
+      [:added, :before, "David"],
+      [:added, :after, "David"],
+      [:added, :before, "Bob"],
+      [:added, :after, "Bob"],
+      [:added, :before, "Lary"],
+      [:added, :after, "Lary"]
+    ],log.last(6)
+
+    post.people_with_callbacks.build(:first_name => "Ted")
+    assert_equal [
+      [:added, :before, "Ted"],
+      [:added, :after, "Ted"]
+    ], log.last(2)
+
+    post.people_with_callbacks.create(:first_name => "Sam")
+    assert_equal [
+      [:added, :before, "Sam"],
+      [:added, :after, "Sam"]
+    ], log.last(2)
+
+    post.people_with_callbacks = [people(:michael),people(:david), Person.new(:first_name => "Julian"), Person.create!(:first_name => "Roger")]
+    assert_equal (%w(Ted Bob Sam Lary) * 2).sort, log[-12..-5].collect(&:last).sort
+    assert_equal [
+      [:added, :before, "Julian"],
+      [:added, :after, "Julian"],
+      [:added, :before, "Roger"],
+      [:added, :after, "Roger"]
+    ], log.last(4)
+
+    post.people_with_callbacks.clear
+    assert_equal (%w(Michael David Julian Roger) * 2).sort, log.last(8).collect(&:last).sort
+  end
+end
+
 class HasManyAssociationsTest < ActiveRecord::TestCase
-  fixtures :accounts, :companies, :developers, :projects,
+  fixtures :accounts, :categories, :companies, :developers, :projects,
            :developers_projects, :topics, :authors, :comments, :author_addresses,
            :people, :posts
 
@@ -1291,8 +1464,23 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal [comments(:eager_other_comment1).id], authors(:mary).comment_ids
   end
 
-  def test_assign_ids_for_through
-    assert_raise(NoMethodError) { authors(:mary).comment_ids = [123] }
+  def test_modifying_a_through_a_has_many_should_raise
+    [
+      lambda { authors(:mary).comment_ids = [comments(:greetings).id, comments(:more_greetings).id] },
+      lambda { authors(:mary).comments = [comments(:greetings), comments(:more_greetings)] },
+      lambda { authors(:mary).comments << Comment.create!(:body => "Yay", :post_id => 424242) },
+      lambda { authors(:mary).comments.delete(authors(:mary).comments.first) },
+    ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasManyReflection, &block) }
+  end
+
+
+  def test_assign_ids_for_through_a_belongs_to
+    post = Post.new(:title => "Assigning IDs works!", :body => "You heared it here first, folks!")
+    post.person_ids = [people(:david).id, people(:michael).id]
+    post.save
+    post.reload
+    assert_equal 2, post.people.length
+    assert post.people.include?(people(:david))
   end
 
   def test_dynamic_find_should_respect_association_order_for_through
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index fcfe260d4a..22c5a645b8 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -46,6 +46,20 @@ class Post < ActiveRecord::Base
 
   has_many :readers
   has_many :people, :through => :readers
+  has_many :people_with_callbacks, :source=>:person, :through => :readers,
+              :before_add    => lambda {|owner, reader| log(:added,   :before, reader.first_name) },
+              :after_add     => lambda {|owner, reader| log(:added,   :after,  reader.first_name) },
+              :before_remove => lambda {|owner, reader| log(:removed, :before, reader.first_name) },
+              :after_remove  => lambda {|owner, reader| log(:removed, :after,  reader.first_name) }
+
+  def self.reset_log
+    @log = []
+  end
+  
+  def self.log(message=nil, side=nil, new_record=nil)
+    return @log if message.nil?
+    @log << [message, side, new_record]
+  end
 
   def self.what_are_you
     'a post...'