Skip to content

Commit

Permalink
Merge pull request #80 from philippfranke/rails4_compatibility
Browse files Browse the repository at this point in the history
Rails4 compatibility
  • Loading branch information
swanandp committed Jul 28, 2013
2 parents 7c1016f + bdb47ce commit d8d4155
Show file tree
Hide file tree
Showing 10 changed files with 314 additions and 381 deletions.
5 changes: 0 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
language: ruby
rvm:
- 1.8.7
- 1.9.2
- 1.9.3
- 2.0.0
Expand All @@ -13,7 +12,3 @@ matrix:
gemfile: gemfiles/rails4/Gemfile
- rvm: 1.9.2
gemfile: gemfiles/rails4/Gemfile
- rvm: 1.9.3
gemfile: gemfiles/rails4/Gemfile
- rvm: 2.0.0
gemfile: gemfiles/rails4/Gemfile
2 changes: 1 addition & 1 deletion gemfiles/rails4/Gemfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
source "https://rubygems.org"

gem 'activerecord', '>= 4.0.0.beta', '< 5'
gem 'activerecord', '>= 4.0.0', '< 5'

# Specify your gem's dependencies in acts_as_list.gemspec
gemspec :path => File.join('..', '..')
Expand Down
22 changes: 10 additions & 12 deletions lib/acts_as_list/active_record/acts/list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ def self.included(base)
# Todo list example:
#
# class TodoList < ActiveRecord::Base
# has_many :todo_items, :order => "position"
# has_many :todo_items, order: "position"
# end
#
# class TodoItem < ActiveRecord::Base
# belongs_to :todo_list
# acts_as_list :scope => :todo_list
# acts_as_list scope: :todo_list
# end
#
# todo_list.first.move_to_bottom
Expand All @@ -29,13 +29,13 @@ module ClassMethods
# * +scope+ - restricts what is to be considered a list. Given a symbol, it'll attach <tt>_id</tt>
# (if it hasn't already been added) and use that as the foreign key restriction. It's also possible
# to give it an entire string that is interpolated if you need a tighter scope than just a foreign key.
# Example: <tt>acts_as_list :scope => 'todo_list_id = #{todo_list_id} AND completed = 0'</tt>
# Example: <tt>acts_as_list scope: 'todo_list_id = #{todo_list_id} AND completed = 0'</tt>
# * +top_of_list+ - defines the integer used for the top of the list. Defaults to 1. Use 0 to make the collection
# act more like an array in its indexing.
# * +add_new_at+ - specifies whether objects get added to the :top or :bottom of the list. (default: +bottom+)
# `nil` will result in new items not being added to the list on create
def acts_as_list(options = {})
configuration = { :column => "position", :scope => "1 = 1", :top_of_list => 1, :add_new_at => :bottom}
configuration = { column: "position", scope: "1 = 1", top_of_list: 1, add_new_at: :bottom}
configuration.update(options) if options.is_a?(Hash)

configuration[:scope] = "#{configuration[:scope]}_id".intern if configuration[:scope].is_a?(Symbol) && configuration[:scope].to_s !~ /_id$/
Expand Down Expand Up @@ -214,10 +214,9 @@ def last?
# Return the next higher item in the list.
def higher_item
return nil unless in_list?
acts_as_list_class.unscoped.find(:first, :conditions =>
"#{scope_condition} AND #{position_column} < #{(send(position_column).to_i).to_s}",
:order => "#{acts_as_list_class.table_name}.#{position_column} DESC"
)
acts_as_list_class.unscoped.
where("#{scope_condition} AND #{position_column} < #{(send(position_column).to_i).to_s}").
order("#{acts_as_list_class.table_name}.#{position_column} DESC").first
end

# Return the next n higher items in the list
Expand All @@ -235,10 +234,9 @@ def higher_items(limit=nil)
# Return the next lower item in the list.
def lower_item
return nil unless in_list?
acts_as_list_class.unscoped.find(:first, :conditions =>
"#{scope_condition} AND #{position_column} > #{(send(position_column).to_i).to_s}",
:order => "#{acts_as_list_class.table_name}.#{position_column} ASC"
)
acts_as_list_class.unscoped.
where("#{scope_condition} AND #{position_column} > #{(send(position_column).to_i).to_s}").
order("#{acts_as_list_class.table_name}.#{position_column} ASC").first
end

# Return the next n lower items in the list
Expand Down
124 changes: 62 additions & 62 deletions test/shared_array_scope_list.rb
Original file line number Diff line number Diff line change
@@ -1,87 +1,87 @@
module Shared
module ArrayScopeList
def setup
(1..4).each { |counter| ArrayScopeListMixin.create! :pos => counter, :parent_id => 5, :parent_type => 'ParentClass' }
(1..4).each { |counter| ArrayScopeListMixin.create! :pos => counter, :parent_id => 6, :parent_type => 'ParentClass' }
(1..4).each { |counter| ArrayScopeListMixin.create! pos: counter, parent_id: 5, parent_type: 'ParentClass' }
(1..4).each { |counter| ArrayScopeListMixin.create! pos: counter, parent_id: 6, parent_type: 'ParentClass' }
end

def test_reordering
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(2).move_lower
assert_equal [1, 3, 2, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
ArrayScopeListMixin.where(id: 2).first.move_lower
assert_equal [1, 3, 2, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(2).move_higher
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
ArrayScopeListMixin.where(id: 2).first.move_higher
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(1).move_to_bottom
assert_equal [2, 3, 4, 1], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
ArrayScopeListMixin.where(id: 1).first.move_to_bottom
assert_equal [2, 3, 4, 1], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(1).move_to_top
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
ArrayScopeListMixin.where(id: 1).first.move_to_top
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(2).move_to_bottom
assert_equal [1, 3, 4, 2], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
ArrayScopeListMixin.where(id: 2).first.move_to_bottom
assert_equal [1, 3, 4, 2], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(4).move_to_top
assert_equal [4, 1, 3, 2], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
ArrayScopeListMixin.where(id: 4).first.move_to_top
assert_equal [4, 1, 3, 2], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(4).insert_at(4)
assert_equal [1, 3, 2, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:pos)
ArrayScopeListMixin.where(id: 4).first.insert_at(4)
assert_equal [1, 3, 2, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:pos)
end

def test_move_to_bottom_with_next_to_last_item
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
ArrayScopeListMixin.find(3).move_to_bottom
assert_equal [1, 2, 4, 3], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)
ArrayScopeListMixin.where(id: 3).first.move_to_bottom
assert_equal [1, 2, 4, 3], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)
end

def test_next_prev
assert_equal ArrayScopeListMixin.find(2), ArrayScopeListMixin.find(1).lower_item
assert_nil ArrayScopeListMixin.find(1).higher_item
assert_equal ArrayScopeListMixin.find(3), ArrayScopeListMixin.find(4).higher_item
assert_nil ArrayScopeListMixin.find(4).lower_item
assert_equal ArrayScopeListMixin.where(id: 2).first, ArrayScopeListMixin.where(id: 1).first.lower_item
assert_nil ArrayScopeListMixin.where(id: 1).first.higher_item
assert_equal ArrayScopeListMixin.where(id: 3).first, ArrayScopeListMixin.where(id: 4).first.higher_item
assert_nil ArrayScopeListMixin.where(id: 4).first.lower_item
end

def test_injection
item = ArrayScopeListMixin.new(:parent_id => 1, :parent_type => 'ParentClass')
item = ArrayScopeListMixin.new(parent_id: 1, parent_type: 'ParentClass')
assert_equal "pos", item.position_column
end

def test_insert
new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 1, new.pos
assert new.first?
assert new.last?

new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 2, new.pos
assert !new.first?
assert new.last?

new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 3, new.pos
assert !new.first?
assert new.last?

new = ArrayScopeListMixin.create(:parent_id => 0, :parent_type => 'ParentClass')
new = ArrayScopeListMixin.create(parent_id: 0, parent_type: 'ParentClass')
assert_equal 1, new.pos
assert new.first?
assert new.last?
end

def test_insert_at
new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 1, new.pos

new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 2, new.pos

new = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 3, new.pos

new4 = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new4 = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 4, new4.pos

new4.insert_at(3)
Expand All @@ -96,7 +96,7 @@ def test_insert_at
new4.reload
assert_equal 4, new4.pos

new5 = ArrayScopeListMixin.create(:parent_id => 20, :parent_type => 'ParentClass')
new5 = ArrayScopeListMixin.create(parent_id: 20, parent_type: 'ParentClass')
assert_equal 5, new5.pos

new5.insert_at(1)
Expand All @@ -107,54 +107,54 @@ def test_insert_at
end

def test_delete_middle
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(2).destroy
ArrayScopeListMixin.where(id: 2).first.destroy

assert_equal [1, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

assert_equal 1, ArrayScopeListMixin.find(1).pos
assert_equal 2, ArrayScopeListMixin.find(3).pos
assert_equal 3, ArrayScopeListMixin.find(4).pos
assert_equal 1, ArrayScopeListMixin.where(id: 1).first.pos
assert_equal 2, ArrayScopeListMixin.where(id: 3).first.pos
assert_equal 3, ArrayScopeListMixin.where(id: 4).first.pos

ArrayScopeListMixin.find(1).destroy
ArrayScopeListMixin.where(id: 1).first.destroy

assert_equal [3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

assert_equal 1, ArrayScopeListMixin.find(3).pos
assert_equal 2, ArrayScopeListMixin.find(4).pos
assert_equal 1, ArrayScopeListMixin.where(id: 3).first.pos
assert_equal 2, ArrayScopeListMixin.where(id: 4).first.pos
end

def test_remove_from_list_should_then_fail_in_list?
assert_equal true, ArrayScopeListMixin.find(1).in_list?
ArrayScopeListMixin.find(1).remove_from_list
assert_equal false, ArrayScopeListMixin.find(1).in_list?
assert_equal true, ArrayScopeListMixin.where(id: 1).first.in_list?
ArrayScopeListMixin.where(id: 1).first.remove_from_list
assert_equal false, ArrayScopeListMixin.where(id: 1).first.in_list?
end

def test_remove_from_list_should_set_position_to_nil
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(2).remove_from_list
ArrayScopeListMixin.where(id: 2).first.remove_from_list

assert_equal [2, 1, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [2, 1, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

assert_equal 1, ArrayScopeListMixin.find(1).pos
assert_equal nil, ArrayScopeListMixin.find(2).pos
assert_equal 2, ArrayScopeListMixin.find(3).pos
assert_equal 3, ArrayScopeListMixin.find(4).pos
assert_equal 1, ArrayScopeListMixin.where(id: 1).first.pos
assert_equal nil, ArrayScopeListMixin.where(id: 2).first.pos
assert_equal 2, ArrayScopeListMixin.where(id: 3).first.pos
assert_equal 3, ArrayScopeListMixin.where(id: 4).first.pos
end

def test_remove_before_destroy_does_not_shift_lower_items_twice
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 2, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

ArrayScopeListMixin.find(2).remove_from_list
ArrayScopeListMixin.find(2).destroy
ArrayScopeListMixin.where(id: 2).first.remove_from_list
ArrayScopeListMixin.where(id: 2).first.destroy

assert_equal [1, 3, 4], ArrayScopeListMixin.find(:all, :conditions => "parent_id = 5 AND parent_type = 'ParentClass'", :order => 'pos').map(&:id)
assert_equal [1, 3, 4], ArrayScopeListMixin.where(parent_id: 5, parent_type: 'ParentClass').order('pos').map(&:id)

assert_equal 1, ArrayScopeListMixin.find(1).pos
assert_equal 2, ArrayScopeListMixin.find(3).pos
assert_equal 3, ArrayScopeListMixin.find(4).pos
assert_equal 1, ArrayScopeListMixin.where(id: 1).first.pos
assert_equal 2, ArrayScopeListMixin.where(id: 3).first.pos
assert_equal 3, ArrayScopeListMixin.where(id: 4).first.pos
end
end
end
Loading

5 comments on commit d8d4155

@tvdeyen
Copy link
Contributor

@tvdeyen tvdeyen commented on d8d4155 Aug 1, 2013

Choose a reason for hiding this comment

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

❤️
:shipit:

@conzett
Copy link
Contributor

@conzett conzett commented on d8d4155 Aug 1, 2013

Choose a reason for hiding this comment

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

👏 Thanks @swanandp and @philippfranke

@swanandp
Copy link
Contributor Author

@swanandp swanandp commented on d8d4155 Aug 2, 2013 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 d8d4155 Aug 2, 2013 via email

Choose a reason for hiding this comment

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

@philippfranke
Copy link
Contributor

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.