Skip to content

Commit

Permalink
Merge pull request #38166 from jules2689/master
Browse files Browse the repository at this point in the history
Only assign @new_record_before_save once in autosave_association
  • Loading branch information
rafaelfranca committed Jan 6, 2020
2 parents ebfe400 + d93f1ab commit 00b277b
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
4 changes: 3 additions & 1 deletion activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -365,7 +365,9 @@ def normalize_reflection_attribute(indexed_attribute, reflection, index, attribu
# Is used as a before_save callback to check while saving a collection
# association whether or not the parent was a new record before saving.
def before_save_collection_association
@new_record_before_save = new_record?
unless defined?(@new_record_before_save)
@new_record_before_save = new_record?
end
end

def after_save_collection_association
Expand Down
24 changes: 24 additions & 0 deletions activerecord/test/cases/autosave_association_test.rb
Expand Up @@ -35,6 +35,30 @@
require "models/reply"

class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase
def test_autosave_works_even_when_other_callbacks_update_the_parent_model
reference = Class.new(ActiveRecord::Base) do
self.table_name = "references"
def self.name; "Reference"; end
end

person = Class.new(ActiveRecord::Base) do
self.table_name = "people"
def self.name; "Person"; end

# It is necessary that the after_create is before the has_many _and_ that it updates the model.
# This replicates a bug found in https://github.com/rails/rails/issues/38120
after_create { update(first_name: "first name") }
has_many :references, autosave: true, anonymous_class: reference
end

reference_instance = reference.create!
person_instance = person.create!(first_name: "foo", references: [reference_instance])

reference_instance.reload
assert_equal person_instance.id, reference_instance.person_id
assert_equal "first name", person_instance.first_name # Make sure the after_create is actually called
end

def test_autosave_does_not_pass_through_non_custom_validation_contexts
person = Class.new(ActiveRecord::Base) {
self.table_name = "people"
Expand Down
2 changes: 2 additions & 0 deletions guides/source/active_record_callbacks.md
Expand Up @@ -117,6 +117,8 @@ Here is a list with all the available Active Record callbacks, listed in the sam

WARNING. `after_save` runs both on create and update, but always _after_ the more specific callbacks `after_create` and `after_update`, no matter the order in which the macro calls were executed.

WARNING. Care should be taken in callbacks to avoid updating attributes. For example, avoid running `update(attribute: "value")` and similar code during callbacks. This can alter the state of the model and may result in unexpected side effects during commit. Instead, you should try to assign values in the `before_create` or earlier callbacks.

NOTE: `before_destroy` callbacks should be placed before `dependent: :destroy`
associations (or use the `prepend: true` option), to ensure they execute before
the records are deleted by `dependent: :destroy`.
Expand Down

0 comments on commit 00b277b

Please sign in to comment.