Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Deprecate update_attribute #6739

Merged
merged 1 commit into from

2 participants

Steve Klabnik Xavier Noria
Steve Klabnik
Collaborator

Historically, update_attribute and update_attributes are similar, but
with one big difference: update_attribute does not run validations.
These two methods are really easy to confuse given their similar
names. Therefore, update_attribute is being deprecated in favor of
update_column, and will be removed in Rails 4.

See the discussion on rails-core here:
https://groups.google.com/d/topic/rubyonrails-core/BWPUTK7WvYA/discussion

Xavier Noria fxn was assigned
Xavier Noria
Owner

I think the deprecation message should warn a little about the differences. Something like "use update_column if you do not need mass-assignment protection, callbacks, or touching updated_at, otherwise switch to update_attributes"... something in that line.

Steve Klabnik steveklabnik Deprecate update_attribute.
Historically, update_attribute and update_attributes are similar, but
with one big difference: update_attribute does not run validations.
These two methods are really easy to confuse given their similar
names. Therefore, update_attribute is being deprecated in favor of
update_column, and will be removed in Rails 4.

See the discussion on rails-core here:
https://groups.google.com/d/topic/rubyonrails-core/BWPUTK7WvYA/discussion
b081f6b
Steve Klabnik
Collaborator

Added in a better message, also silenced warnings from the tests.

Xavier Noria fxn merged commit 63bcfbb into from
Brendon Murphy bemurphy referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 14, 2012
  1. Steve Klabnik

    Deprecate update_attribute.

    steveklabnik authored
    Historically, update_attribute and update_attributes are similar, but
    with one big difference: update_attribute does not run validations.
    These two methods are really easy to confuse given their similar
    names. Therefore, update_attribute is being deprecated in favor of
    update_column, and will be removed in Rails 4.
    
    See the discussion on rails-core here:
    https://groups.google.com/d/topic/rubyonrails-core/BWPUTK7WvYA/discussion
This page is out of date. Refresh to see the latest.
2  activerecord/lib/active_record/associations/has_one_association.rb
View
@@ -36,7 +36,7 @@ def delete(method = options[:dependent])
when :destroy
target.destroy
when :nullify
- target.update_attribute(reflection.foreign_key, nil)
+ target.update_column(reflection.foreign_key, nil)
end
end
end
4 activerecord/lib/active_record/migration.rb
View
@@ -232,7 +232,7 @@ def initialize(name)
# add_column :people, :salary, :integer
# Person.reset_column_information
# Person.all.each do |p|
- # p.update_attribute :salary, SalaryCalculator.compute(p)
+ # p.update_column :salary, SalaryCalculator.compute(p)
# end
# end
# end
@@ -252,7 +252,7 @@ def initialize(name)
# ...
# say_with_time "Updating salaries..." do
# Person.all.each do |p|
- # p.update_attribute :salary, SalaryCalculator.compute(p)
+ # p.update_column :salary, SalaryCalculator.compute(p)
# end
# end
# ...
10 activerecord/lib/active_record/persistence.rb
View
@@ -174,8 +174,12 @@ def becomes(klass)
# * updated_at/updated_on column is updated if that column is available.
# * Updates all the attributes that are dirty in this object.
#
+ # This method has been deprecated in favor of <tt>update_column</tt> due to
+ # its similarity with <tt>update_attributes</tt>.
+ #
def update_attribute(name, value)
name = name.to_s
+ ActiveSupport::Deprecation.warn("update_attribute is deprecated and will be removed in Rails 4. If you want to skip mass-assignment protection, callbacks, and modifying updated_at, use update_column. If you do want those things, use update_attributes.")
raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name)
send("#{name}=", value)
save(:validate => false)
@@ -239,7 +243,7 @@ def increment(attribute, by = 1)
# Saving is not subjected to validation checks. Returns +true+ if the
# record could be saved.
def increment!(attribute, by = 1)
- increment(attribute, by).update_attribute(attribute, self[attribute])
+ increment(attribute, by).update_column(attribute, self[attribute])
end
# Initializes +attribute+ to zero if +nil+ and subtracts the value passed as +by+ (default is 1).
@@ -256,7 +260,7 @@ def decrement(attribute, by = 1)
# Saving is not subjected to validation checks. Returns +true+ if the
# record could be saved.
def decrement!(attribute, by = 1)
- decrement(attribute, by).update_attribute(attribute, self[attribute])
+ decrement(attribute, by).update_column(attribute, self[attribute])
end
# Assigns to +attribute+ the boolean opposite of <tt>attribute?</tt>. So
@@ -273,7 +277,7 @@ def toggle(attribute)
# Saving is not subjected to validation checks. Returns +true+ if the
# record could be saved.
def toggle!(attribute)
- toggle(attribute).update_attribute(attribute, self[attribute])
+ toggle(attribute).update_column(attribute, self[attribute])
end
# Reloads the attributes of this object from the database.
2  activerecord/test/cases/base_test.rb
View
@@ -2127,7 +2127,7 @@ def test_cache_key_format_for_existing_record_with_updated_at
def test_cache_key_format_for_existing_record_with_nil_updated_at
dev = Developer.first
- dev.update_attribute(:updated_at, nil)
+ dev.update_column(:updated_at, nil)
assert_match(/\/#{dev.id}$/, dev.cache_key)
end
2  activerecord/test/cases/dirty_test.rb
View
@@ -498,7 +498,7 @@ def test_previous_changes
assert !pirate.previous_changes.key?('created_on')
pirate = Pirate.find_by_catchphrase("Ahoy!")
- pirate.update_attribute(:catchphrase, "Ninjas suck!")
+ pirate.update_column(:catchphrase, "Ninjas suck!")
assert_equal 2, pirate.previous_changes.size
assert_equal ["Ahoy!", "Ninjas suck!"], pirate.previous_changes['catchphrase']
34 activerecord/test/cases/persistence_test.rb
View
@@ -355,10 +355,17 @@ def test_destroy_record_with_associations
def test_update_attribute
assert !Topic.find(1).approved?
- Topic.find(1).update_attribute("approved", true)
+
+ ActiveSupport::Deprecation.silence do
+ Topic.find(1).update_attribute("approved", true)
+ end
+
assert Topic.find(1).approved?
- Topic.find(1).update_attribute(:approved, false)
+ ActiveSupport::Deprecation.silence do
+ Topic.find(1).update_attribute(:approved, false)
+ end
+
assert !Topic.find(1).approved?
end
@@ -368,7 +375,10 @@ def test_update_attribute_does_not_choke_on_nil
def test_update_attribute_for_readonly_attribute
minivan = Minivan.find('m1')
- assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') }
+
+ ActiveSupport::Deprecation.silence do
+ assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') }
+ end
end
# This test is correct, but it is hard to fix it since
@@ -395,7 +405,11 @@ def test_update_attribute_for_readonly_attribute
def test_update_attribute_with_one_updated
t = Topic.first
title = t.title
- t.update_attribute(:title, 'super_title')
+
+ ActiveSupport::Deprecation.silence do
+ t.update_attribute(:title, 'super_title')
+ end
+
assert_equal 'super_title', t.title
assert !t.changed?, "topic should not have changed"
assert !t.title_changed?, "title should not have changed"
@@ -409,10 +423,16 @@ def test_update_attribute_for_updated_at_on
developer = Developer.find(1)
prev_month = Time.now.prev_month
- developer.update_attribute(:updated_at, prev_month)
+ ActiveSupport::Deprecation.silence do
+ developer.update_attribute(:updated_at, prev_month)
+ end
+
assert_equal prev_month, developer.updated_at
- developer.update_attribute(:salary, 80001)
+ ActiveSupport::Deprecation.silence do
+ developer.update_attribute(:salary, 80001)
+ end
+
assert_not_equal prev_month, developer.updated_at
developer.reload
@@ -450,7 +470,7 @@ def test_update_column_should_raise_exception_if_new_record
def test_update_column_should_not_leave_the_object_dirty
topic = Topic.find(1)
- topic.update_attribute("content", "Have a nice day")
+ topic.update_column("content", "Have a nice day")
topic.reload
topic.update_column(:content, "You too")
4 activerecord/test/cases/timestamp_test.rb
View
@@ -11,7 +11,7 @@ class TimestampTest < ActiveRecord::TestCase
def setup
@developer = Developer.order(:id).first
- @developer.update_attribute(:updated_at, Time.now.prev_month)
+ @developer.update_column(:updated_at, Time.now.prev_month)
@previously_updated_at = @developer.updated_at
end
@@ -133,7 +133,7 @@ def test_touching_a_record_with_a_belongs_to_that_uses_a_counter_cache_should_up
pet = Pet.first
owner = pet.owner
- owner.update_attribute(:happy_at, 3.days.ago)
+ owner.update_column(:happy_at, 3.days.ago)
previously_owner_updated_at = owner.updated_at
pet.name = "I'm a parrot"
Something went wrong with that request. Please try again.