Allow ActiveRecord::Relation#update to run on result of a relation with callbacks and validations

- Right now, there is no method to update multiple records with
  validations and callbacks.
- Changed the behavior of existing `update` method so that when `id`
  attribute is not given and the method is called on an `Relation`
  object, it will execute update for every record of the `Relation` and
  will run validations and callbacks for every record.
- Added test case for validating that the callbacks run when `update` is
  called on a `Relation`.
- Changed test_create_columns_not_equal_attributes test from
  persistence_test to include author_name column on topics table as it
  it used in before_update callback.
- This change introduces performance issues when a large number of
  records are to be updated because it runs UPDATE query for every
  record of the result. The `update_all` method can be used in that case
  if callbacks are not required because it will only run single UPDATE
  for all the records.

4 files changed, 51 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 6494c7374e..1ade4f8751 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*
+
 *   Introduce `force: :cascade` option for `create_table`. Using this option
     will recreate tables even if they have dependent objects (like foreign keys).
     `db/schema.rb` now uses `force: :cascade`. This makes it possible to
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index daafb0b645..5ca28033bf 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 6fc4731f01..67a929ba09 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 3a0398d08d..17943f5457 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')