Permalink
Browse files

Replaced usage of update_attribute with update_attribute!

Fixed deprecation warning in Rails 3.2.7

Closes #44
  • Loading branch information...
1 parent da082ae commit 7efbbaf4c04ef688a0f027ee1ba6f77111ef6342 @kevmoo kevmoo committed Jul 27, 2012
Showing with 15 additions and 15 deletions.
  1. +7 −7 lib/acts_as_list/active_record/acts/list.rb
  2. +8 −8 test/test_list.rb
@@ -136,20 +136,20 @@ def move_to_top
def remove_from_list
if in_list?
decrement_positions_on_lower_items
- update_attribute position_column, nil
+ update_attributes! position_column => nil
end
end
# Increase the position of this item without adjusting the rest of the list.
def increment_position
return unless in_list?
- update_attribute position_column, self.send(position_column).to_i + 1
+ update_attributes! position_column => self.send(position_column).to_i + 1
end
# Decrease the position of this item without adjusting the rest of the list.
def decrement_position
return unless in_list?
- update_attribute position_column, self.send(position_column).to_i - 1
+ update_attributes! position_column => self.send(position_column).to_i - 1
end
# Return +true+ if this object is the first in the list.
@@ -230,12 +230,12 @@ def bottom_item(except = nil)
# Forces item to assume the bottom position in the list.
def assume_bottom_position
- update_attribute(position_column, bottom_position_in_list(self).to_i + 1)
+ update_attributes!(position_column => bottom_position_in_list(self).to_i + 1)
end
# Forces item to assume the top position in the list.
def assume_top_position
- update_attribute(position_column, acts_as_list_top)
+ update_attributes!(position_column => acts_as_list_top)
end
# This has the effect of moving all the higher items up one.
@@ -307,14 +307,14 @@ def insert_at_position(position)
else
increment_positions_on_lower_items(position)
end
- self.update_attribute(position_column, position)
+ self.update_attributes!(position_column => position)
end
# used by insert_at_position instead of remove_from_list, as postgresql raises error if position_column has non-null constraint
def store_at_0
if in_list?
old_position = send(position_column).to_i
- update_attribute(position_column, 0)
+ update_attributes!(position_column => 0)
decrement_positions_on_lower_items(old_position)
end
end
View
@@ -243,13 +243,13 @@ def test_insert_at
def test_update_position
assert_equal [1, 2, 3, 4], DefaultScopedMixin.find(:all).map(&:id)
- DefaultScopedMixin.find(2).update_attribute(:pos, 4)
+ DefaultScopedMixin.find(2).update_attributes!(:pos => 4)
assert_equal [1, 3, 4, 2], DefaultScopedMixin.find(:all).map(&:id)
- DefaultScopedMixin.find(2).update_attribute(:pos, 2)
+ DefaultScopedMixin.find(2).update_attributes!(:pos => 2)
assert_equal [1, 2, 3, 4], DefaultScopedMixin.find(:all).map(&:id)
- DefaultScopedMixin.find(1).update_attribute(:pos, 4)
+ DefaultScopedMixin.find(1).update_attributes!(:pos => 4)
assert_equal [2, 3, 4, 1], DefaultScopedMixin.find(:all).map(&:id)
- DefaultScopedMixin.find(1).update_attribute(:pos, 1)
+ DefaultScopedMixin.find(1).update_attributes!(:pos => 1)
assert_equal [1, 2, 3, 4], DefaultScopedMixin.find(:all).map(&:id)
end
@@ -337,13 +337,13 @@ def test_insert_at
def test_update_position
assert_equal [1, 2, 3, 4], DefaultScopedWhereMixin.where(:active => false).find(:all).map(&:id)
- DefaultScopedWhereMixin.where(:active => false).find(2).update_attribute(:pos, 4)
+ DefaultScopedWhereMixin.where(:active => false).find(2).update_attributes!(:pos => 4)
assert_equal [1, 3, 4, 2], DefaultScopedWhereMixin.where(:active => false).find(:all).map(&:id)
- DefaultScopedWhereMixin.where(:active => false).find(2).update_attribute(:pos, 2)
+ DefaultScopedWhereMixin.where(:active => false).find(2).update_attributes!(:pos => 2)
assert_equal [1, 2, 3, 4], DefaultScopedWhereMixin.where(:active => false).find(:all).map(&:id)
- DefaultScopedWhereMixin.where(:active => false).find(1).update_attribute(:pos, 4)
+ DefaultScopedWhereMixin.where(:active => false).find(1).update_attributes!(:pos => 4)
assert_equal [2, 3, 4, 1], DefaultScopedWhereMixin.where(:active => false).find(:all).map(&:id)
- DefaultScopedWhereMixin.where(:active => false).find(1).update_attribute(:pos, 1)
+ DefaultScopedWhereMixin.where(:active => false).find(1).update_attributes!(:pos => 1)
assert_equal [1, 2, 3, 4], DefaultScopedWhereMixin.where(:active => false).find(:all).map(&:id)
end

7 comments on commit 7efbbaf

Wouldn't it be possible to pass some role (e.g. :acts_as_list) to those update_attributes!? Because I don't think it's a good thing to whitelist position column by default in whole application.

I think that using extra role would be a quite good solution in this unpleasant situation. Or couldn't acts_as_list whitelist this column for itself automatically? That would make sense as it can't work without it anyway.

Owner

swanandp replied Jul 28, 2012

Good catch, let me think over this. I haven't considered the whole whitelist / blacklist issues at all.

@swanandp, Could you release a new version? The deprecation warning is annoying.

Collaborator

fabn replied Aug 8, 2012

+1 for the release of a new version...

+1 for releasing it on rubygems.

Owner

swanandp replied Aug 9, 2012

Contributor

tvdeyen replied Aug 9, 2012

👍

Please sign in to comment.