Skip to content

Commit

Permalink
Replaced usage of update_attribute with update_attribute!
Browse files Browse the repository at this point in the history
Fixed deprecation warning in Rails 3.2.7

Closes #44
  • Loading branch information
kevmoo committed Jul 27, 2012
1 parent da082ae commit 7efbbaf
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 15 deletions.
14 changes: 7 additions & 7 deletions lib/acts_as_list/active_record/acts/list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions test/test_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down

7 comments on commit 7efbbaf

@xHire
Copy link

@xHire xHire commented on 7efbbaf Jul 28, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@swanandp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@zhengjia
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@fabn
Copy link
Contributor

@fabn fabn commented on 7efbbaf Aug 8, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@robinboening
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for releasing it on rubygems.

@swanandp
Copy link
Contributor

@swanandp swanandp commented on 7efbbaf Aug 9, 2012 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tvdeyen
Copy link
Contributor

@tvdeyen tvdeyen commented on 7efbbaf Aug 9, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Please sign in to comment.