diff options
| -rw-r--r-- | activerecord/lib/active_record/associations/collection_association.rb | 12 | ||||
| -rw-r--r-- | activerecord/test/cases/associations/has_many_associations_test.rb | 45 |
2 files changed, 54 insertions, 3 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 7e1a41e84d..7a8c0bf59f 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -117,7 +117,7 @@ module ActiveRecord result = true load_target if owner.new_record? - transaction do + block = lambda do records.flatten.each do |record| raise_on_type_mismatch(record) add_to_target(record) do |r| @@ -126,6 +126,8 @@ module ActiveRecord end end + owner.new_record? ? block.call : transaction(&block) + result && records end @@ -295,7 +297,7 @@ module ActiveRecord other_array.each { |val| raise_on_type_mismatch(val) } original_target = load_target.dup - transaction do + block = lambda do delete(target - other_array) unless concat(other_array - target) @@ -304,6 +306,8 @@ module ActiveRecord "new records could not be saved." end end + + owner.new_record? ? block.call : transaction(&block) end def include?(record) @@ -444,7 +448,7 @@ module ActiveRecord records.each { |record| raise_on_type_mismatch(record) } existing_records = records.reject { |r| r.new_record? } - transaction do + block = lambda do records.each { |record| callback(:before_remove, record) } delete_records(existing_records, method) if existing_records.any? @@ -452,6 +456,8 @@ module ActiveRecord records.each { |record| callback(:after_remove, record) } end + + existing_records.any? ? transaction(&block) : block.call end # Delete the given records from the association, using one of the methods :destroy, diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 49999630b6..79b56d81df 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -537,6 +537,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 3, companies(:first_firm).clients_of_firm(true).size end + def test_transactions_when_adding_to_persisted + force_signal37_to_load_all_clients_of_firm + Client.expects(:transaction) + companies(:first_firm).clients_of_firm.concat(Client.new("name" => "Natural Company")) + end + + def test_transactions_when_adding_to_new_record + Client.expects(:transaction).never + firm = Firm.new + firm.clients_of_firm.concat(Client.new("name" => "Natural Company")) + end + def test_new_aliased_to_build company = companies(:first_firm) new_client = assert_no_queries { company.clients_of_firm.new("name" => "Another Client") } @@ -778,6 +790,21 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 0, companies(:first_firm).clients_of_firm(true).size end + def test_transaction_when_deleting_persisted + force_signal37_to_load_all_clients_of_firm + client = companies(:first_firm).clients_of_firm.create("name" => "Another Client") + Client.expects(:transaction) + companies(:first_firm).clients_of_firm.delete(client) + end + + def test_transaction_when_deleting_new_record + client = Client.new("name" => "New Client") + firm = Firm.new + firm.clients_of_firm << client + Client.expects(:transaction).never + firm.clients_of_firm.delete(client) + end + def test_clearing_an_association_collection firm = companies(:first_firm) client_id = firm.clients_of_firm.first.id @@ -1111,6 +1138,24 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal orig_accounts, firm.accounts end + def test_transactions_when_replacing_on_persisted + firm = Firm.find(:first, :order => "id") + firm.clients = [companies(:first_client)] + assert firm.save, "Could not save firm" + firm.reload + + Client.expects(:transaction) + firm.clients_of_firm = [Client.new("name" => "Natural Company")] + end + + def test_transactions_when_replacing_on_new_record + firm = Firm.new + firm.clients_of_firm << Client.new("name" => "Natural Company") + + Client.expects(:transaction).never + firm.clients_of_firm = [Client.new("name" => "New Client")] + end + def test_get_ids assert_equal [companies(:first_client).id, companies(:second_client).id], companies(:first_firm).client_ids end |