Merge pull request #11898 from prathamesh-sonpatki/patch-update

Changed ActiveRecord::Relation#update behavior so that it will work on Relation objects without giving id

Conflicts:
	activerecord/CHANGELOG.md

4 files changed, 51 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index ea8c8088a9..f473e84575 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,19 @@
+*   Change `ActiveRecord::Relation#update` behavior so that it can
+    be called without passing ids of the records to be updated.
+
+    This change allows to update multiple records returned by
+    `ActiveRecord::Relation` with callbacks and validations.
+
+        # Before
+        # ArgumentError: wrong number of arguments (1 for 2)
+        Comment.where(group: 'expert').update(body: "Group of Rails Experts")
+
+        # After
+        # Comments with group expert updated with body "Group of Rails Experts"
+        Comment.where(group: 'expert').update(body: "Group of Rails Experts")
+
+    *Prathamesh Sonpatki*
+
 *   Fix `reaping_frequency` option when the value is a string.
 
     This usually happens when it is configured using `DATABASE_URL`.
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index cdafa6a50a..ab3debc03b 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -362,9 +362,21 @@ module ActiveRecord
     #   # Updates multiple records
     #   people = { 1 => { "first_name" => "David" }, 2 => { "first_name" => "Jeremy" } }
     #   Person.update(people.keys, people.values)
-    def update(id, attributes)
+    #
+    #   # Updates multiple records from the result of a relation
+    #   people = Person.where(group: 'expert')
+    #   people.update(group: 'masters')
+    #
+    #   Note: Updating a large number of records will run a
+    #   UPDATE query for each record, which may cause a performance
+    #   issue. So if it is not needed to run callbacks for each update, it is
+    #   preferred to use <tt>update_all</tt> for updating all records using
+    #   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]) }
+      elsif id == :all
+        to_a.each { |record| record.update(attributes) }
       else
         object = find(id)
         object.update(attributes)
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index cf5a5de3a0..d6816041bc 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -252,8 +252,10 @@ class PersistenceTest < ActiveRecord::TestCase
 
   def test_create_columns_not_equal_attributes
     topic = Topic.instantiate(
-      'title'          => 'Another New Topic',
-      'does_not_exist' => 'test'
+      'attributes' => {
+        'title'          => 'Another New Topic',
+        'does_not_exist' => 'test'
+      }
     )
     assert_nothing_raised { topic.save }
   end
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index edb2d7fa7d..0210ae6487 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -22,6 +22,11 @@ class RelationTest < ActiveRecord::TestCase
   fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments,
     :tags, :taggings, :cars, :minivans
 
+  class TopicWithCallbacks < ActiveRecord::Base
+    self.table_name = :topics
+    before_update { |topic| topic.author_name = 'David' if topic.author_name.blank? }
+  end
+
   def test_do_not_double_quote_string_id
     van = Minivan.last
     assert van
@@ -1429,6 +1434,19 @@ class RelationTest < ActiveRecord::TestCase
     assert_equal posts(:welcome),  comments(:greetings).post
   end
 
+  def test_update_on_relation
+    topic1 = TopicWithCallbacks.create! title: 'arel', author_name: nil
+    topic2 = TopicWithCallbacks.create! title: 'activerecord', author_name: nil
+    topics = TopicWithCallbacks.where(id: [topic1.id, topic2.id])
+    topics.update(title: 'adequaterecord')
+
+    assert_equal 'adequaterecord', topic1.reload.title
+    assert_equal 'adequaterecord', topic2.reload.title
+    # Testing that the before_update callbacks have run
+    assert_equal 'David', topic1.reload.author_name
+    assert_equal 'David', topic2.reload.author_name
+  end
+
   def test_distinct
     tag1 = Tag.create(:name => 'Foo')
     tag2 = Tag.create(:name => 'Foo')