From f5806e357cd00a22d86c56b576c95cdb9ec3450a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Po=C5=9Bli=C5=84ski?= Date: Mon, 20 Jun 2016 11:12:58 +0200 Subject: [PATCH 1/4] Fix position in case of not serial positioning in db --- lib/acts_as_list/active_record/acts/list.rb | 34 +++++++++++++-------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/acts_as_list/active_record/acts/list.rb b/lib/acts_as_list/active_record/acts/list.rb index 2dfe9188..977b30b7 100644 --- a/lib/acts_as_list/active_record/acts/list.rb +++ b/lib/acts_as_list/active_record/acts/list.rb @@ -137,10 +137,8 @@ def self.update_all_with_touch(updates) attr_reader :position_changed before_validation :check_top_position - before_destroy :lock! after_destroy :decrement_positions_on_lower_items - before_update :check_scope after_update :update_positions @@ -169,8 +167,12 @@ def move_lower return unless lower_item acts_as_list_class.transaction do - lower_item.decrement_position - increment_position + if lower_item.send(position_column) != self.send(position_column) + swap_positions(lower_item, self) + else + lower_item.decrement_position + increment_position + end end end @@ -179,8 +181,12 @@ def move_higher return unless higher_item acts_as_list_class.transaction do - higher_item.increment_position - decrement_position + if higher_item.send(position_column) != self.send(position_column) + swap_positions(higher_item, self) + else + higher_item.increment_position + decrement_position + end end end @@ -231,16 +237,14 @@ def decrement_position set_list_position(self.send(position_column).to_i - 1) end - # Return +true+ if this object is the first in the list. def first? return false unless in_list? - self.send(position_column) == acts_as_list_top + !higher_item end - # Return +true+ if this object is the last in the list. def last? return false unless in_list? - self.send(position_column) == bottom_position_in_list + !lower_item end # Return the next higher item in the list. @@ -256,9 +260,8 @@ def higher_items(limit=nil) position_value = send(position_column) acts_as_list_list. where("#{quoted_table_name}.#{quoted_position_column} < ?", position_value). - where("#{quoted_table_name}.#{quoted_position_column} >= ?", position_value - limit). limit(limit). - order("#{quoted_table_name}.#{quoted_position_column} ASC") + order("#{quoted_table_name}.#{quoted_position_column} DESC") end # Return the next lower item in the list. @@ -274,7 +277,6 @@ def lower_items(limit=nil) position_value = send(position_column) acts_as_list_list. where("#{quoted_table_name}.#{quoted_position_column} > ?", position_value). - where("#{quoted_table_name}.#{quoted_position_column} <= ?", position_value + limit). limit(limit). order("#{quoted_table_name}.#{quoted_position_column} ASC") end @@ -303,6 +305,12 @@ def set_list_position(new_position) end private + + def swap_positions(item1, item2) + item1.set_list_position(item2.send(position_column)) + item2.set_list_position(item1.send("#{position_column}_was")) + end + def acts_as_list_list acts_as_list_class.unscoped do acts_as_list_class.where(scope_condition) From c674513c59ab10b81378cc54c706a8920622f9b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dawid=20Po=C5=9Bli=C5=84ski?= Date: Wed, 29 Jun 2016 11:29:03 +0200 Subject: [PATCH 2/4] New specs --- lib/acts_as_list/active_record/acts/list.rb | 8 +++---- test/shared_list_sub.rb | 26 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/acts_as_list/active_record/acts/list.rb b/lib/acts_as_list/active_record/acts/list.rb index 977b30b7..20c2829e 100644 --- a/lib/acts_as_list/active_record/acts/list.rb +++ b/lib/acts_as_list/active_record/acts/list.rb @@ -260,8 +260,8 @@ def higher_items(limit=nil) position_value = send(position_column) acts_as_list_list. where("#{quoted_table_name}.#{quoted_position_column} < ?", position_value). - limit(limit). - order("#{quoted_table_name}.#{quoted_position_column} DESC") + order("#{quoted_table_name}.#{quoted_position_column} DESC"). + limit(limit) end # Return the next lower item in the list. @@ -277,8 +277,8 @@ def lower_items(limit=nil) position_value = send(position_column) acts_as_list_list. where("#{quoted_table_name}.#{quoted_position_column} > ?", position_value). - limit(limit). - order("#{quoted_table_name}.#{quoted_position_column} ASC") + order("#{quoted_table_name}.#{quoted_position_column} ASC"). + limit(limit) end # Test if this record is in a list diff --git a/test/shared_list_sub.rb b/test/shared_list_sub.rb index 32c2b77a..18f171fa 100644 --- a/test/shared_list_sub.rb +++ b/test/shared_list_sub.rb @@ -43,6 +43,32 @@ def test_next_prev assert_nil ListMixin.where(id: 4).first.lower_item end + def test_next_prev_not_regular_sequence + ListMixin.all.map do |item| + item.update(pos: item.pos * 5) + end + + assert_equal [1, 2, 3, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id) + + ListMixin.where(id: 2).first.move_lower + assert_equal [1, 3, 2, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id) + + ListMixin.where(id: 2).first.move_higher + assert_equal [1, 2, 3, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id) + + ListMixin.where(id: 1).first.move_to_bottom + assert_equal [2, 3, 4, 1], ListMixin.where(parent_id: 5000).order('pos').map(&:id) + + ListMixin.where(id: 1).first.move_to_top + assert_equal [1, 2, 3, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id) + + ListMixin.where(id: 2).first.move_to_bottom + assert_equal [1, 3, 4, 2], ListMixin.where(parent_id: 5000).order('pos').map(&:id) + + ListMixin.where(id: 4).first.move_to_top + assert_equal [4, 1, 3, 2], ListMixin.where(parent_id: 5000).order('pos').map(&:id) + end + def test_next_prev_groups li1 = ListMixin.where(id: 1).first li2 = ListMixin.where(id: 2).first From 2330682326d4bca9328bbc2d649ce7cd3030db65 Mon Sep 17 00:00:00 2001 From: Justas Palumickas Date: Fri, 19 Aug 2016 12:55:40 +0300 Subject: [PATCH 3/4] Update tests --- test/shared_list_sub.rb | 4 ++-- test/test_joined_list.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/shared_list_sub.rb b/test/shared_list_sub.rb index 18f171fa..8bac8935 100644 --- a/test/shared_list_sub.rb +++ b/test/shared_list_sub.rb @@ -79,9 +79,9 @@ def test_next_prev_groups assert_equal [li2, li3], li1.lower_items(2) assert_equal [], li4.lower_items - assert_equal [li1, li2], li3.higher_items + assert_equal [li2, li1], li3.higher_items assert_equal [li1], li2.higher_items - assert_equal [li2, li3], li4.higher_items(2) + assert_equal [li3, li2], li4.higher_items(2) assert_equal [], li1.higher_items end diff --git a/test/test_joined_list.rb b/test/test_joined_list.rb index 680223c0..c81337b5 100644 --- a/test/test_joined_list.rb +++ b/test/test_joined_list.rb @@ -51,7 +51,7 @@ def test_higher_items item1 = Item.create section: section item2 = Item.create section: section item3 = Item.create section: section - assert_equal item3.higher_items.visible, [item1, item2] + assert_equal item3.higher_items.visible, [item2, item1] end def test_lower_items From 2abb8467054c992ae648dedb2b7ea3f480cded7c Mon Sep 17 00:00:00 2001 From: Justas Palumickas Date: Fri, 19 Aug 2016 13:29:21 +0300 Subject: [PATCH 4/4] Use update_attributes instead of update --- test/shared_list_sub.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/shared_list_sub.rb b/test/shared_list_sub.rb index 8bac8935..c0c8d2b6 100644 --- a/test/shared_list_sub.rb +++ b/test/shared_list_sub.rb @@ -44,8 +44,8 @@ def test_next_prev end def test_next_prev_not_regular_sequence - ListMixin.all.map do |item| - item.update(pos: item.pos * 5) + ListMixin.all.each do |item| + item.update_attributes(pos: item.pos * 5) end assert_equal [1, 2, 3, 4], ListMixin.where(parent_id: 5000).order('pos').map(&:id)