diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1f1131b32bf1d..10d741914dc39 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Deprecate using `insert_all`/`upsert_all` with unpersisted records in associations. + + Using these methods on associations containing unpersisted records will now + show a deprecation warning, as the unpersisted records will be lost after + the operation. This will become an error in Rails 8.2. + + *Nick Schwaderer* + * Introduce versions formatter for the schema dumper. It is now possible to override how schema dumper formats versions information inside the diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 930179ed0ecd4..4f07ca5cc1079 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -1128,7 +1128,18 @@ def pretty_print(pp) # :nodoc: %w(insert insert_all insert! insert_all! upsert upsert_all).each do |method| class_eval <<~RUBY, __FILE__, __LINE__ + 1 def #{method}(...) - scope.#{method}(...).tap { reset } + if loaded? && @association&.target&.any? { |r| r.new_record? } + ActiveRecord.deprecator.warn(<<~MSG) + Using #{method} on association with unpersisted records + (\#{to_a.reject(&:persisted?).map(&:class).uniq.join(', ')}) + is deprecated. The unpersisted records will be lost after this operation. + Please either persist your records first or store them separately before + calling #{method}. This will become an error in Rails 8.1. + MSG + scope.#{method}(...) + else + scope.#{method}(...).tap { reset } + end end RUBY end diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index fb39dce3d1f32..4823c969ec56b 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -16,7 +16,7 @@ class ReadonlyNameBook < Book end class InsertAllTest < ActiveRecord::TestCase - fixtures :books + fixtures :books, :authors def setup Arel::Table.engine = nil # should not rely on the global Arel::Table.engine @@ -863,6 +863,32 @@ def test_insert_all_when_table_name_contains_database end end + def test_insert_all_with_unpersisted_records_deprecation + author = Author.create!(name: "Rafael") + author.books.build(title: "Unpersisted Book") + author.books.load + + assert_deprecated(ActiveRecord.deprecator) do + author.books.insert_all([{ title: "New Book" }]) + end + + assert_equal 1, author.books.size + assert_equal ["Unpersisted Book"], author.books.pluck(:title) + end + + def test_insert_all_resets_without_unpersisted_records + author = Author.create!(name: "Rafael") + author.books.load + + assert_not_deprecated(ActiveRecord.deprecator) do + author.books.insert_all([{ title: "New Book" }]) + end + + assert_not_predicate author.books, :loaded? + assert_equal 1, author.books.size + assert_equal ["New Book"], author.books.pluck(:title) + end + private def capture_log_output output = StringIO.new