Skip to content
Browse files

let `insert_record` actuall save the object.

`before_add` callbacks are fired before the record is saved on
`has_and_belongs_to_many` assocations *and* on `has_many :through`
associations.  Before this change, `before_add` callbacks would be fired
before the record was saved on `has_and_belongs_to_many` associations, but
*not* on `has_many :through` associations.

Fixes #14144
  • Loading branch information...
1 parent 7b98746 commit 759c99b23dfd3318a867b07769f389f5bd24b224 @tenderlove tenderlove committed Feb 25, 2014
View
8 activerecord/CHANGELOG.md
@@ -1,3 +1,11 @@
+* `before_add` callbacks are fired before the record is saved on
+ `has_and_belongs_to_many` assocations *and* on `has_many :through`
+ associations. Before this change, `before_add` callbacks would be fired
+ before the record was saved on `has_and_belongs_to_many` associations, but
+ *not* on `has_many :through` associations.
+
+ Fixes #14144
+
* Fixed STI classes not defining an attribute method if there is a
conflicting private method defined on its ancestors.
View
4 activerecord/lib/active_record/associations/collection_association.rb
@@ -513,13 +513,13 @@ def replace_records(new_target, original_target)
target
end
- def concat_records(records)
+ def concat_records(records, should_raise = false)
result = true
records.flatten.each do |record|
raise_on_type_mismatch!(record)
add_to_target(record) do |rec|
- result &&= insert_record(rec) unless owner.new_record?
+ result &&= insert_record(rec, true, should_raise) unless owner.new_record?
end
end
View
3 activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -30,7 +30,6 @@ def concat(*records)
unless owner.new_record?
records.flatten.each do |record|
raise_on_type_mismatch!(record)
- record.save! if record.new_record?
end
end
@@ -40,7 +39,7 @@ def concat(*records)
def concat_records(records)
ensure_not_nested
- records = super
+ records = super(records, true)
if owner.new_record? && records
records.flatten.each do |record|
View
21 activerecord/test/cases/associations/callbacks_test.rb
@@ -101,6 +101,27 @@ def test_has_and_belongs_to_many_add_callback
"after_adding#{david.id}"], ar.developers_log
end
+ def test_has_and_belongs_to_many_before_add_called_before_save
+ dev = nil
+ new_dev = nil
+ klass = Class.new(Project) do
+ def self.name; Project.name; end
+ has_and_belongs_to_many :developers_with_callbacks,
+ :class_name => "Developer",
+ :before_add => lambda { |o,r|
+ dev = r
+ new_dev = r.new_record?
+ }
+ end
+ rec = klass.create!
+ alice = Developer.new(:name => 'alice')
+ rec.developers_with_callbacks << alice
+ assert_equal alice, dev
+ assert_not_nil new_dev
+ assert new_dev, "record should not have been saved"
+ assert_not alice.new_record?
+ end
+
def test_has_and_belongs_to_many_after_add_called_after_save
ar = projects(:active_record)
assert ar.developers_log.empty?

0 comments on commit 759c99b

Please sign in to comment.
Something went wrong with that request. Please try again.