From c83e30da27eafde79164ecb376e8a28ccc8d841f Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Tue, 31 Jul 2018 08:17:45 +0900 Subject: Avoid extra scoping when using `Relation#update` Since 9ac7dd4, class level `update`, `destroy`, and `delete` were placed in the `Persistence` module as class methods. But `Relation#update` without passing ids which was introduced at #11898 is not a class method, and it was caused the extra scoping regression #33470. I moved the relation method back into the `Relation` to fix the regression. Fixes #33470. --- activerecord/lib/active_record/persistence.rb | 4 +--- activerecord/lib/active_record/relation.rb | 4 ++++ activerecord/test/cases/relations_test.rb | 4 ++++ 3 files changed, 9 insertions(+), 3 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 155d67fd8f..05963e5546 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -107,13 +107,11 @@ module ActiveRecord # When running callbacks is not needed for each record update, # it is preferred to use {update_all}[rdoc-ref:Relation#update_all] # for updating all records in a single query. - def update(id = :all, attributes) + def update(id, attributes) if id.is_a?(Array) 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 if ActiveRecord::Base === id raise ArgumentError, diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index a7523563e2..2d3e1eaa08 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -375,6 +375,10 @@ module ActiveRecord @klass.connection.update stmt, "#{@klass} Update All" end + def update(attributes) # :nodoc: + each { |record| record.update(attributes) } + end + def update_counters(counters) # :nodoc: touch = counters.delete(:touch) diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 952d2dd5d9..5412ab5def 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -30,7 +30,9 @@ class RelationTest < ActiveRecord::TestCase class TopicWithCallbacks < ActiveRecord::Base self.table_name = :topics + cattr_accessor :topic_count before_update { |topic| topic.author_name = "David" if topic.author_name.blank? } + after_update { |topic| topic.class.topic_count = topic.class.count } end def test_do_not_double_quote_string_id @@ -1576,6 +1578,8 @@ class RelationTest < ActiveRecord::TestCase topics = TopicWithCallbacks.where(id: [topic1.id, topic2.id]) topics.update(title: "adequaterecord") + assert_equal TopicWithCallbacks.count, TopicWithCallbacks.topic_count + assert_equal "adequaterecord", topic1.reload.title assert_equal "adequaterecord", topic2.reload.title # Testing that the before_update callbacks have run -- cgit v1.2.3