Use update_columns to implemente the update_column

2 files changed, 54 insertions, 19 deletions

diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index bd5d8de69c..e34dc11606 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -167,22 +167,6 @@ module ActiveRecord
       became
     end
 
-    # Updates a single attribute of an object, without calling save.
-    #
-    # * Validation is skipped.
-    # * Callbacks are skipped.
-    # * updated_at/updated_on column is not updated if that column is available.
-    #
-    # Raises an +ActiveRecordError+ when called on new objects, or when the +name+
-    # attribute is marked as readonly.
-    def update_column(name, value)
-      name = name.to_s
-      verify_readonly_attribute(name)
-      raise ActiveRecordError, "can not update on a new record object" unless persisted?
-      raw_write_attribute(name, value)
-      self.class.where(self.class.primary_key => id).update_all(name => value) == 1
-    end
-
     # Updates the attributes of the model from the passed-in hash and saves the
     # record, all wrapped in a transaction. If the object is invalid, the saving
     # will fail and false will be returned.
@@ -211,6 +195,18 @@ module ActiveRecord
       end
     end
 
+    # Updates a single attribute of an object, without calling save.
+    #
+    # * Validation is skipped.
+    # * Callbacks are skipped.
+    # * updated_at/updated_on column is not updated if that column is available.
+    #
+    # Raises an +ActiveRecordError+ when called on new objects, or when the +name+
+    # attribute is marked as readonly.
+    def update_column(name, value)
+      update_columns(name => value)
+    end
+
     # Updates the attributes from the passed-in hash, without calling save.
     #
     # * Validation is skipped.
@@ -221,10 +217,15 @@ module ActiveRecord
     # one of the attributes is marked as readonly.
     def update_columns(attributes)
       raise ActiveRecordError, "can not update on a new record object" unless persisted?
-      attributes.each_key {|key| raise ActiveRecordError, "#{key.to_s} is marked as readonly" if self.class.readonly_attributes.include?(key.to_s) }
+
+      attributes.each_key do |key|
+        raise ActiveRecordError, "#{key.to_s} is marked as readonly" if self.class.readonly_attributes.include?(key.to_s)
+      end
+
       attributes.each do |k,v|
         raw_write_attribute(k,v)
       end
+
       self.class.where(self.class.primary_key => id).update_all(attributes) == 1
     end
 
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 77a5458b2a..1f6c224a11 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -471,6 +471,17 @@ class PersistencesTest < ActiveRecord::TestCase
     assert_equal "Sebastian Topic", topic.title
   end
 
+  def test_update_columns_should_not_use_setter_method
+    dev = Developer.find(1)
+    dev.instance_eval { def salary=(value); write_attribute(:salary, value * 2); end }
+
+    dev.update_columns(salary: 80000)
+    assert_equal 80000, dev.salary
+
+    dev.reload
+    assert_equal 80000, dev.salary
+  end
+
   def test_update_columns_should_raise_exception_if_new_record
     topic = Topic.new
     assert_raises(ActiveRecord::ActiveRecordError) { topic.update_columns({ approved: false }) }
@@ -489,6 +500,14 @@ class PersistencesTest < ActiveRecord::TestCase
     assert_equal [], topic.changed
   end
 
+  def test_update_columns_with_model_having_primary_key_other_than_id
+    minivan = Minivan.find('m1')
+    new_name = 'sebavan'
+
+    minivan.update_columns(name: new_name)
+    assert_equal new_name, minivan.name
+  end
+
   def test_update_columns_with_one_readonly_attribute
     minivan = Minivan.find('m1')
     prev_color = minivan.color
@@ -506,10 +525,10 @@ class PersistencesTest < ActiveRecord::TestCase
     developer = Developer.find(1)
     prev_month = Time.now.prev_month
 
-    developer.update_column(:updated_at, prev_month)
+    developer.update_columns(updated_at: prev_month)
     assert_equal prev_month, developer.updated_at
 
-    developer.update_columns({ salary: 80000 })
+    developer.update_columns(salary: 80000)
     assert_equal prev_month, developer.updated_at
     assert_equal 80000, developer.salary
 
@@ -518,6 +537,21 @@ class PersistencesTest < ActiveRecord::TestCase
     assert_equal 80000, developer.salary
   end
 
+  def test_update_columns_with_one_changed_and_one_updated
+    t = Topic.order('id').limit(1).first
+    author_name = t.author_name
+    t.author_name = 'John'
+    t.update_columns(title: 'super_title')
+    assert_equal 'John', t.author_name
+    assert_equal 'super_title', t.title
+    assert t.changed?, "topic should have changed"
+    assert t.author_name_changed?, "author_name should have changed"
+
+    t.reload
+    assert_equal author_name, t.author_name
+    assert_equal 'super_title', t.title
+  end
+
   def test_update_attributes
     topic = Topic.find(1)
     assert !topic.approved?