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 Jan 21, 2025
1 parent 83311f4 commit 1524683
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 1 deletion.
43 changes: 43 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,46 @@
* 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*

* Make column name optional for `index_exists?`.

This aligns well with `remove_index` signature as well, where
index name doesn't need to be derived from the column names.

*Ali Ismayiliov*

* Change the payload name of `sql.active_record` notification for eager
loading from "SQL" to "#{model.name} Eager Load".

*zzak*

* Enable automatically retrying idempotent `#exists?` queries on connection
errors.

*Hartley McGuire*, *classidied*

* Deprecate usage of unsupported methods in conjunction with `update_all`:

`update_all` will now print a deprecation message if a query includes either `WITH`,
`WITH RECURSIVE` or `DISTINCT` statements. Those were never supported and were ignored
when generating the SQL query.

An error will be raised in a future Rails release. This behaviour will be consistent
with `delete_all` which currently raises an error for unsupported statements.

*Edouard Chin*

* The table columns inside `schema.rb` are now sorted alphabetically.

Previously they'd be sorted by creation order, which can cause merge conflicts when two
branches modify the same table concurrently.

*John Duff*

* Introduce versions formatter for the schema dumper.

It is now possible to override how schema dumper formats versions information inside the
Expand Down
13 changes: 12 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,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
Expand Down
100 changes: 100 additions & 0 deletions activerecord/test/cases/insert_all_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1524683

Please sign in to comment.