diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index ed8fc28b4dda..348205e956d6 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* + * Serialized attributes can now be marked as comparable. A not rare issue when working with serialized attributes is that the serialized representation of an object diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 930179ed0ecd..65127ef1cce7 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 @association&.target&.any? { |r| r.new_record? } + association_name = @association.reflection.name + ActiveRecord.deprecator.warn(<<~MSG) + Using #{method} on association \#{association_name} with unpersisted records + is deprecated. The unpersisted records will be lost after this operation. + Please either persist your records first or store them separately before + calling #{method}. + 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 fb39dce3d1f3..c102d6967045 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -863,6 +863,106 @@ def test_insert_all_when_table_name_contains_database end end + def test_insert_all_with_unpersisted_records_triggers_deprecation + author = Author.create!(name: "Rafael") + author.books.build(title: "Unpersisted Book") + + assert_deprecated(ActiveRecord.deprecator) do + author.books.insert({ title: "New Book" }) + end + + author.books.load + assert_includes author.books.pluck(:title), "Unpersisted Book" + end + + def test_insert_all_without_unpersisted_records_has_no_deprecation + author = Author.create!(name: "Rafael") + + assert_not_deprecated(ActiveRecord.deprecator) do + author.books.insert_all([{ title: "New Book" }]) + end + end + + def test_insert_with_unpersisted_records_triggers_deprecation + author = Author.create!(name: "Rafael") + author.books.build(title: "Unpersisted Book") + + assert_deprecated(ActiveRecord.deprecator) do + author.books.insert({ title: "New Book" }) + end + + author.books.load + assert_includes author.books.pluck(:title), "Unpersisted Book" + end + + def test_insert_without_unpersisted_records_has_no_deprecation + author = Author.create!(name: "Rafael") + + assert_not_deprecated(ActiveRecord.deprecator) do + author.books.insert({ title: "New Book" }) + end + end + + def test_insert_all_bang_with_unpersisted_record_triggers_deprecation + author = Author.create!(name: "Rafael") + author.books.build(title: "Unpersisted Book") + + assert_deprecated(ActiveRecord.deprecator) do + author.books.insert_all!([{ title: "New Book" }]) + end + + author.books.load + assert_includes author.books.pluck(:title), "Unpersisted Book" + end + + def test_insert_all_bang_without_unpersisted_records_has_no_deprecation + author = Author.create!(name: "Rafael") + + assert_not_deprecated(ActiveRecord.deprecator) do + author.books.insert_all!([{ title: "New Book" }]) + end + end + + def test_upsert_all_with_unpersisted_record_triggers_deprecation + author = Author.create!(name: "Rafael") + author.books.build(title: "Unpersisted Book") + + assert_deprecated(ActiveRecord.deprecator) do + author.books.upsert_all([{ title: "New Book" }]) + end + + author.books.load + assert_includes author.books.pluck(:title), "Unpersisted Book" + end + + def test_upsert_all_without_unpersisted_records_has_no_deprecation + author = Author.create!(name: "Rafael") + + assert_not_deprecated(ActiveRecord.deprecator) do + author.books.upsert_all([{ title: "New Book" }]) + end + end + + def test_upsert_with_unpersisted_record_triggers_deprecation + author = Author.create!(name: "Rafael") + author.books.build(title: "Unpersisted Book") + + assert_deprecated(ActiveRecord.deprecator) do + author.books.upsert({ title: "New Book" }) + end + + author.books.load + assert_includes author.books.pluck(:title), "Unpersisted Book" + end + + def test_upsert_without_unpersisted_records_has_no_deprecation + author = Author.create!(name: "Rafael") + + assert_not_deprecated(ActiveRecord.deprecator) do + author.books.upsert({ title: "New Book" }) + end + end + private def capture_log_output output = StringIO.new