Fixed that autosave should validate associations even if master is invalid [#1930 status:committed]

1 files changed, 442 insertions, 8 deletions

diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index 381249c0c2..6ced84e0b6 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -1,10 +1,20 @@
-require "cases/helper"
-require "models/pirate"
-require "models/ship"
-require "models/ship_part"
-require "models/bird"
-require "models/parrot"
-require "models/treasure"
+require 'cases/helper'
+require 'models/bird'
+require 'models/company'
+require 'models/customer'
+require 'models/developer'
+require 'models/order'
+require 'models/parrot'
+require 'models/person'
+require 'models/pirate'
+require 'models/post'
+require 'models/reader'
+require 'models/ship'
+require 'models/ship_part'
+require 'models/treasure'
+
+# TODO:
+# - add test case for new parent and children with invalid data and saving with validate = false
 
 class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
   def test_autosave_should_be_a_valid_option_for_has_one
@@ -30,6 +40,383 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
   end
 end
 
+class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
+  def test_save_fails_for_invalid_has_one
+    firm = Firm.find(:first)
+    assert firm.valid?
+
+    firm.account = Account.new
+
+    assert !firm.account.valid?
+    assert !firm.valid?
+    assert !firm.save
+    assert_equal "is invalid", firm.errors.on("account")
+  end
+
+  def test_save_succeeds_for_invalid_has_one_with_validate_false
+    firm = Firm.find(:first)
+    assert firm.valid?
+
+    firm.unvalidated_account = Account.new
+
+    assert !firm.unvalidated_account.valid?
+    assert firm.valid?
+    assert firm.save
+  end
+
+  def test_build_before_child_saved
+    firm = Firm.find(1)
+
+    account = firm.account.build("credit_limit" => 1000)
+    assert_equal account, firm.account
+    assert account.new_record?
+    assert firm.save
+    assert_equal account, firm.account
+    assert !account.new_record?
+  end
+
+  def test_build_before_either_saved
+    firm = Firm.new("name" => "GlobalMegaCorp")
+
+    firm.account = account = Account.new("credit_limit" => 1000)
+    assert_equal account, firm.account
+    assert account.new_record?
+    assert firm.save
+    assert_equal account, firm.account
+    assert !account.new_record?
+  end
+
+  def test_assignment_before_parent_saved
+    firm = Firm.new("name" => "GlobalMegaCorp")
+    firm.account = a = Account.find(1)
+    assert firm.new_record?
+    assert_equal a, firm.account
+    assert firm.save
+    assert_equal a, firm.account
+    assert_equal a, firm.account(true)
+  end
+
+  def test_assignment_before_either_saved
+    firm = Firm.new("name" => "GlobalMegaCorp")
+    firm.account = a = Account.new("credit_limit" => 1000)
+    assert firm.new_record?
+    assert a.new_record?
+    assert_equal a, firm.account
+    assert firm.save
+    assert !firm.new_record?
+    assert !a.new_record?
+    assert_equal a, firm.account
+    assert_equal a, firm.account(true)
+  end
+
+  def test_not_resaved_when_unchanged
+    firm = Firm.find(:first, :include => :account)
+    firm.name += '-changed'
+    assert_queries(1) { firm.save! }
+
+    firm = Firm.find(:first)
+    firm.account = Account.find(:first)
+    assert_queries(Firm.partial_updates? ? 0 : 1) { firm.save! }
+
+    firm = Firm.find(:first).clone
+    firm.account = Account.find(:first)
+    assert_queries(2) { firm.save! }
+
+    firm = Firm.find(:first).clone
+    firm.account = Account.find(:first).clone
+    assert_queries(2) { firm.save! }
+  end
+end
+
+class TestDefaultAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
+  def test_save_fails_for_invalid_belongs_to
+    assert log = AuditLog.create(:developer_id => 0, :message => "")
+
+    log.developer = Developer.new
+    assert !log.developer.valid?
+    assert !log.valid?
+    assert !log.save
+    assert_equal "is invalid", log.errors.on("developer")
+  end
+
+  def test_save_succeeds_for_invalid_belongs_to_with_validate_false
+    assert log = AuditLog.create(:developer_id => 0, :message=> "")
+
+    log.unvalidated_developer = Developer.new
+    assert !log.unvalidated_developer.valid?
+    assert log.valid?
+    assert log.save
+  end
+
+  def test_assignment_before_parent_saved
+    client = Client.find(:first)
+    apple = Firm.new("name" => "Apple")
+    client.firm = apple
+    assert_equal apple, client.firm
+    assert apple.new_record?
+    assert client.save
+    assert apple.save
+    assert !apple.new_record?
+    assert_equal apple, client.firm
+    assert_equal apple, client.firm(true)
+  end
+
+  def test_assignment_before_either_saved
+    final_cut = Client.new("name" => "Final Cut")
+    apple = Firm.new("name" => "Apple")
+    final_cut.firm = apple
+    assert final_cut.new_record?
+    assert apple.new_record?
+    assert final_cut.save
+    assert !final_cut.new_record?
+    assert !apple.new_record?
+    assert_equal apple, final_cut.firm
+    assert_equal apple, final_cut.firm(true)
+  end
+
+  def test_store_two_association_with_one_save
+    num_orders = Order.count
+    num_customers = Customer.count
+    order = Order.new
+
+    customer1 = order.billing = Customer.new
+    customer2 = order.shipping = Customer.new
+    assert order.save
+    assert_equal customer1, order.billing
+    assert_equal customer2, order.shipping
+
+    order.reload
+
+    assert_equal customer1, order.billing
+    assert_equal customer2, order.shipping
+
+    assert_equal num_orders +1, Order.count
+    assert_equal num_customers +2, Customer.count
+  end
+
+  def test_store_association_in_two_relations_with_one_save
+    num_orders = Order.count
+    num_customers = Customer.count
+    order = Order.new
+
+    customer = order.billing = order.shipping = Customer.new
+    assert order.save
+    assert_equal customer, order.billing
+    assert_equal customer, order.shipping
+
+    order.reload
+
+    assert_equal customer, order.billing
+    assert_equal customer, order.shipping
+
+    assert_equal num_orders +1, Order.count
+    assert_equal num_customers +1, Customer.count
+  end
+
+  def test_store_association_in_two_relations_with_one_save_in_existing_object
+    num_orders = Order.count
+    num_customers = Customer.count
+    order = Order.create
+
+    customer = order.billing = order.shipping = Customer.new
+    assert order.save
+    assert_equal customer, order.billing
+    assert_equal customer, order.shipping
+
+    order.reload
+
+    assert_equal customer, order.billing
+    assert_equal customer, order.shipping
+
+    assert_equal num_orders +1, Order.count
+    assert_equal num_customers +1, Customer.count
+  end
+
+  def test_store_association_in_two_relations_with_one_save_in_existing_object_with_values
+    num_orders = Order.count
+    num_customers = Customer.count
+    order = Order.create
+
+    customer = order.billing = order.shipping = Customer.new
+    assert order.save
+    assert_equal customer, order.billing
+    assert_equal customer, order.shipping
+
+    order.reload
+
+    customer = order.billing = order.shipping = Customer.new
+
+    assert order.save
+    order.reload
+
+    assert_equal customer, order.billing
+    assert_equal customer, order.shipping
+
+    assert_equal num_orders +1, Order.count
+    assert_equal num_customers +2, Customer.count
+  end
+end
+
+class TestDefaultAutosaveAssociationOnAHasManyAssociation < ActiveRecord::TestCase
+  fixtures :companies, :people
+
+  def test_invalid_adding
+    firm = Firm.find(1)
+    assert !(firm.clients_of_firm << c = Client.new)
+    assert c.new_record?
+    assert !firm.valid?
+    assert !firm.save
+    assert c.new_record?
+  end
+
+  def test_invalid_adding_before_save
+    no_of_firms = Firm.count
+    no_of_clients = Client.count
+    new_firm = Firm.new("name" => "A New Firm, Inc")
+    new_firm.clients_of_firm.concat([c = Client.new, Client.new("name" => "Apple")])
+    assert c.new_record?
+    assert !c.valid?
+    assert !new_firm.valid?
+    assert !new_firm.save
+    assert c.new_record?
+    assert new_firm.new_record?
+  end
+
+  def test_invalid_adding_with_validate_false
+    firm = Firm.find(:first)
+    client = Client.new
+    firm.unvalidated_clients_of_firm << client
+
+    assert firm.valid?
+    assert !client.valid?
+    assert firm.save
+    assert client.new_record?
+  end
+
+  def test_valid_adding_with_validate_false
+    no_of_clients = Client.count
+
+    firm = Firm.find(:first)
+    client = Client.new("name" => "Apple")
+
+    assert firm.valid?
+    assert client.valid?
+    assert client.new_record?
+
+    firm.unvalidated_clients_of_firm << client
+
+    assert firm.save
+    assert !client.new_record?
+    assert_equal no_of_clients+1, Client.count
+  end
+
+  def test_invalid_build
+    new_client = companies(:first_firm).clients_of_firm.build
+    assert new_client.new_record?
+    assert !new_client.valid?
+    assert_equal new_client, companies(:first_firm).clients_of_firm.last
+    assert !companies(:first_firm).save
+    assert new_client.new_record?
+    assert_equal 1, companies(:first_firm).clients_of_firm(true).size
+  end
+
+  def test_adding_before_save
+    no_of_firms = Firm.count
+    no_of_clients = Client.count
+
+    new_firm = Firm.new("name" => "A New Firm, Inc")
+    c = Client.new("name" => "Apple")
+
+    new_firm.clients_of_firm.push Client.new("name" => "Natural Company")
+    assert_equal 1, new_firm.clients_of_firm.size
+    new_firm.clients_of_firm << c
+    assert_equal 2, new_firm.clients_of_firm.size
+
+    assert_equal no_of_firms, Firm.count      # Firm was not saved to database.
+    assert_equal no_of_clients, Client.count  # Clients were not saved to database.
+    assert new_firm.save
+    assert !new_firm.new_record?
+    assert !c.new_record?
+    assert_equal new_firm, c.firm
+    assert_equal no_of_firms+1, Firm.count      # Firm was saved to database.
+    assert_equal no_of_clients+2, Client.count  # Clients were saved to database.
+
+    assert_equal 2, new_firm.clients_of_firm.size
+    assert_equal 2, new_firm.clients_of_firm(true).size
+  end
+
+  def test_assign_ids
+    firm = Firm.new("name" => "Apple")
+    firm.client_ids = [companies(:first_client).id, companies(:second_client).id]
+    firm.save
+    firm.reload
+    assert_equal 2, firm.clients.length
+    assert firm.clients.include?(companies(:second_client))
+  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_build_before_save
+    company = companies(:first_firm)
+    new_client = assert_no_queries { company.clients_of_firm.build("name" => "Another Client") }
+    assert !company.clients_of_firm.loaded?
+
+    company.name += '-changed'
+    assert_queries(2) { assert company.save }
+    assert !new_client.new_record?
+    assert_equal 2, company.clients_of_firm(true).size
+  end
+
+  def test_build_many_before_save
+    company = companies(:first_firm)
+    new_clients = assert_no_queries { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) }
+
+    company.name += '-changed'
+    assert_queries(3) { assert company.save }
+    assert_equal 3, company.clients_of_firm(true).size
+  end
+
+  def test_build_via_block_before_save
+    company = companies(:first_firm)
+    new_client = assert_no_queries { company.clients_of_firm.build {|client| client.name = "Another Client" } }
+    assert !company.clients_of_firm.loaded?
+
+    company.name += '-changed'
+    assert_queries(2) { assert company.save }
+    assert !new_client.new_record?
+    assert_equal 2, company.clients_of_firm(true).size
+  end
+
+  def test_build_many_via_block_before_save
+    company = companies(:first_firm)
+    new_clients = assert_no_queries do
+      company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) do |client|
+        client.name = "changed"
+      end
+    end
+
+    company.name += '-changed'
+    assert_queries(3) { assert company.save }
+    assert_equal 3, company.clients_of_firm(true).size
+  end
+
+  def test_replace_on_new_object
+    firm = Firm.new("name" => "New Firm")
+    firm.clients = [companies(:second_client), Client.new("name" => "New Client")]
+    assert firm.save
+    firm.reload
+    assert_equal 2, firm.clients.length
+    assert firm.clients.include?(Client.find_by_name("New Client"))
+  end
+end
+
 class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
   self.use_transactional_fixtures = false
 
@@ -181,6 +568,14 @@ class TestAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase
     assert !@pirate.errors.on(:ship_name).blank?
   end
 
+  def test_should_merge_errors_on_the_associated_models_onto_the_parent_even_if_it_is_not_valid
+    @pirate.ship.name   = nil
+    @pirate.catchphrase = nil
+    assert !@pirate.valid?
+    assert !@pirate.errors.on(:ship_name).blank?
+    assert !@pirate.errors.on(:catchphrase).blank?
+  end
+
   def test_should_still_allow_to_bypass_validations_on_the_associated_model
     @pirate.catchphrase = ''
     @pirate.ship.name = ''
@@ -263,6 +658,14 @@ class TestAutosaveAssociationOnABelongsToAssociation < ActiveRecord::TestCase
     assert !@ship.errors.on(:pirate_catchphrase).blank?
   end
 
+  def test_should_merge_errors_on_the_associated_model_onto_the_parent_even_if_it_is_not_valid
+    @ship.name = nil
+    @ship.pirate.catchphrase = nil
+    assert !@ship.valid?
+    assert !@ship.errors.on(:name).blank?
+    assert !@ship.errors.on(:pirate_catchphrase).blank?
+  end
+
   def test_should_still_allow_to_bypass_validations_on_the_associated_model
     @ship.pirate.catchphrase = ''
     @ship.name = ''
@@ -326,7 +729,24 @@ module AutosaveAssociationOnACollectionAssociationTests
     assert @pirate.errors.on(@association_name).blank?
   end
 
-  def test_should_still_allow_to_bypass_validations_on_the_associated_models
+  def test_should_not_use_default_invalid_error_on_associated_models
+    @pirate.send(@association_name).build(:name => '')
+
+    assert !@pirate.valid?
+    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}_name")
+    assert @pirate.errors.on(@association_name).blank?
+  end
+
+  def test_should_merge_errors_on_the_associated_models_onto_the_parent_even_if_it_is_not_valid
+    @pirate.send(@association_name).each { |child| child.name = '' }
+    @pirate.catchphrase = nil
+
+    assert !@pirate.valid?
+    assert_equal "can't be blank", @pirate.errors.on("#{@association_name}_name")
+    assert !@pirate.errors.on(:catchphrase).blank?
+  end
+
+  def test_should_allow_to_bypass_validations_on_the_associated_models_on_update
     @pirate.catchphrase = ''
     @pirate.send(@association_name).each { |child| child.name = '' }
 
@@ -338,6 +758,20 @@ module AutosaveAssociationOnACollectionAssociationTests
     ]
   end
 
+  def test_should_validation_the_associated_models_on_create
+    assert_no_difference("#{ @association_name == :birds ? 'Bird' : 'Parrot' }.count") do
+      2.times { @pirate.send(@association_name).build }
+      @pirate.save(true)
+    end
+  end
+
+  def test_should_allow_to_bypass_validations_on_the_associated_models_on_create
+    assert_difference("#{ @association_name == :birds ? 'Bird' : 'Parrot' }.count", +2) do
+      2.times { @pirate.send(@association_name).build }
+      @pirate.save(false)
+    end
+  end
+
   def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
     before = [@pirate.catchphrase, *@pirate.send(@association_name).map(&:name)]
     new_names = ['Grace OMalley', 'Privateers Greed']