Skip to content

Commit 5eeb9a7

Browse files
Schwadrafaelfranca
authored andcommitted
Deprecate using insert_all/upsert_all with unpersisted records in associations.
1 parent e25d738 commit 5eeb9a7

File tree

3 files changed

+120
-1
lines changed

3 files changed

+120
-1
lines changed

activerecord/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
* Deprecate using `insert_all`/`upsert_all` with unpersisted records in associations.
2+
3+
Using these methods on associations containing unpersisted records will now
4+
show a deprecation warning, as the unpersisted records will be lost after
5+
the operation. This will become an error in Rails 8.2.
6+
7+
*Nick Schwaderer*
8+
19
* Make column name optional for `index_exists?`.
210

311
This aligns well with `remove_index` signature as well, where

activerecord/lib/active_record/associations/collection_proxy.rb

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1128,7 +1128,18 @@ def pretty_print(pp) # :nodoc:
11281128
%w(insert insert_all insert! insert_all! upsert upsert_all).each do |method|
11291129
class_eval <<~RUBY, __FILE__, __LINE__ + 1
11301130
def #{method}(...)
1131-
scope.#{method}(...).tap { reset }
1131+
if @association&.target&.any? { |r| r.new_record? }
1132+
association_name = @association.reflection.name
1133+
ActiveRecord.deprecator.warn(<<~MSG)
1134+
Using #{method} on association \#{association_name} with unpersisted records
1135+
is deprecated. The unpersisted records will be lost after this operation.
1136+
Please either persist your records first or store them separately before
1137+
calling #{method}.
1138+
MSG
1139+
scope.#{method}(...)
1140+
else
1141+
scope.#{method}(...).tap { reset }
1142+
end
11321143
end
11331144
RUBY
11341145
end

activerecord/test/cases/insert_all_test.rb

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -863,6 +863,106 @@ def test_insert_all_when_table_name_contains_database
863863
end
864864
end
865865

866+
def test_insert_all_with_unpersisted_records_triggers_deprecation
867+
author = Author.create!(name: "Rafael")
868+
author.books.build(title: "Unpersisted Book")
869+
870+
assert_deprecated(ActiveRecord.deprecator) do
871+
author.books.insert({ title: "New Book" })
872+
end
873+
874+
author.books.load
875+
assert_includes author.books.pluck(:title), "Unpersisted Book"
876+
end
877+
878+
def test_insert_all_without_unpersisted_records_has_no_deprecation
879+
author = Author.create!(name: "Rafael")
880+
881+
assert_not_deprecated(ActiveRecord.deprecator) do
882+
author.books.insert_all([{ title: "New Book" }])
883+
end
884+
end
885+
886+
def test_insert_with_unpersisted_records_triggers_deprecation
887+
author = Author.create!(name: "Rafael")
888+
author.books.build(title: "Unpersisted Book")
889+
890+
assert_deprecated(ActiveRecord.deprecator) do
891+
author.books.insert({ title: "New Book" })
892+
end
893+
894+
author.books.load
895+
assert_includes author.books.pluck(:title), "Unpersisted Book"
896+
end
897+
898+
def test_insert_without_unpersisted_records_has_no_deprecation
899+
author = Author.create!(name: "Rafael")
900+
901+
assert_not_deprecated(ActiveRecord.deprecator) do
902+
author.books.insert({ title: "New Book" })
903+
end
904+
end
905+
906+
def test_insert_all_bang_with_unpersisted_record_triggers_deprecation
907+
author = Author.create!(name: "Rafael")
908+
author.books.build(title: "Unpersisted Book")
909+
910+
assert_deprecated(ActiveRecord.deprecator) do
911+
author.books.insert_all!([{ title: "New Book" }])
912+
end
913+
914+
author.books.load
915+
assert_includes author.books.pluck(:title), "Unpersisted Book"
916+
end
917+
918+
def test_insert_all_bang_without_unpersisted_records_has_no_deprecation
919+
author = Author.create!(name: "Rafael")
920+
921+
assert_not_deprecated(ActiveRecord.deprecator) do
922+
author.books.insert_all!([{ title: "New Book" }])
923+
end
924+
end
925+
926+
def test_upsert_all_with_unpersisted_record_triggers_deprecation
927+
author = Author.create!(name: "Rafael")
928+
author.books.build(title: "Unpersisted Book")
929+
930+
assert_deprecated(ActiveRecord.deprecator) do
931+
author.books.upsert_all([{ title: "New Book" }])
932+
end
933+
934+
author.books.load
935+
assert_includes author.books.pluck(:title), "Unpersisted Book"
936+
end
937+
938+
def test_upsert_all_without_unpersisted_records_has_no_deprecation
939+
author = Author.create!(name: "Rafael")
940+
941+
assert_not_deprecated(ActiveRecord.deprecator) do
942+
author.books.upsert_all([{ title: "New Book" }])
943+
end
944+
end
945+
946+
def test_upsert_with_unpersisted_record_triggers_deprecation
947+
author = Author.create!(name: "Rafael")
948+
author.books.build(title: "Unpersisted Book")
949+
950+
assert_deprecated(ActiveRecord.deprecator) do
951+
author.books.upsert({ title: "New Book" })
952+
end
953+
954+
author.books.load
955+
assert_includes author.books.pluck(:title), "Unpersisted Book"
956+
end
957+
958+
def test_upsert_without_unpersisted_records_has_no_deprecation
959+
author = Author.create!(name: "Rafael")
960+
961+
assert_not_deprecated(ActiveRecord.deprecator) do
962+
author.books.upsert({ title: "New Book" })
963+
end
964+
end
965+
866966
private
867967
def capture_log_output
868968
output = StringIO.new

0 commit comments

Comments
 (0)