Skip to content

Commit

Permalink
Deprecate using insert_all/upsert_all with unpersisted records in ass…
Browse files Browse the repository at this point in the history
…ociations.
  • Loading branch information
Schwad committed Dec 11, 2024
1 parent 410ad1c commit cac6527
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 1 deletion.
8 changes: 8 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.1.

*Nick Schwaderer*

* SQLite extensions can be configured in `config/database.yml`.

The database configuration option `extensions:` allows an application to load SQLite extensions
Expand Down
14 changes: 13 additions & 1 deletion activerecord/lib/active_record/associations/collection_proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,19 @@ 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? }
unpersisted_records = @association.target.reject(&:persisted?)
ActiveRecord.deprecator.warn(<<~MSG)
Using #{method} on association with unpersisted records
(#{@association.target.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
Expand Down
24 changes: 24 additions & 0 deletions activerecord/test/cases/insert_all_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,30 @@ def test_insert_all_when_table_name_contains_database
end
end

def test_insert_all_with_unpersisted_records_deprecation
author = Author.create!(name: "DHH")
author.books.build(title: "Unpersisted Book")
author.books.load

assert_deprecated(ActiveRecord.deprecator) do
author.books.insert_all([{ title: "New Book" }])
end

assert_equal 2, author.books.to_a.size
assert_equal ["Unpersisted Book", "New Book"], author.books.pluck(:title)
end

def test_insert_all_resets_without_unpersisted_records
author = Author.create!(name: "DHH")
author.books.load

assert_not_deprecated(ActiveRecord.deprecator) do
author.books.insert_all([{ title: "New Book" }])
end

assert_not author.books.loaded?
end

private
def capture_log_output
output = StringIO.new
Expand Down

0 comments on commit cac6527

Please sign in to comment.