aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--activerecord/CHANGELOG.md5
-rw-r--r--activerecord/lib/active_record/associations/has_one_association.rb2
-rw-r--r--activerecord/lib/active_record/migration.rb4
-rw-r--r--activerecord/lib/active_record/persistence.rb9
-rw-r--r--activerecord/test/cases/base_test.rb2
-rw-r--r--activerecord/test/cases/dirty_test.rb10
-rw-r--r--activerecord/test/cases/persistence_test.rb35
-rw-r--r--activerecord/test/cases/timestamp_test.rb4
8 files changed, 32 insertions, 39 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 18d7871a82..f9a7febd85 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,10 @@
## Rails 3.2.9 (unreleased)
+* Fix `increment!`, `decrement!`, `toggle!` that was skipping callbacks.
+ Fixes #7306.
+
+ *Rafael Mendonça França*
+
* Fix AR#create to return an unsaved record when AR::RecordInvalid is
raised. Fixes #3217.
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 08f66eed3f..501ebe7c5b 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -38,7 +38,7 @@ module ActiveRecord
when :destroy
target.destroy
when :nullify
- target.update_column(reflection.foreign_key, nil)
+ target.update_attribute(reflection.foreign_key, nil)
end
end
end
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index b4b4667ce0..e6686597b7 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -232,7 +232,7 @@ module ActiveRecord
# add_column :people, :salary, :integer
# Person.reset_column_information
# Person.all.each do |p|
- # p.update_column :salary, SalaryCalculator.compute(p)
+ # p.update_attribute :salary, SalaryCalculator.compute(p)
# end
# end
# end
@@ -252,7 +252,7 @@ module ActiveRecord
# ...
# say_with_time "Updating salaries..." do
# Person.all.each do |p|
- # p.update_column :salary, SalaryCalculator.compute(p)
+ # p.update_attribute :salary, SalaryCalculator.compute(p)
# end
# end
# ...
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index d6249db380..038355deaa 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -174,9 +174,6 @@ module ActiveRecord
# * 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
raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name)
@@ -242,7 +239,7 @@ module ActiveRecord
# Saving is not subjected to validation checks. Returns +true+ if the
# record could be saved.
def increment!(attribute, by = 1)
- increment(attribute, by).update_column(attribute, self[attribute])
+ increment(attribute, by).update_attribute(attribute, self[attribute])
end
# Initializes +attribute+ to zero if +nil+ and subtracts the value passed as +by+ (default is 1).
@@ -259,7 +256,7 @@ module ActiveRecord
# Saving is not subjected to validation checks. Returns +true+ if the
# record could be saved.
def decrement!(attribute, by = 1)
- decrement(attribute, by).update_column(attribute, self[attribute])
+ decrement(attribute, by).update_attribute(attribute, self[attribute])
end
# Assigns to +attribute+ the boolean opposite of <tt>attribute?</tt>. So
@@ -276,7 +273,7 @@ module ActiveRecord
# Saving is not subjected to validation checks. Returns +true+ if the
# record could be saved.
def toggle!(attribute)
- toggle(attribute).update_column(attribute, self[attribute])
+ toggle(attribute).update_attribute(attribute, self[attribute])
end
# Reloads the attributes of this object from the database.
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index db42f17eeb..2fd0d59e17 100644
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -2133,7 +2133,7 @@ class BasicsTest < ActiveRecord::TestCase
def test_cache_key_format_for_existing_record_with_nil_updated_at
dev = Developer.first
- dev.update_column(:updated_at, nil)
+ dev.update_attribute(:updated_at, nil)
assert_match(/\/#{dev.id}$/, dev.cache_key)
end
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index d939e13d9a..26afd203d2 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -509,6 +509,16 @@ class DirtyTest < ActiveRecord::TestCase
assert_not_nil pirate.previous_changes['updated_on'][1]
assert !pirate.previous_changes.key?('parrot_id')
assert !pirate.previous_changes.key?('created_on')
+
+ pirate = Pirate.find_by_catchphrase("Ahoy!")
+ pirate.update_attribute(:catchphrase, "Ninjas suck!")
+
+ assert_equal 2, pirate.previous_changes.size
+ assert_equal ["Ahoy!", "Ninjas suck!"], pirate.previous_changes['catchphrase']
+ assert_not_nil pirate.previous_changes['updated_on'][0]
+ assert_not_nil pirate.previous_changes['updated_on'][1]
+ assert !pirate.previous_changes.key?('parrot_id')
+ assert !pirate.previous_changes.key?('created_on')
end
private
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index aec4f99e71..e4b8caae52 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -355,17 +355,10 @@ class PersistencesTest < ActiveRecord::TestCase
def test_update_attribute
assert !Topic.find(1).approved?
-
- ActiveSupport::Deprecation.silence do
- Topic.find(1).update_attribute("approved", true)
- end
-
+ Topic.find(1).update_attribute("approved", true)
assert Topic.find(1).approved?
- ActiveSupport::Deprecation.silence do
- Topic.find(1).update_attribute(:approved, false)
- end
-
+ Topic.find(1).update_attribute(:approved, false)
assert !Topic.find(1).approved?
end
@@ -375,10 +368,7 @@ class PersistencesTest < ActiveRecord::TestCase
def test_update_attribute_for_readonly_attribute
minivan = Minivan.find('m1')
-
- ActiveSupport::Deprecation.silence do
- assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') }
- end
+ assert_raises(ActiveRecord::ActiveRecordError) { minivan.update_attribute(:color, 'black') }
end
# This test is correct, but it is hard to fix it since
@@ -404,11 +394,8 @@ class PersistencesTest < ActiveRecord::TestCase
def test_update_attribute_with_one_updated
t = Topic.first
-
- ActiveSupport::Deprecation.silence do
- t.update_attribute(:title, 'super_title')
- end
-
+ title = t.title
+ t.update_attribute(:title, 'super_title')
assert_equal 'super_title', t.title
assert !t.changed?, "topic should not have changed"
assert !t.title_changed?, "title should not have changed"
@@ -422,16 +409,10 @@ class PersistencesTest < ActiveRecord::TestCase
developer = Developer.find(1)
prev_month = Time.now.prev_month
- ActiveSupport::Deprecation.silence do
- developer.update_attribute(:updated_at, prev_month)
- end
-
+ developer.update_attribute(:updated_at, prev_month)
assert_equal prev_month, developer.updated_at
- ActiveSupport::Deprecation.silence do
- developer.update_attribute(:salary, 80001)
- end
-
+ developer.update_attribute(:salary, 80001)
assert_not_equal prev_month, developer.updated_at
developer.reload
@@ -469,7 +450,7 @@ class PersistencesTest < ActiveRecord::TestCase
def test_update_column_should_not_leave_the_object_dirty
topic = Topic.find(1)
- topic.update_column("content", "Have a nice day")
+ topic.update_attribute("content", "Have a nice day")
topic.reload
topic.update_column(:content, "You too")
diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb
index c965371a49..28543a5a3a 100644
--- a/activerecord/test/cases/timestamp_test.rb
+++ b/activerecord/test/cases/timestamp_test.rb
@@ -11,7 +11,7 @@ class TimestampTest < ActiveRecord::TestCase
def setup
@developer = Developer.order(:id).first
- @developer.update_column(:updated_at, Time.now.prev_month)
+ @developer.update_attribute(:updated_at, Time.now.prev_month)
@previously_updated_at = @developer.updated_at
end
@@ -133,7 +133,7 @@ class TimestampTest < ActiveRecord::TestCase
pet = Pet.first
owner = pet.owner
- owner.update_column(:happy_at, 3.days.ago)
+ owner.update_attribute(:happy_at, 3.days.ago)
previously_owner_updated_at = owner.updated_at
pet.name = "I'm a parrot"