From 0dc3b6535dfb8127f568bb4abc2fb09118ce4b96 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 1 Dec 2017 11:49:36 +0900 Subject: Maintain raising `RecordNotFound` for class level `update` and` destroy` In 35836019, class level `update` and `destroy` suppressed `RecordNotFound` to ensure returning affected objects. But `RecordNotFound` is a common exception caught by a `rescue_from` handler. So changing the behavior when a typical `params[:id]` is passed has a compatibility problem. The previous behavior should not be changed. Fixes #31301. --- activerecord/lib/active_record/persistence.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'activerecord/lib/active_record/persistence.rb') diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 4e1b05dbf6..17459f2505 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -99,7 +99,11 @@ 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| update(one_id, attributes[idx]) }.compact + id.map.with_index { |one_id, idx| + object = find_by(primary_key => one_id) + object.update(attributes[idx]) if object + object + }.compact elsif id == :all all.each { |record| record.update(attributes) } else @@ -112,7 +116,6 @@ module ActiveRecord object.update(attributes) object end - rescue RecordNotFound end # Destroy an object (or multiple objects) that has the given id. The object is instantiated first, @@ -136,11 +139,13 @@ module ActiveRecord # Todo.destroy(todos) def destroy(id) if id.is_a?(Array) - id.map { |one_id| destroy(one_id) }.compact + id.map { |one_id| + object = find_by(primary_key => one_id) + object.destroy if object + }.compact else find(id).destroy end - rescue RecordNotFound end # Deletes the row with a primary key matching the +id+ argument, using a -- cgit v1.2.3 From 695ec1fef0b7fdb3125a3e2e4364a7f449265c1b Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 1 Dec 2017 18:21:21 +0900 Subject: Class level `update` and `destroy` checks all the records exist before making changes (#31306) It makes more sense than ignoring invalid IDs. --- activerecord/lib/active_record/persistence.rb | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) (limited to 'activerecord/lib/active_record/persistence.rb') diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 17459f2505..a13b0d0181 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -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 -- cgit v1.2.3