diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index aac1c856f0..8dd92a1e8b 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* acts_as_list plays nicely with inheritance by remembering the class which declared it. #2811 [rephorm@rephorm.com] + * Fix sqlite adaptor's detection of missing dbfile or database declaration. [Nicholas Seckar] * Fixed acts_as_list for definitions without an explicit :order #2803 [jonathan@bluewire.net.nz] diff --git a/activerecord/lib/active_record/acts/list.rb b/activerecord/lib/active_record/acts/list.rb index 551c41fb6a..560c74dfff 100644 --- a/activerecord/lib/active_record/acts/list.rb +++ b/activerecord/lib/active_record/acts/list.rb @@ -54,6 +54,10 @@ module ActiveRecord class_eval <<-EOV include ActiveRecord::Acts::List::InstanceMethods + def acts_as_list_class + #{self.name} + end + def position_column '#{configuration[:column]}' end @@ -78,7 +82,7 @@ module ActiveRecord def move_lower return unless lower_item - self.class.transaction do + acts_as_list_class.transaction do lower_item.decrement_position increment_position end @@ -87,7 +91,7 @@ module ActiveRecord def move_higher return unless higher_item - self.class.transaction do + acts_as_list_class.transaction do higher_item.increment_position decrement_position end @@ -95,7 +99,7 @@ module ActiveRecord def move_to_bottom return unless in_list? - self.class.transaction do + acts_as_list_class.transaction do decrement_positions_on_lower_items assume_bottom_position end @@ -103,7 +107,7 @@ module ActiveRecord def move_to_top return unless in_list? - self.class.transaction do + acts_as_list_class.transaction do increment_positions_on_higher_items assume_top_position end @@ -135,14 +139,14 @@ module ActiveRecord def higher_item return nil unless in_list? - self.class.find(:first, :conditions => + acts_as_list_class.find(:first, :conditions => "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i - 1).to_s}" ) end def lower_item return nil unless in_list? - self.class.find(:first, :conditions => + acts_as_list_class.find(:first, :conditions => "#{scope_condition} AND #{position_column} = #{(send(position_column).to_i + 1).to_s}" ) end @@ -171,7 +175,7 @@ module ActiveRecord def bottom_item(except = nil) conditions = scope_condition conditions = "#{conditions} AND id != #{except.id}" if except - self.class.find(:first, :conditions => conditions, :order => "#{position_column} DESC") + acts_as_list_class.find(:first, :conditions => conditions, :order => "#{position_column} DESC") end def assume_bottom_position @@ -184,7 +188,7 @@ module ActiveRecord # This has the effect of moving all the higher items up one. def decrement_positions_on_higher_items(position) - self.class.update_all( + acts_as_list_class.update_all( "#{position_column} = (#{position_column} - 1)", "#{scope_condition} AND #{position_column} <= #{position}" ) end @@ -192,7 +196,7 @@ module ActiveRecord # This has the effect of moving all the lower items up one. def decrement_positions_on_lower_items return unless in_list? - self.class.update_all( + acts_as_list_class.update_all( "#{position_column} = (#{position_column} - 1)", "#{scope_condition} AND #{position_column} > #{send(position_column).to_i}" ) end @@ -200,20 +204,20 @@ module ActiveRecord # This has the effect of moving all the higher items down one. def increment_positions_on_higher_items return unless in_list? - self.class.update_all( + acts_as_list_class.update_all( "#{position_column} = (#{position_column} + 1)", "#{scope_condition} AND #{position_column} < #{send(position_column).to_i}" ) end # This has the effect of moving all the lower items down one. def increment_positions_on_lower_items(position) - self.class.update_all( + acts_as_list_class.update_all( "#{position_column} = (#{position_column} + 1)", "#{scope_condition} AND #{position_column} >= #{position}" ) end def increment_positions_on_all_items - self.class.update_all( + acts_as_list_class.update_all( "#{position_column} = (#{position_column} + 1)", "#{scope_condition}" ) end diff --git a/activerecord/test/fixtures/mixin.rb b/activerecord/test/fixtures/mixin.rb index a0a92c4f64..0411c0d87e 100644 --- a/activerecord/test/fixtures/mixin.rb +++ b/activerecord/test/fixtures/mixin.rb @@ -16,6 +16,12 @@ class ListMixin < Mixin def self.table_name() "mixins" end end +class ListMixinSub1 < ListMixin +end + +class ListMixinSub2 < ListMixin +end + class ListWithStringScopeMixin < ActiveRecord::Base acts_as_list :column => "pos", :scope => 'parent_id = #{parent_id}' diff --git a/activerecord/test/fixtures/mixins.yml b/activerecord/test/fixtures/mixins.yml index 7b20965512..cb21349cd0 100644 --- a/activerecord/test/fixtures/mixins.yml +++ b/activerecord/test/fixtures/mixins.yml @@ -78,3 +78,12 @@ tree_<%= set[0] %>: root_id: 42 <% end %> + +# subclasses of list items +<% (1..4).each do |i| %> +list_sub_<%= i %>: + id: <%= i + 5000 %> + pos: <%= i %> + parent_id: 5000 + type: <%= (i % 2 == 1) ? ListMixinSub1 : ListMixinSub2 %> +<% end %> diff --git a/activerecord/test/mixin_test.rb b/activerecord/test/mixin_test.rb index 82b2a143fd..4a59b649dd 100644 --- a/activerecord/test/mixin_test.rb +++ b/activerecord/test/mixin_test.rb @@ -195,6 +195,7 @@ class ListTest < Test::Unit::TestCase new2.move_higher assert_equal [new2, new1, new3], ListMixin.find(:all, :conditions => 'parent_id IS NULL', :order => 'pos') end + end class TreeTest < Test::Unit::TestCase @@ -353,3 +354,165 @@ class TouchTest < Test::Unit::TestCase end end + + +class ListSubTest < Test::Unit::TestCase + fixtures :mixins + + def test_reordering + + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_2), + mixins(:list_sub_3), + mixins(:list_sub_4)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + mixins(:list_sub_2).move_lower + + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_3), + mixins(:list_sub_2), + mixins(:list_sub_4)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + mixins(:list_sub_2).move_higher + + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_2), + mixins(:list_sub_3), + mixins(:list_sub_4)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + mixins(:list_sub_1).move_to_bottom + + assert_equal [mixins(:list_sub_2), + mixins(:list_sub_3), + mixins(:list_sub_4), + mixins(:list_sub_1)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + mixins(:list_sub_1).move_to_top + + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_2), + mixins(:list_sub_3), + mixins(:list_sub_4)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + + mixins(:list_sub_2).move_to_bottom + + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_3), + mixins(:list_sub_4), + mixins(:list_sub_2)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + mixins(:list_sub_4).move_to_top + + assert_equal [mixins(:list_sub_4), + mixins(:list_sub_1), + mixins(:list_sub_3), + mixins(:list_sub_2)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + end + + def test_move_to_bottom_with_next_to_last_item + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_2), + mixins(:list_sub_3), + mixins(:list_sub_4)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + mixins(:list_sub_3).move_to_bottom + + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_2), + mixins(:list_sub_4), + mixins(:list_sub_3)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + end + + def test_next_prev + assert_equal mixins(:list_sub_2), mixins(:list_sub_1).lower_item + assert_nil mixins(:list_sub_1).higher_item + assert_equal mixins(:list_sub_3), mixins(:list_sub_4).higher_item + assert_nil mixins(:list_sub_4).lower_item + end + + + def test_injection + item = ListMixin.new("parent_id"=>1) + assert_equal "parent_id = 1", item.scope_condition + assert_equal "pos", item.position_column + end + + + def test_insert_at + new = ListMixin.create("parent_id" => 20) + assert_equal 1, new.pos + + new = ListMixinSub1.create("parent_id" => 20) + assert_equal 2, new.pos + + new = ListMixinSub2.create("parent_id" => 20) + assert_equal 3, new.pos + + new4 = ListMixin.create("parent_id" => 20) + assert_equal 4, new4.pos + + new4.insert_at(3) + assert_equal 3, new4.pos + + new.reload + assert_equal 4, new.pos + + new.insert_at(2) + assert_equal 2, new.pos + + new4.reload + assert_equal 4, new4.pos + + new5 = ListMixinSub1.create("parent_id" => 20) + assert_equal 5, new5.pos + + new5.insert_at(1) + assert_equal 1, new5.pos + + new4.reload + assert_equal 5, new4.pos + end + + def test_delete_middle + + assert_equal [mixins(:list_sub_1), + mixins(:list_sub_2), + mixins(:list_sub_3), + mixins(:list_sub_4)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + mixins(:list_sub_2).destroy + + assert_equal [mixins(:list_sub_1, :reload), + mixins(:list_sub_3, :reload), + mixins(:list_sub_4, :reload)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + assert_equal 1, mixins(:list_sub_1).pos + assert_equal 2, mixins(:list_sub_3).pos + assert_equal 3, mixins(:list_sub_4).pos + + mixins(:list_sub_1).destroy + + assert_equal [mixins(:list_sub_3, :reload), + mixins(:list_sub_4, :reload)], + ListMixin.find(:all, :conditions => 'parent_id = 5000', :order => 'pos') + + assert_equal 1, mixins(:list_sub_3).pos + assert_equal 2, mixins(:list_sub_4).pos + + end + +end +