mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Class level update
and destroy
checks all the records exist before making changes (#31306)
It makes more sense than ignoring invalid IDs.
This commit is contained in:
parent
d041a1dcba
commit
695ec1fef0
2 changed files with 41 additions and 13 deletions
|
@ -99,11 +99,9 @@ module ActiveRecord
|
|||
# for updating all records in a single query.
|
||||
def update(id = :all, attributes)
|
||||
if id.is_a?(Array)
|
||||
id.map.with_index { |one_id, idx|
|
||||
object = find_by(primary_key => one_id)
|
||||
object.update(attributes[idx]) if object
|
||||
object
|
||||
}.compact
|
||||
id.map { |one_id| find(one_id) }.each_with_index { |object, idx|
|
||||
object.update(attributes[idx])
|
||||
}
|
||||
elsif id == :all
|
||||
all.each { |record| record.update(attributes) }
|
||||
else
|
||||
|
@ -139,10 +137,7 @@ module ActiveRecord
|
|||
# Todo.destroy(todos)
|
||||
def destroy(id)
|
||||
if id.is_a?(Array)
|
||||
id.map { |one_id|
|
||||
object = find_by(primary_key => one_id)
|
||||
object.destroy if object
|
||||
}.compact
|
||||
find(id).each(&:destroy)
|
||||
else
|
||||
find(id).destroy
|
||||
end
|
||||
|
|
|
@ -70,7 +70,7 @@ class PersistenceTest < ActiveRecord::TestCase
|
|||
end
|
||||
|
||||
def test_update_many
|
||||
topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, nil => {} }
|
||||
topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } }
|
||||
updated = Topic.update(topic_data.keys, topic_data.values)
|
||||
|
||||
assert_equal [1, 2], updated.map(&:id)
|
||||
|
@ -78,10 +78,33 @@ class PersistenceTest < ActiveRecord::TestCase
|
|||
assert_equal "2 updated", Topic.find(2).content
|
||||
end
|
||||
|
||||
def test_update_many_with_duplicated_ids
|
||||
updated = Topic.update([1, 1, 2], [
|
||||
{ "content" => "1 duplicated" }, { "content" => "1 updated" }, { "content" => "2 updated" }
|
||||
])
|
||||
|
||||
assert_equal [1, 1, 2], updated.map(&:id)
|
||||
assert_equal "1 updated", Topic.find(1).content
|
||||
assert_equal "2 updated", Topic.find(2).content
|
||||
end
|
||||
|
||||
def test_update_many_with_invalid_id
|
||||
topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" }, 99999 => {} }
|
||||
|
||||
assert_raise(ActiveRecord::RecordNotFound) do
|
||||
Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) }
|
||||
end
|
||||
|
||||
assert_not_equal "1 updated", Topic.find(1).content
|
||||
assert_not_equal "2 updated", Topic.find(2).content
|
||||
end
|
||||
|
||||
def test_class_level_update_is_affected_by_scoping
|
||||
topic_data = { 1 => { "content" => "1 updated" }, 2 => { "content" => "2 updated" } }
|
||||
|
||||
assert_equal [], Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) }
|
||||
assert_raise(ActiveRecord::RecordNotFound) do
|
||||
Topic.where("1=0").scoping { Topic.update(topic_data.keys, topic_data.values) }
|
||||
end
|
||||
|
||||
assert_not_equal "1 updated", Topic.find(1).content
|
||||
assert_not_equal "2 updated", Topic.find(2).content
|
||||
|
@ -175,15 +198,25 @@ class PersistenceTest < ActiveRecord::TestCase
|
|||
end
|
||||
|
||||
def test_destroy_many
|
||||
clients = Client.all.merge!(order: "id").find([2, 3])
|
||||
clients = Client.find([2, 3])
|
||||
|
||||
assert_difference("Client.count", -2) do
|
||||
destroyed = Client.destroy([2, 3, nil]).sort_by(&:id)
|
||||
destroyed = Client.destroy([2, 3])
|
||||
assert_equal clients, destroyed
|
||||
assert destroyed.all?(&:frozen?), "destroyed clients should be frozen"
|
||||
end
|
||||
end
|
||||
|
||||
def test_destroy_many_with_invalid_id
|
||||
clients = Client.find([2, 3])
|
||||
|
||||
assert_raise(ActiveRecord::RecordNotFound) do
|
||||
Client.destroy([2, 3, 99999])
|
||||
end
|
||||
|
||||
assert_equal clients, Client.find([2, 3])
|
||||
end
|
||||
|
||||
def test_becomes
|
||||
assert_kind_of Reply, topics(:first).becomes(Reply)
|
||||
assert_equal "The First Topic", topics(:first).becomes(Reply).title
|
||||
|
|
Loading…
Reference in a new issue