aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md23
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb8
-rw-r--r--[-rwxr-xr-x]activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb0
-rw-r--r--[-rwxr-xr-x]activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb0
-rw-r--r--[-rwxr-xr-x]activerecord/lib/active_record/connection_adapters/mysql_adapter.rb0
-rw-r--r--activerecord/lib/active_record/migration/command_recorder.rb3
-rw-r--r--activerecord/lib/active_record/nested_attributes.rb10
-rw-r--r--activerecord/lib/active_record/railties/databases.rake5
-rw-r--r--[-rwxr-xr-x]activerecord/test/cases/base_test.rb0
-rw-r--r--activerecord/test/cases/migration/command_recorder_test.rb10
-rw-r--r--activerecord/test/cases/nested_attributes_with_callbacks_test.rb144
11 files changed, 191 insertions, 12 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index f53cb6ad51..e8ec99511e 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,26 @@
+* `add_to_target` now accepts a second optional `skip_callbacks` argument
+
+ If truthy, it will skip the :before_add and :after_add callbacks.
+
+ *Ben Woosley*
+
+* Fix interactions between `:before_add` callbacks and nested attributes
+ assignment of `has_many` associations, when the association was not
+ yet loaded:
+
+ - A `:before_add` callback was being called when a nested attributes
+ assignment assigned to an existing record.
+
+ - Nested Attributes assignment did not affect the record in the
+ association target when a `:before_add` callback triggered the
+ loading of the association
+
+ *Jörg Schray*
+
+* Allow enable_extension migration method to be revertible.
+
+ *Eric Tipton*
+
* Type cast hstore values on write, so that the value is consistent
with reading from the database.
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 8ce02afef8..228c500f0a 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -290,7 +290,7 @@ module ActiveRecord
# Returns true if the collection is empty.
#
- # If the collection has been loaded
+ # If the collection has been loaded
# it is equivalent to <tt>collection.size.zero?</tt>. If the
# collection has not been loaded, it is equivalent to
# <tt>collection.exists?</tt>. If the collection has not already been
@@ -366,8 +366,8 @@ module ActiveRecord
target
end
- def add_to_target(record)
- callback(:before_add, record)
+ def add_to_target(record, skip_callbacks = false)
+ callback(:before_add, record) unless skip_callbacks
yield(record) if block_given?
if association_scope.distinct_value && index = @target.index(record)
@@ -376,7 +376,7 @@ module ActiveRecord
@target << record
end
- callback(:after_add, record)
+ callback(:after_add, record) unless skip_callbacks
set_inverse_instance(record)
record
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 4b11ea795c..4b11ea795c 100755..100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
index 92796c996e..92796c996e 100755..100644
--- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 15b5452850..15b5452850 100755..100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb
index 9782a48055..01c73be849 100644
--- a/activerecord/lib/active_record/migration/command_recorder.rb
+++ b/activerecord/lib/active_record/migration/command_recorder.rb
@@ -73,7 +73,7 @@ module ActiveRecord
[:create_table, :create_join_table, :rename_table, :add_column, :remove_column,
:rename_index, :rename_column, :add_index, :remove_index, :add_timestamps, :remove_timestamps,
:change_column_default, :add_reference, :remove_reference, :transaction,
- :drop_join_table, :drop_table, :execute_block,
+ :drop_join_table, :drop_table, :execute_block, :enable_extension,
:change_column, :execute, :remove_columns, # irreversible methods need to be here too
].each do |method|
class_eval <<-EOV, __FILE__, __LINE__ + 1
@@ -100,6 +100,7 @@ module ActiveRecord
add_column: :remove_column,
add_timestamps: :remove_timestamps,
add_reference: :remove_reference,
+ enable_extension: :disable_extension
}.each do |cmd, inv|
[[inv, cmd], [cmd, inv]].uniq.each do |method, inverse|
class_eval <<-EOV, __FILE__, __LINE__ + 1
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index e53e8553ad..df28451bb7 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -465,19 +465,17 @@ module ActiveRecord
association.build(attributes.except(*UNASSIGNABLE_KEYS))
end
elsif existing_record = existing_records.detect { |record| record.id.to_s == attributes['id'].to_s }
- unless association.loaded? || call_reject_if(association_name, attributes)
+ unless call_reject_if(association_name, attributes)
# Make sure we are operating on the actual object which is in the association's
# proxy_target array (either by finding it, or adding it if not found)
- target_record = association.target.detect { |record| record == existing_record }
-
+ # Take into account that the proxy_target may have changed due to callbacks
+ target_record = association.target.detect { |record| record.id.to_s == attributes['id'].to_s }
if target_record
existing_record = target_record
else
- association.add_to_target(existing_record)
+ association.add_to_target(existing_record, :skip_callbacks)
end
- end
- if !call_reject_if(association_name, attributes)
assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy])
end
else
diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index 8a311039d5..daccab762f 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -320,11 +320,14 @@ db_namespace = namespace :db do
# desc "Recreate the test database from an existent schema.rb file"
task :load_schema => 'db:test:purge' do
begin
+ should_reconnect = ActiveRecord::Base.connection_pool.active_connection?
ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations['test'])
ActiveRecord::Schema.verbose = false
db_namespace["schema:load"].invoke
ensure
- ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[ActiveRecord::Tasks::DatabaseTasks.env])
+ if should_reconnect
+ ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[ActiveRecord::Tasks::DatabaseTasks.env])
+ end
end
end
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index c91cf89f6d..c91cf89f6d 100755..100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb
index 2cad8a6d96..1b205d372f 100644
--- a/activerecord/test/cases/migration/command_recorder_test.rb
+++ b/activerecord/test/cases/migration/command_recorder_test.rb
@@ -242,6 +242,16 @@ module ActiveRecord
add = @recorder.inverse_of :remove_belongs_to, [:table, :user]
assert_equal [:add_reference, [:table, :user], nil], add
end
+
+ def test_invert_enable_extension
+ disable = @recorder.inverse_of :enable_extension, ['uuid-ossp']
+ assert_equal [:disable_extension, ['uuid-ossp'], nil], disable
+ end
+
+ def test_invert_disable_extension
+ enable = @recorder.inverse_of :disable_extension, ['uuid-ossp']
+ assert_equal [:enable_extension, ['uuid-ossp'], nil], enable
+ end
end
end
end
diff --git a/activerecord/test/cases/nested_attributes_with_callbacks_test.rb b/activerecord/test/cases/nested_attributes_with_callbacks_test.rb
new file mode 100644
index 0000000000..43a69928b6
--- /dev/null
+++ b/activerecord/test/cases/nested_attributes_with_callbacks_test.rb
@@ -0,0 +1,144 @@
+require "cases/helper"
+require "models/pirate"
+require "models/bird"
+
+class NestedAttributesWithCallbacksTest < ActiveRecord::TestCase
+ Pirate.has_many(:birds_with_add_load,
+ :class_name => "Bird",
+ :before_add => proc { |p,b|
+ @@add_callback_called << b
+ p.birds_with_add_load.to_a
+ })
+ Pirate.has_many(:birds_with_add,
+ :class_name => "Bird",
+ :before_add => proc { |p,b| @@add_callback_called << b })
+
+ Pirate.accepts_nested_attributes_for(:birds_with_add_load,
+ :birds_with_add,
+ :allow_destroy => true)
+
+ def setup
+ @@add_callback_called = []
+ @pirate = Pirate.new.tap do |pirate|
+ pirate.catchphrase = "Don't call me!"
+ pirate.birds_attributes = [{:name => 'Bird1'},{:name => 'Bird2'}]
+ pirate.save!
+ end
+ @birds = @pirate.birds.to_a
+ end
+
+ def bird_to_update
+ @birds[0]
+ end
+
+ def bird_to_destroy
+ @birds[1]
+ end
+
+ def existing_birds_attributes
+ @birds.map do |bird|
+ bird.attributes.slice("id","name")
+ end
+ end
+
+ def new_birds
+ @pirate.birds_with_add.to_a - @birds
+ end
+
+ def new_bird_attributes
+ [{'name' => "New Bird"}]
+ end
+
+ def destroy_bird_attributes
+ [{'id' => bird_to_destroy.id.to_s, "_destroy" => true}]
+ end
+
+ def update_new_and_destroy_bird_attributes
+ [{'id' => @birds[0].id.to_s, 'name' => 'New Name'},
+ {'name' => "New Bird"},
+ {'id' => bird_to_destroy.id.to_s, "_destroy" => true}]
+ end
+
+ # Characterizing when :before_add callback is called
+ test ":before_add called for new bird when not loaded" do
+ assert_not @pirate.birds_with_add.loaded?
+ @pirate.birds_with_add_attributes = new_bird_attributes
+ assert_new_bird_with_callback_called
+ end
+
+ test ":before_add called for new bird when loaded" do
+ @pirate.birds_with_add.load_target
+ @pirate.birds_with_add_attributes = new_bird_attributes
+ assert_new_bird_with_callback_called
+ end
+
+ def assert_new_bird_with_callback_called
+ assert_equal(1, new_birds.size)
+ assert_equal(new_birds, @@add_callback_called)
+ end
+
+ test ":before_add not called for identical assignment when not loaded" do
+ assert_not @pirate.birds_with_add.loaded?
+ @pirate.birds_with_add_attributes = existing_birds_attributes
+ assert_callbacks_not_called
+ end
+
+ test ":before_add not called for identical assignment when loaded" do
+ @pirate.birds_with_add.load_target
+ @pirate.birds_with_add_attributes = existing_birds_attributes
+ assert_callbacks_not_called
+ end
+
+ test ":before_add not called for destroy assignment when not loaded" do
+ assert_not @pirate.birds_with_add.loaded?
+ @pirate.birds_with_add_attributes = destroy_bird_attributes
+ assert_callbacks_not_called
+ end
+
+ test ":before_add not called for deletion assignment when loaded" do
+ @pirate.birds_with_add.load_target
+ @pirate.birds_with_add_attributes = destroy_bird_attributes
+ assert_callbacks_not_called
+ end
+
+ def assert_callbacks_not_called
+ assert_empty new_birds
+ assert_empty @@add_callback_called
+ end
+
+ # Ensuring that the records in the association target are updated,
+ # whether the association is loaded before or not
+ test "Assignment updates records in target when not loaded" do
+ assert_not @pirate.birds_with_add.loaded?
+ @pirate.birds_with_add_attributes = update_new_and_destroy_bird_attributes
+ assert_assignment_affects_records_in_target(:birds_with_add)
+ end
+
+ test "Assignment updates records in target when loaded" do
+ @pirate.birds_with_add.load_target
+ @pirate.birds_with_add_attributes = update_new_and_destroy_bird_attributes
+ assert_assignment_affects_records_in_target(:birds_with_add)
+ end
+
+ test("Assignment updates records in target when not loaded" +
+ " and callback loads target") do
+ assert_not @pirate.birds_with_add_load.loaded?
+ @pirate.birds_with_add_load_attributes = update_new_and_destroy_bird_attributes
+ assert_assignment_affects_records_in_target(:birds_with_add_load)
+ end
+
+ test("Assignment updates records in target when loaded" +
+ " and callback loads target") do
+ @pirate.birds_with_add_load.load_target
+ @pirate.birds_with_add_load_attributes = update_new_and_destroy_bird_attributes
+ assert_assignment_affects_records_in_target(:birds_with_add_load)
+ end
+
+ def assert_assignment_affects_records_in_target(association_name)
+ association = @pirate.send(association_name)
+ assert association.detect {|b| b == bird_to_update }.name_changed?,
+ 'Update record not updated'
+ assert association.detect {|b| b == bird_to_destroy }.marked_for_destruction?,
+ 'Destroy record not marked for destruction'
+ end
+end
generated by cgit v1.2.3 (git 2.25.1) at 2025-10-18 06:23:29 +0000